This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to branch 4.20
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.20 by this push:
new 9ae696d1c82 Preserve VM settings on Instance Snapshot revert for
Custom Service Offering (#12555)
9ae696d1c82 is described below
commit 9ae696d1c82efb6ade3a365600f909bba2660197
Author: Abhisar Sinha <[email protected]>
AuthorDate: Tue Feb 3 18:45:09 2026 +0530
Preserve VM settings on Instance Snapshot revert for Custom Service
Offering (#12555)
---
.../cloud/vm/snapshot/VMSnapshotManagerImpl.java | 23 +++++++-----
.../cloud/vm/snapshot/VMSnapshotManagerTest.java | 42 +++++++++++++++-------
2 files changed, 44 insertions(+), 21 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 43270e6a029..cdbb7119e9e 100644
--- a/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
+++ b/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
@@ -23,6 +23,7 @@ import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import javax.inject.Inject;
import javax.naming.ConfigurationException;
@@ -182,6 +183,11 @@ public class VMSnapshotManagerImpl extends
MutualExclusiveIdsManagerBase impleme
Long.class, "vm.job.check.interval", "3000",
"Interval in milliseconds to check if the job is complete", false);
+ private static final Set<String>
VM_SNAPSHOT_CUSTOM_SERVICE_OFFERING_DETAILS = Set.of(
+ VmDetailConstants.CPU_NUMBER.toLowerCase(),
+ VmDetailConstants.CPU_SPEED.toLowerCase(),
+ VmDetailConstants.MEMORY.toLowerCase());
+
@Override
public boolean configure(String name, Map<String, Object> params) throws
ConfigurationException {
_name = name;
@@ -473,7 +479,8 @@ public class VMSnapshotManagerImpl extends
MutualExclusiveIdsManagerBase impleme
}
/**
- * Add entries on vm_snapshot_details if service offering is dynamic. This
will allow setting details when revert to vm snapshot
+ * Add entries about cpu, cpu_speed and memory in vm_snapshot_details if
service offering is dynamic.
+ * This will allow setting details when revert to vm snapshot.
* @param vmId vm id
* @param serviceOfferingId service offering id
* @param vmSnapshotId vm snapshot id
@@ -484,7 +491,7 @@ public class VMSnapshotManagerImpl extends
MutualExclusiveIdsManagerBase impleme
List<UserVmDetailVO> vmDetails =
_userVmDetailsDao.listDetails(vmId);
List<VMSnapshotDetailsVO> vmSnapshotDetails = new
ArrayList<VMSnapshotDetailsVO>();
for (UserVmDetailVO detail : vmDetails) {
-
if(detail.getName().equalsIgnoreCase(VmDetailConstants.CPU_NUMBER) ||
detail.getName().equalsIgnoreCase(VmDetailConstants.CPU_SPEED) ||
detail.getName().equalsIgnoreCase(VmDetailConstants.MEMORY)) {
+ if
(VM_SNAPSHOT_CUSTOM_SERVICE_OFFERING_DETAILS.contains(detail.getName().toLowerCase()))
{
vmSnapshotDetails.add(new
VMSnapshotDetailsVO(vmSnapshotId, detail.getName(), detail.getValue(),
detail.isDisplay()));
}
}
@@ -931,7 +938,7 @@ public class VMSnapshotManagerImpl extends
MutualExclusiveIdsManagerBase impleme
Transaction.execute(new
TransactionCallbackWithExceptionNoReturn<CloudRuntimeException>() {
@Override
public void doInTransactionWithoutResult(TransactionStatus
status) throws CloudRuntimeException {
- revertUserVmDetailsFromVmSnapshot(userVm, vmSnapshotVo);
+ revertCustomServiceOfferingDetailsFromVmSnapshot(userVm,
vmSnapshotVo);
updateUserVmServiceOffering(userVm, vmSnapshotVo);
}
});
@@ -943,19 +950,19 @@ public class VMSnapshotManagerImpl extends
MutualExclusiveIdsManagerBase impleme
}
/**
- * Update or add user vm details from vm snapshot for vms with custom
service offerings
+ * Update or add user vm details (cpu, cpu_speed and memory) from vm
snapshot for vms with custom service offerings
* @param userVm user vm
* @param vmSnapshotVo vm snapshot
*/
- protected void revertUserVmDetailsFromVmSnapshot(UserVmVO userVm,
VMSnapshotVO vmSnapshotVo) {
+ protected void revertCustomServiceOfferingDetailsFromVmSnapshot(UserVmVO
userVm, VMSnapshotVO vmSnapshotVo) {
ServiceOfferingVO serviceOfferingVO =
_serviceOfferingDao.findById(vmSnapshotVo.getServiceOfferingId());
if (serviceOfferingVO.isDynamic()) {
List<VMSnapshotDetailsVO> vmSnapshotDetails =
_vmSnapshotDetailsDao.listDetails(vmSnapshotVo.getId());
- List<UserVmDetailVO> userVmDetails = new
ArrayList<UserVmDetailVO>();
for (VMSnapshotDetailsVO detail : vmSnapshotDetails) {
- userVmDetails.add(new UserVmDetailVO(userVm.getId(),
detail.getName(), detail.getValue(), detail.isDisplay()));
+ if
(VM_SNAPSHOT_CUSTOM_SERVICE_OFFERING_DETAILS.contains(detail.getName().toLowerCase()))
{
+ _userVmDetailsDao.addDetail(userVm.getId(),
detail.getName(), detail.getValue(), detail.isDisplay());
+ }
}
- _userVmDetailsDao.saveDetails(userVmDetails);
}
}
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 06c917a1244..8d48fc4dac5 100644
--- a/server/src/test/java/com/cloud/vm/snapshot/VMSnapshotManagerTest.java
+++ b/server/src/test/java/com/cloud/vm/snapshot/VMSnapshotManagerTest.java
@@ -51,6 +51,7 @@ import com.cloud.vm.UserVmManager;
import com.cloud.vm.UserVmVO;
import com.cloud.vm.VirtualMachine.State;
import com.cloud.vm.VirtualMachineManager;
+import com.cloud.vm.VmDetailConstants;
import com.cloud.vm.dao.UserVmDao;
import com.cloud.vm.dao.UserVmDetailsDao;
import com.cloud.vm.dao.VMInstanceDao;
@@ -67,7 +68,6 @@ import org.junit.Before;
import org.junit.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.Captor;
-import org.mockito.ArgumentMatchers;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.mockito.Spy;
@@ -79,13 +79,18 @@ import java.util.List;
import java.util.Map;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -225,13 +230,13 @@ public class VMSnapshotManagerTest {
when(_serviceOfferingDao.findById(SERVICE_OFFERING_ID)).thenReturn(serviceOffering);
for (ResourceDetail detail : Arrays.asList(userVmDetailCpuNumber,
vmSnapshotDetailCpuNumber)) {
- when(detail.getName()).thenReturn("cpuNumber");
+ when(detail.getName()).thenReturn(VmDetailConstants.CPU_NUMBER);
when(detail.getValue()).thenReturn("2");
when(detail.isDisplay()).thenReturn(true);
}
for (ResourceDetail detail : Arrays.asList(userVmDetailMemory,
vmSnapshotDetailMemory)) {
- when(detail.getName()).thenReturn("memory");
+ when(detail.getName()).thenReturn(VmDetailConstants.MEMORY);
when(detail.getValue()).thenReturn("2048");
when(detail.isDisplay()).thenReturn(true);
}
@@ -348,12 +353,12 @@ public class VMSnapshotManagerTest {
@Test
public void testUpdateUserVmServiceOfferingDifferentServiceOffering()
throws ConcurrentOperationException, ResourceUnavailableException,
ManagementServerException, VirtualMachineMigrationException {
when(userVm.getServiceOfferingId()).thenReturn(SERVICE_OFFERING_DIFFERENT_ID);
-
when(_userVmManager.upgradeVirtualMachine(ArgumentMatchers.eq(TEST_VM_ID),
ArgumentMatchers.eq(SERVICE_OFFERING_ID),
mapDetailsCaptor.capture())).thenReturn(true);
+ when(_userVmManager.upgradeVirtualMachine(eq(TEST_VM_ID),
eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture())).thenReturn(true);
_vmSnapshotMgr.updateUserVmServiceOffering(userVm, vmSnapshotVO);
verify(_vmSnapshotMgr).changeUserVmServiceOffering(userVm,
vmSnapshotVO);
verify(_vmSnapshotMgr).getVmMapDetails(userVm);
-
verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(ArgumentMatchers.eq(userVm),
ArgumentMatchers.eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture());
+ verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(eq(userVm),
eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture());
}
@Test
@@ -368,18 +373,18 @@ public class VMSnapshotManagerTest {
@Test
public void testChangeUserVmServiceOffering() throws
ConcurrentOperationException, ResourceUnavailableException,
ManagementServerException, VirtualMachineMigrationException {
-
when(_userVmManager.upgradeVirtualMachine(ArgumentMatchers.eq(TEST_VM_ID),
ArgumentMatchers.eq(SERVICE_OFFERING_ID),
mapDetailsCaptor.capture())).thenReturn(true);
+ when(_userVmManager.upgradeVirtualMachine(eq(TEST_VM_ID),
eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture())).thenReturn(true);
_vmSnapshotMgr.changeUserVmServiceOffering(userVm, vmSnapshotVO);
verify(_vmSnapshotMgr).getVmMapDetails(userVm);
-
verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(ArgumentMatchers.eq(userVm),
ArgumentMatchers.eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture());
+ verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(eq(userVm),
eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture());
}
@Test(expected=CloudRuntimeException.class)
public void
testChangeUserVmServiceOfferingFailOnUpgradeVMServiceOffering() throws
ConcurrentOperationException, ResourceUnavailableException,
ManagementServerException, VirtualMachineMigrationException {
-
when(_userVmManager.upgradeVirtualMachine(ArgumentMatchers.eq(TEST_VM_ID),
ArgumentMatchers.eq(SERVICE_OFFERING_ID),
mapDetailsCaptor.capture())).thenReturn(false);
+ when(_userVmManager.upgradeVirtualMachine(eq(TEST_VM_ID),
eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture())).thenReturn(false);
_vmSnapshotMgr.changeUserVmServiceOffering(userVm, vmSnapshotVO);
verify(_vmSnapshotMgr).getVmMapDetails(userVm);
-
verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(ArgumentMatchers.eq(userVm),
ArgumentMatchers.eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture());
+ verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(eq(userVm),
eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture());
}
@Test
@@ -396,16 +401,27 @@ public class VMSnapshotManagerTest {
@Test
public void
testRevertUserVmDetailsFromVmSnapshotNotDynamicServiceOffering() {
- _vmSnapshotMgr.revertUserVmDetailsFromVmSnapshot(vmMock, vmSnapshotVO);
+
_vmSnapshotMgr.revertCustomServiceOfferingDetailsFromVmSnapshot(vmMock,
vmSnapshotVO);
verify(_vmSnapshotDetailsDao, never()).listDetails(anyLong());
}
@Test
public void testRevertUserVmDetailsFromVmSnapshotDynamicServiceOffering() {
when(serviceOffering.isDynamic()).thenReturn(true);
- _vmSnapshotMgr.revertUserVmDetailsFromVmSnapshot(vmMock, vmSnapshotVO);
+ VMSnapshotDetailsVO uefiSnapshotDetail = new
VMSnapshotDetailsVO(VM_SNAPSHOT_ID, "UEFI", "SECURE", true);
+ List<VMSnapshotDetailsVO> snapshotDetailsWithUefi = Arrays.asList(
+ vmSnapshotDetailCpuNumber, vmSnapshotDetailMemory,
uefiSnapshotDetail);
+
when(_vmSnapshotDetailsDao.listDetails(VM_SNAPSHOT_ID)).thenReturn(snapshotDetailsWithUefi);
+
+
_vmSnapshotMgr.revertCustomServiceOfferingDetailsFromVmSnapshot(vmMock,
vmSnapshotVO);
+
verify(_vmSnapshotDetailsDao).listDetails(VM_SNAPSHOT_ID);
-
verify(_userVmDetailsDao).saveDetails(listUserVmDetailsCaptor.capture());
+ verify(_userVmDetailsDao, never()).saveDetails(any());
+ ArgumentCaptor<String> detailNameCaptor =
ArgumentCaptor.forClass(String.class);
+ verify(_userVmDetailsDao, times(2)).addDetail(eq(TEST_VM_ID),
detailNameCaptor.capture(), anyString(), anyBoolean());
+ List<String> appliedNames = detailNameCaptor.getAllValues();
+ assertTrue(appliedNames.contains(VmDetailConstants.CPU_NUMBER));
+ assertTrue(appliedNames.contains(VmDetailConstants.MEMORY));
+ assertFalse("UEFI must not be applied from snapshot so that existing
UEFI setting is preserved", appliedNames.contains("UEFI"));
}
-
}
