From a64ad9d9b7a75fee5a7be218251c5a63f08a9cf1 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Mon, 12 Apr 2021 08:09:37 +0530 Subject: [PATCH] server: Prevent vm snapshots being indefinitely stuck in Expunging state on deletion failure (#4898) Fixes #4201 This PR addresses the issue of a vm snapshot being indefinitely stuck is Expunging state in case deletion fails. Co-authored-by: Pearl Dsilva --- .../java/com/cloud/vm/snapshot/VMSnapshot.java | 1 + .../vmsnapshot/DefaultVMSnapshotStrategy.java | 15 +++++++++------ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/api/src/main/java/com/cloud/vm/snapshot/VMSnapshot.java b/api/src/main/java/com/cloud/vm/snapshot/VMSnapshot.java index c398e583fae..3897df2d5e6 100644 --- a/api/src/main/java/com/cloud/vm/snapshot/VMSnapshot.java +++ b/api/src/main/java/com/cloud/vm/snapshot/VMSnapshot.java @@ -59,6 +59,7 @@ public interface VMSnapshot extends ControlledEntity, Identity, InternalIdentity s_fsm.addTransition(Error, Event.ExpungeRequested, Expunging); s_fsm.addTransition(Expunging, Event.ExpungeRequested, Expunging); s_fsm.addTransition(Expunging, Event.OperationSucceeded, Removed); + s_fsm.addTransition(Expunging, Event.OperationFailed, Error); } } diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java index 84236249cd2..d60e623661d 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java @@ -230,11 +230,10 @@ public class DefaultVMSnapshotStrategy extends ManagerBase implements VMSnapshot } else { String errMsg = (answer == null) ? null : answer.getDetails(); s_logger.error("Delete vm snapshot " + vmSnapshot.getName() + " of vm " + userVm.getInstanceName() + " failed due to " + errMsg); + processAnswer(vmSnapshotVO, userVm, answer, hostId); throw new CloudRuntimeException("Delete vm snapshot " + vmSnapshot.getName() + " of vm " + userVm.getInstanceName() + " failed due to " + errMsg); } - } catch (OperationTimedoutException e) { - throw new CloudRuntimeException("Delete vm snapshot " + vmSnapshot.getName() + " of vm " + userVm.getInstanceName() + " failed due to " + e.getMessage()); - } catch (AgentUnavailableException e) { + } catch (OperationTimedoutException | AgentUnavailableException e) { throw new CloudRuntimeException("Delete vm snapshot " + vmSnapshot.getName() + " of vm " + userVm.getInstanceName() + " failed due to " + e.getMessage()); } } @@ -254,9 +253,13 @@ public class DefaultVMSnapshotStrategy extends ManagerBase implements VMSnapshot finalizeRevert(vmSnapshot, answer.getVolumeTOs()); vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationSucceeded); } else if (as instanceof DeleteVMSnapshotAnswer) { - DeleteVMSnapshotAnswer answer = (DeleteVMSnapshotAnswer)as; - finalizeDelete(vmSnapshot, answer.getVolumeTOs()); - vmSnapshotDao.remove(vmSnapshot.getId()); + if (as.getResult()) { + DeleteVMSnapshotAnswer answer = (DeleteVMSnapshotAnswer) as; + finalizeDelete(vmSnapshot, answer.getVolumeTOs()); + vmSnapshotDao.remove(vmSnapshot.getId()); + } else { + vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationFailed); + } } } });