GutoVeronezi commented on a change in pull request #5297:
URL: https://github.com/apache/cloudstack/pull/5297#discussion_r694225751



##########
File path: 
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java
##########
@@ -100,20 +111,23 @@ public Answer execute(final RevertSnapshotCommand 
command, final LibvirtComputin
                 rbd.close(image);
                 rados.ioCtxDestroy(io);
             } else {
-                NfsTO nfsImageStore = (NfsTO)snapshotImageStore;
-                String secondaryStoragePoolUrl = nfsImageStore.getUrl();
-                secondaryStoragePool = 
storagePoolMgr.getStoragePoolByURI(secondaryStoragePoolUrl);
-                String ssPmountPath = secondaryStoragePool.getLocalPath();
-                snapshotPath = ssPmountPath + File.separator + snapshotRelPath;
-
-                Script cmd = new 
Script(libvirtComputingResource.manageSnapshotPath(), 
libvirtComputingResource.getCmdsTimeout(), s_logger);
-                cmd.add("-v", snapshotPath);
-                cmd.add("-n", snapshotDisk.getName());
-                cmd.add("-p", snapshotDisk.getPath());
-                String result = cmd.execute();
-                if (result != null) {
-                    s_logger.debug("Failed to revert snaptshot: " + result);
-                    return new Answer(command, false, result);
+                KVMStoragePool secondaryStoragePool = null;
+                if (snapshotImageStore != null && DataStoreRole.Primary != 
snapshotImageStore.getRole()) {
+                    
storagePoolMgr.getStoragePoolByURI(snapshotImageStore.getUrl());
+                }
+
+                if (primaryPool.getType() == StoragePoolType.CLVM) {

Review comment:
       @nvazquez reverting snapshots that were only in primary storage was not 
working correctly for KVM. The `RBD` validation was added in PR #3502 to allow 
`Ceph` to revert snapshots that were only in primary storage, however the rest 
kept not working. This PR fixed this error to some others storage pool types 
(which were added to the validation), however `CLVM` was not our focus, 
therefore we tried to not touch in it.




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