Fix Revert Instance to Snapshot with custom service offering (#12885)

* Fix revertVM with custom svc offering
This commit is contained in:
Abhisar Sinha 2026-03-27 10:19:52 +05:30 committed by GitHub
parent 4b7370a601
commit b22dbbe2d7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 80 additions and 24 deletions

View File

@ -754,11 +754,17 @@ public class VMSnapshotManagerImpl extends MutualExclusiveIdsManagerBase impleme
"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
@ -811,20 +817,36 @@ public class VMSnapshotManagerImpl extends MutualExclusiveIdsManagerBase impleme
}
/**
* 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;
}
/**
@ -944,7 +966,9 @@ public class VMSnapshotManagerImpl extends MutualExclusiveIdsManagerBase impleme
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);
}
});

View File

@ -228,6 +228,7 @@ public class VMSnapshotManagerTest {
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 class VMSnapshotManagerTest {
}
@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(_vmSnapshotMgr).changeUserVmServiceOffering(userVm, vmSnapshotVO);
verify(_serviceOfferingDao).findByIdIncludingRemoved(TEST_VM_ID, SERVICE_OFFERING_ID);
verify(_serviceOfferingDao, never()).getComputeOffering(any(ServiceOfferingVO.class), any());
}
@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