[GitHub] koushik-das commented on a change in pull request #2081: CLOUDSTACK-9894 Separate creation and backup operations for a volume snapshot

[gi...@git.apache.org]

koushik-das commented on a change in pull request #2081: CLOUDSTACK-9894 Separate creation and backup operations for a volume snapshot
URL: https://github.com/apache/cloudstack/pull/2081#discussion_r123682731
 
 

 ##########
 File path: engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java
 ##########
 @@ -380,57 +390,44 @@ public SnapshotInfo takeSnapshot(SnapshotInfo snapshot) {
                     volumeInfo.stateTransit(Volume.Event.OperationFailed);
                 }
             }
-
-            snapshot = result.getSnapshot();
-            DataStore primaryStore = snapshot.getDataStore();
-            boolean backupFlag = Boolean.parseBoolean(configDao.getValue(Config.BackupSnapshotAfterTakingSnapshot.toString()));
-
-            SnapshotInfo backupedSnapshot;
-            if(backupFlag) {
-                backupedSnapshot = backupSnapshot(snapshot);
-            } else {
-                // Fake it to get the transitions to fire in the proper order
-                s_logger.debug("skipping backup of snapshot due to configuration "+Config.BackupSnapshotAfterTakingSnapshot.toString());
-
-                SnapshotObject snapObj = (SnapshotObject)snapshot;
-                try {
-                    snapObj.processEvent(Snapshot.Event.OperationNotPerformed);
-                } catch (NoTransitionException e) {
-                    s_logger.debug("Failed to change state: " + snapshot.getId() + ": " + e.toString());
-                    throw new CloudRuntimeException(e.toString());
-                }
-                backupedSnapshot = snapshot;
+            snapshotOnPrimary = result.getSnapshot();
+            snapshotOnPrimary.addPayload(snapshot.getPayload());
+            _snapshotDetailsDao.addDetail(((SnapshotObject)snapshotOnPrimary).getId(), MSID, Long.toString(MacAddress.getMacAddress().toLong()), false);
+            return snapshotOnPrimary;
+        } finally {
+            if (snapshotVO != null) {
+                snapshotDao.releaseFromLockTable(snapshotOnPrimary.getId());
             }
+        }
+    }
 
-            try {
-                SnapshotInfo parent = snapshot.getParent();
-                if (backupedSnapshot != null && parent != null && primaryStore instanceof PrimaryDataStoreImpl) {
-                    if (((PrimaryDataStoreImpl)primaryStore).getPoolType() != StoragePoolType.RBD) {
-                        Long parentSnapshotId = parent.getId();
-                        while (parentSnapshotId != null && parentSnapshotId != 0L) {
-                            SnapshotDataStoreVO snapshotDataStoreVO = snapshotStoreDao.findByStoreSnapshot(primaryStore.getRole(), primaryStore.getId(), parentSnapshotId);
-                            if (snapshotDataStoreVO != null) {
-                                parentSnapshotId = snapshotDataStoreVO.getParentSnapshotId();
-                                snapshotStoreDao.remove(snapshotDataStoreVO.getId());
-                            } else {
-                                parentSnapshotId = null;
-                            }
+    @Override
+    public void cleanSnapshotAfterCreation(SnapshotInfo snapshotOnPrimary) {
+        _snapshotDetailsDao.removeDetail(((SnapshotObject)snapshotOnPrimary).getId(), MSID);
 
 Review comment:
   Again since there are multiple DB operations, better to do them in a tx
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services