diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDao.java b/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDao.java index 737e4a5a53ca..2659f41d5620 100755 --- a/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDao.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDao.java @@ -53,6 +53,8 @@ public interface SnapshotDao extends GenericDao, StateDao listByStatusNotIn(long volumeId, Snapshot.State... status); + SnapshotVO findByVolumeIdAndNameNotInStatus(long volumeId, String name, Snapshot.State... status); + /** * Retrieves a list of snapshots filtered by ids. * @param ids Snapshot ids. diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDaoImpl.java index 238ae54e07f3..0f6576e996a8 100755 --- a/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDaoImpl.java @@ -65,6 +65,7 @@ public class SnapshotDaoImpl extends GenericDaoBase implements private SearchBuilder InstanceIdSearch; private SearchBuilder StatusSearch; private SearchBuilder notInStatusSearch; + private SearchBuilder volumeIdNameNotInStatusSearch; private GenericSearchBuilder CountSnapshotsByAccount; @Inject ResourceTagDao _tagsDao; @@ -159,6 +160,12 @@ protected void init() { notInStatusSearch.and("status", notInStatusSearch.entity().getState(), SearchCriteria.Op.NOTIN); notInStatusSearch.done(); + volumeIdNameNotInStatusSearch = createSearchBuilder(); + volumeIdNameNotInStatusSearch.and("volumeId", volumeIdNameNotInStatusSearch.entity().getVolumeId(), SearchCriteria.Op.EQ); + volumeIdNameNotInStatusSearch.and("name", volumeIdNameNotInStatusSearch.entity().getName(), SearchCriteria.Op.EQ); + volumeIdNameNotInStatusSearch.and("status", volumeIdNameNotInStatusSearch.entity().getState(), SearchCriteria.Op.NOTIN); + volumeIdNameNotInStatusSearch.done(); + CountSnapshotsByAccount = createSearchBuilder(Long.class); CountSnapshotsByAccount.select(null, Func.COUNT, null); CountSnapshotsByAccount.and("account", CountSnapshotsByAccount.entity().getAccountId(), SearchCriteria.Op.EQ); @@ -295,6 +302,15 @@ public List listByStatusNotIn(long volumeId, Snapshot.State... statu return listBy(sc, null); } + @Override + public SnapshotVO findByVolumeIdAndNameNotInStatus(long volumeId, String name, Snapshot.State... status) { + SearchCriteria sc = volumeIdNameNotInStatusSearch.create(); + sc.setParameters("volumeId", volumeId); + sc.setParameters("name", name); + sc.setParameters("status", (Object[]) status); + return findOneBy(sc); + } + @Override public List searchByVolumes(List volumeIds) { if (CollectionUtils.isEmpty(volumeIds)) { diff --git a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java index 0d6e9de509fa..ed1af687c563 100755 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -1721,6 +1721,10 @@ public Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, if (snapshotName == null) snapshotName = vmDisplayName + "_" + volume.getName() + "_" + timeString; + if (_snapshotDao.findByVolumeIdAndNameNotInStatus(volumeId, snapshotName, Snapshot.State.Destroyed, Snapshot.State.Error) != null) { + throw new InvalidParameterValueException(String.format("A snapshot with name [%s] already exists for volume %s.", snapshotName, volume)); + } + HypervisorType hypervisorType = HypervisorType.None; StoragePoolVO storagePool = _storagePoolDao.findById(volume.getDataStore().getId()); if (storagePool.getScope() == ScopeType.ZONE) { diff --git a/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerImplTest.java b/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerImplTest.java index 32b103f39d3f..c8eb9cbc0e5e 100644 --- a/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerImplTest.java +++ b/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerImplTest.java @@ -28,8 +28,13 @@ import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotResult; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotService; import org.apache.cloudstack.framework.async.AsyncCallFuture; +import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; @@ -45,10 +50,12 @@ import com.cloud.dc.dao.DataCenterDao; import com.cloud.event.ActionEventUtils; import com.cloud.exception.InvalidParameterValueException; +import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.exception.PermissionDeniedException; import com.cloud.exception.ResourceUnavailableException; import com.cloud.org.Grouping; import com.cloud.storage.DataStoreRole; +import com.cloud.storage.ScopeType; import com.cloud.storage.Snapshot; import com.cloud.storage.SnapshotVO; import com.cloud.storage.VolumeVO; @@ -59,8 +66,10 @@ import com.cloud.user.AccountManager; import com.cloud.user.AccountVO; import com.cloud.user.ResourceLimitService; +import com.cloud.user.User; import com.cloud.user.dao.AccountDao; import com.cloud.utils.Pair; +import com.cloud.vm.dao.UserVmDao; @RunWith(MockitoJUnitRunner.class) public class SnapshotManagerImplTest { @@ -86,9 +95,42 @@ public class SnapshotManagerImplTest { SnapshotZoneDao snapshotZoneDao; @Mock VolumeDao volumeDao; + @Mock + PrimaryDataStoreDao primaryDataStoreDao; + @Mock + VolumeDataFactory volFactory; + @Mock + UserVmDao userVmDao; @InjectMocks SnapshotManagerImpl snapshotManager = new SnapshotManagerImpl(); + @Test + public void testAllocSnapshotRejectsDuplicateNameForVolume() { + long volumeId = 1L; + CallContext.register(Mockito.mock(User.class), Mockito.mock(Account.class)); + try { + VolumeInfo volume = Mockito.mock(VolumeInfo.class); + Mockito.when(volFactory.getVolume(volumeId)).thenReturn(volume); + DataStore dataStore = Mockito.mock(DataStore.class); + Mockito.when(volume.getDataStore()).thenReturn(dataStore); + Mockito.when(dataStore.getId()).thenReturn(10L); + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + Mockito.when(primaryDataStoreDao.findById(10L)).thenReturn(pool); + Mockito.when(pool.getScope()).thenReturn(ScopeType.ZONE); + Mockito.when(pool.getHypervisor()).thenReturn(HypervisorType.None); + Mockito.when(volume.getInstanceId()).thenReturn(null); + Mockito.when(volume.getAccountId()).thenReturn(2L); + + Mockito.when(snapshotDao.findByVolumeIdAndNameNotInStatus(volumeId, "dup", Snapshot.State.Destroyed, Snapshot.State.Error)) + .thenReturn(Mockito.mock(SnapshotVO.class)); + + Assert.assertThrows(InvalidParameterValueException.class, () -> + snapshotManager.allocSnapshot(volumeId, Snapshot.MANUAL_POLICY_ID, "dup", null)); + } finally { + CallContext.unregister(); + } + } + @Test public void testGetSnapshotZoneImageStoreValid() { final long snapshotId = 1L;