slavkap commented on a change in pull request #5349:
URL: https://github.com/apache/cloudstack/pull/5349#discussion_r693795743



##########
File path: 
plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java
##########
@@ -287,20 +288,45 @@ public void copyAsync(DataObject srcdata, DataObject 
destData, AsyncCompletionCa
                 }
                 CopyCommandResult result = new CopyCommandResult("", answer);
                 callback.complete(result);
+            } else if (srcdata.getType() == DataObjectType.SNAPSHOT && 
destData.getType() == DataObjectType.VOLUME) {
+                SnapshotObjectTO srcTO = (SnapshotObjectTO) srcdata.getTO();
+                CopyCommand cmd = new CopyCommand(srcTO, destData.getTO(), 
StorageManager.PRIMARY_STORAGE_DOWNLOAD_WAIT.value(), true);
+                EndPoint ep = epSelector.select(srcdata, destData);
+                CopyCmdAnswer answer = null;
+                if (ep == null) {
+                    String errMsg = "No remote endpoint to send command, check 
if host or ssvm is down?";
+                    s_logger.error(errMsg);
+                    answer = new CopyCmdAnswer(errMsg);
+                } else {
+                    answer = (CopyCmdAnswer) ep.sendMessage(cmd);
+                }
+                CopyCommandResult result = new CopyCommandResult("", answer);
+                callback.complete(result);
             }
         }
     }
 
     @Override
     public boolean canCopy(DataObject srcData, DataObject destData) {
         //BUG fix for CLOUDSTACK-4618
-        DataStore store = destData.getDataStore();
-        if (store.getRole() == DataStoreRole.Primary && srcData.getType() == 
DataObjectType.TEMPLATE
+        DataStore destStore = destData.getDataStore();
+        if (destStore.getRole() == DataStoreRole.Primary && srcData.getType() 
== DataObjectType.TEMPLATE
                 && (destData.getType() == DataObjectType.TEMPLATE || 
destData.getType() == DataObjectType.VOLUME)) {
-            StoragePoolVO storagePoolVO = 
primaryStoreDao.findById(store.getId());
+            StoragePoolVO storagePoolVO = 
primaryStoreDao.findById(destStore.getId());
             if (storagePoolVO != null && storagePoolVO.getPoolType() == 
Storage.StoragePoolType.CLVM) {
                 return true;
             }
+        } else if (srcData.getType() == DataObjectType.SNAPSHOT && 
destData.getType() == DataObjectType.VOLUME) {
+            DataStore srcStore = srcData.getDataStore();
+            if (srcStore.getRole() == DataStoreRole.Primary && 
destStore.getRole() == DataStoreRole.Primary) {
+                StoragePoolVO srcStoragePoolVO = 
primaryStoreDao.findById(srcStore.getId());
+                StoragePoolVO dstStoragePoolVO = 
primaryStoreDao.findById(destStore.getId());
+                if (srcStoragePoolVO != null && srcStoragePoolVO.getPoolType() 
== StoragePoolType.RBD

Review comment:
       @DaanHoogland, thanks for the review! I will make the rest of the 
comments, but I can't entirely agree here. I mean that for me, it's better to 
compare enums with `==`. In your case, it's not safe to invoke `equals()` if I 
remove the check for a null value.
    Which is the preferred way in ACS to compare enums? 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to