Multiple Secondary Storage Issue

api/src/com/cloud/agent/api/BackupSnapshotCommand.java

@@ -35,6 +35,7 @@ public class BackupSnapshotCommand extends SnapshotCommand {
     private SwiftTO swift;
     private S3TO s3;
     StorageFilerTO pool;
+    private Long secHostId;
 
     protected BackupSnapshotCommand() {
 
@@ -49,12 +50,14 @@ public class BackupSnapshotCommand extends SnapshotCommand {
      * @param firstBackupUuid          This is the backup of the first ever snapshot taken by the volume.
      * @param isFirstSnapshotOfRootVolume true if this is the first snapshot of a root volume. Set the parent of the backup to null.
      * @param isVolumeInactive         True if the volume belongs to a VM that is not running or is detached.
+     * @param secHostId                This is the Id of the secondary storage.
      */
     public BackupSnapshotCommand(String secondaryStoragePoolURL,
                                  Long   dcId,
                                  Long   accountId,
                                  Long   volumeId,
                                  Long   snapshotId,
+                                 Long   secHostId,
                                  String volumePath,
                                  StoragePool pool,
                                  String snapshotUuid,
@@ -71,6 +74,7 @@ public class BackupSnapshotCommand extends SnapshotCommand {
         this.prevBackupUuid = prevBackupUuid;
         this.isVolumeInactive = isVolumeInactive;
         this.vmName = vmName;
+        this.secHostId = secHostId;
         setVolumePath(volumePath);
         setWait(wait);
     }
@@ -111,4 +115,7 @@ public class BackupSnapshotCommand extends SnapshotCommand {
         return snapshotId;
     }
 
+    public Long getSecHostId() {
+        return secHostId;
+    }
 }

api/test/org/apache/cloudstack/api/agent/test/BackupSnapshotAnswerTest.java

@@ -37,7 +37,7 @@ public class BackupSnapshotAnswerTest {
         StoragePool pool = Mockito.mock(StoragePool.class);
 
         bsc = new BackupSnapshotCommand(
-                "secondaryStoragePoolURL", 101L, 102L, 103L, 104L,
+                "secondaryStoragePoolURL", 101L, 102L, 103L, 104L, 105L,
                 "volumePath", pool, "snapshotUuid", "snapshotName",
                 "prevSnapshotUuid", "prevBackupUuid", false, "vmName", 5);
         bsa = new BackupSnapshotAnswer(bsc, true, "results", "bussname", false);

api/test/org/apache/cloudstack/api/agent/test/BackupSnapshotCommandTest.java

@@ -147,14 +147,14 @@ public class BackupSnapshotCommandTest {
 
     BackupSnapshotCommand bsc = new BackupSnapshotCommand(
             "http://secondary.Storage.Url",
-            101L, 102L, 103L, 104L, "vPath", pool,
+            101L, 102L, 103L, 104L, 105L, "vPath", pool,
             "420fa39c-4ef1-a83c-fd93-46dc1ff515ae", "sName",
             "9012793e-0657-11e2-bebc-0050568b0057",
             "7167e0b2-f5b0-11e1-8414-0050568b0057", false, "vmName", 5);
 
     BackupSnapshotCommand bsc1 = new BackupSnapshotCommand(
             "http://secondary.Storage.Url",
-            101L, 102L, 103L, 104L, "vPath", pool,
+            101L, 102L, 103L, 104L, 105L,"vPath", pool,
             "420fa39c-4ef1-a83c-fd93-46dc1ff515ae", "sName",
             "9012793e-0657-11e2-bebc-0050568b0057",
             "7167e0b2-f5b0-11e1-8414-0050568b0057", false, "vmName", 5);

engine/storage/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java

@@ -660,7 +660,7 @@ public class AncientDataMotionStrategy implements DataMotionStrategy {
          Long accountId = baseVolume.getAccountId();
          
          HostVO secHost = getSecHost(baseVolume.getId(), baseVolume.getDataCenterId());
-         
+         Long secHostId = secHost.getId();
          String secondaryStoragePoolUrl = secHost.getStorageUrl();
          String snapshotUuid = srcSnapshot.getPath();
          // In order to verify that the snapshot is not empty,
@@ -693,7 +693,7 @@ public class AncientDataMotionStrategy implements DataMotionStrategy {
          StoragePool srcPool = (StoragePool)dataStoreMgr.getPrimaryDataStore(baseVolume.getPoolId());
          String value = configDao.getValue(Config.BackupSnapshotWait.toString());
          int _backupsnapshotwait = NumbersUtil.parseInt(value, Integer.parseInt(Config.BackupSnapshotWait.getDefaultValue()));
-         BackupSnapshotCommand backupSnapshotCommand = new BackupSnapshotCommand(secondaryStoragePoolUrl, dcId, accountId, baseVolume.getId(), srcSnapshot.getId(), baseVolume.getPath(), srcPool, snapshotUuid,
+         BackupSnapshotCommand backupSnapshotCommand = new BackupSnapshotCommand(secondaryStoragePoolUrl, dcId, accountId, baseVolume.getId(), srcSnapshot.getId(), secHostId, baseVolume.getPath(), srcPool, snapshotUuid,
         		 srcSnapshot.getName(), prevSnapshotUuid, prevBackupUuid, isVolumeInactive, vmName, _backupsnapshotwait);
 
          if ( swift != null ) {

plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java

@@ -3552,7 +3552,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
 
 
     protected String backupSnapshot(Connection conn, String primaryStorageSRUuid, Long dcId, Long accountId,
-            Long volumeId, String secondaryStorageMountPath, String snapshotUuid, String prevBackupUuid, Boolean isISCSI, int wait) {
+            Long volumeId, String secondaryStorageMountPath, String snapshotUuid, String prevBackupUuid, Boolean isISCSI, int wait, Long secHostId) {
         String backupSnapshotUuid = null;
 
         if (prevBackupUuid == null) {
@@ -3565,7 +3565,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
         String results = callHostPluginAsync(conn, "vmopsSnapshot", "backupSnapshot", wait,
                 "primaryStorageSRUuid", primaryStorageSRUuid, "dcId", dcId.toString(), "accountId", accountId.toString(),
                 "volumeId", volumeId.toString(), "secondaryStorageMountPath", secondaryStorageMountPath,
-                "snapshotUuid", snapshotUuid, "prevBackupUuid", prevBackupUuid, "backupUuid", backupUuid, "isISCSI", isISCSI.toString());
+                "snapshotUuid", snapshotUuid, "prevBackupUuid", prevBackupUuid, "backupUuid", backupUuid, "isISCSI", isISCSI.toString(), "secHostId", secHostId.toString());
         String errMsg = null;
         if (results == null || results.isEmpty()) {
             errMsg = "Could not copy backupUuid: " + backupSnapshotUuid + " of volumeId: " + volumeId
@@ -6837,6 +6837,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
         String prevBackupUuid = cmd.getPrevBackupUuid();
         String prevSnapshotUuid = cmd.getPrevSnapshotUuid();
         int wait = cmd.getWait();
+        Long secHostId = cmd.getSecHostId();
         // By default assume failure
         String details = null;
         boolean success = false;
@@ -6915,7 +6916,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
                 } else if (cmd.getS3() != null) {
                     backupSnapshotToS3(conn, cmd.getS3(), primaryStorageSRUuid, snapshotPaUuid, isISCSI, wait);
                 } else {
-                    snapshotBackupUuid = backupSnapshot(conn, primaryStorageSRUuid, dcId, accountId, volumeId, secondaryStorageMountPath, snapshotUuid, prevBackupUuid, isISCSI, wait);
+                    snapshotBackupUuid = backupSnapshot(conn, primaryStorageSRUuid, dcId, accountId, volumeId, secondaryStorageMountPath, snapshotUuid, prevBackupUuid, isISCSI, wait, secHostId);
                     success = (snapshotBackupUuid != null);
                 }
             }

scripts/vm/hypervisor/xenserver/vmopsSnapshot

@@ -321,25 +321,24 @@ def umount(localDir):
     util.SMlog("Successfully unmounted " + localDir)
     return
 
-def mountSnapshotsDir(secondaryStorageMountPath, relativeDir, dcId, accountId, instanceId):
+def mountSnapshotsDir(secondaryStorageMountPath, relativeDir, dcId, accountId, instanceId, secHostId):
     # The aim is to mount secondaryStorageMountPath on 
     # And create <accountId>/<instanceId> dir on it, if it doesn't exist already.
     # Assuming that secondaryStorageMountPath  exists remotely
 
-    # Alex's suggestion and currently implemented:
+    # Just mount secondaryStorageMountPath/<relativeDir>/SecondaryStorageHost/ everytime
-    # Just mount secondaryStorageMountPath/<relativeDir> everytime
     # Never unmount.
     snapshotsDir = os.path.join(secondaryStorageMountPath, relativeDir)
 
     # Mkdir local mount point dir, if it doesn't exist.
     localMountPointPath = os.path.join(CLOUD_DIR, dcId)
-    localMountPointPath = os.path.join(localMountPointPath, relativeDir)
+    localMountPointPath = os.path.join(localMountPointPath, relativeDir, secHostId)
 
     makedirs(localMountPointPath)
-    # if something is not mounted already on localMountPointPath, 
+    # if something is not mounted already on localMountPointPath,
     # mount secondaryStorageMountPath on localMountPath
     if os.path.ismount(localMountPointPath):
-        # There is only one secondary storage per zone. 
+        # There is more than one secondary storage per zone.
         # And we are mounting each sec storage under a zone-specific directory
         # So two secondary storage snapshot dirs will never get mounted on the same point on the same XenServer.
         util.SMlog("The remote snapshots directory has already been mounted on " + localMountPointPath)
@@ -352,11 +351,22 @@ def mountSnapshotsDir(secondaryStorageMountPath, relativeDir, dcId, accountId, i
     makedirs(backupsDir)
     return backupsDir
 
+def unmountAll(path):
+    try:
+        for dir in os.listdir(path):
+            if dir.isdigit():
+                util.SMlog("Unmounting Sub-Directory: " + dir)
+                localMountPointPath = os.path.join(path, dir)
+                umount(localMountPointPath)
+    except:
+        util.SMlog("Ignoring the error while trying to unmount the snapshots dir")
+
 @echo
 def unmountSnapshotsDir(session, args):
     dcId = args['dcId']
     localMountPointPath = os.path.join(CLOUD_DIR, dcId)
     localMountPointPath = os.path.join(localMountPointPath, "snapshots")
+    unmountAll(localMountPointPath)
     try:
         umount(localMountPointPath)
     except:
@@ -482,7 +492,8 @@ def backupSnapshot(session, args):
     snapshotUuid              = args['snapshotUuid']
     prevBackupUuid            = args['prevBackupUuid']
     backupUuid                = args['backupUuid']
-    isISCSI                   = getIsTrueString(args['isISCSI']) 
+    isISCSI                   = getIsTrueString(args['isISCSI'])
+    secHostId                 = args['secHostId']
 
     primarySRPath = getPrimarySRPath(primaryStorageSRUuid, isISCSI)
     util.SMlog("primarySRPath: " + primarySRPath)
@@ -496,10 +507,10 @@ def backupSnapshot(session, args):
     # Mount secondary storage mount path on XenServer along the path
     # /var/run/sr-mount/<dcId>/snapshots/ and create <accountId>/<volumeId> dir
     # on it.
-    backupsDir = mountSnapshotsDir(secondaryStorageMountPath, "snapshots", dcId, accountId, volumeId)
+    backupsDir = mountSnapshotsDir(secondaryStorageMountPath, "snapshots", dcId, accountId, volumeId, secHostId)
     util.SMlog("Backups dir " + backupsDir)
 
-    # Check existence of snapshot on primary storage 
+    # Check existence of snapshot on primary storage
     isfile(baseCopyPath, isISCSI)
     if prevBackupUuid:
         # Check existence of prevBackupFile

scripts/vm/hypervisor/xenserver/xcposs/vmopsSnapshot

@@ -321,25 +321,24 @@ def umount(localDir):
     util.SMlog("Successfully unmounted " + localDir)
     return
 
-def mountSnapshotsDir(secondaryStorageMountPath, relativeDir, dcId, accountId, instanceId):
+def mountSnapshotsDir(secondaryStorageMountPath, relativeDir, dcId, accountId, instanceId, secHostId):
     # The aim is to mount secondaryStorageMountPath on 
     # And create <accountId>/<instanceId> dir on it, if it doesn't exist already.
     # Assuming that secondaryStorageMountPath  exists remotely
 
-    # Alex's suggestion and currently implemented:
+    # Just mount secondaryStorageMountPath/<relativeDir>/SecondaryStorageHost/ everytime
-    # Just mount secondaryStorageMountPath/<relativeDir> everytime
     # Never unmount.
     snapshotsDir = os.path.join(secondaryStorageMountPath, relativeDir)
 
     # Mkdir local mount point dir, if it doesn't exist.
     localMountPointPath = os.path.join(CLOUD_DIR, dcId)
-    localMountPointPath = os.path.join(localMountPointPath, relativeDir)
+    localMountPointPath = os.path.join(localMountPointPath, relativeDir, secHostId)
 
     makedirs(localMountPointPath)
     # if something is not mounted already on localMountPointPath, 
     # mount secondaryStorageMountPath on localMountPath
     if os.path.ismount(localMountPointPath):
-        # There is only one secondary storage per zone. 
+        # There can be more than one secondary storage per zone.
         # And we are mounting each sec storage under a zone-specific directory
         # So two secondary storage snapshot dirs will never get mounted on the same point on the same XenServer.
         util.SMlog("The remote snapshots directory has already been mounted on " + localMountPointPath)
@@ -352,11 +351,22 @@ def mountSnapshotsDir(secondaryStorageMountPath, relativeDir, dcId, accountId, i
     makedirs(backupsDir)
     return backupsDir
 
+def unmountAll(path):
+    try:
+        for dir in os.listdir(path):
+            if dir.isdigit():
+                util.SMlog("Unmounting Sub-Directory: " + dir)
+                localMountPointPath = os.path.join(path, dir)
+                umount(localMountPointPath)
+    except:
+        util.SMlog("Ignoring the error while trying to unmount the snapshots dir")
+
 @echo
 def unmountSnapshotsDir(session, args):
     dcId = args['dcId']
     localMountPointPath = os.path.join(CLOUD_DIR, dcId)
     localMountPointPath = os.path.join(localMountPointPath, "snapshots")
+    unmountAll(localMountPointPath)
     try:
         umount(localMountPointPath)
     except:
@@ -490,7 +500,8 @@ def backupSnapshot(session, args):
     snapshotUuid              = args['snapshotUuid']
     prevBackupUuid            = args['prevBackupUuid']
     backupUuid                = args['backupUuid']
-    isISCSI                   = getIsTrueString(args['isISCSI']) 
+    isISCSI                   = getIsTrueString(args['isISCSI'])
+    secHostId                 = args['secHostId']
 
     primarySRPath = getPrimarySRPath(session, primaryStorageSRUuid, isISCSI)
     util.SMlog("primarySRPath: " + primarySRPath)
@@ -504,10 +515,10 @@ def backupSnapshot(session, args):
     # Mount secondary storage mount path on XenServer along the path
     # /var/run/sr-mount/<dcId>/snapshots/ and create <accountId>/<volumeId> dir
     # on it.
-    backupsDir = mountSnapshotsDir(secondaryStorageMountPath, "snapshots", dcId, accountId, volumeId)
+    backupsDir = mountSnapshotsDir(secondaryStorageMountPath, "snapshots", dcId, accountId, volumeId, secHostId)
     util.SMlog("Backups dir " + backupsDir)
 
-    # Check existence of snapshot on primary storage 
+    # Check existence of snapshot on primary storage
     isfile(baseCopyPath, isISCSI)
     if prevBackupUuid:
         # Check existence of prevBackupFile