From f72263deed9ce02f72e00d7b09cee90b6cf4e0ac Mon Sep 17 00:00:00 2001 From: Slavka Peleva Date: Fri, 1 Oct 2021 11:55:40 +0300 Subject: [PATCH 1/7] Fix of revert RBD snapshots If snapshot is taken only on Primary storage with the option "snapshot.backup.to.secondary" set to true, when you set this option to false the revert will fail. Added check if the snapshot is not on Secondary to check for it on Primary --- .../java/com/cloud/storage/snapshot/SnapshotManagerImpl.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 d94811a53a3b..09612cb9cd37 100755 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -312,7 +312,10 @@ public Snapshot revertSnapshot(Long snapshotId) { SnapshotInfo snapshotInfo = snapshotFactory.getSnapshot(snapshotId, dataStoreRole); if (snapshotInfo == null) { - throw new CloudRuntimeException("snapshot:" + snapshotId + " not exist in data store"); + snapshotInfo = snapshotFactory.getSnapshot(snapshotId, DataStoreRole.Primary); + if (snapshotInfo == null) { + throw new CloudRuntimeException(String.format("snapshot [%s] does not exists in data store", snapshotId)); + } } SnapshotStrategy snapshotStrategy = _storageStrategyFactory.getSnapshotStrategy(snapshot, SnapshotOperation.REVERT); From d86f383f0fe6f7e1a7a7e6644c81b55cda16a87d Mon Sep 17 00:00:00 2001 From: Slavka Peleva Date: Wed, 6 Oct 2021 08:40:16 +0300 Subject: [PATCH 2/7] Check if snapshot is on primary storage Will check first if the snapshot is on Primary storage, if not will return Image as data store --- .../storage/snapshot/SnapshotManagerImpl.java | 43 +++---------------- 1 file changed, 7 insertions(+), 36 deletions(-) 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 09612cb9cd37..d0bd0de9cdf2 100755 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -38,7 +38,6 @@ import org.apache.cloudstack.api.command.user.snapshot.UpdateSnapshotPolicyCmd; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; -import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreCapabilities; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint; import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector; @@ -60,6 +59,7 @@ 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.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.MapUtils; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -100,7 +100,6 @@ import com.cloud.storage.SnapshotVO; import com.cloud.storage.Storage; import com.cloud.storage.Storage.ImageFormat; -import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePool; import com.cloud.storage.VMTemplateVO; @@ -312,10 +311,7 @@ public Snapshot revertSnapshot(Long snapshotId) { SnapshotInfo snapshotInfo = snapshotFactory.getSnapshot(snapshotId, dataStoreRole); if (snapshotInfo == null) { - snapshotInfo = snapshotFactory.getSnapshot(snapshotId, DataStoreRole.Primary); - if (snapshotInfo == null) { - throw new CloudRuntimeException(String.format("snapshot [%s] does not exists in data store", snapshotId)); - } + throw new CloudRuntimeException(String.format("snapshot [%s] does not exists in data store", snapshotId)); } SnapshotStrategy snapshotStrategy = _storageStrategyFactory.getSnapshotStrategy(snapshot, SnapshotOperation.REVERT); @@ -1245,11 +1241,7 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationExc SnapshotDataStoreVO snapshotStoreRef = _snapshotStoreDao.findBySnapshot(snapshotId, dataStoreRole); if (snapshotStoreRef == null) { - // The snapshot was not backed up to secondary. Find the snap on primary - snapshotStoreRef = _snapshotStoreDao.findBySnapshot(snapshotId, DataStoreRole.Primary); - if (snapshotStoreRef == null) { - throw new CloudRuntimeException("Could not find snapshot"); - } + throw new CloudRuntimeException("Could not find snapshot"); } UsageEventUtils.publishUsageEvent(EventTypes.EVENT_SNAPSHOT_CREATE, snapshot.getAccountId(), snapshot.getDataCenterId(), snapshotId, snapshot.getName(), null, null, snapshotStoreRef.getPhysicalSize(), volume.getSize(), snapshot.getClass().getName(), snapshot.getUuid()); @@ -1336,32 +1328,11 @@ private void updateSnapshotPayload(long storagePoolId, CreateSnapshotPayload pay } private DataStoreRole getDataStoreRole(Snapshot snapshot, SnapshotDataStoreDao snapshotStoreDao, DataStoreManager dataStoreMgr) { - SnapshotDataStoreVO snapshotStore = snapshotStoreDao.findBySnapshot(snapshot.getId(), DataStoreRole.Primary); - - if (snapshotStore == null) { - return DataStoreRole.Image; + List snapshots = snapshotStoreDao.findBySnapshotId(snapshot.getId()); + if (CollectionUtils.isEmpty(snapshots)) { + return null; } - - long storagePoolId = snapshotStore.getDataStoreId(); - DataStore dataStore = dataStoreMgr.getDataStore(storagePoolId, DataStoreRole.Primary); - - Map mapCapabilities = dataStore.getDriver().getCapabilities(); - - if (mapCapabilities != null) { - String value = mapCapabilities.get(DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT.toString()); - Boolean supportsStorageSystemSnapshots = new Boolean(value); - - if (supportsStorageSystemSnapshots) { - return DataStoreRole.Primary; - } - } - - StoragePoolVO storagePoolVO = _storagePoolDao.findById(storagePoolId); - if ((storagePoolVO.getPoolType() == StoragePoolType.RBD || storagePoolVO.getPoolType() == StoragePoolType.PowerFlex) && !BackupSnapshotAfterTakingSnapshot.value()) { - return DataStoreRole.Primary; - } - - return DataStoreRole.Image; + return snapshots.stream().map(store -> store.getRole()).filter(store -> DataStoreRole.Primary.equals(store)).findFirst().orElse(DataStoreRole.Image); } @Override From 8a238cf2558dbb288cf8dfd9aeae32e66e6f9d4e Mon Sep 17 00:00:00 2001 From: Slavka Peleva Date: Wed, 6 Oct 2021 14:22:48 +0300 Subject: [PATCH 3/7] Fix unit tests --- .../storage/snapshot/SnapshotManagerTest.java | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java b/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java index 37b4488376bf..44dd9c9087aa 100755 --- a/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java +++ b/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java @@ -23,6 +23,9 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.ArrayList; import java.util.List; import java.util.UUID; @@ -30,6 +33,7 @@ import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; @@ -149,6 +153,8 @@ public class SnapshotManagerTest { SnapshotDataStoreVO snapshotStoreMock; @Mock SnapshotService snapshotSrv; + @Mock + DataStoreManager dataStoreManager; @Mock GlobalLock globalLockMock; @@ -326,22 +332,24 @@ public void testRevertSnapshotF1() { // vm on Xenserver, return null @Test - public void testRevertSnapshotF2() { + public void testRevertSnapshotF2() throws NoSuchMethodException, IllegalAccessException, InvocationTargetException, InstantiationException { when(_vmDao.findById(anyLong())).thenReturn(vmMock); when(vmMock.getState()).thenReturn(State.Stopped); when(vmMock.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.XenServer); when(volumeMock.getFormat()).thenReturn(ImageFormat.VHD); + getDataStore(); Snapshot snapshot = _snapshotMgr.revertSnapshot(TEST_SNAPSHOT_ID); Assert.assertNull(snapshot); } // vm on KVM, successful @Test - public void testRevertSnapshotF3() { + public void testRevertSnapshotF3() throws NoSuchMethodException, IllegalAccessException, InvocationTargetException, InstantiationException { when(_vmDao.findById(anyLong())).thenReturn(vmMock); when(vmMock.getState()).thenReturn(State.Stopped); when(vmMock.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); when(volumeMock.getFormat()).thenReturn(ImageFormat.QCOW2); + getDataStore(); when (snapshotStrategy.revertSnapshot(Mockito.any(SnapshotInfo.class))).thenReturn(true); when(_volumeDao.update(anyLong(), any(VolumeVO.class))).thenReturn(true); Snapshot snapshot = _snapshotMgr.revertSnapshot(TEST_SNAPSHOT_ID); @@ -526,4 +534,15 @@ public void validatePersistSnapshotPolicyLockAquiredUpdateSnapshotPolicy() { Mockito.any(DateUtil.IntervalType.class), Mockito.anyInt(), Mockito.anyBoolean(), Mockito.anyBoolean()); Mockito.verify(_snapshotMgr, timesVerification).createTagsForSnapshotPolicy(Mockito.any(), Mockito.any()); } + + private void getDataStore() + throws NoSuchMethodException, IllegalAccessException, InvocationTargetException, InstantiationException { + Method declaredMethod = SnapshotManagerImpl.class.getDeclaredMethod("getDataStoreRole", Snapshot.class, SnapshotDataStoreDao.class, DataStoreManager.class); + declaredMethod.setAccessible(true); + List dataStoreSnapshots = new ArrayList<>(); + dataStoreSnapshots.add(snapshotStoreMock); + when(snapshotStoreDao.findBySnapshotId(anyLong())).thenReturn(dataStoreSnapshots); + Class snapshotMgrClass = SnapshotManagerImpl.class; + when(declaredMethod.invoke(snapshotMgrClass.newInstance(),snapshotMock, snapshotStoreDao, dataStoreManager)).thenReturn(DataStoreRole.Primary); + } } From 04cc48fbbf84e4535b2e980226eab9a17a03efe6 Mon Sep 17 00:00:00 2001 From: Slavka Peleva Date: Tue, 11 Jan 2022 12:53:07 +0200 Subject: [PATCH 4/7] removed unused method's params --- .../cloud/storage/snapshot/SnapshotManagerImpl.java | 10 +++++----- .../cloud/storage/snapshot/SnapshotManagerTest.java | 5 ++--- 2 files changed, 7 insertions(+), 8 deletions(-) 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 d0bd0de9cdf2..96cfe8d0042b 100755 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -307,7 +307,7 @@ public Snapshot revertSnapshot(Long snapshotId) { } } - DataStoreRole dataStoreRole = getDataStoreRole(snapshot, _snapshotStoreDao, dataStoreMgr); + DataStoreRole dataStoreRole = getDataStoreRole(snapshot); SnapshotInfo snapshotInfo = snapshotFactory.getSnapshot(snapshotId, dataStoreRole); if (snapshotInfo == null) { @@ -586,7 +586,7 @@ public boolean deleteSnapshot(long snapshotId) { return false; } - DataStoreRole dataStoreRole = getDataStoreRole(snapshotCheck, _snapshotStoreDao, dataStoreMgr); + DataStoreRole dataStoreRole = getDataStoreRole(snapshotCheck); SnapshotDataStoreVO snapshotStoreRef = _snapshotStoreDao.findBySnapshot(snapshotId, dataStoreRole); @@ -1237,7 +1237,7 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationExc try { postCreateSnapshot(volume.getId(), snapshotId, payload.getSnapshotPolicyId()); - DataStoreRole dataStoreRole = getDataStoreRole(snapshot, _snapshotStoreDao, dataStoreMgr); + DataStoreRole dataStoreRole = getDataStoreRole(snapshot); SnapshotDataStoreVO snapshotStoreRef = _snapshotStoreDao.findBySnapshot(snapshotId, dataStoreRole); if (snapshotStoreRef == null) { @@ -1327,8 +1327,8 @@ private void updateSnapshotPayload(long storagePoolId, CreateSnapshotPayload pay } } - private DataStoreRole getDataStoreRole(Snapshot snapshot, SnapshotDataStoreDao snapshotStoreDao, DataStoreManager dataStoreMgr) { - List snapshots = snapshotStoreDao.findBySnapshotId(snapshot.getId()); + private DataStoreRole getDataStoreRole(Snapshot snapshot) { + List snapshots = _snapshotStoreDao.findBySnapshotId(snapshot.getId()); if (CollectionUtils.isEmpty(snapshots)) { return null; } diff --git a/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java b/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java index 44dd9c9087aa..34c350761f76 100755 --- a/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java +++ b/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java @@ -537,12 +537,11 @@ public void validatePersistSnapshotPolicyLockAquiredUpdateSnapshotPolicy() { private void getDataStore() throws NoSuchMethodException, IllegalAccessException, InvocationTargetException, InstantiationException { - Method declaredMethod = SnapshotManagerImpl.class.getDeclaredMethod("getDataStoreRole", Snapshot.class, SnapshotDataStoreDao.class, DataStoreManager.class); + Method declaredMethod = SnapshotManagerImpl.class.getDeclaredMethod("getDataStoreRole", Snapshot.class); declaredMethod.setAccessible(true); List dataStoreSnapshots = new ArrayList<>(); dataStoreSnapshots.add(snapshotStoreMock); when(snapshotStoreDao.findBySnapshotId(anyLong())).thenReturn(dataStoreSnapshots); - Class snapshotMgrClass = SnapshotManagerImpl.class; - when(declaredMethod.invoke(snapshotMgrClass.newInstance(),snapshotMock, snapshotStoreDao, dataStoreManager)).thenReturn(DataStoreRole.Primary); + when(declaredMethod.invoke(_snapshotMgr, snapshotMock)).thenReturn(DataStoreRole.Primary); } } From 3c4ef895c3da6bbed019d9351ee00f9141ecb762 Mon Sep 17 00:00:00 2001 From: Slavka Peleva Date: Thu, 20 Jan 2022 11:07:03 +0200 Subject: [PATCH 5/7] Formatted error message and added the snapshot ID to it --- .../java/com/cloud/storage/snapshot/SnapshotManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 96cfe8d0042b..6a5fa2cdc2f5 100755 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -1241,7 +1241,7 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationExc SnapshotDataStoreVO snapshotStoreRef = _snapshotStoreDao.findBySnapshot(snapshotId, dataStoreRole); if (snapshotStoreRef == null) { - throw new CloudRuntimeException("Could not find snapshot"); + throw new CloudRuntimeException(String.format("Could not find snapshot with id [%] on [%]", snapshotId, snapshot.getLocationType())); } UsageEventUtils.publishUsageEvent(EventTypes.EVENT_SNAPSHOT_CREATE, snapshot.getAccountId(), snapshot.getDataCenterId(), snapshotId, snapshot.getName(), null, null, snapshotStoreRef.getPhysicalSize(), volume.getSize(), snapshot.getClass().getName(), snapshot.getUuid()); From 709e04214f9241aec2d9c9727f9d8196eb7641fb Mon Sep 17 00:00:00 2001 From: Slavka Peleva Date: Mon, 7 Feb 2022 08:20:58 +0200 Subject: [PATCH 6/7] Return to the old logic, the fix will only apply to RBD --- .../storage/snapshot/SnapshotManagerImpl.java | 33 ++++++++++++++++--- .../storage/snapshot/SnapshotManagerTest.java | 22 ++----------- 2 files changed, 30 insertions(+), 25 deletions(-) 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 6a5fa2cdc2f5..283a45ac4bb6 100755 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -38,6 +38,7 @@ import org.apache.cloudstack.api.command.user.snapshot.UpdateSnapshotPolicyCmd; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreCapabilities; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint; import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector; @@ -59,7 +60,6 @@ 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.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.MapUtils; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -100,6 +100,7 @@ import com.cloud.storage.SnapshotVO; import com.cloud.storage.Storage; import com.cloud.storage.Storage.ImageFormat; +import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePool; import com.cloud.storage.VMTemplateVO; @@ -310,6 +311,7 @@ public Snapshot revertSnapshot(Long snapshotId) { DataStoreRole dataStoreRole = getDataStoreRole(snapshot); SnapshotInfo snapshotInfo = snapshotFactory.getSnapshot(snapshotId, dataStoreRole); + if (snapshotInfo == null) { throw new CloudRuntimeException(String.format("snapshot [%s] does not exists in data store", snapshotId)); } @@ -1328,11 +1330,32 @@ private void updateSnapshotPayload(long storagePoolId, CreateSnapshotPayload pay } private DataStoreRole getDataStoreRole(Snapshot snapshot) { - List snapshots = _snapshotStoreDao.findBySnapshotId(snapshot.getId()); - if (CollectionUtils.isEmpty(snapshots)) { - return null; + SnapshotDataStoreVO snapshotStore = _snapshotStoreDao.findBySnapshot(snapshot.getId(), DataStoreRole.Primary); + + if (snapshotStore == null) { + return DataStoreRole.Image; + } + + long storagePoolId = snapshotStore.getDataStoreId(); + DataStore dataStore = dataStoreMgr.getDataStore(storagePoolId, DataStoreRole.Primary); + + Map mapCapabilities = dataStore.getDriver().getCapabilities(); + + if (mapCapabilities != null) { + String value = mapCapabilities.get(DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT.toString()); + Boolean supportsStorageSystemSnapshots = Boolean.valueOf(value); + + if (supportsStorageSystemSnapshots) { + return DataStoreRole.Primary; + } + } + + StoragePoolVO storagePoolVO = _storagePoolDao.findById(storagePoolId); + if (storagePoolVO.getPoolType() == StoragePoolType.RBD) { + return DataStoreRole.Primary; } - return snapshots.stream().map(store -> store.getRole()).filter(store -> DataStoreRole.Primary.equals(store)).findFirst().orElse(DataStoreRole.Image); + + return DataStoreRole.Image; } @Override diff --git a/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java b/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java index 34c350761f76..37b4488376bf 100755 --- a/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java +++ b/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java @@ -23,9 +23,6 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; -import java.util.ArrayList; import java.util.List; import java.util.UUID; @@ -33,7 +30,6 @@ import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; -import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; @@ -153,8 +149,6 @@ public class SnapshotManagerTest { SnapshotDataStoreVO snapshotStoreMock; @Mock SnapshotService snapshotSrv; - @Mock - DataStoreManager dataStoreManager; @Mock GlobalLock globalLockMock; @@ -332,24 +326,22 @@ public void testRevertSnapshotF1() { // vm on Xenserver, return null @Test - public void testRevertSnapshotF2() throws NoSuchMethodException, IllegalAccessException, InvocationTargetException, InstantiationException { + public void testRevertSnapshotF2() { when(_vmDao.findById(anyLong())).thenReturn(vmMock); when(vmMock.getState()).thenReturn(State.Stopped); when(vmMock.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.XenServer); when(volumeMock.getFormat()).thenReturn(ImageFormat.VHD); - getDataStore(); Snapshot snapshot = _snapshotMgr.revertSnapshot(TEST_SNAPSHOT_ID); Assert.assertNull(snapshot); } // vm on KVM, successful @Test - public void testRevertSnapshotF3() throws NoSuchMethodException, IllegalAccessException, InvocationTargetException, InstantiationException { + public void testRevertSnapshotF3() { when(_vmDao.findById(anyLong())).thenReturn(vmMock); when(vmMock.getState()).thenReturn(State.Stopped); when(vmMock.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); when(volumeMock.getFormat()).thenReturn(ImageFormat.QCOW2); - getDataStore(); when (snapshotStrategy.revertSnapshot(Mockito.any(SnapshotInfo.class))).thenReturn(true); when(_volumeDao.update(anyLong(), any(VolumeVO.class))).thenReturn(true); Snapshot snapshot = _snapshotMgr.revertSnapshot(TEST_SNAPSHOT_ID); @@ -534,14 +526,4 @@ public void validatePersistSnapshotPolicyLockAquiredUpdateSnapshotPolicy() { Mockito.any(DateUtil.IntervalType.class), Mockito.anyInt(), Mockito.anyBoolean(), Mockito.anyBoolean()); Mockito.verify(_snapshotMgr, timesVerification).createTagsForSnapshotPolicy(Mockito.any(), Mockito.any()); } - - private void getDataStore() - throws NoSuchMethodException, IllegalAccessException, InvocationTargetException, InstantiationException { - Method declaredMethod = SnapshotManagerImpl.class.getDeclaredMethod("getDataStoreRole", Snapshot.class); - declaredMethod.setAccessible(true); - List dataStoreSnapshots = new ArrayList<>(); - dataStoreSnapshots.add(snapshotStoreMock); - when(snapshotStoreDao.findBySnapshotId(anyLong())).thenReturn(dataStoreSnapshots); - when(declaredMethod.invoke(_snapshotMgr, snapshotMock)).thenReturn(DataStoreRole.Primary); - } } From d4e0ecc35ef7c7848085817b4ce2d32f642c93c1 Mon Sep 17 00:00:00 2001 From: Slavka Peleva Date: Wed, 9 Feb 2022 14:05:18 +0200 Subject: [PATCH 7/7] Formatted Exception's messages --- .../java/com/cloud/storage/snapshot/SnapshotManagerImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 283a45ac4bb6..ffa393bd8664 100755 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -313,7 +313,7 @@ public Snapshot revertSnapshot(Long snapshotId) { SnapshotInfo snapshotInfo = snapshotFactory.getSnapshot(snapshotId, dataStoreRole); if (snapshotInfo == null) { - throw new CloudRuntimeException(String.format("snapshot [%s] does not exists in data store", snapshotId)); + throw new CloudRuntimeException(String.format("snapshot %s [%s] does not exists in data store", snapshot.getName(), snapshot.getUuid())); } SnapshotStrategy snapshotStrategy = _storageStrategyFactory.getSnapshotStrategy(snapshot, SnapshotOperation.REVERT); @@ -1243,7 +1243,7 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationExc SnapshotDataStoreVO snapshotStoreRef = _snapshotStoreDao.findBySnapshot(snapshotId, dataStoreRole); if (snapshotStoreRef == null) { - throw new CloudRuntimeException(String.format("Could not find snapshot with id [%] on [%]", snapshotId, snapshot.getLocationType())); + throw new CloudRuntimeException(String.format("Could not find snapshot %s [%s] on [%s]", snapshot.getName(), snapshot.getUuid(), snapshot.getLocationType())); } UsageEventUtils.publishUsageEvent(EventTypes.EVENT_SNAPSHOT_CREATE, snapshot.getAccountId(), snapshot.getDataCenterId(), snapshotId, snapshot.getName(), null, null, snapshotStoreRef.getPhysicalSize(), volume.getSize(), snapshot.getClass().getName(), snapshot.getUuid());