Invoke grantAccess and revokeAccess one time each using a SnapshotInfo (where previously a VolumeInfo was being passed in)

Handle the case where a SnapshotInfo is passed into the grantAccess and revokeAccess methods

api/src/com/cloud/agent/api/to/DiskTO.java

@@ -35,7 +35,6 @@ public class DiskTO {
     public static final String MOUNT_POINT = "mountpoint";
     public static final String PROTOCOL_TYPE = "protocoltype";
     public static final String PATH = "path";
-    public static final String VOLUME_ID = "volumeId";
 
     private DataTO data;
     private Long diskSeq;

engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java

@@ -224,16 +224,7 @@ public class StorageSystemSnapshotStrategy extends SnapshotStrategyBase {
                         _volService.grantAccess(volumeInfo, hostVO, dataStore);
                     }
 
-                    VolumeVO volume = _volumeDao.findById(volumeInfo.getId());
+                    _volService.grantAccess(snapshotInfo, hostVO, dataStore);
-
-                    // the Folder field is used by grantAccess(VolumeInfo, Host, DataStore) when that method
-                    // connects the host(s) to the volume
-                    // this Folder change is NOT to be written to the DB; it is only temporarily used here so that
-                    // the connect method can be passed in the expected data and do its work (on the volume that backs
-                    // the snapshot)
-                    volume.setFolder(destDetails.get(DiskTO.VOLUME_ID));
-
-                    _volService.grantAccess(volumeInfo, hostVO, dataStore);
 
                     snapshotAndCopyAnswer = (SnapshotAndCopyAnswer)_agentMgr.send(hostVO.getId(), snapshotAndCopyCommand);
                 }
@@ -242,16 +233,7 @@ public class StorageSystemSnapshotStrategy extends SnapshotStrategyBase {
                 }
                 finally {
                     try {
-                        VolumeVO volume = _volumeDao.findById(volumeInfo.getId());
+                        _volService.revokeAccess(snapshotInfo, hostVO, dataStore);
-
-                        // the Folder field is used by revokeAccess(VolumeInfo, Host, DataStore) when that method
-                        // disconnects the host(s) from the volume
-                        // this Folder change is NOT to be written to the DB; it is only temporarily used here so that
-                        // the disconnect method can be passed in the expected data and do its work (on the volume that backs
-                        // the snapshot)
-                        volume.setFolder(destDetails.get(DiskTO.VOLUME_ID));
-
-                        _volService.revokeAccess(volumeInfo, hostVO, dataStore);
 
                         // if sourceDetails != null, we need to disconnect the host(s) from the volume
                         if (sourceDetails != null) {
@@ -324,7 +306,6 @@ public class StorageSystemSnapshotStrategy extends SnapshotStrategyBase {
 
         long snapshotId = snapshotInfo.getId();
 
-        destDetails.put(DiskTO.VOLUME_ID, getProperty(snapshotId, DiskTO.VOLUME_ID));
         destDetails.put(DiskTO.IQN, getProperty(snapshotId, DiskTO.IQN));
 
         destDetails.put(DiskTO.CHAP_INITIATOR_USERNAME, getProperty(snapshotId, DiskTO.CHAP_INITIATOR_USERNAME));

plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidFirePrimaryDataStoreDriver.java

@@ -128,13 +128,11 @@ public class SolidFirePrimaryDataStoreDriver implements PrimaryDataStoreDriver {
     @Override
     public synchronized boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore)
     {
-        VolumeInfo volumeInfo = (VolumeInfo)dataObject;
+        if (dataObject == null || host == null || dataStore == null) {
-
-        if (volumeInfo == null || host == null || dataStore == null) {
             return false;
         }
 
-        long sfVolumeId = Long.parseLong(volumeInfo.getFolder());
+        long sfVolumeId = getSolidFireVolumeId(dataObject);
         long clusterId = host.getClusterId();
         long storagePoolId = dataStore.getId();
 
@@ -173,13 +171,11 @@ public class SolidFirePrimaryDataStoreDriver implements PrimaryDataStoreDriver {
     @Override
     public synchronized void revokeAccess(DataObject dataObject, Host host, DataStore dataStore)
     {
-        VolumeInfo volumeInfo = (VolumeInfo)dataObject;
+        if (dataObject == null || host == null || dataStore == null) {
-
-        if (volumeInfo == null || host == null || dataStore == null) {
             return;
         }
 
-        long sfVolumeId = Long.parseLong(volumeInfo.getFolder());
+        long sfVolumeId = getSolidFireVolumeId(dataObject);
         long clusterId = host.getClusterId();
         long storagePoolId = dataStore.getId();
 
@@ -201,6 +197,24 @@ public class SolidFirePrimaryDataStoreDriver implements PrimaryDataStoreDriver {
         }
     }
 
+    private long getSolidFireVolumeId(DataObject dataObject) {
+        if (dataObject.getType() == DataObjectType.VOLUME) {
+            return Long.parseLong(((VolumeInfo)dataObject).getFolder());
+        }
+
+        if (dataObject.getType() == DataObjectType.SNAPSHOT) {
+            SnapshotDetailsVO snapshotDetails = _snapshotDetailsDao.findDetail(dataObject.getId(), SolidFireUtil.VOLUME_ID);
+
+            if (snapshotDetails == null || snapshotDetails.getValue() == null) {
+                throw new CloudRuntimeException("Unable to locate the volume ID associated with the following snapshot ID: " + dataObject.getId());
+            }
+
+            return Long.parseLong(snapshotDetails.getValue());
+        }
+
+        throw new CloudRuntimeException("Invalid DataObjectType (" + dataObject.getType() + ") passed to getSolidFireVolumeId(DataObject)");
+    }
+
     private long getDefaultMinIops(long storagePoolId) {
         StoragePoolDetailVO storagePoolDetail = _storagePoolDetailsDao.findDetail(storagePoolId, SolidFireUtil.CLUSTER_DEFAULT_MIN_IOPS);
 
@@ -559,7 +573,7 @@ public class SolidFirePrimaryDataStoreDriver implements PrimaryDataStoreDriver {
 
     private void updateSnapshotDetails(long csSnapshotId, long sfNewVolumeId, long storagePoolId, long sfNewVolumeSize, String sfNewVolumeIqn) {
         SnapshotDetailsVO snapshotDetail = new SnapshotDetailsVO(csSnapshotId,
-                DiskTO.VOLUME_ID,
+                SolidFireUtil.VOLUME_ID,
                 String.valueOf(sfNewVolumeId),
                 false);
 
@@ -596,7 +610,7 @@ public class SolidFirePrimaryDataStoreDriver implements PrimaryDataStoreDriver {
         try {
             SolidFireUtil.SolidFireConnection sfConnection = SolidFireUtil.getSolidFireConnection(storagePoolId, _storagePoolDetailsDao);
 
-            SnapshotDetailsVO snapshotDetails = _snapshotDetailsDao.findDetail(snapshotId, DiskTO.VOLUME_ID);
+            SnapshotDetailsVO snapshotDetails = _snapshotDetailsDao.findDetail(snapshotId, SolidFireUtil.VOLUME_ID);
 
             long volumeId = Long.parseLong(snapshotDetails.getValue());