Skip to content

Fix Revert Instance to Snapshot with custom service offering #12885

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
abh1sar wants to merge 2 commits into
base: 4.20
Choose a base branch
from
+80 −24
Open

Fix Revert Instance to Snapshot with custom service offering #12885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5e066a1
Fix revertVM with custom svc offering
abh1sar Mar 25, 2026
e5ee0de
Update server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerIm…
abh1sar Mar 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 39 additions & 15 deletions server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 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.");
}

// if snapshot is not created, error out
Expand Down Expand Up @@ -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 <code>changeUserVmServiceOffering</code> 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<String, String> 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;
}

/**
Expand Down Expand Up @@ -944,7 +966,9 @@ private UserVm orchestrateRevertToVMSnapshot(Long vmSnapshotId) throws Insuffici
Transaction.execute(new TransactionCallbackWithExceptionNoReturn<CloudRuntimeException>() {
@Override
public void doInTransactionWithoutResult(TransactionStatus status) throws CloudRuntimeException {
updateUserVmServiceOffering(userVm, vmSnapshotVo);
if (userVmServiceOfferingNeedsChange(userVm, vmSnapshotVo)) {
changeUserVmServiceOffering(userVm, vmSnapshotVo);
}
revertCustomServiceOfferingDetailsFromVmSnapshot(userVm, vmSnapshotVo);
}
});
Expand Down
50 changes: 41 additions & 9 deletions server/src/test/java/com/cloud/vm/snapshot/VMSnapshotManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
Loading