From 86d02eaa155cfdbb0a8cf31c8eae47711fbf5c2b Mon Sep 17 00:00:00 2001 From: anthony Date: Wed, 3 Nov 2010 15:59:16 -0700 Subject: [PATCH] bug 6882: relink the child to parent after delete snapshot change dd block size from 512 to 1M, make taking snapstho faster for iscsi primary storage status 6882: resolved fixed --- .../agent/api/BackupSnapshotCommand.java | 10 +++ .../api/DeleteSnapshotBackupCommand.java | 10 ++- .../xen/resource/CitrixResourceBase.java | 66 +++++++++++-------- scripts/vm/hypervisor/xenserver/vmops | 15 +++-- .../storage/snapshot/SnapshotManagerImpl.java | 14 ++-- 5 files changed, 77 insertions(+), 38 deletions(-) diff --git a/core/src/com/cloud/agent/api/BackupSnapshotCommand.java b/core/src/com/cloud/agent/api/BackupSnapshotCommand.java index 72c24317395..3db363f3520 100644 --- a/core/src/com/cloud/agent/api/BackupSnapshotCommand.java +++ b/core/src/com/cloud/agent/api/BackupSnapshotCommand.java @@ -30,6 +30,7 @@ public class BackupSnapshotCommand extends SnapshotCommand { private boolean isFirstSnapshotOfRootVolume; private boolean isVolumeInactive; private String firstBackupUuid; + private String volumeUUID; protected BackupSnapshotCommand() { @@ -50,6 +51,7 @@ public class BackupSnapshotCommand extends SnapshotCommand { Long dcId, Long accountId, Long volumeId, + String volumeUUID, String snapshotUuid, String prevSnapshotUuid, String prevBackupUuid, @@ -63,6 +65,7 @@ public class BackupSnapshotCommand extends SnapshotCommand { this.firstBackupUuid = firstBackupUuid; this.isFirstSnapshotOfRootVolume = isFirstSnapshotOfRootVolume; this.isVolumeInactive = isVolumeInactive; + this.volumeUUID = volumeUUID; } public String getPrevSnapshotUuid() { @@ -84,5 +87,12 @@ public class BackupSnapshotCommand extends SnapshotCommand { public boolean isVolumeInactive() { return isVolumeInactive; } + + /** + * @return the volumeUUID + */ + public String getVolumeUUID() { + return volumeUUID; + } } \ No newline at end of file diff --git a/core/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java b/core/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java index 1f6cd9624f8..f1ac31f9385 100644 --- a/core/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java +++ b/core/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java @@ -23,6 +23,7 @@ package com.cloud.agent.api; */ public class DeleteSnapshotBackupCommand extends SnapshotCommand { private String childUUID; + private String childChildUUID; protected DeleteSnapshotBackupCommand() { @@ -59,10 +60,12 @@ public class DeleteSnapshotBackupCommand extends SnapshotCommand { Long accountId, Long volumeId, String backupUUID, - String childUUID) + String childUUID, + String childChildUUID) { super(primaryStoragePoolNameLabel, secondaryStoragePoolURL, backupUUID, dcId, accountId, volumeId); this.childUUID = childUUID; + this.childChildUUID = childChildUUID; } /** @@ -72,4 +75,9 @@ public class DeleteSnapshotBackupCommand extends SnapshotCommand { return childUUID; } + + public String getChildChildUUID() { + return childChildUUID; + } + } \ No newline at end of file diff --git a/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java b/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java index faca9462c4a..33d2f2cedcb 100644 --- a/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java +++ b/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java @@ -399,30 +399,6 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR return null; } - protected boolean currentlyAttached(SR sr, SR.Record rec, PBD pbd, PBD.Record pbdr) { - String status = null; - if (SRType.NFS.equals(rec.type)) { - //status = callHostPlugin("checkMount", "mount", rec.uuid); - return true; - } else if (SRType.LVMOISCSI.equals(rec.type) ) { - String scsiid = pbdr.deviceConfig.get("SCSIid"); - if (scsiid.isEmpty()) { - return false; - } - status = callHostPlugin("checkIscsi", "scsiid", scsiid); - } else { - return true; - } - - if (status != null && status.equalsIgnoreCase("1")) { - s_logger.debug("currently attached " + pbdr.uuid); - return true; - } else { - s_logger.debug("currently not attached " + pbdr.uuid); - return false; - } - } - protected boolean pingdomr(String host, String port) { String status; status = callHostPlugin("pingdomr", "host", host, "port", port); @@ -5383,7 +5359,9 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR // new one // and muddle the vhd chain on the secondary storage. details = "Successfully backedUp the snapshotUuid: " + snapshotUuid + " to secondary storage."; - destroySnapshotOnPrimaryStorage(prevSnapshotUuid); + String volumeUuid = cmd.getVolumeUUID(); + destroySnapshotOnPrimaryStorageExceptThis(volumeUuid, snapshotUuid); + } } catch (XenAPIException e) { @@ -5519,6 +5497,7 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR String secondaryStoragePoolURL = cmd.getSecondaryStoragePoolURL(); String backupUUID = cmd.getSnapshotUuid(); String childUUID = cmd.getChildUUID(); + String childChildUUID = cmd.getChildChildUUID(); String primaryStorageNameLabel = cmd.getPrimaryStoragePoolNameLabel(); String details = null; @@ -5556,7 +5535,7 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR if (secondaryStorageMountPath == null) { details = "Couldn't delete snapshot because the URL passed: " + secondaryStoragePoolURL + " is invalid."; } else { - details = deleteSnapshotBackup(dcId, accountId, volumeId, secondaryStorageMountPath, backupUUID, childUUID, isISCSI); + details = deleteSnapshotBackup(dcId, accountId, volumeId, secondaryStorageMountPath, backupUUID, childUUID, childChildUUID, isISCSI); success = (details != null && details.equals("1")); if (success) { s_logger.debug("Successfully deleted snapshot backup " + backupUUID); @@ -5879,6 +5858,37 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR return backupSnapshotUuid; } + + private boolean destroySnapshotOnPrimaryStorageExceptThis(String volumeUuid, String avoidSnapshotUuid){ + try { + Connection conn = getConnection(); + VDI volume = getVDIbyUuid(volumeUuid); + if (volume == null) { + throw new InternalErrorException("Could not destroy snapshot on volume " + volumeUuid + " due to can not find it"); + } + Set snapshots = volume.getSnapshots(conn); + for( VDI snapshot : snapshots ) { + try { + if(! snapshot.getUuid(conn).equals(avoidSnapshotUuid)) { + snapshot.destroy(conn); + } + } catch (Exception e) { + String msg = "Destroying snapshot: " + snapshot+ " on primary storage failed due to " + e.toString(); + s_logger.warn(msg, e); + } + } + s_logger.debug("Successfully destroyed snapshot on volume: " + volumeUuid + " execept this current snapshot "+ avoidSnapshotUuid ); + return true; + } catch (XenAPIException e) { + String msg = "Destroying snapshot on volume: " + volumeUuid + " execept this current snapshot "+ avoidSnapshotUuid + " failed due to " + e.toString(); + s_logger.error(msg, e); + } catch (Exception e) { + String msg = "Destroying snapshot on volume: " + volumeUuid + " execept this current snapshot "+ avoidSnapshotUuid + " failed due to " + e.toString(); + s_logger.warn(msg, e); + } + + return false; + } protected boolean destroySnapshotOnPrimaryStorage(String snapshotUuid) { // Precondition snapshotUuid != null @@ -5902,10 +5912,10 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR return false; } - protected String deleteSnapshotBackup(Long dcId, Long accountId, Long volumeId, String secondaryStorageMountPath, String backupUUID, String childUUID, Boolean isISCSI) { + protected String deleteSnapshotBackup(Long dcId, Long accountId, Long volumeId, String secondaryStorageMountPath, String backupUUID, String childUUID, String childChildUUID, Boolean isISCSI) { // If anybody modifies the formatting below again, I'll skin them - String result = callHostPlugin("deleteSnapshotBackup", "backupUUID", backupUUID, "childUUID", childUUID, "dcId", dcId.toString(), "accountId", accountId.toString(), + String result = callHostPlugin("deleteSnapshotBackup", "backupUUID", backupUUID, "childUUID", childUUID, "childChildUUID", childChildUUID, "dcId", dcId.toString(), "accountId", accountId.toString(), "volumeId", volumeId.toString(), "secondaryStorageMountPath", secondaryStorageMountPath, "isISCSI", isISCSI.toString()); return result; diff --git a/scripts/vm/hypervisor/xenserver/vmops b/scripts/vm/hypervisor/xenserver/vmops index 06b0d00af6f..5cebcff4013 100755 --- a/scripts/vm/hypervisor/xenserver/vmops +++ b/scripts/vm/hypervisor/xenserver/vmops @@ -580,7 +580,7 @@ def copyfile(fromFile, toFile, isISCSI): errMsg = '' if isISCSI: try: - cmd = ['dd', 'if=' + fromFile, 'of=' + toFile] + cmd = ['dd', 'if=' + fromFile, 'of=' + toFile , 'bs=1M'] txt = util.pread2(cmd) except: txt = '' @@ -685,7 +685,7 @@ def rename(originalVHD, newVHD): raise xs_errors.XenError(errMsg) return -def coalesceToChild(backupVHD, childUUID): +def coalesceToChild(backupVHD, childUUID, childChildUUID): # coalesce childVHD with its parent childVHD = getVHD(childUUID, False) util.SMlog("childVHD: " + childVHD) @@ -698,6 +698,9 @@ def coalesceToChild(backupVHD, childUUID): # rename the existing backupVHD file to childVHD # childVHD file automatically gets overwritten rename(backupVHD, childVHD) + if childChildUUID: + childChildVHD = getVHD(childChildUUID, False) + setParent(childVHD, childChildVHD) # parent of the newly coalesced file still remains the same. # child of childVHD has it's parent name still set to childVHD. @@ -1110,7 +1113,8 @@ def backupSnapshot(session, args): # Check existence of snapshot on primary storage isfile(baseCopyPath, isISCSI) # copy baseCopyPath to backupsDir - backupVHD = getVHD(baseCopyUuid, False) + backupUuid = util.gen_uuid() + backupVHD = getVHD(backupUuid, False) backupFile = os.path.join(backupsDir, backupVHD) copyfile(baseCopyPath, backupFile, isISCSI) @@ -1136,7 +1140,7 @@ def backupSnapshot(session, args): prevBackupVHD = getVHD(prevBackupUuid, False) setParent(prevBackupVHD, backupFile) - txt = "1#" + baseCopyUuid + txt = "1#" + backupUuid return txt def createDummyVHD(baseCopyPath, backupsDir): @@ -1166,6 +1170,7 @@ def deleteSnapshotBackup(session, args): secondaryStorageMountPath = args['secondaryStorageMountPath'] backupUUID = args['backupUUID'] childUUID = args['childUUID'] + childChildUUID = args['childChildUUID'] backupsDir = mountSnapshotsDir(secondaryStorageMountPath, "snapshots", dcId, accountId, volumeId) # chdir to the backupsDir for convenience @@ -1182,7 +1187,7 @@ def deleteSnapshotBackup(session, args): # Case 1) childUUID exists if childUUID: - coalesceToChild(backupVHD, childUUID) + coalesceToChild(backupVHD, childUUID, childChildUUID) else: # Just delete the backupVHD try: diff --git a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java index fb1078bb3f7..43884edda2c 100644 --- a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -592,6 +592,7 @@ public class SnapshotManagerImpl implements SnapshotManager { dcId, accountId, volumeId, + volume.getPath(), snapshotUuid, prevSnapshotUuid, prevBackupUuid, @@ -937,8 +938,11 @@ public class SnapshotManagerImpl implements SnapshotManager { String backupOfSnapshot = snapshot.getBackupSnapshotId(); String backupOfNextSnapshot = null; - if (nextSnapshot != null) { - backupOfNextSnapshot = nextSnapshot.getBackupSnapshotId(); + backupOfNextSnapshot = nextSnapshot.getBackupSnapshotId(); + String backupOfNextNextSnapshot = null; + SnapshotVO nextNextSnapshot = _snapshotDao.findNextSnapshot(nextSnapshot.getId()); + if( nextNextSnapshot != null ) { + backupOfNextNextSnapshot = nextNextSnapshot.getBackupSnapshotId(); } DeleteSnapshotBackupCommand cmd = @@ -948,7 +952,8 @@ public class SnapshotManagerImpl implements SnapshotManager { accountId, volumeId, backupOfSnapshot, - backupOfNextSnapshot); + backupOfNextSnapshot, + backupOfNextNextSnapshot); details = "Failed to destroy snapshot id:" + snapshotId + " for volume: " + volume.getId(); Answer answer = _storageMgr.sendToHostsOnStoragePool(volume.getPoolId(), @@ -1294,7 +1299,8 @@ public class SnapshotManagerImpl implements SnapshotManager { accountId, volumeId, backupOfSnapshot, - backupOfNextSnapshot); + backupOfNextSnapshot, + null); String basicErrMsg = "Failed to destroy snapshot id: " + snapshotId + " for volume id: " + volumeId; Answer answer = _storageMgr.sendToHostsOnStoragePool(volume.getPoolId(), cmd, basicErrMsg, _totalRetries, _pauseInterval, _shouldBeSnapshotCapable);