From ae770af112c74291db5f3fbf842a3a5158733a0f Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Wed, 7 Apr 2021 15:54:07 +0530 Subject: [PATCH 1/2] VM Snapshot: Prevent snapshot indefinitely being stuck in Expunging state on deletion failure --- .../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 c398e583fae5..60338a429bec 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 String getDescription() { 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, Ready); } } 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 84236249cd20..d60e623661dc 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 boolean deleteVMSnapshot(VMSnapshot 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 void doInTransactionWithoutResult(TransactionStatus status) throws NoTran 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); + } } } }); From 06bf5f90db7dfd420159fa9fbcf6692ff4c32d17 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 8 Apr 2021 11:56:31 +0530 Subject: [PATCH 2/2] Change state transition on vm snap deletion failure --- api/src/main/java/com/cloud/vm/snapshot/VMSnapshot.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 60338a429bec..3897df2d5e67 100644 --- a/api/src/main/java/com/cloud/vm/snapshot/VMSnapshot.java +++ b/api/src/main/java/com/cloud/vm/snapshot/VMSnapshot.java @@ -59,7 +59,7 @@ public String getDescription() { 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, Ready); + s_fsm.addTransition(Expunging, Event.OperationFailed, Error); } }