From 8d1a670026ac052cb4121a69e17ca907909c26f7 Mon Sep 17 00:00:00 2001
From: Alena Prokharchyk <alena.prokharchyk@citrix.com>
Date: Tue, 23 Jul 2013 14:38:40 -0700
Subject: [PATCH] CLOUDSTACK-3478: fixed volume destroy. #1 - don't call
 destroy() when the volume is in Expunged/Expunging/Destroy state. #2 - added
 state transition for Expunged state

---
 api/src/com/cloud/storage/Volume.java         |  3 +++
 .../ObjectInDataStoreStateMachine.java        | 26 ++++++++++++++-----
 .../com/cloud/storage/VolumeManagerImpl.java  | 10 ++++---
 3 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/api/src/com/cloud/storage/Volume.java b/api/src/com/cloud/storage/Volume.java
index 342dfd3ab2e..9319da9d29b 100755
--- a/api/src/com/cloud/storage/Volume.java
+++ b/api/src/com/cloud/storage/Volume.java
@@ -93,6 +93,9 @@ public interface Volume extends ControlledEntity, Identity, InternalIdentity, Ba
             s_fsm.addTransition(UploadOp, Event.OperationSucceeded, Uploaded);
             s_fsm.addTransition(UploadOp, Event.OperationFailed, Allocated);
             s_fsm.addTransition(Uploaded, Event.DestroyRequested, Destroy);
+            s_fsm.addTransition(Expunged, Event.ExpungingRequested, Expunged);
+            s_fsm.addTransition(Expunged, Event.OperationSucceeded, Expunged);
+            s_fsm.addTransition(Expunged, Event.OperationFailed, Expunged);
         }
     }
 
diff --git a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/ObjectInDataStoreStateMachine.java b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/ObjectInDataStoreStateMachine.java
index 04522f628d5..b21616a5094 100644
--- a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/ObjectInDataStoreStateMachine.java
+++ b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/ObjectInDataStoreStateMachine.java
@@ -22,11 +22,16 @@ import com.cloud.utils.fsm.StateObject;
 
 public interface ObjectInDataStoreStateMachine extends StateObject<ObjectInDataStoreStateMachine.State> {
     enum State {
-        Allocated("The initial state"), Creating2("This is only used with createOnlyRequested event"), Creating(
-                "The object is being creating on data store"), Created("The object is created"), Ready(
-                "Template downloading is accomplished"), Copying("The object is being coping"), Migrating(
-                "The object is being migrated"), Destroying("Template is destroying"), Destroyed(
-                "Template is destroyed"), Failed("Failed to download template");
+        Allocated("The initial state"),
+        Creating2("This is only used with createOnlyRequested event"),
+        Creating("The object is being creating on data store"),
+        Created("The object is created"),
+        Ready("Template downloading is accomplished"),
+        Copying("The object is being coping"),
+        Migrating("The object is being migrated"),
+        Destroying("Template is destroying"),
+        Destroyed("Template is destroyed"),
+        Failed("Failed to download template");
         String _description;
 
         private State(String description) {
@@ -39,7 +44,14 @@ public interface ObjectInDataStoreStateMachine extends StateObject<ObjectInDataS
     }
 
     enum Event {
-        CreateRequested, CreateOnlyRequested, DestroyRequested, OperationSuccessed, OperationFailed, CopyingRequested, MigrationRequested, ResizeRequested, ExpungeRequested
-
+        CreateRequested,
+        CreateOnlyRequested,
+        DestroyRequested,
+        OperationSuccessed,
+        OperationFailed,
+        CopyingRequested,
+        MigrationRequested,
+        ResizeRequested,
+        ExpungeRequested
     }
 }
diff --git a/server/src/com/cloud/storage/VolumeManagerImpl.java b/server/src/com/cloud/storage/VolumeManagerImpl.java
index 4074229b623..1c133cf72dd 100644
--- a/server/src/com/cloud/storage/VolumeManagerImpl.java
+++ b/server/src/com/cloud/storage/VolumeManagerImpl.java
@@ -2088,10 +2088,14 @@ public class VolumeManagerImpl extends ManagerBase implements VolumeManager {
         txn.start();
         for (VolumeVO vol : volumesForVm) {
             if (vol.getVolumeType().equals(Type.ROOT)) {
-                // This check is for VM in Error state (volume is already
-                // destroyed)
-                if (!vol.getState().equals(Volume.State.Destroy)) {
+                // Destroy volume if not already destroyed
+                boolean volumeAlreadyDestroyed = (vol.getState() == Volume.State.Destroy || 
+                        vol.getState() == Volume.State.Expunged ||
+                        vol.getState() == Volume.State.Expunging);
+                if (!volumeAlreadyDestroyed) {
                     volService.destroyVolume(vol.getId());
+                } else {
+                    s_logger.debug("Skipping destroy for the volume " + vol + " as its in state " + vol.getState().toString());
                 }
                 toBeExpunged.add(vol);
             } else {