From 66ea5ed4fbb0ed63f181a053167271a1afefe7f7 Mon Sep 17 00:00:00 2001
From: Mice Xia <mice@apache.org>
Date: Wed, 15 Aug 2012 11:06:25 +0800
Subject: [PATCH] CS-15823 Failed snapshot should be marked as Error and
 cleanup asynchronously

---
 server/src/com/cloud/storage/StorageManagerImpl.java  | 11 +++++++++++
 server/src/com/cloud/storage/dao/SnapshotDao.java     |  1 +
 server/src/com/cloud/storage/dao/SnapshotDaoImpl.java |  7 +++++++
 .../cloud/storage/snapshot/SnapshotManagerImpl.java   |  9 ++-------
 4 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/server/src/com/cloud/storage/StorageManagerImpl.java b/server/src/com/cloud/storage/StorageManagerImpl.java
index 20dde8bd0fe..3fe4605ea37 100755
--- a/server/src/com/cloud/storage/StorageManagerImpl.java
+++ b/server/src/com/cloud/storage/StorageManagerImpl.java
@@ -2260,6 +2260,17 @@ public class StorageManagerImpl implements StorageManager, Manager, ClusterManag
                             s_logger.warn("Unable to destroy " + vol.getId(), e);
                         }
                     }
+                    
+                    // remove snapshots in Error state
+                    List<SnapshotVO> snapshots = _snapshotDao.listAllByStatus(Snapshot.Status.Error);
+                    for (SnapshotVO snapshotVO : snapshots) {
+                        try{
+                            _snapshotDao.expunge(snapshotVO.getId());
+                        }catch (Exception e) {
+                            s_logger.warn("Unable to destroy " + snapshotVO.getId(), e);
+                        }
+                    }
+                    
                 } finally {
                     scanLock.unlock();
                 }
diff --git a/server/src/com/cloud/storage/dao/SnapshotDao.java b/server/src/com/cloud/storage/dao/SnapshotDao.java
index 4bf54ded52d..b32d2782b6a 100644
--- a/server/src/com/cloud/storage/dao/SnapshotDao.java
+++ b/server/src/com/cloud/storage/dao/SnapshotDao.java
@@ -41,4 +41,5 @@ public interface SnapshotDao extends GenericDao<SnapshotVO, Long> {
     public Long countSnapshotsForAccount(long accountId);
 	List<SnapshotVO> listByInstanceId(long instanceId, Snapshot.Status... status);
 	List<SnapshotVO> listByStatus(long volumeId, Snapshot.Status... status);
+    List<SnapshotVO> listAllByStatus(Snapshot.Status... status);
 }
diff --git a/server/src/com/cloud/storage/dao/SnapshotDaoImpl.java b/server/src/com/cloud/storage/dao/SnapshotDaoImpl.java
index ac4bb6fc46d..65e2f5f1d76 100644
--- a/server/src/com/cloud/storage/dao/SnapshotDaoImpl.java
+++ b/server/src/com/cloud/storage/dao/SnapshotDaoImpl.java
@@ -307,4 +307,11 @@ public class SnapshotDaoImpl extends GenericDaoBase<SnapshotVO, Long> implements
         txn.commit();
         return result;
     }
+    
+    @Override
+    public List<SnapshotVO> listAllByStatus(Snapshot.Status... status) {
+        SearchCriteria<SnapshotVO> sc = this.StatusSearch.create();
+        sc.setParameters("status", (Object[])status);
+        return listBy(sc, null);
+    }
 }
diff --git a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
index 6e3f9c14116..2f0c64068e5 100755
--- a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
+++ b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
@@ -372,9 +372,6 @@ public class SnapshotManagerImpl implements SnapshotManager, SnapshotService, Ma
             if (answer != null) {
                 s_logger.error(answer.getDetails());
             }
-
-            // delete from the snapshots table
-            _snapshotDao.expunge(snapshotId);
             throw new CloudRuntimeException("Creating snapshot for volume " + volumeId + " on primary storage failed.");
         }
 
@@ -424,7 +421,6 @@ public class SnapshotManagerImpl implements SnapshotManager, SnapshotService, Ma
                 if (hosts != null && !hosts.isEmpty()) {
                     HostVO host = hosts.get(0);
                     if (!hostSupportSnapsthot(host)) {
-                        _snapshotDao.expunge(snapshotId);
                         throw new CloudRuntimeException("KVM Snapshot is not supported on cluster: " + host.getId());
                     }
                 }
@@ -435,14 +431,13 @@ public class SnapshotManagerImpl implements SnapshotManager, SnapshotService, Ma
                 UserVmVO userVm = _vmDao.findById(volume.getInstanceId());
                 if (userVm != null) {
                     if (userVm.getState().equals(State.Destroyed) || userVm.getState().equals(State.Expunging)) {
-                        _snapshotDao.expunge(snapshotId);
                         throw new CloudRuntimeException("Creating snapshot failed due to volume:" + volumeId + " is associated with vm:" + userVm.getInstanceName() + " is in "
                                 + userVm.getState().toString() + " state");
                     }
                     
                     if(userVm.getHypervisorType() == HypervisorType.VMware || userVm.getHypervisorType() == HypervisorType.KVM) {
-                    	List<SnapshotVO> activeSnapshots = _snapshotDao.listByInstanceId(volume.getInstanceId(), Snapshot.Status.Creating,  Snapshot.Status.CreatedOnPrimary,  Snapshot.Status.BackingUp);
-                    	if(activeSnapshots.size() > 1)
+                        List<SnapshotVO> activeSnapshots = _snapshotDao.listByInstanceId(volume.getInstanceId(), Snapshot.Status.Creating,  Snapshot.Status.CreatedOnPrimary,  Snapshot.Status.BackingUp);
+                        if(activeSnapshots.size() > 1)
                             throw new CloudRuntimeException("There is other active snapshot tasks on the instance to which the volume is attached, please try again later");
                     }
                 }