nvazquez commented on code in PR #6661:
URL: https://github.com/apache/cloudstack/pull/6661#discussion_r1008000568


##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java:
##########
@@ -627,33 +627,26 @@ public void copyAsync(DataObject srcData, DataObject 
dstData, AsyncCompletionCal
                 final String name = vinfo.getUuid();
                 SpConnectionDesc conn = 
StorPoolUtil.getSpConnection(vinfo.getDataStore().getUuid(), 
vinfo.getDataStore().getId(), storagePoolDetailsDao, primaryStoreDao);
 
-                Long snapshotSize = StorPoolUtil.snapshotSize(parentName, 
conn);
-                if (snapshotSize == null) {
-                    err = String.format("Snapshot=%s does not exist on 
StorPool. Will recreate it first on primary", parentName);
-                    vmTemplatePoolDao.remove(templStoragePoolVO.getId());
+                Long snapshotSize = templStoragePoolVO.getTemplateSize();
+                long size = vinfo.getSize();
+                if (size < snapshotSize) {

Review Comment:
   I think keeping the null check for snapshotSize can be useful



##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java:
##########
@@ -792,6 +775,71 @@ public void copyAsync(DataObject srcData, DataObject 
dstData, AsyncCompletionCal
         callback.complete(res);
     }
 
+    private Answer migrateVolumeToStorPool(DataObject srcData, DataObject 
dstData, VolumeInfo srcInfo,
+            VolumeObjectTO srcTO, final String volumeName) {
+        Answer answer;
+        SpConnectionDesc conn = 
StorPoolUtil.getSpConnection(dstData.getDataStore().getUuid(), 
dstData.getDataStore().getId(), storagePoolDetailsDao, primaryStoreDao);
+        String baseOn = 
StorPoolStorageAdaptor.getVolumeNameFromPath(srcTO.getPath(), true);
+
+        String vmUuid = null;
+        String vcPolicyTag = null;
+
+        VMInstanceVO vm = null;
+        if (srcInfo.getInstanceId() != null) {
+            vm = vmInstanceDao.findById(srcInfo.getInstanceId());
+        }
+
+        if (vm != null) {

Review Comment:
   Maybe the logic to get the VMinstanceVO can be moved to a method?



##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java:
##########
@@ -627,33 +627,26 @@ public void copyAsync(DataObject srcData, DataObject 
dstData, AsyncCompletionCal
                 final String name = vinfo.getUuid();
                 SpConnectionDesc conn = 
StorPoolUtil.getSpConnection(vinfo.getDataStore().getUuid(), 
vinfo.getDataStore().getId(), storagePoolDetailsDao, primaryStoreDao);
 
-                Long snapshotSize = StorPoolUtil.snapshotSize(parentName, 
conn);
-                if (snapshotSize == null) {
-                    err = String.format("Snapshot=%s does not exist on 
StorPool. Will recreate it first on primary", parentName);
-                    vmTemplatePoolDao.remove(templStoragePoolVO.getId());
+                Long snapshotSize = templStoragePoolVO.getTemplateSize();
+                long size = vinfo.getSize();
+                if (size < snapshotSize) {
+                    StorPoolUtil.spLog(String.format("provided size is too 
small for snapshot. Provided %d, snapshot %d. Using snapshot size", size, 
snapshotSize));
+                    size = snapshotSize;
                 }
-                if (err == null) {
-                    long size = vinfo.getSize();
-                    if( size < snapshotSize )
-                    {
-                        StorPoolUtil.spLog(String.format("provided size is too 
small for snapshot. Provided %d, snapshot %d. Using snapshot size", size, 
snapshotSize));
-                        size = snapshotSize;
-                    }
-                    StorPoolUtil.spLog(String.format("volume size is: %d", 
size));
-                    Long vmId = vinfo.getInstanceId();
-                    SpApiResponse resp = StorPoolUtil.volumeCreate(name, 
parentName, size, getVMInstanceUUID(vmId),
-                            getVcPolicyTag(vmId), "volume", 
vinfo.getMaxIops(), conn);
-                    if (resp.getError() == null) {
-                        updateStoragePool(dstData.getDataStore().getId(), 
vinfo.getSize());
-
-                        VolumeObjectTO to = (VolumeObjectTO) vinfo.getTO();
-                        to.setSize(vinfo.getSize());
-                        
to.setPath(StorPoolUtil.devPath(StorPoolUtil.getNameFromResponse(resp, false)));
-
-                        answer = new CopyCmdAnswer(to);
-                    } else {
-                        err = String.format("Could not create Storpool volume 
%s. Error: %s", name, resp.getError());
-                    }
+                StorPoolUtil.spLog(String.format("volume size is: %d", size));
+                Long vmId = vinfo.getInstanceId();
+                SpApiResponse resp = StorPoolUtil.volumeCreate(name, 
parentName, size, getVMInstanceUUID(vmId),
+                        getVcPolicyTag(vmId), "volume", vinfo.getMaxIops(), 
conn);
+                if (resp.getError() == null) {
+                    updateStoragePool(dstData.getDataStore().getId(), 
vinfo.getSize());
+
+                    VolumeObjectTO to = (VolumeObjectTO) vinfo.getTO();
+                    to.setSize(vinfo.getSize());
+                    
to.setPath(StorPoolUtil.devPath(StorPoolUtil.getNameFromResponse(resp, false)));
+
+                    answer = new CopyCmdAnswer(to);
+                } else {
+                    err = String.format("Could not create Storpool volume %s. 
Error: %s", name, resp.getError());

Review Comment:
   Could be useful to also log the resp.getError()?



-- 
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