diff --git a/engine/schema/src/com/cloud/storage/dao/SnapshotDao.java b/engine/schema/src/com/cloud/storage/dao/SnapshotDao.java old mode 100644 new mode 100755 index ff2e4457441..fe635586370 --- a/engine/schema/src/com/cloud/storage/dao/SnapshotDao.java +++ b/engine/schema/src/com/cloud/storage/dao/SnapshotDao.java @@ -37,6 +37,8 @@ public interface SnapshotDao extends GenericDao, StateDao listByVolumeIdType(long volumeId, Type type); + List listByVolumeIdTypeNotDestroyed(long volumeId, Type type); + List listByVolumeIdIncludingRemoved(long volumeId); List listByBackupUuid(long volumeId, String backupUuid); diff --git a/engine/schema/src/com/cloud/storage/dao/SnapshotDaoImpl.java b/engine/schema/src/com/cloud/storage/dao/SnapshotDaoImpl.java old mode 100644 new mode 100755 index 9483f1ca82c..a6941cf5165 --- a/engine/schema/src/com/cloud/storage/dao/SnapshotDaoImpl.java +++ b/engine/schema/src/com/cloud/storage/dao/SnapshotDaoImpl.java @@ -62,6 +62,7 @@ public class SnapshotDaoImpl extends GenericDaoBase implements private SearchBuilder VolumeIdSearch; private SearchBuilder VolumeIdTypeSearch; + private SearchBuilder VolumeIdTypeNotDestroyedSearch; private SearchBuilder ParentIdSearch; private SearchBuilder backupUuidSearch; private SearchBuilder VolumeIdVersionSearch; @@ -95,6 +96,15 @@ public class SnapshotDaoImpl extends GenericDaoBase implements return listByVolumeIdType(null, volumeId, type); } + @Override + public List listByVolumeIdTypeNotDestroyed(long volumeId, Type type) { + SearchCriteria sc = VolumeIdTypeNotDestroyedSearch.create(); + sc.setParameters("volumeId", volumeId); + sc.setParameters("type", type.ordinal()); + sc.setParameters("status", State.Destroyed); + return listBy(sc, null); + } + @Override public List listByVolumeIdVersion(long volumeId, String version) { return listByVolumeIdVersion(null, volumeId, version); @@ -147,6 +157,12 @@ public class SnapshotDaoImpl extends GenericDaoBase implements VolumeIdTypeSearch.and("type", VolumeIdTypeSearch.entity().getsnapshotType(), SearchCriteria.Op.EQ); VolumeIdTypeSearch.done(); + VolumeIdTypeNotDestroyedSearch = createSearchBuilder(); + VolumeIdTypeNotDestroyedSearch.and("volumeId", VolumeIdTypeNotDestroyedSearch.entity().getVolumeId(), SearchCriteria.Op.EQ); + VolumeIdTypeNotDestroyedSearch.and("type", VolumeIdTypeNotDestroyedSearch.entity().getsnapshotType(), SearchCriteria.Op.EQ); + VolumeIdTypeNotDestroyedSearch.and("status", VolumeIdTypeNotDestroyedSearch.entity().getState(), SearchCriteria.Op.NEQ); + VolumeIdTypeNotDestroyedSearch.done(); + VolumeIdVersionSearch = createSearchBuilder(); VolumeIdVersionSearch.and("volumeId", VolumeIdVersionSearch.entity().getVolumeId(), SearchCriteria.Op.EQ); VolumeIdVersionSearch.and("version", VolumeIdVersionSearch.entity().getVersion(), SearchCriteria.Op.EQ); diff --git a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java old mode 100644 new mode 100755 index 2c7b4a0bf49..92937ba35c6 --- a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -481,7 +481,7 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement Type type = spstVO.getRecurringType(); int maxSnaps = type.getMax(); - List snaps = listSnapsforVolumeType(volumeId, type); + List snaps = listSnapsforVolumeTypeNotDestroyed(volumeId, type); SnapshotPolicyVO policy = _snapshotPolicyDao.findById(policyId); if (policy != null && policy.getMaxSnaps() < maxSnaps) { maxSnaps = policy.getMaxSnaps(); @@ -514,6 +514,10 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement throw new InvalidParameterValueException("unable to find a snapshot with id " + snapshotId); } + if (snapshotCheck.getState() == Snapshot.State.Destroyed) { + throw new InvalidParameterValueException("Snapshot with id: " + snapshotId + " is already destroyed"); + } + _accountMgr.checkAccess(caller, null, true, snapshotCheck); SnapshotStrategy snapshotStrategy = _storageStrategyFactory.getSnapshotStrategy(snapshotCheck, SnapshotOperation.DELETE); @@ -898,8 +902,8 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement return _snapshotDao.listByVolumeId(volumeId); } - private List listSnapsforVolumeType(long volumeId, Type type) { - return _snapshotDao.listByVolumeIdType(volumeId, type); + private List listSnapsforVolumeTypeNotDestroyed(long volumeId, Type type) { + return _snapshotDao.listByVolumeIdTypeNotDestroyed(volumeId, type); } @Override diff --git a/server/test/com/cloud/storage/snapshot/SnapshotManagerTest.java b/server/test/com/cloud/storage/snapshot/SnapshotManagerTest.java old mode 100644 new mode 100755 index 58ee2e72d72..685f4954a2b --- a/server/test/com/cloud/storage/snapshot/SnapshotManagerTest.java +++ b/server/test/com/cloud/storage/snapshot/SnapshotManagerTest.java @@ -248,14 +248,13 @@ public class SnapshotManagerTest { _snapshotMgr.allocSnapshot(TEST_VOLUME_ID, Snapshot.MANUAL_POLICY_ID, null, null); } - @Test + @Test(expected = InvalidParameterValueException.class) public void testDeleteSnapshotF1() { when(snapshotStrategy.deleteSnapshot(TEST_SNAPSHOT_ID)).thenReturn(true); when(snapshotMock.getState()).thenReturn(Snapshot.State.Destroyed); when(snapshotMock.getAccountId()).thenReturn(2L); when(snapshotMock.getDataCenterId()).thenReturn(2L); - boolean result =_snapshotMgr.deleteSnapshot(TEST_SNAPSHOT_ID); - Assert.assertTrue(result); + _snapshotMgr.deleteSnapshot(TEST_SNAPSHOT_ID); } // vm state not stopped