[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #5349: Fix of creating volumes from snapshots without backup to secondary storage

[GitBox <gi...@apache.org>]

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



##########
File path: engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java
##########
@@ -497,6 +492,25 @@ public VolumeInfo createVolumeFromSnapshot(Volume volume, Snapshot snapshot, Use
 
     }
 
+    private SnapshotInfo backupSnapshotIfNeeded(Snapshot snapshot, DataStoreRole dataStoreRole, SnapshotInfo snapInfo) {
+        boolean backupSnapToSecondary = SnapshotManager.BackupSnapshotAfterTakingSnapshot.value() == null || SnapshotManager.BackupSnapshotAfterTakingSnapshot.value();
+
+        StoragePoolVO srcPool = _storagePoolDao.findById(snapInfo.getBaseVolume().getPoolId());
+        // We need to copy the snapshot onto secondary.
+        //Skipping the backup to secondary storage with NFS/FS could be supported when CLOUDSTACK-5297 is accepted with small enhancement in:
+        //KVMStorageProcessor::createVolumeFromSnapshot and CloudStackPrimaryDataStoreDriverImpl::copyAsync/createAsync
+        if ((!backupSnapToSecondary && (StoragePoolType.NetworkFilesystem.equals(srcPool.getPoolType()) || StoragePoolType.Filesystem.equals(srcPool.getPoolType())))) {

Review comment:
       I think that there is an extra parenthesis in the IF.
   From:
   `if ((!backupSnapToSecondary && (StoragePoolType.NetworkFilesystem.equals(srcPool.getPoolType()) || StoragePoolType.Filesystem.equals(srcPool.getPoolType())))) {`
   
   To:
   `if (!backupSnapToSecondary && (StoragePoolType.NetworkFilesystem.equals(srcPool.getPoolType()) || StoragePoolType.Filesystem.equals(srcPool.getPoolType()))) {`
   
   Maybe this conditional could be extracted into a method just have a cleaner code.




-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org