From 03b0adf3183beaf45f8706c3ccb409a2e1585845 Mon Sep 17 00:00:00 2001
From: Abhisar Sinha <63767682+abh1sar@users.noreply.github.com>
Date: Wed, 25 Mar 2026 09:45:00 +0530
Subject: [PATCH 1/2] Fix revertVM with custom svc offering
---
.../vm/snapshot/VMSnapshotManagerImpl.java | 54 +++++++++++++------
.../vm/snapshot/VMSnapshotManagerTest.java | 50 +++++++++++++----
2 files changed, 80 insertions(+), 24 deletions(-)
diff --git a/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java b/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
index 8e7ce351246a..c81f4b277541 100644
--- a/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
+++ b/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
@@ -754,11 +754,17 @@ public UserVm revertToSnapshot(Long vmSnapshotId) throws InsufficientCapacityExc
"Instance Snapshot reverting failed because the Instance is not in Running or Stopped state.");
}
- if (userVm.getState() == VirtualMachine.State.Running && vmSnapshotVo.getType() == VMSnapshot.Type.Disk || userVm.getState() == VirtualMachine.State.Stopped
- && vmSnapshotVo.getType() == VMSnapshot.Type.DiskAndMemory) {
+ if (userVm.getState() == VirtualMachine.State.Running && vmSnapshotVo.getType() == VMSnapshot.Type.Disk) {
throw new InvalidParameterValueException(
- "Reverting to the Instance Snapshot is not allowed for running Instances as this would result in a Instance state change. For running Instances only Snapshots with memory can be reverted. In order to revert to a Snapshot without memory you need to first stop the Instance."
- + " Snapshot");
+ "Reverting to the Instance Snapshot is not allowed for running Instances as this would result in an Instance state change. " +
+ "For running Instances only Snapshots with memory can be reverted. " +
+ "In order to revert to a Snapshot without memory you need to first stop the Instance.");
+ }
+
+ if ( userVm.getState() == VirtualMachine.State.Stopped && vmSnapshotVo.getType() == VMSnapshot.Type.DiskAndMemory) {
+ throw new InvalidParameterValueException(
+ "Reverting to the Instance Snapshot is not allowed for stopped Instances if the Snapshot contains memory. " +
+ "In order to revert to a Snapshot with memory you need to first start the Instance.");
}
// if snapshot is not created, error out
@@ -811,20 +817,36 @@ else if (jobResult instanceof Throwable)
}
/**
- * If snapshot was taken with a different service offering than actual used in vm, should change it back to it.
- * We also call changeUserVmServiceOffering in case the service offering is dynamic in order to
- * perform resource limit validation, as the amount of CPUs or memory may have been changed.
- * @param vmSnapshotVo vm snapshot
+ * Check if service offering change is needed for user vm when reverting to vm snapshot.
+ * Service offering change is needed when snapshot was taken with a different service offering than actual used in vm.
+ * Service offering change is also needed when service offering is dynamic and the amount of cpu, memory or cpu speed
+ * has been changed since snapshot was taken.
+ * @param userVm
+ * @param vmSnapshotVo
+ * @return true if service offering change is needed; false otherwise
*/
- protected void updateUserVmServiceOffering(UserVm userVm, VMSnapshotVO vmSnapshotVo) {
+ protected boolean userVmServiceOfferingNeedsChange(UserVm userVm, VMSnapshotVO vmSnapshotVo) {
if (vmSnapshotVo.getServiceOfferingId() != userVm.getServiceOfferingId()) {
- changeUserVmServiceOffering(userVm, vmSnapshotVo);
- return;
+ return true;
}
- ServiceOfferingVO serviceOffering = _serviceOfferingDao.findById(userVm.getServiceOfferingId());
- if (serviceOffering.isDynamic()) {
- changeUserVmServiceOffering(userVm, vmSnapshotVo);
+
+ ServiceOfferingVO currentServiceOffering = _serviceOfferingDao.findByIdIncludingRemoved(userVm.getId(), userVm.getServiceOfferingId());
+ if (currentServiceOffering.isDynamic()) {
+ Map vmDetails = getVmMapDetails(vmSnapshotVo);
+ ServiceOfferingVO newServiceOffering = _serviceOfferingDao.getComputeOffering(currentServiceOffering, vmDetails);
+
+ int newCpu = newServiceOffering.getCpu();
+ int newMemory = newServiceOffering.getRamSize();
+ int newSpeed = newServiceOffering.getSpeed();
+ int currentCpu = currentServiceOffering.getCpu();
+ int currentMemory = currentServiceOffering.getRamSize();
+ int currentSpeed = currentServiceOffering.getSpeed();
+
+ if (newCpu != currentCpu || newMemory != currentMemory || newSpeed != currentSpeed) {
+ return true;
+ }
}
+ return false;
}
/**
@@ -944,7 +966,9 @@ private UserVm orchestrateRevertToVMSnapshot(Long vmSnapshotId) throws Insuffici
Transaction.execute(new TransactionCallbackWithExceptionNoReturn() {
@Override
public void doInTransactionWithoutResult(TransactionStatus status) throws CloudRuntimeException {
- updateUserVmServiceOffering(userVm, vmSnapshotVo);
+ if (userVmServiceOfferingNeedsChange(userVm, vmSnapshotVo)) {
+ changeUserVmServiceOffering(userVm, vmSnapshotVo);
+ }
revertCustomServiceOfferingDetailsFromVmSnapshot(userVm, vmSnapshotVo);
}
});
diff --git a/server/src/test/java/com/cloud/vm/snapshot/VMSnapshotManagerTest.java b/server/src/test/java/com/cloud/vm/snapshot/VMSnapshotManagerTest.java
index 01512b448b25..26f452cb9f42 100644
--- a/server/src/test/java/com/cloud/vm/snapshot/VMSnapshotManagerTest.java
+++ b/server/src/test/java/com/cloud/vm/snapshot/VMSnapshotManagerTest.java
@@ -228,6 +228,7 @@ public void setup() {
when(vmSnapshotVO.getId()).thenReturn(VM_SNAPSHOT_ID);
when(serviceOffering.isDynamic()).thenReturn(false);
when(_serviceOfferingDao.findById(SERVICE_OFFERING_ID)).thenReturn(serviceOffering);
+ when(_serviceOfferingDao.findByIdIncludingRemoved(TEST_VM_ID, SERVICE_OFFERING_ID)).thenReturn(serviceOffering);
for (ResourceDetail detail : Arrays.asList(userVmDetailCpuNumber, vmSnapshotDetailCpuNumber)) {
when(detail.getName()).thenReturn(VmDetailConstants.CPU_NUMBER);
@@ -345,20 +346,51 @@ public void testAddSupportForCustomServiceOfferingDynamicServiceOffering() {
}
@Test
- public void testUpdateUserVmServiceOfferingSameServiceOffering() {
- _vmSnapshotMgr.updateUserVmServiceOffering(userVm, vmSnapshotVO);
- verify(_vmSnapshotMgr, never()).changeUserVmServiceOffering(userVm, vmSnapshotVO);
+ public void testUserVmServiceOfferingNeedsChangeWhenSnapshotOfferingDiffers() {
+ when(userVm.getServiceOfferingId()).thenReturn(SERVICE_OFFERING_DIFFERENT_ID);
+ when(vmSnapshotVO.getServiceOfferingId()).thenReturn(SERVICE_OFFERING_ID);
+
+ assertTrue(_vmSnapshotMgr.userVmServiceOfferingNeedsChange(userVm, vmSnapshotVO));
+
+ verify(_serviceOfferingDao, never()).findByIdIncludingRemoved(anyLong(), anyLong());
+ verify(_serviceOfferingDao, never()).getComputeOffering(any(ServiceOfferingVO.class), any());
}
@Test
- public void testUpdateUserVmServiceOfferingDifferentServiceOffering() throws ConcurrentOperationException, ResourceUnavailableException, ManagementServerException, VirtualMachineMigrationException {
- when(userVm.getServiceOfferingId()).thenReturn(SERVICE_OFFERING_DIFFERENT_ID);
- when(_userVmManager.upgradeVirtualMachine(eq(TEST_VM_ID), eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture())).thenReturn(true);
- _vmSnapshotMgr.updateUserVmServiceOffering(userVm, vmSnapshotVO);
+ public void testUserVmServiceOfferingNeedsChangeWhenSameNonDynamicOffering() {
+ assertFalse(_vmSnapshotMgr.userVmServiceOfferingNeedsChange(userVm, vmSnapshotVO));
+
+ verify(_serviceOfferingDao).findByIdIncludingRemoved(TEST_VM_ID, SERVICE_OFFERING_ID);
+ verify(_serviceOfferingDao, never()).getComputeOffering(any(ServiceOfferingVO.class), any());
+ }
- verify(_vmSnapshotMgr).changeUserVmServiceOffering(userVm, vmSnapshotVO);
+ @Test
+ public void testUserVmServiceOfferingNeedsChangeWhenDynamicOfferingMatchesSnapshot() {
+ when(serviceOffering.isDynamic()).thenReturn(true);
+ when(serviceOffering.getCpu()).thenReturn(2);
+ when(serviceOffering.getRamSize()).thenReturn(2048);
+ when(serviceOffering.getSpeed()).thenReturn(1000);
+ when(_serviceOfferingDao.getComputeOffering(eq(serviceOffering), any())).thenReturn(serviceOffering);
+
+ assertFalse(_vmSnapshotMgr.userVmServiceOfferingNeedsChange(userVm, vmSnapshotVO));
+
+ verify(_serviceOfferingDao).getComputeOffering(eq(serviceOffering), any());
verify(_vmSnapshotMgr).getVmMapDetails(vmSnapshotVO);
- verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(eq(userVm), eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture());
+ }
+
+ @Test
+ public void testUserVmServiceOfferingNeedsChangeWhenDynamicCpuDiffersFromSnapshot() {
+ when(serviceOffering.isDynamic()).thenReturn(true);
+ when(serviceOffering.getCpu()).thenReturn(2);
+ when(serviceOffering.getRamSize()).thenReturn(2048);
+ when(serviceOffering.getSpeed()).thenReturn(1000);
+ ServiceOfferingVO fromSnapshot = mock(ServiceOfferingVO.class);
+ when(fromSnapshot.getCpu()).thenReturn(4);
+ when(fromSnapshot.getRamSize()).thenReturn(2048);
+ when(fromSnapshot.getSpeed()).thenReturn(1000);
+ when(_serviceOfferingDao.getComputeOffering(eq(serviceOffering), any())).thenReturn(fromSnapshot);
+
+ assertTrue(_vmSnapshotMgr.userVmServiceOfferingNeedsChange(userVm, vmSnapshotVO));
}
@Test
From 64c9a9656a958432cda0d0d8303d24e1577d062d Mon Sep 17 00:00:00 2001
From: Abhisar Sinha <63767682+abh1sar@users.noreply.github.com>
Date: Thu, 26 Mar 2026 11:23:26 +0530
Subject: [PATCH 2/2] Update
server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
Co-authored-by: Fabricio Duarte
---
.../java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java b/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
index c81f4b277541..b2ac831a633e 100644
--- a/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
+++ b/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
@@ -761,9 +761,9 @@ public UserVm revertToSnapshot(Long vmSnapshotId) throws InsufficientCapacityExc
"In order to revert to a Snapshot without memory you need to first stop the Instance.");
}
- if ( userVm.getState() == VirtualMachine.State.Stopped && vmSnapshotVo.getType() == VMSnapshot.Type.DiskAndMemory) {
+ if (userVm.getState() == VirtualMachine.State.Stopped && vmSnapshotVo.getType() == VMSnapshot.Type.DiskAndMemory) {
throw new InvalidParameterValueException(
- "Reverting to the Instance Snapshot is not allowed for stopped Instances if the Snapshot contains memory. " +
+ "Reverting to the Instance Snapshot is not allowed for stopped Instances when the Snapshot contains memory as this would result in an Instance state change. " +
"In order to revert to a Snapshot with memory you need to first start the Instance.");
}