From fb8364ad0c5e6ea0ef356465c6ea5b55b3b9afce Mon Sep 17 00:00:00 2001 From: anthony Date: Tue, 10 Jan 2012 15:51:13 -0800 Subject: [PATCH] bug 10363 : cleanup vhd in secondary storage if backsnapshot fails Conflicts: core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java --- .../xen/resource/CitrixResourceBase.java | 52 ++++++++++--------- scripts/vm/hypervisor/xenserver/vmopsSnapshot | 37 +++++++------ 2 files changed, 45 insertions(+), 44 deletions(-) diff --git a/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java b/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java index 314ab431d62..32b5231bef8 100755 --- a/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java +++ b/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java @@ -3075,35 +3075,37 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe // Each argument is put in a separate line for readability. // Using more lines does not harm the environment. + String backupUuid = UUID.randomUUID().toString(); String results = callHostPluginAsync(conn, "vmopsSnapshot", "backupSnapshot", wait, - "primaryStorageSRUuid", primaryStorageSRUuid, "dcId", dcId.toString(), "accountId", accountId - .toString(), "volumeId", volumeId.toString(), "secondaryStorageMountPath", - secondaryStorageMountPath, "snapshotUuid", snapshotUuid, "prevBackupUuid", prevBackupUuid, "isISCSI", - isISCSI.toString()); - + "primaryStorageSRUuid", primaryStorageSRUuid, "dcId", dcId.toString(), "accountId", accountId.toString(), + "volumeId", volumeId.toString(), "secondaryStorageMountPath", secondaryStorageMountPath, + "snapshotUuid", snapshotUuid, "prevBackupUuid", prevBackupUuid, "backupUuid", backupUuid, "isISCSI", isISCSI.toString()); + String errMsg = null; if (results == null || results.isEmpty()) { - // errString is already logged. - return null; - } - - String[] tmp = results.split("#"); - String status = tmp[0]; - backupSnapshotUuid = tmp[1]; - - // status == "1" if and only if backupSnapshotUuid != null - // So we don't rely on status value but return backupSnapshotUuid as an - // indicator of success. - String failureString = "Could not copy backupUuid: " + backupSnapshotUuid + " of volumeId: " + volumeId - + " from primary storage " + primaryStorageSRUuid + " to secondary storage " - + secondaryStorageMountPath; - if (status != null && status.equalsIgnoreCase("1") && backupSnapshotUuid != null) { - s_logger.debug("Successfully copied backupUuid: " + backupSnapshotUuid + " of volumeId: " + volumeId - + " to secondary storage"); + errMsg = "Could not copy backupUuid: " + backupSnapshotUuid + " of volumeId: " + volumeId + + " from primary storage " + primaryStorageSRUuid + " to secondary storage " + + secondaryStorageMountPath + " due to null"; } else { - s_logger.debug(failureString + ". Failed with status: " + status); - return null; + + String[] tmp = results.split("#"); + String status = tmp[0]; + backupSnapshotUuid = tmp[1]; + // status == "1" if and only if backupSnapshotUuid != null + // So we don't rely on status value but return backupSnapshotUuid as an + // indicator of success. + if (status != null && status.equalsIgnoreCase("1") && backupSnapshotUuid != null) { + s_logger.debug("Successfully copied backupUuid: " + backupSnapshotUuid + " of volumeId: " + volumeId + + " to secondary storage"); + return backupSnapshotUuid; + } else { + errMsg = "Could not copy backupUuid: " + backupSnapshotUuid + " of volumeId: " + volumeId + + " from primary storage " + primaryStorageSRUuid + " to secondary storage " + + secondaryStorageMountPath + " due to " + tmp[1]; + } } - return backupSnapshotUuid; + s_logger.warn(errMsg); + return null; + } protected String callHostPluginAsync(Connection conn, String plugin, String cmd, int wait, String... params) { diff --git a/scripts/vm/hypervisor/xenserver/vmopsSnapshot b/scripts/vm/hypervisor/xenserver/vmopsSnapshot index 67c7921c270..9b011c9edf1 100755 --- a/scripts/vm/hypervisor/xenserver/vmopsSnapshot +++ b/scripts/vm/hypervisor/xenserver/vmopsSnapshot @@ -113,8 +113,8 @@ def post_create_private_template(session, args): local_mount_path = os.path.join(CLOUD_DIR, util.gen_uuid()) mount(templatePath, local_mount_path) # Retrieve args - filename = args["templateFilename"] - name = args["templateName"] + filename = args["templateFilename"] + name = args["templateName"] description = args["templateDescription"] checksum = args["checksum"] file_size = args["size"] @@ -175,23 +175,18 @@ def isfile(path, isISCSI): def copyfile(fromFile, toFile, isISCSI): util.SMlog("Starting to copy " + fromFile + " to " + toFile) errMsg = '' - if isISCSI: + try: + cmd = ['dd', 'if=' + fromFile, 'of=' + toFile, 'bs=4M'] + txt = util.pread2(cmd) + except: try: - cmd = ['dd', 'if=' + fromFile, 'of=' + toFile, 'bs=1M'] - txt = util.pread2(cmd) + os.system("rm -f " + toFile) except: - txt = '' - errMsg = "Error while copying " + fromFile + " to " + toFile + " in ISCSI mode" - util.SMlog(errMsg) - raise xs_errors.XenError(errMsg) - else: - try: - shutil.copy2(fromFile, toFile) - except OSError, (errno, strerror): - errMsg = "Error while copying " + fromFile + " to " + toFile + " with errno: " + str(errno) + " and strerr: " + strerror - util.SMlog(errMsg) - raise xs_errors.XenError(errMsg) + txt = '' + errMsg = "Error while copying " + fromFile + " to " + toFile + " in secondary storage" + util.SMlog(errMsg) + raise xs_errors.XenError(errMsg) util.SMlog("Successfully copied " + fromFile + " to " + toFile) return errMsg @@ -469,6 +464,7 @@ def backupSnapshot(session, args): secondaryStorageMountPath = args['secondaryStorageMountPath'] snapshotUuid = args['snapshotUuid'] prevBackupUuid = args['prevBackupUuid'] + backupUuid = args['backupUuid'] isISCSI = getIsTrueString(args['isISCSI']) primarySRPath = getPrimarySRPath(primaryStorageSRUuid, isISCSI) @@ -488,8 +484,13 @@ def backupSnapshot(session, args): # Check existence of snapshot on primary storage isfile(baseCopyPath, isISCSI) + if prevBackupUuid: + # Check existence of prevBackupFile + prevBackupVHD = getBackupVHD(prevBackupUuid) + prevBackupFile = os.path.join(backupsDir, prevBackupVHD) + isfile(prevBackupFile, False) + # copy baseCopyPath to backupsDir with new uuid - backupUuid = util.gen_uuid() backupVHD = getBackupVHD(backupUuid) backupFile = os.path.join(backupsDir, backupVHD) util.SMlog("Back up " + baseCopyUuid + " to Secondary Storage as " + backupUuid) @@ -501,8 +502,6 @@ def backupSnapshot(session, args): # So set the parent of the current baseCopyVHD to prevBackupVHD if prevBackupUuid: # If there was a previous snapshot - prevBackupVHD = getBackupVHD(prevBackupUuid) - prevBackupFile = os.path.join(backupsDir, prevBackupVHD) setParent(prevBackupFile, backupFile) txt = "1#" + backupUuid