bug 10363 : cleanup vhd in secondary storage if backsnapshot fails

Conflicts:

	core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java

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
+                "primaryStorageSRUuid", primaryStorageSRUuid, "dcId", dcId.toString(), "accountId", accountId.toString(),
-                .toString(), "volumeId", volumeId.toString(), "secondaryStorageMountPath",
+                "volumeId", volumeId.toString(), "secondaryStorageMountPath", secondaryStorageMountPath,
-                secondaryStorageMountPath, "snapshotUuid", snapshotUuid, "prevBackupUuid", prevBackupUuid, "isISCSI",
+                "snapshotUuid", snapshotUuid, "prevBackupUuid", prevBackupUuid, "backupUuid", backupUuid, "isISCSI", isISCSI.toString());
-                isISCSI.toString());
+        String errMsg = null;
-
         if (results == null || results.isEmpty()) {
-            // errString is already logged.
+            errMsg = "Could not copy backupUuid: " + backupSnapshotUuid + " of volumeId: " + volumeId
-            return null;
+                    + " from primary storage " + primaryStorageSRUuid + " to secondary storage "
-        }
+                    + secondaryStorageMountPath + " due to 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");
         } 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) {

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"]
+            filename = args["templateFilename"]
-	    name = args["templateName"]
+            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']
+            os.system("rm -f " + toFile)
-            txt = util.pread2(cmd)
         except:
-            txt = ''
-            errMsg = "Error while copying " + fromFile + " to " + toFile + " in ISCSI mode"
-            util.SMlog(errMsg)
-            raise xs_errors.XenError(errMsg)
 
-    else:
+        txt = ''
-        try:
+        errMsg = "Error while copying " + fromFile + " to " + toFile + " in secondary storage"
-            shutil.copy2(fromFile, toFile)
+        util.SMlog(errMsg)
-        except OSError, (errno, strerror):
+        raise xs_errors.XenError(errMsg)
-            errMsg = "Error while copying " + fromFile + " to " + toFile + " with errno: " + str(errno) + " and strerr: " + strerror
-            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