This is an automated email from the ASF dual-hosted git repository.
sureshanaparti pushed a commit to branch 4.22
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.22 by this push:
new d75acb6efcc Fix rollback disk snapshots on instance snapshot failure
(#12949)
d75acb6efcc is described below
commit d75acb6efcc2f80b878248b7d125102612eed7b2
Author: Suresh Kumar Anaparti <[email protected]>
AuthorDate: Tue Apr 14 15:21:05 2026 +0530
Fix rollback disk snapshots on instance snapshot failure (#12949)
---
.../storage/vmsnapshot/StorageVMSnapshotStrategy.java | 19 ++++++++++++-------
.../storage/vmsnapshot/VMSnapshotStrategyKVMTest.java | 4 ++--
2 files changed, 14 insertions(+), 9 deletions(-)
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<SnapshotInfo> forRollback = new ArrayList<>();
+ List<SnapshotInfo> 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<SnapshotInfo> forRollback, VolumeInfo vol) {
+ protected SnapshotInfo createDiskSnapshot(VMSnapshot vmSnapshot,
List<SnapshotInfo> 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<SnapshotInfo> forRollback = new ArrayList<>();
+ List<SnapshotInfo> 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);
}
