[GitHub] [cloudstack] ggoodrich-ipp commented on a change in pull request #4032: Suspending the VM prior to deleting snapshots to avoid corruption, th…

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

ggoodrich-ipp commented on a change in pull request #4032: Suspending the VM prior to deleting snapshots to avoid corruption, th…
URL: https://github.com/apache/cloudstack/pull/4032#discussion_r409551472
 
 

 ##########
 File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteVMSnapshotCommandWrapper.java
 ##########
 @@ -93,12 +108,26 @@ public Answer execute(final DeleteVMSnapshotCommand cmd, final LibvirtComputingR
             } else if (snapshot == null) {
                 s_logger.debug("Can not find vm snapshot " + cmd.getTarget().getSnapshotName() + " on vm: " + vmName + ", return true");
                 return new DeleteVMSnapshotAnswer(cmd, cmd.getVolumeTOs());
+            } else if (didDelete) {
+                s_logger.error("Failed to resume vm after delete snapshot " + cmd.getTarget().getSnapshotName() + " on vm: " + vmName + " return true : " + e);
+                return new DeleteVMSnapshotAnswer(cmd, cmd.getVolumeTOs());
             }
 
             s_logger.warn(msg, e);
             return new DeleteVMSnapshotAnswer(cmd, false, msg);
         } finally {
             if (dm != null) {
+                // Make sure if the VM is paused, then resume it, in case we got an exception during our delete() and didn't have the chance before
+                try {
+                    dm = libvirtComputingResource.getDomain(conn, vmName);
 
 Review comment:
   I was under the impression that dm needed to be refreshed after a state change, as I saw this same thing being done in KVMStorageProcessor -> backupSnapshot(). Rather than validate that it was necessary or not, I decided that safer is always better, as refreshing the domain variable won't cause harm.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services