diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/StorageVMSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/StorageVMSnapshotStrategy.java index e3f28a7012c..31b13fc279e 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/StorageVMSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/StorageVMSnapshotStrategy.java @@ -111,7 +111,7 @@ public class StorageVMSnapshotStrategy extends DefaultVMSnapshotStrategy { FreezeThawVMAnswer freezeAnswer = null; FreezeThawVMCommand thawCmd = null; FreezeThawVMAnswer thawAnswer = null; - List forRollback = new ArrayList<>(); + List snapshotsForRollback = new ArrayList<>(); long startFreeze = 0; try { vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshotVO, VMSnapshot.Event.CreateRequested); @@ -165,7 +165,7 @@ public class StorageVMSnapshotStrategy extends DefaultVMSnapshotStrategy { logger.info("The virtual machine is frozen"); for (VolumeInfo vol : vinfos) { long startSnapshtot = System.nanoTime(); - SnapshotInfo snapInfo = createDiskSnapshot(vmSnapshot, forRollback, vol); + SnapshotInfo snapInfo = createDiskSnapshot(vmSnapshot, snapshotsForRollback, vol); if (snapInfo == null) { thawAnswer = (FreezeThawVMAnswer) agentMgr.send(hostId, thawCmd); @@ -222,7 +222,7 @@ public class StorageVMSnapshotStrategy extends DefaultVMSnapshotStrategy { } } if (!result) { - for (SnapshotInfo snapshotInfo : forRollback) { + for (SnapshotInfo snapshotInfo : snapshotsForRollback) { rollbackDiskSnapshot(snapshotInfo); } try { @@ -388,10 +388,16 @@ public class StorageVMSnapshotStrategy extends DefaultVMSnapshotStrategy { //Rollback if one of disks snapshot fails protected void rollbackDiskSnapshot(SnapshotInfo snapshotInfo) { + if (snapshotInfo == null) { + return; + } Long snapshotID = snapshotInfo.getId(); SnapshotVO snapshot = snapshotDao.findById(snapshotID); + if (snapshot == null) { + return; + } deleteSnapshotByStrategy(snapshot); - logger.debug("Rollback is executed: deleting snapshot with id:" + snapshotID); + logger.debug("Rollback is executed: deleting snapshot with id: {}", snapshotID); } protected void deleteSnapshotByStrategy(SnapshotVO snapshot) { @@ -434,7 +440,7 @@ public class StorageVMSnapshotStrategy extends DefaultVMSnapshotStrategy { } } - protected SnapshotInfo createDiskSnapshot(VMSnapshot vmSnapshot, List forRollback, VolumeInfo vol) { + protected SnapshotInfo createDiskSnapshot(VMSnapshot vmSnapshot, List snapshotsForRollback, VolumeInfo vol) { String snapshotName = vmSnapshot.getId() + "_" + vol.getUuid(); SnapshotVO snapshot = new SnapshotVO(vol.getDataCenterId(), vol.getAccountId(), vol.getDomainId(), vol.getId(), vol.getDiskOfferingId(), snapshotName, (short) Snapshot.Type.GROUP.ordinal(), Snapshot.Type.GROUP.name(), vol.getSize(), vol.getMinIops(), vol.getMaxIops(), Hypervisor.HypervisorType.KVM, null); @@ -448,6 +454,7 @@ public class StorageVMSnapshotStrategy extends DefaultVMSnapshotStrategy { vol.addPayload(setPayload(vol, snapshot, quiescevm)); SnapshotInfo snapshotInfo = snapshotDataFactory.getSnapshot(snapshot.getId(), vol.getDataStore()); snapshotInfo.addPayload(vol.getpayload()); + snapshotsForRollback.add(snapshotInfo); SnapshotStrategy snapshotStrategy = storageStrategyFactory.getSnapshotStrategy(snapshotInfo, SnapshotOperation.TAKE); if (snapshotStrategy == null) { throw new CloudRuntimeException("Could not find strategy for snapshot uuid:" + snapshotInfo.getUuid()); @@ -455,8 +462,6 @@ public class StorageVMSnapshotStrategy extends DefaultVMSnapshotStrategy { snapshotInfo = snapshotStrategy.takeSnapshot(snapshotInfo); if (snapshotInfo == null) { throw new CloudRuntimeException("Failed to create snapshot"); - } else { - forRollback.add(snapshotInfo); } vmSnapshotDetailsDao.persist(new VMSnapshotDetailsVO(vmSnapshot.getId(), STORAGE_SNAPSHOT, String.valueOf(snapshot.getId()), true)); snapshotInfo.markBackedUp(); diff --git a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyKVMTest.java b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyKVMTest.java index 609a1225118..46ea36eea7e 100644 --- a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyKVMTest.java +++ b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyKVMTest.java @@ -153,7 +153,7 @@ public class VMSnapshotStrategyKVMTest extends TestCase{ @Test public void testCreateDiskSnapshotBasedOnStrategy() throws Exception { VMSnapshotVO vmSnapshot = Mockito.mock(VMSnapshotVO.class); - List forRollback = new ArrayList<>(); + List snapshotsForRollback = new ArrayList<>(); VolumeInfo vol = Mockito.mock(VolumeInfo.class); SnapshotInfo snapshotInfo = Mockito.mock(SnapshotInfo.class); SnapshotStrategy strategy = Mockito.mock(SnapshotStrategy.class); @@ -177,7 +177,7 @@ public class VMSnapshotStrategyKVMTest extends TestCase{ VMSnapshotDetailsVO vmDetails = new VMSnapshotDetailsVO(vmSnapshot.getId(), volUuid, String.valueOf(snapshot.getId()), false); when(vmSnapshotDetailsDao.persist(any())).thenReturn(vmDetails); - info = vmStrategy.createDiskSnapshot(vmSnapshot, forRollback, vol); + info = vmStrategy.createDiskSnapshot(vmSnapshot, snapshotsForRollback, vol); assertNotNull(info); }