diff --git a/PendingReleaseNotes b/PendingReleaseNotes index 9670b6e7c13a..fb382e4e8478 100644 --- a/PendingReleaseNotes +++ b/PendingReleaseNotes @@ -39,3 +39,12 @@ example.ver.1 > example.ver.2: which can now be attached to Instances. This is to prevent the Secondary Storage to grow to enormous sizes as Linux Distributions keep growing in size while a stripped down Linux should fit on a 2.88MB floppy. + +4.22.0.0 > 4.22.0.1: + * Disk-only instance snapshots for KVM UEFI VMs now include a sidecar copy of + the active NVRAM state so revert operations restore both disk and firmware + boot state consistently. + + * UEFI disk-only instance snapshots taken before this change do not contain an + NVRAM sidecar and cannot be safely reverted. Take a new snapshot after + upgrading before relying on revert for UEFI VMs. diff --git a/agent/conf/agent.properties b/agent/conf/agent.properties index 013c2e5bf2af..ba4a3874664a 100644 --- a/agent/conf/agent.properties +++ b/agent/conf/agent.properties @@ -472,4 +472,3 @@ iscsi.session.cleanup.enabled=false # Optional vCenter SHA1 thumbprint for VMware to KVM conversion via VDDK, passed as # -io vddk-thumbprint=. If unset, CloudStack computes it on the KVM host via openssl. #vddk.thumbprint= - diff --git a/api/src/main/java/com/cloud/host/Host.java b/api/src/main/java/com/cloud/host/Host.java index b52348201516..a7b89b8a2b85 100644 --- a/api/src/main/java/com/cloud/host/Host.java +++ b/api/src/main/java/com/cloud/host/Host.java @@ -55,6 +55,7 @@ public static String[] toStrings(Host.Type... types) { } String HOST_UEFI_ENABLE = "host.uefi.enable"; + String HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM = "host.kvm.diskonlyvmsnapshot.nvram"; String HOST_VOLUME_ENCRYPTION = "host.volume.encryption"; String HOST_INSTANCE_CONVERSION = "host.instance.conversion"; String HOST_VDDK_SUPPORT = "host.vddk.support"; diff --git a/core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotAnswer.java b/core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotAnswer.java index 4d61249c7cbc..ffa4eaff2963 100644 --- a/core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotAnswer.java @@ -27,13 +27,24 @@ public class CreateDiskOnlyVmSnapshotAnswer extends Answer { protected Map> mapVolumeToSnapshotSizeAndNewVolumePath; + private String nvramSnapshotPath; public CreateDiskOnlyVmSnapshotAnswer(Command command, boolean success, String details, Map> mapVolumeToSnapshotSizeAndNewVolumePath) { + this(command, success, details, mapVolumeToSnapshotSizeAndNewVolumePath, null); + } + + public CreateDiskOnlyVmSnapshotAnswer(Command command, boolean success, String details, Map> mapVolumeToSnapshotSizeAndNewVolumePath, + String nvramSnapshotPath) { super(command, success, details); this.mapVolumeToSnapshotSizeAndNewVolumePath = mapVolumeToSnapshotSizeAndNewVolumePath; + this.nvramSnapshotPath = nvramSnapshotPath; } public Map> getMapVolumeToSnapshotSizeAndNewVolumePath() { return mapVolumeToSnapshotSizeAndNewVolumePath; } + + public String getNvramSnapshotPath() { + return nvramSnapshotPath; + } } diff --git a/core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotCommand.java b/core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotCommand.java index 952bf0c971de..7f328bab81db 100644 --- a/core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotCommand.java +++ b/core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotCommand.java @@ -29,13 +29,30 @@ public class CreateDiskOnlyVmSnapshotCommand extends VMSnapshotBaseCommand { protected VirtualMachine.State vmState; + private final String vmUuid; + private final boolean uefiEnabled; public CreateDiskOnlyVmSnapshotCommand(String vmName, VMSnapshotTO snapshot, List volumeTOs, String guestOSType, VirtualMachine.State vmState) { + this(vmName, null, snapshot, volumeTOs, guestOSType, vmState, false); + } + + public CreateDiskOnlyVmSnapshotCommand(String vmName, String vmUuid, VMSnapshotTO snapshot, List volumeTOs, String guestOSType, + VirtualMachine.State vmState, boolean uefiEnabled) { super(vmName, snapshot, volumeTOs, guestOSType); + this.vmUuid = vmUuid; this.vmState = vmState; + this.uefiEnabled = uefiEnabled; } public VirtualMachine.State getVmState() { return vmState; } + + public String getVmUuid() { + return vmUuid; + } + + public boolean isUefiEnabled() { + return uefiEnabled; + } } diff --git a/core/src/main/java/com/cloud/agent/api/storage/DeleteDiskOnlyVmSnapshotCommand.java b/core/src/main/java/com/cloud/agent/api/storage/DeleteDiskOnlyVmSnapshotCommand.java index bf7bdd597360..1ac0a0ff719b 100644 --- a/core/src/main/java/com/cloud/agent/api/storage/DeleteDiskOnlyVmSnapshotCommand.java +++ b/core/src/main/java/com/cloud/agent/api/storage/DeleteDiskOnlyVmSnapshotCommand.java @@ -19,24 +19,43 @@ package com.cloud.agent.api.storage; import com.cloud.agent.api.Command; - import com.cloud.agent.api.to.DataTO; - +import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; import java.util.List; public class DeleteDiskOnlyVmSnapshotCommand extends Command { - List snapshots; + private final List snapshots; + private final String nvramSnapshotPath; + private final PrimaryDataStoreTO primaryDataStore; public DeleteDiskOnlyVmSnapshotCommand(List snapshots) { + this(snapshots, null); + } + + public DeleteDiskOnlyVmSnapshotCommand(List snapshots, String nvramSnapshotPath) { + this(snapshots, nvramSnapshotPath, null); + } + + public DeleteDiskOnlyVmSnapshotCommand(List snapshots, String nvramSnapshotPath, PrimaryDataStoreTO primaryDataStore) { this.snapshots = snapshots; + this.nvramSnapshotPath = nvramSnapshotPath; + this.primaryDataStore = primaryDataStore; } public List getSnapshots() { return snapshots; } + public String getNvramSnapshotPath() { + return nvramSnapshotPath; + } + + public PrimaryDataStoreTO getPrimaryDataStore() { + return primaryDataStore; + } + @Override public boolean executeInSequence() { return false; diff --git a/core/src/main/java/com/cloud/agent/api/storage/RevertDiskOnlyVmSnapshotCommand.java b/core/src/main/java/com/cloud/agent/api/storage/RevertDiskOnlyVmSnapshotCommand.java index 72bb92bcb10d..3c9859aa44af 100644 --- a/core/src/main/java/com/cloud/agent/api/storage/RevertDiskOnlyVmSnapshotCommand.java +++ b/core/src/main/java/com/cloud/agent/api/storage/RevertDiskOnlyVmSnapshotCommand.java @@ -27,11 +27,21 @@ public class RevertDiskOnlyVmSnapshotCommand extends Command { private List snapshotObjectTos; private String vmName; + private final String vmUuid; + private final boolean uefiEnabled; + private final String nvramSnapshotPath; public RevertDiskOnlyVmSnapshotCommand(List snapshotObjectTos, String vmName) { + this(snapshotObjectTos, vmName, null, false, null); + } + + public RevertDiskOnlyVmSnapshotCommand(List snapshotObjectTos, String vmName, String vmUuid, boolean uefiEnabled, String nvramSnapshotPath) { super(); this.snapshotObjectTos = snapshotObjectTos; this.vmName = vmName; + this.vmUuid = vmUuid; + this.uefiEnabled = uefiEnabled; + this.nvramSnapshotPath = nvramSnapshotPath; } public List getSnapshotObjectTos() { @@ -42,6 +52,18 @@ public String getVmName() { return vmName; } + public String getVmUuid() { + return vmUuid; + } + + public boolean isUefiEnabled() { + return uefiEnabled; + } + + public String getNvramSnapshotPath() { + return nvramSnapshotPath; + } + @Override public boolean executeInSequence() { return false; diff --git a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java index 8c69dcdc4828..ecb789a15bd5 100644 --- a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java @@ -108,10 +108,12 @@ import com.cloud.exception.UnsupportedVersionException; import com.cloud.ha.HighAvailabilityManager; import com.cloud.host.Host; +import com.cloud.host.DetailVO; import com.cloud.host.HostVO; import com.cloud.host.Status; import com.cloud.host.Status.Event; import com.cloud.host.dao.HostDao; +import com.cloud.host.dao.HostDetailsDao; import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.hypervisor.HypervisorGuruManager; import com.cloud.org.Cluster; @@ -167,6 +169,8 @@ public class AgentManagerImpl extends ManagerBase implements AgentManager, Handl @Inject protected HostDao _hostDao = null; @Inject + protected HostDetailsDao _hostDetailsDao = null; + @Inject private ManagementServerHostDao _mshostDao; @Inject protected OutOfBandManagementDao outOfBandManagementDao; @@ -802,18 +806,24 @@ protected AgentAttache notifyMonitorsOfConnection(final AgentAttache attache, fi ReadyAnswer readyAnswer = (ReadyAnswer)answer; Map detailsMap = readyAnswer.getDetailsMap(); if (detailsMap != null) { + _hostDao.loadDetails(host); + if (host.getDetails() == null) { + host.setDetails(new HashMap<>()); + } String uefiEnabled = detailsMap.get(Host.HOST_UEFI_ENABLE); + String diskOnlyVmSnapshotNvramSupport = detailsMap.get(Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM); String virtv2vVersion = detailsMap.get(Host.HOST_VIRTV2V_VERSION); String ovftoolVersion = detailsMap.get(Host.HOST_OVFTOOL_VERSION); String vddkSupport = detailsMap.get(Host.HOST_VDDK_SUPPORT); String vddkLibDir = detailsMap.get(Host.HOST_VDDK_LIB_DIR); String vddkVersion = detailsMap.get(Host.HOST_VDDK_VERSION); logger.debug("Got HOST_UEFI_ENABLE [{}] for host [{}]:", uefiEnabled, host); - if (ObjectUtils.anyNotNull(uefiEnabled, virtv2vVersion, ovftoolVersion, vddkSupport, vddkLibDir, vddkVersion)) { - _hostDao.loadDetails(host); + if (ObjectUtils.anyNotNull(uefiEnabled, diskOnlyVmSnapshotNvramSupport, virtv2vVersion, ovftoolVersion, vddkSupport, vddkLibDir, vddkVersion)) { boolean updateNeeded = false; - if (StringUtils.isNotBlank(uefiEnabled) && !uefiEnabled.equals(host.getDetails().get(Host.HOST_UEFI_ENABLE))) { - host.getDetails().put(Host.HOST_UEFI_ENABLE, uefiEnabled); + if (syncBooleanHostCapability(host, Host.HOST_UEFI_ENABLE, uefiEnabled)) { + updateNeeded = true; + } + if (syncBooleanHostCapability(host, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM, diskOnlyVmSnapshotNvramSupport)) { updateNeeded = true; } if (StringUtils.isNotBlank(virtv2vVersion) && !virtv2vVersion.equals(host.getDetails().get(Host.HOST_VIRTV2V_VERSION))) { @@ -856,6 +866,26 @@ protected AgentAttache notifyMonitorsOfConnection(final AgentAttache attache, fi return attache; } + protected boolean syncBooleanHostCapability(HostVO host, String capabilityName, String advertisedValue) { + if (StringUtils.isNotBlank(advertisedValue)) { + if (!advertisedValue.equals(host.getDetails().get(capabilityName))) { + host.getDetails().put(capabilityName, advertisedValue); + return true; + } + return false; + } + + if (host.getDetails().containsKey(capabilityName)) { + host.getDetails().remove(capabilityName); + DetailVO hostDetail = _hostDetailsDao.findDetail(host.getId(), capabilityName); + if (hostDetail != null) { + _hostDetailsDao.remove(hostDetail.getId()); + } + return true; + } + return false; + } + @Override public boolean start() { ManagementServerHostVO msHost = _mshostDao.findByMsid(_nodeId); diff --git a/engine/orchestration/src/test/java/com/cloud/agent/manager/AgentManagerImplTest.java b/engine/orchestration/src/test/java/com/cloud/agent/manager/AgentManagerImplTest.java index 43d83a672c0f..2377bbaefbc8 100644 --- a/engine/orchestration/src/test/java/com/cloud/agent/manager/AgentManagerImplTest.java +++ b/engine/orchestration/src/test/java/com/cloud/agent/manager/AgentManagerImplTest.java @@ -18,14 +18,17 @@ import com.cloud.agent.Listener; import com.cloud.agent.api.Answer; +import com.cloud.agent.api.ReadyAnswer; import com.cloud.agent.api.ReadyCommand; import com.cloud.agent.api.StartupCommand; import com.cloud.agent.api.StartupRoutingCommand; import com.cloud.exception.ConnectionException; +import com.cloud.host.DetailVO; import com.cloud.host.Host; import com.cloud.host.HostVO; import com.cloud.host.Status; import com.cloud.host.dao.HostDao; +import com.cloud.host.dao.HostDetailsDao; import com.cloud.hypervisor.Hypervisor; import com.cloud.utils.Pair; import org.junit.Assert; @@ -34,10 +37,13 @@ import org.mockito.Mockito; import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; public class AgentManagerImplTest { private HostDao hostDao; + private HostDetailsDao hostDetailsDao; private Listener storagePoolMonitor; private AgentAttache attache; private AgentManagerImpl mgr = Mockito.spy(new AgentManagerImpl()); @@ -46,15 +52,18 @@ public class AgentManagerImplTest { @Before public void setUp() throws Exception { - host = new HostVO("some-Uuid"); + host = Mockito.spy(new HostVO("some-Uuid")); + Mockito.when(host.getId()).thenReturn(1L); host.setDataCenterId(1L); cmds = new StartupCommand[]{new StartupRoutingCommand()}; attache = new ConnectedAgentAttache(null, 1L, "uuid", "kvm-attache", Hypervisor.HypervisorType.KVM, null, false); hostDao = Mockito.mock(HostDao.class); + hostDetailsDao = Mockito.mock(HostDetailsDao.class); storagePoolMonitor = Mockito.mock(Listener.class); mgr._hostDao = hostDao; + mgr._hostDetailsDao = hostDetailsDao; mgr._hostMonitors = new ArrayList<>(); mgr._hostMonitors.add(new Pair<>(0, storagePoolMonitor)); } @@ -86,6 +95,32 @@ public void testNotifyMonitorsOfConnectionWhenStoragePoolConnectionHostFailure() Mockito.verify(mgr, Mockito.times(1)).handleDisconnectWithoutInvestigation(Mockito.any(attache.getClass()), Mockito.eq(Status.Event.AgentDisconnected), Mockito.eq(true), Mockito.eq(true)); } + @Test + public void testNotifyMonitorsOfConnectionClearsStaleNvramCapabilityOnReconnect() throws ConnectionException { + DetailVO staleNvramCapability = Mockito.mock(DetailVO.class); + ReadyAnswer readyAnswer = Mockito.mock(ReadyAnswer.class); + host.setDetails(new HashMap<>(Map.of(Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString(), + Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM, Boolean.TRUE.toString()))); + + Mockito.when(staleNvramCapability.getId()).thenReturn(11L); + Mockito.when(hostDao.findById(Mockito.anyLong())).thenReturn(host); + Mockito.doNothing().when(hostDao).loadDetails(host); + Mockito.when(hostDetailsDao.findDetail(host.getId(), Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)).thenReturn(staleNvramCapability); + Mockito.doNothing().when(storagePoolMonitor).processConnect(Mockito.eq(host), Mockito.eq(cmds[0]), Mockito.eq(false)); + Mockito.doReturn(true).when(mgr).handleDisconnectWithoutInvestigation(Mockito.any(attache.getClass()), Mockito.any(Status.Event.class), Mockito.anyBoolean(), Mockito.anyBoolean()); + Mockito.when(readyAnswer.getResult()).thenReturn(true); + Mockito.when(readyAnswer.getDetailsMap()).thenReturn(Map.of(Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString())); + Mockito.doReturn(readyAnswer).when(mgr).easySend(Mockito.anyLong(), Mockito.any(ReadyCommand.class)); + Mockito.doReturn(true).when(mgr).agentStatusTransitTo(Mockito.eq(host), Mockito.eq(Status.Event.Ready), Mockito.anyLong()); + + final AgentAttache agentAttache = mgr.notifyMonitorsOfConnection(attache, cmds, false); + + Assert.assertTrue(agentAttache.isReady()); + Assert.assertFalse(host.getDetails().containsKey(Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)); + Mockito.verify(hostDetailsDao).remove(11L); + Mockito.verify(hostDao).saveDetails(host); + } + @Test public void testGetTimeoutWithPositiveTimeout() { Commands commands = Mockito.mock(Commands.class); diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java index 003065e394f5..ad2be6f4fb84 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java @@ -28,9 +28,13 @@ import com.cloud.agent.api.storage.RevertDiskOnlyVmSnapshotCommand; import com.cloud.agent.api.storage.SnapshotMergeTreeTO; import com.cloud.agent.api.to.DataTO; +import com.cloud.alert.AlertManager; import com.cloud.configuration.Resource; import com.cloud.event.EventTypes; import com.cloud.event.UsageEventUtils; +import com.cloud.host.DetailVO; +import com.cloud.host.Host; +import com.cloud.host.dao.HostDetailsDao; import com.cloud.hypervisor.Hypervisor; import com.cloud.storage.DataStoreRole; import com.cloud.storage.Snapshot; @@ -46,9 +50,11 @@ import com.cloud.utils.fsm.NoTransitionException; import com.cloud.vm.UserVmVO; import com.cloud.vm.VirtualMachine; +import com.cloud.vm.dao.VMInstanceDetailsDao; import com.cloud.vm.snapshot.VMSnapshot; import com.cloud.vm.snapshot.VMSnapshotDetailsVO; import com.cloud.vm.snapshot.VMSnapshotVO; +import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.backup.BackupOfferingVO; import org.apache.cloudstack.backup.dao.BackupOfferingDao; import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; @@ -59,10 +65,12 @@ import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; import org.apache.cloudstack.storage.snapshot.SnapshotObject; import org.apache.cloudstack.storage.to.SnapshotObjectTO; import org.apache.cloudstack.storage.to.VolumeObjectTO; import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.lang3.StringUtils; import javax.inject.Inject; import java.util.ArrayList; @@ -76,6 +84,7 @@ public class KvmFileBasedStorageVmSnapshotStrategy extends StorageVMSnapshotStrategy { private static final List supportedStoragePoolTypes = List.of(Storage.StoragePoolType.Filesystem, Storage.StoragePoolType.NetworkFilesystem, Storage.StoragePoolType.SharedMountPoint); + private static final String KVM_FILE_BASED_STORAGE_SNAPSHOT_NVRAM = "kvmFileBasedStorageSnapshotNvram"; @Inject protected SnapshotDataStoreDao snapshotDataStoreDao; @@ -86,6 +95,15 @@ public class KvmFileBasedStorageVmSnapshotStrategy extends StorageVMSnapshotStra @Inject protected BackupOfferingDao backupOfferingDao; + @Inject + protected VMInstanceDetailsDao vmInstanceDetailsDao; + + @Inject + protected HostDetailsDao hostDetailsDao; + + @Inject + protected AlertManager alertManager; + @Override public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { Map volumeInfoToSnapshotObjectMap = new HashMap<>(); @@ -117,6 +135,7 @@ public boolean deleteVMSnapshot(VMSnapshot vmSnapshot) { UserVmVO userVm = userVmDao.findById(vmSnapshot.getVmId()); VMSnapshotVO vmSnapshotBeingDeleted = (VMSnapshotVO) vmSnapshot; Long hostId = vmSnapshotHelper.pickRunningHost(vmSnapshotBeingDeleted.getVmId()); + validateHostSupportsNvramSidecarCleanup(vmSnapshotBeingDeleted, hostId, "delete"); long virtualSize = 0; boolean isCurrent = vmSnapshotBeingDeleted.getCurrent(); @@ -124,6 +143,7 @@ public boolean deleteVMSnapshot(VMSnapshot vmSnapshot) { List volumeTOs = vmSnapshotHelper.getVolumeTOList(vmSnapshotBeingDeleted.getVmId()); List snapshotChildren = vmSnapshotDao.listByParentAndStateIn(vmSnapshotBeingDeleted.getId(), VMSnapshot.State.Ready, VMSnapshot.State.Hidden); + PrimaryDataStoreTO nvramPrimaryDataStore = getPrimaryDataStoreForNvramCleanup(vmSnapshotBeingDeleted, volumeTOs); long realSize = getVMSnapshotRealSize(vmSnapshotBeingDeleted); int numberOfChildren = snapshotChildren.size(); @@ -157,6 +177,8 @@ public boolean deleteVMSnapshot(VMSnapshot vmSnapshot) { return true; } + deleteNvramSnapshotIfNeeded(vmSnapshotBeingDeleted, hostId, nvramPrimaryDataStore); + transitStateWithoutThrow(vmSnapshotBeingDeleted, VMSnapshot.Event.OperationSucceeded); vmSnapshotDetailsDao.removeDetails(vmSnapshotBeingDeleted.getId()); @@ -176,6 +198,7 @@ public boolean revertVMSnapshot(VMSnapshot vmSnapshot) { VMSnapshotVO vmSnapshotBeingReverted = (VMSnapshotVO) vmSnapshot; Long hostId = vmSnapshotHelper.pickRunningHost(vmSnapshotBeingReverted.getVmId()); + validateHostSupportsUefiNvramAwareDiskOnlySnapshots(hostId, userVm, "revert"); transitStateWithoutThrow(vmSnapshotBeingReverted, VMSnapshot.Event.RevertRequested); @@ -184,7 +207,9 @@ public boolean revertVMSnapshot(VMSnapshot vmSnapshot) { .map(snapshot -> (SnapshotObjectTO) snapshotDataFactory.getSnapshot(snapshot.getSnapshotId(), snapshot.getDataStoreId(), DataStoreRole.Primary).getTO()) .collect(Collectors.toList()); - RevertDiskOnlyVmSnapshotCommand revertDiskOnlyVMSnapshotCommand = new RevertDiskOnlyVmSnapshotCommand(volumeSnapshotTos, userVm.getName()); + RevertDiskOnlyVmSnapshotCommand revertDiskOnlyVMSnapshotCommand = + new RevertDiskOnlyVmSnapshotCommand(volumeSnapshotTos, userVm.getName(), userVm.getUuid(), isUefiVm(userVm), + getNvramSnapshotPath(vmSnapshotBeingReverted)); Answer answer = agentMgr.easySend(hostId, revertDiskOnlyVMSnapshotCommand); if (answer == null || !answer.getResult()) { @@ -204,6 +229,11 @@ public boolean revertVMSnapshot(VMSnapshot vmSnapshot) { publishUsageEvent(EventTypes.EVENT_VM_SNAPSHOT_REVERT, vmSnapshotBeingReverted, userVm, volumeObjectTo); } + if (isUefiVm(userVm)) { + userVm.setLastHostId(hostId); + userVmDao.update(userVm.getId(), userVm); + } + transitStateWithoutThrow(vmSnapshotBeingReverted, VMSnapshot.Event.OperationSucceeded); VMSnapshotVO currentVmSnapshot = vmSnapshotDao.findCurrentSnapshotByVmId(userVm.getId()); @@ -248,6 +278,8 @@ private void mergeOldSiblingWithOldParentIfOldParentIsDead(VMSnapshotVO oldParen return; } + validateHostSupportsNvramSidecarCleanup(oldParent, hostId, "clean up"); + PrimaryDataStoreTO nvramPrimaryDataStore = getPrimaryDataStoreForNvramCleanup(oldParent, volumeTOs); List snapshotVos; if (oldParent.getCurrent()) { @@ -276,6 +308,8 @@ private void mergeOldSiblingWithOldParentIfOldParentIsDead(VMSnapshotVO oldParen snapshotDao.update(snapshotVO.getId(), snapshotVO); } + deleteNvramSnapshotIfNeeded(oldParent, hostId, nvramPrimaryDataStore); + vmSnapshotDetailsDao.removeDetails(oldParent.getId()); oldParent.setRemoved(DateUtil.now()); @@ -347,12 +381,13 @@ public StrategyPriority canHandle(Long vmId, Long rootPoolId, boolean snapshotMe } private List deleteSnapshot(VMSnapshotVO vmSnapshotVO, Long hostId) { + validateHostSupportsNvramSidecarCleanup(vmSnapshotVO, hostId, "delete"); List volumeSnapshots = getVolumeSnapshotsAssociatedWithVmSnapshot(vmSnapshotVO); List volumeSnapshotTOList = volumeSnapshots.stream() .map(snapshotDataStoreVO -> snapshotDataFactory.getSnapshot(snapshotDataStoreVO.getSnapshotId(), snapshotDataStoreVO.getDataStoreId(), DataStoreRole.Primary).getTO()) .collect(Collectors.toList()); - DeleteDiskOnlyVmSnapshotCommand deleteSnapshotCommand = new DeleteDiskOnlyVmSnapshotCommand(volumeSnapshotTOList); + DeleteDiskOnlyVmSnapshotCommand deleteSnapshotCommand = new DeleteDiskOnlyVmSnapshotCommand(volumeSnapshotTOList, getNvramSnapshotPath(vmSnapshotVO)); Answer answer = agentMgr.easySend(hostId, deleteSnapshotCommand); if (answer == null || !answer.getResult()) { logger.error("Failed to delete VM snapshot [{}] due to {}.", vmSnapshotVO.getUuid(), answer != null ? answer.getDetails() : "Communication failure"); @@ -368,6 +403,21 @@ private List deleteSnapshot(VMSnapshotVO vmSnapshotVO, Long hostId) return snapshotVOList; } + protected void deleteNvramSnapshotIfNeeded(VMSnapshotVO vmSnapshotVO, Long hostId, PrimaryDataStoreTO primaryDataStore) { + String nvramSnapshotPath = getNvramSnapshotPath(vmSnapshotVO); + if (StringUtils.isBlank(nvramSnapshotPath) || primaryDataStore == null) { + return; + } + + validateHostSupportsNvramSidecarCleanup(vmSnapshotVO, hostId, "delete"); + DeleteDiskOnlyVmSnapshotCommand deleteSnapshotCommand = new DeleteDiskOnlyVmSnapshotCommand(List.of(), nvramSnapshotPath, primaryDataStore); + Answer answer = agentMgr.easySend(hostId, deleteSnapshotCommand); + if (answer == null || !answer.getResult()) { + logger.warn("Failed to delete the NVRAM sidecar of VM snapshot [{}] due to {}.", vmSnapshotVO.getUuid(), + answer != null ? answer.getDetails() : "communication failure"); + } + } + private List mergeSnapshots(VMSnapshotVO vmSnapshotVO, VMSnapshotVO childSnapshot, UserVmVO userVm, List volumeObjectTOS, Long hostId) { logger.debug("Merging VM snapshot [{}] with its child [{}].", vmSnapshotVO.getUuid(), childSnapshot.getUuid()); @@ -471,6 +521,7 @@ protected VMSnapshot takeVmSnapshotInternal(VMSnapshot vmSnapshot, Map volumeTOs = vmSnapshotHelper.getVolumeTOList(userVm.getId()); @@ -493,14 +544,18 @@ protected VMSnapshot takeVmSnapshotInternal(VMSnapshot vmSnapshot, Map volumeTOs) { + return (PrimaryDataStoreTO) volumeTOs.stream() + .filter(volumeObjectTO -> Volume.Type.ROOT.equals(volumeObjectTO.getVolumeType())) + .findFirst() + .orElseThrow(() -> new CloudRuntimeException("Failed to locate the root volume while handling the VM snapshot.")) + .getDataStore(); + } + + protected PrimaryDataStoreTO getRootVolumePrimaryDataStoreForCleanup(VMSnapshotVO vmSnapshot, List volumeTOs) { + try { + return getRootVolumePrimaryDataStore(volumeTOs); + } catch (CloudRuntimeException e) { + logger.warn("Failed to locate the root volume while cleaning up the NVRAM sidecar for VM snapshot [{}].", vmSnapshot.getUuid(), e); + return null; + } + } + + protected PrimaryDataStoreTO getPrimaryDataStoreForNvramCleanup(VMSnapshotVO vmSnapshot, List volumeTOs) { + PrimaryDataStoreTO rootSnapshotPrimaryDataStore = getRootSnapshotPrimaryDataStoreForCleanup(vmSnapshot); + return rootSnapshotPrimaryDataStore != null ? rootSnapshotPrimaryDataStore : getRootVolumePrimaryDataStoreForCleanup(vmSnapshot, volumeTOs); + } + + protected PrimaryDataStoreTO getRootSnapshotPrimaryDataStoreForCleanup(VMSnapshotVO vmSnapshot) { + try { + return (PrimaryDataStoreTO) getVolumeSnapshotsAssociatedWithVmSnapshot(vmSnapshot).stream() + .map(snapshotDataStoreVO -> (SnapshotObjectTO) snapshotDataFactory.getSnapshot(snapshotDataStoreVO.getSnapshotId(), + snapshotDataStoreVO.getDataStoreId(), DataStoreRole.Primary).getTO()) + .filter(snapshotObjectTO -> Volume.Type.ROOT.equals(snapshotObjectTO.getVolume().getVolumeType())) + .findFirst() + .orElseThrow(() -> new CloudRuntimeException("Failed to locate the root volume snapshot while handling the VM snapshot.")) + .getDataStore(); + } catch (CloudRuntimeException e) { + logger.warn("Failed to locate the root volume snapshot while cleaning up the NVRAM sidecar for VM snapshot [{}].", vmSnapshot.getUuid(), e); + return null; + } + } + + protected String getNvramSnapshotPath(VMSnapshotVO vmSnapshot) { + VMSnapshotDetailsVO nvramDetail = vmSnapshotDetailsDao.findDetail(vmSnapshot.getId(), KVM_FILE_BASED_STORAGE_SNAPSHOT_NVRAM); + return nvramDetail != null ? nvramDetail.getValue() : null; + } + + protected void validateHostSupportsUefiNvramAwareDiskOnlySnapshots(Long hostId, UserVm userVm, String operation) { + if (!isUefiVm(userVm)) { + return; + } + + if (!isHostCapabilityEnabled(hostId, Host.HOST_UEFI_ENABLE)) { + throw new CloudRuntimeException(String.format("Cannot %s a disk-only snapshot for UEFI VM [%s] on host [%s] because the host does not advertise " + + "UEFI support. Ensure the host is configured with UEFI support and retry.", operation, userVm.getUuid(), hostId)); + } + + if (!isHostCapabilityEnabled(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)) { + throw new CloudRuntimeException(String.format("Cannot %s a disk-only snapshot for UEFI VM [%s] on host [%s] because the KVM agent does not advertise " + + "NVRAM-aware disk-only snapshot support. Upgrade the host and retry.", operation, userVm.getUuid(), hostId)); + } + } + + protected boolean isHostCapabilityEnabled(Long hostId, String capabilityName) { + DetailVO hostCapability = hostDetailsDao.findDetail(hostId, capabilityName); + return hostCapability != null && Boolean.parseBoolean(hostCapability.getValue()); + } + + protected void validateHostSupportsNvramSidecarCleanup(VMSnapshotVO vmSnapshotVO, Long hostId, String operation) { + if (StringUtils.isBlank(getNvramSnapshotPath(vmSnapshotVO))) { + return; + } + + if (!isHostCapabilityEnabled(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)) { + throw new CloudRuntimeException(String.format("Cannot %s VM snapshot [%s] on host [%s] because the KVM agent does not advertise " + + "NVRAM-aware disk-only snapshot support and the snapshot has an NVRAM sidecar that must be cleaned up. Upgrade the host and retry.", + operation, vmSnapshotVO.getUuid(), hostId)); + } + } + + protected void notifyGuestRecoveryIssueIfNeeded(CreateDiskOnlyVmSnapshotAnswer answer, UserVm userVm, VMSnapshotVO vmSnapshot) { + if (StringUtils.isBlank(answer.getDetails())) { + return; + } + + String subject = String.format("Disk-only VM snapshot [%s] completed with guest recovery warnings", vmSnapshot.getUuid()); + String message = String.format("Disk-only VM snapshot [%s] for UEFI VM [%s] completed, but post-snapshot guest recovery reported: %s", + vmSnapshot.getUuid(), userVm.getUuid(), answer.getDetails()); + logger.error(message); + try { + alertManager.sendAlert(AlertManager.AlertType.ALERT_TYPE_VM_SNAPSHOT, userVm.getDataCenterId(), userVm.getPodIdToDeployIn(), subject, message); + } catch (Exception e) { + logger.warn("Failed to send post-snapshot guest recovery alert for VM snapshot [{}].", vmSnapshot.getUuid(), e); + } + } + /** * Given a list of VM snapshots, will remove any that are part of the current direct backing chain (all the direct ancestors of the current vm snapshot). * This is done because, when using virDomainBlockCommit}, Libvirt will maintain diff --git a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategyTest.java b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategyTest.java new file mode 100644 index 000000000000..aeed1ab91974 --- /dev/null +++ b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategyTest.java @@ -0,0 +1,413 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.cloudstack.storage.vmsnapshot; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.lang.reflect.Method; +import java.util.Collections; +import java.util.List; + +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; +import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO; +import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; +import org.apache.cloudstack.storage.to.SnapshotObjectTO; +import org.apache.cloudstack.storage.to.VolumeObjectTO; +import org.apache.cloudstack.api.ApiConstants; +import org.junit.Before; +import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; + +import com.cloud.agent.AgentManager; +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.storage.CreateDiskOnlyVmSnapshotAnswer; +import com.cloud.agent.api.storage.DeleteDiskOnlyVmSnapshotCommand; +import com.cloud.agent.api.storage.RevertDiskOnlyVmSnapshotAnswer; +import com.cloud.agent.api.storage.RevertDiskOnlyVmSnapshotCommand; +import com.cloud.alert.AlertManager; +import com.cloud.host.DetailVO; +import com.cloud.host.Host; +import com.cloud.host.dao.HostDetailsDao; +import com.cloud.storage.DataStoreRole; +import com.cloud.storage.Volume; +import com.cloud.storage.dao.VolumeDao; +import com.cloud.user.ResourceLimitService; +import com.cloud.uservm.UserVm; +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.vm.VMInstanceDetailVO; +import com.cloud.vm.UserVmVO; +import com.cloud.vm.VirtualMachine; +import com.cloud.vm.dao.VMInstanceDetailsDao; +import com.cloud.vm.dao.UserVmDao; +import com.cloud.vm.snapshot.VMSnapshot; +import com.cloud.vm.snapshot.VMSnapshotDetailsVO; +import com.cloud.vm.snapshot.VMSnapshotVO; +import com.cloud.vm.snapshot.dao.VMSnapshotDao; +import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao; + +public class KvmFileBasedStorageVmSnapshotStrategyTest { + + private KvmFileBasedStorageVmSnapshotStrategy strategy; + private VMSnapshotDetailsDao vmSnapshotDetailsDao; + private VMSnapshotDao vmSnapshotDao; + private VMSnapshotHelper vmSnapshotHelper; + private AgentManager agentMgr; + private SnapshotDataStoreDao snapshotDataStoreDao; + private HostDetailsDao hostDetailsDao; + + @Before + public void setup() { + strategy = Mockito.spy(new KvmFileBasedStorageVmSnapshotStrategy()); + vmSnapshotDetailsDao = mock(VMSnapshotDetailsDao.class); + vmSnapshotDao = mock(VMSnapshotDao.class); + vmSnapshotHelper = mock(VMSnapshotHelper.class); + agentMgr = mock(AgentManager.class); + snapshotDataStoreDao = mock(SnapshotDataStoreDao.class); + hostDetailsDao = mock(HostDetailsDao.class); + + strategy.vmSnapshotDetailsDao = vmSnapshotDetailsDao; + strategy.vmSnapshotDao = vmSnapshotDao; + strategy.vmSnapshotHelper = vmSnapshotHelper; + strategy.agentMgr = agentMgr; + strategy.snapshotDataStoreDao = snapshotDataStoreDao; + strategy.resourceLimitManager = mock(ResourceLimitService.class); + strategy.snapshotDataFactory = mock(SnapshotDataFactory.class); + strategy.userVmDao = mock(UserVmDao.class); + strategy.volumeDao = mock(VolumeDao.class); + strategy.vmInstanceDetailsDao = mock(VMInstanceDetailsDao.class); + strategy.hostDetailsDao = hostDetailsDao; + strategy.alertManager = mock(AlertManager.class); + doNothing().when(strategy).publishUsageEvent(anyString(), any(VMSnapshot.class), any(UserVm.class), anyLong(), anyLong()); + doNothing().when(strategy).publishUsageEvent(anyString(), any(VMSnapshot.class), any(UserVm.class), any(VolumeObjectTO.class)); + } + + @Test + public void testProcessCreateVmSnapshotAnswerPersistsNvramPath() throws Exception { + VMSnapshot vmSnapshot = mock(VMSnapshot.class); + CreateDiskOnlyVmSnapshotAnswer answer = mock(CreateDiskOnlyVmSnapshotAnswer.class); + UserVm userVm = mock(UserVm.class); + VMSnapshotVO vmSnapshotVO = mock(VMSnapshotVO.class); + + when(vmSnapshot.getId()).thenReturn(42L); + when(vmSnapshot.getUuid()).thenReturn("vm-snapshot"); + when(answer.getMapVolumeToSnapshotSizeAndNewVolumePath()).thenReturn(Collections.emptyMap()); + when(answer.getNvramSnapshotPath()).thenReturn("nvram/42.fd"); + + Method method = KvmFileBasedStorageVmSnapshotStrategy.class.getDeclaredMethod("processCreateVmSnapshotAnswer", VMSnapshot.class, java.util.Map.class, + CreateDiskOnlyVmSnapshotAnswer.class, UserVm.class, VMSnapshotVO.class, long.class, VMSnapshotVO.class); + method.setAccessible(true); + method.invoke(strategy, vmSnapshot, Collections.emptyMap(), answer, userVm, vmSnapshotVO, 0L, null); + + verify(vmSnapshotDetailsDao).addDetail(42L, "kvmFileBasedStorageSnapshotNvram", "nvram/42.fd", false); + } + + @Test + public void testRevertVMSnapshotPassesNvramPathToAgentCommand() { + long vmId = 10L; + long snapshotId = 20L; + long dataStoreId = 30L; + long hostId = 40L; + + UserVmVO userVm = mock(UserVmVO.class); + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + VMSnapshotVO currentVmSnapshot = mock(VMSnapshotVO.class); + SnapshotDataStoreVO snapshotDataStoreVO = mock(SnapshotDataStoreVO.class); + SnapshotInfo snapshotInfo = mock(SnapshotInfo.class); + SnapshotObjectTO snapshotObjectTO = mock(SnapshotObjectTO.class); + VMSnapshotDetailsVO volumeSnapshotDetail = new VMSnapshotDetailsVO(snapshotId, "kvmFileBasedStorageSnapshot", String.valueOf(snapshotId), true); + VMSnapshotDetailsVO nvramDetail = new VMSnapshotDetailsVO(snapshotId, "kvmFileBasedStorageSnapshotNvram", "nvram/42.fd", false); + + when(vmSnapshot.getVmId()).thenReturn(vmId); + when(vmSnapshot.getId()).thenReturn(snapshotId); + when(vmSnapshot.getUuid()).thenReturn("vm-snapshot"); + when(userVm.getState()).thenReturn(VirtualMachine.State.Stopped); + when(userVm.getName()).thenReturn("i-10-VM"); + when(userVm.getUuid()).thenReturn("vm-uuid"); + when(userVm.getId()).thenReturn(vmId); + when(strategy.userVmDao.findById(vmId)).thenReturn(userVm); + when(strategy.vmInstanceDetailsDao.findDetail(vmId, ApiConstants.BootType.UEFI.toString())) + .thenReturn(new VMInstanceDetailVO(vmId, ApiConstants.BootType.UEFI.toString(), "SECURE", true)); + when(vmSnapshotHelper.pickRunningHost(vmId)).thenReturn(hostId); + when(hostDetailsDao.findDetail(hostId, Host.HOST_UEFI_ENABLE)) + .thenReturn(new DetailVO(hostId, Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString())); + when(hostDetailsDao.findDetail(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)) + .thenReturn(new DetailVO(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM, Boolean.TRUE.toString())); + when(vmSnapshotDetailsDao.findDetails(snapshotId, "kvmFileBasedStorageSnapshot")).thenReturn(List.of(volumeSnapshotDetail)); + when(vmSnapshotDetailsDao.findDetail(snapshotId, "kvmFileBasedStorageSnapshotNvram")).thenReturn(nvramDetail); + when(snapshotDataStoreDao.findOneBySnapshotAndDatastoreRole(snapshotId, DataStoreRole.Primary)).thenReturn(snapshotDataStoreVO); + when(snapshotDataStoreVO.getSnapshotId()).thenReturn(snapshotId); + when(snapshotDataStoreVO.getDataStoreId()).thenReturn(dataStoreId); + when(strategy.snapshotDataFactory.getSnapshot(snapshotId, dataStoreId, DataStoreRole.Primary)).thenReturn(snapshotInfo); + when(snapshotInfo.getTO()).thenReturn(snapshotObjectTO); + when(vmSnapshotDao.findCurrentSnapshotByVmId(vmId)).thenReturn(currentVmSnapshot); + when(agentMgr.easySend(eq(hostId), any())).thenAnswer(invocation -> + new RevertDiskOnlyVmSnapshotAnswer((RevertDiskOnlyVmSnapshotCommand) invocation.getArgument(1), Collections.emptyList())); + + ArgumentCaptor commandCaptor = ArgumentCaptor.forClass(RevertDiskOnlyVmSnapshotCommand.class); + + strategy.revertVMSnapshot(vmSnapshot); + + verify(agentMgr).easySend(eq(hostId), commandCaptor.capture()); + verify(userVm).setLastHostId(hostId); + verify(strategy.userVmDao).update(vmId, userVm); + assertEquals("vm-uuid", commandCaptor.getValue().getVmUuid()); + assertEquals(true, commandCaptor.getValue().isUefiEnabled()); + assertEquals("nvram/42.fd", commandCaptor.getValue().getNvramSnapshotPath()); + } + + @Test + public void testDeleteNvramSnapshotIfNeededPassesPrimaryDataStoreToAgentCommand() { + long hostId = 40L; + + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + VMSnapshotDetailsVO nvramDetail = new VMSnapshotDetailsVO(20L, "kvmFileBasedStorageSnapshotNvram", "nvram/42.fd", false); + PrimaryDataStoreTO primaryDataStore = mock(PrimaryDataStoreTO.class); + + when(vmSnapshot.getId()).thenReturn(20L); + when(vmSnapshot.getUuid()).thenReturn("vm-snapshot"); + when(vmSnapshotDetailsDao.findDetail(20L, "kvmFileBasedStorageSnapshotNvram")).thenReturn(nvramDetail); + when(hostDetailsDao.findDetail(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)) + .thenReturn(new DetailVO(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM, Boolean.TRUE.toString())); + when(agentMgr.easySend(eq(hostId), any())).thenReturn(new Answer(null, true, null)); + + strategy.deleteNvramSnapshotIfNeeded(vmSnapshot, hostId, primaryDataStore); + + verify(agentMgr).easySend(eq(hostId), argThat(command -> { + if (!(command instanceof DeleteDiskOnlyVmSnapshotCommand)) { + return false; + } + + DeleteDiskOnlyVmSnapshotCommand deleteCommand = (DeleteDiskOnlyVmSnapshotCommand) command; + return deleteCommand.getSnapshots().isEmpty() + && "nvram/42.fd".equals(deleteCommand.getNvramSnapshotPath()) + && deleteCommand.getPrimaryDataStore() == primaryDataStore; + })); + } + + @Test(expected = CloudRuntimeException.class) + public void testDeleteNvramSnapshotIfNeededFailsWhenHostLacksNvramAwareCleanupCapability() { + long hostId = 40L; + + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + VMSnapshotDetailsVO nvramDetail = new VMSnapshotDetailsVO(20L, "kvmFileBasedStorageSnapshotNvram", "nvram/42.fd", false); + PrimaryDataStoreTO primaryDataStore = mock(PrimaryDataStoreTO.class); + + when(vmSnapshot.getId()).thenReturn(20L); + when(vmSnapshot.getUuid()).thenReturn("vm-snapshot"); + when(vmSnapshotDetailsDao.findDetail(20L, "kvmFileBasedStorageSnapshotNvram")).thenReturn(nvramDetail); + when(hostDetailsDao.findDetail(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)).thenReturn(null); + + try { + strategy.deleteNvramSnapshotIfNeeded(vmSnapshot, hostId, primaryDataStore); + } finally { + verify(agentMgr, never()).easySend(eq(hostId), any()); + } + } + + @Test + public void testGetRootVolumePrimaryDataStoreForCleanupReturnsNullWhenRootVolumeIsMissing() { + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + VolumeObjectTO dataVolume = mock(VolumeObjectTO.class); + + when(vmSnapshot.getUuid()).thenReturn("vm-snapshot"); + when(dataVolume.getVolumeType()).thenReturn(Volume.Type.DATADISK); + + PrimaryDataStoreTO primaryDataStore = strategy.getRootVolumePrimaryDataStoreForCleanup(vmSnapshot, List.of(dataVolume)); + + assertNull(primaryDataStore); + } + + @Test + public void testGetPrimaryDataStoreForNvramCleanupPrefersRootSnapshotPrimaryDataStore() { + long vmSnapshotId = 20L; + long rootSnapshotId = 30L; + long dataStoreId = 40L; + + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + VolumeObjectTO rootVolume = mock(VolumeObjectTO.class); + PrimaryDataStoreTO liveRootVolumePrimaryDataStore = mock(PrimaryDataStoreTO.class); + PrimaryDataStoreTO rootSnapshotPrimaryDataStore = mock(PrimaryDataStoreTO.class); + SnapshotDataStoreVO rootSnapshotDataStore = mock(SnapshotDataStoreVO.class); + SnapshotInfo rootSnapshotInfo = mock(SnapshotInfo.class); + SnapshotObjectTO rootSnapshotObjectTo = mock(SnapshotObjectTO.class); + VolumeObjectTO rootSnapshotVolume = mock(VolumeObjectTO.class); + VMSnapshotDetailsVO volumeSnapshotDetail = new VMSnapshotDetailsVO(vmSnapshotId, "kvmFileBasedStorageSnapshot", String.valueOf(rootSnapshotId), true); + + when(vmSnapshot.getId()).thenReturn(vmSnapshotId); + when(vmSnapshot.getUuid()).thenReturn("vm-snapshot"); + when(vmSnapshotDetailsDao.findDetails(vmSnapshotId, "kvmFileBasedStorageSnapshot")).thenReturn(List.of(volumeSnapshotDetail)); + when(snapshotDataStoreDao.findOneBySnapshotAndDatastoreRole(rootSnapshotId, DataStoreRole.Primary)).thenReturn(rootSnapshotDataStore); + when(rootSnapshotDataStore.getSnapshotId()).thenReturn(rootSnapshotId); + when(rootSnapshotDataStore.getDataStoreId()).thenReturn(dataStoreId); + when(strategy.snapshotDataFactory.getSnapshot(rootSnapshotId, dataStoreId, DataStoreRole.Primary)).thenReturn(rootSnapshotInfo); + when(rootSnapshotInfo.getTO()).thenReturn(rootSnapshotObjectTo); + when(rootSnapshotObjectTo.getVolume()).thenReturn(rootSnapshotVolume); + when(rootSnapshotVolume.getVolumeType()).thenReturn(Volume.Type.ROOT); + when(rootSnapshotObjectTo.getDataStore()).thenReturn(rootSnapshotPrimaryDataStore); + when(rootVolume.getVolumeType()).thenReturn(Volume.Type.ROOT); + when(rootVolume.getDataStore()).thenReturn(liveRootVolumePrimaryDataStore); + + PrimaryDataStoreTO primaryDataStore = strategy.getPrimaryDataStoreForNvramCleanup(vmSnapshot, List.of(rootVolume)); + + assertSame(rootSnapshotPrimaryDataStore, primaryDataStore); + } + + @Test(expected = CloudRuntimeException.class) + public void testTakeVmSnapshotInternalFailsWhenHostLacksNvramAwareSnapshotCapabilityForUefiVm() throws Exception { + long vmId = 10L; + long hostId = 40L; + + UserVmVO userVm = mock(UserVmVO.class); + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + + when(vmSnapshot.getVmId()).thenReturn(vmId); + when(userVm.getId()).thenReturn(vmId); + when(userVm.getUuid()).thenReturn("vm-uuid"); + when(strategy.userVmDao.findById(vmId)).thenReturn(userVm); + when(vmSnapshotHelper.pickRunningHost(vmId)).thenReturn(hostId); + when(strategy.vmInstanceDetailsDao.findDetail(vmId, ApiConstants.BootType.UEFI.toString())) + .thenReturn(new VMInstanceDetailVO(vmId, ApiConstants.BootType.UEFI.toString(), "SECURE", true)); + when(hostDetailsDao.findDetail(hostId, Host.HOST_UEFI_ENABLE)) + .thenReturn(new DetailVO(hostId, Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString())); + when(hostDetailsDao.findDetail(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)).thenReturn(null); + + strategy.takeVmSnapshotInternal(vmSnapshot, Collections.emptyMap()); + } + + @Test(expected = CloudRuntimeException.class) + public void testDeleteVMSnapshotFailsWhenHostLacksNvramAwareCleanupCapabilityForSidecarSnapshot() { + long vmId = 10L; + long vmSnapshotId = 20L; + long hostId = 40L; + + UserVmVO userVm = mock(UserVmVO.class); + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + VMSnapshotDetailsVO nvramDetail = new VMSnapshotDetailsVO(vmSnapshotId, "kvmFileBasedStorageSnapshotNvram", "nvram/42.fd", false); + + when(vmSnapshot.getVmId()).thenReturn(vmId); + when(vmSnapshot.getId()).thenReturn(vmSnapshotId); + when(vmSnapshot.getUuid()).thenReturn("vm-snapshot"); + when(strategy.userVmDao.findById(vmId)).thenReturn(userVm); + when(vmSnapshotHelper.pickRunningHost(vmId)).thenReturn(hostId); + when(vmSnapshotDetailsDao.findDetail(vmSnapshotId, "kvmFileBasedStorageSnapshotNvram")).thenReturn(nvramDetail); + when(hostDetailsDao.findDetail(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)).thenReturn(null); + + strategy.deleteVMSnapshot(vmSnapshot); + } + + @Test(expected = CloudRuntimeException.class) + public void testTakeVmSnapshotInternalFailsWhenHostLacksUefiCapabilityForUefiVm() throws Exception { + long vmId = 10L; + long hostId = 40L; + + UserVmVO userVm = mock(UserVmVO.class); + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + + when(vmSnapshot.getVmId()).thenReturn(vmId); + when(userVm.getId()).thenReturn(vmId); + when(userVm.getUuid()).thenReturn("vm-uuid"); + when(strategy.userVmDao.findById(vmId)).thenReturn(userVm); + when(vmSnapshotHelper.pickRunningHost(vmId)).thenReturn(hostId); + when(strategy.vmInstanceDetailsDao.findDetail(vmId, ApiConstants.BootType.UEFI.toString())) + .thenReturn(new VMInstanceDetailVO(vmId, ApiConstants.BootType.UEFI.toString(), "SECURE", true)); + when(hostDetailsDao.findDetail(hostId, Host.HOST_UEFI_ENABLE)).thenReturn(null); + when(hostDetailsDao.findDetail(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)) + .thenReturn(new DetailVO(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM, Boolean.TRUE.toString())); + + strategy.takeVmSnapshotInternal(vmSnapshot, Collections.emptyMap()); + } + + @Test(expected = CloudRuntimeException.class) + public void testRevertVMSnapshotFailsWhenHostLacksNvramAwareSnapshotCapabilityForUefiVm() { + long vmId = 10L; + long hostId = 40L; + + UserVmVO userVm = mock(UserVmVO.class); + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + + when(vmSnapshot.getVmId()).thenReturn(vmId); + when(userVm.getId()).thenReturn(vmId); + when(userVm.getUuid()).thenReturn("vm-uuid"); + when(userVm.getState()).thenReturn(VirtualMachine.State.Stopped); + when(strategy.userVmDao.findById(vmId)).thenReturn(userVm); + when(vmSnapshotHelper.pickRunningHost(vmId)).thenReturn(hostId); + when(strategy.vmInstanceDetailsDao.findDetail(vmId, ApiConstants.BootType.UEFI.toString())) + .thenReturn(new VMInstanceDetailVO(vmId, ApiConstants.BootType.UEFI.toString(), "SECURE", true)); + when(hostDetailsDao.findDetail(hostId, Host.HOST_UEFI_ENABLE)) + .thenReturn(new DetailVO(hostId, Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString())); + when(hostDetailsDao.findDetail(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)).thenReturn(null); + + strategy.revertVMSnapshot(vmSnapshot); + } + + @Test(expected = CloudRuntimeException.class) + public void testRevertVMSnapshotFailsWhenHostLacksUefiCapabilityForUefiVm() { + long vmId = 10L; + long hostId = 40L; + + UserVmVO userVm = mock(UserVmVO.class); + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + + when(vmSnapshot.getVmId()).thenReturn(vmId); + when(userVm.getId()).thenReturn(vmId); + when(userVm.getUuid()).thenReturn("vm-uuid"); + when(userVm.getState()).thenReturn(VirtualMachine.State.Stopped); + when(strategy.userVmDao.findById(vmId)).thenReturn(userVm); + when(vmSnapshotHelper.pickRunningHost(vmId)).thenReturn(hostId); + when(strategy.vmInstanceDetailsDao.findDetail(vmId, ApiConstants.BootType.UEFI.toString())) + .thenReturn(new VMInstanceDetailVO(vmId, ApiConstants.BootType.UEFI.toString(), "SECURE", true)); + when(hostDetailsDao.findDetail(hostId, Host.HOST_UEFI_ENABLE)).thenReturn(null); + when(hostDetailsDao.findDetail(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)) + .thenReturn(new DetailVO(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM, Boolean.TRUE.toString())); + + strategy.revertVMSnapshot(vmSnapshot); + } + + @Test + public void testNotifyGuestRecoveryIssueIfNeededSendsAlert() { + CreateDiskOnlyVmSnapshotAnswer answer = mock(CreateDiskOnlyVmSnapshotAnswer.class); + UserVm userVm = mock(UserVm.class); + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + + when(answer.getDetails()).thenReturn("VM could not be thawed"); + when(userVm.getUuid()).thenReturn("vm-uuid"); + when(userVm.getDataCenterId()).thenReturn(1L); + when(userVm.getPodIdToDeployIn()).thenReturn(2L); + when(vmSnapshot.getUuid()).thenReturn("snapshot-uuid"); + + strategy.notifyGuestRecoveryIssueIfNeeded(answer, userVm, vmSnapshot); + + verify(strategy.alertManager).sendAlert(eq(AlertManager.AlertType.ALERT_TYPE_VM_SNAPSHOT), eq(1L), eq(2L), anyString(), anyString()); + } +} diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 25162ca9b929..bfc216327fd1 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -17,6 +17,7 @@ package com.cloud.hypervisor.kvm.resource; import static com.cloud.host.Host.HOST_INSTANCE_CONVERSION; +import static com.cloud.host.Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM; import static com.cloud.host.Host.HOST_OVFTOOL_VERSION; import static com.cloud.host.Host.HOST_VDDK_LIB_DIR; import static com.cloud.host.Host.HOST_VDDK_SUPPORT; @@ -4265,6 +4266,7 @@ public StartupCommand[] initialize() { privateIp = cmd.getPrivateIpAddress(); cmd.getHostDetails().putAll(getVersionStrings()); cmd.getHostDetails().put(KeyStoreUtils.SECURED, String.valueOf(isHostSecured()).toLowerCase()); + cmd.getHostDetails().put(HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM, Boolean.TRUE.toString()); cmd.setPool(pool); cmd.setCluster(clusterId); cmd.setGatewayIpAddress(localGateway); @@ -6334,6 +6336,15 @@ public String getSnapshotTemporaryPath(String diskPath, String snapshotName) { return String.join(File.separator, diskPathSplitted); } + public String getUefiNvramPath(String vmUuid) { + String nvramDirectory = uefiProperties.getProperty(LibvirtVMDef.GuestDef.GUEST_NVRAM_PATH); + if (StringUtils.isBlank(nvramDirectory) || StringUtils.isBlank(vmUuid)) { + return null; + } + + return nvramDirectory + vmUuid + ".fd"; + } + public static String generateSecretUUIDFromString(String seed) { return UuidUtils.nameUUIDFromBytes(seed.getBytes()).toString(); } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java index eaa8b7904397..f28b5eda4a63 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java @@ -107,7 +107,7 @@ public Answer execute(ConvertInstanceCommand cmd, LibvirtComputingResource serve String vddkLibDir = resolveVddkSetting(cmd.getVddkLibDir(), serverResource.getVddkLibDir()); if (StringUtils.isBlank(vddkLibDir)) { String err = String.format("VDDK lib dir is not configured on the host. " + - "Set '%s' in agent.properties or in details parameter of the import api calll to use VDDK-based conversion.", "vddk.lib.dir"); + "Set '%s' in agent.properties or in details parameter of the import api call to use VDDK-based conversion.", "vddk.lib.dir"); logger.error("({}) {}", originalVMName, err); return new Answer(cmd, false, err); } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCreateDiskOnlyVMSnapshotCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCreateDiskOnlyVMSnapshotCommandWrapper.java index 84d17a1a1161..4a549b747f85 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCreateDiskOnlyVMSnapshotCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCreateDiskOnlyVMSnapshotCommandWrapper.java @@ -19,6 +19,7 @@ package com.cloud.hypervisor.kvm.resource.wrapper; import com.cloud.agent.api.Answer; +import com.cloud.agent.api.FreezeThawVMCommand; import com.cloud.agent.api.VMSnapshotTO; import com.cloud.agent.api.storage.CreateDiskOnlyVmSnapshotAnswer; import com.cloud.agent.api.storage.CreateDiskOnlyVmSnapshotCommand; @@ -32,24 +33,31 @@ import com.cloud.vm.VirtualMachine; import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; import org.apache.cloudstack.storage.to.VolumeObjectTO; +import org.apache.cloudstack.utils.qemu.QemuCommand; import org.apache.cloudstack.utils.qemu.QemuImg; import org.apache.cloudstack.utils.qemu.QemuImgException; import org.apache.cloudstack.utils.qemu.QemuImgFile; +import org.apache.commons.lang3.StringUtils; import org.libvirt.Connect; import org.libvirt.Domain; import org.libvirt.LibvirtException; +import com.google.gson.JsonParser; import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.UUID; +import com.cloud.storage.Volume; + @ResourceWrapper(handles = CreateDiskOnlyVmSnapshotCommand.class) public class LibvirtCreateDiskOnlyVMSnapshotCommandWrapper extends CommandWrapper { + protected static final String NVRAM_SNAPSHOT_DIR = ".cloudstack-vm-snapshot-nvram"; private static final String SNAPSHOT_XML = "\n" + "%s\n" + @@ -79,6 +87,11 @@ protected Answer takeDiskOnlyVmSnapshotOfRunningVm(CreateDiskOnlyVmSnapshotComma logger.info("Taking disk-only VM snapshot of running VM [{}].", vmName); Domain dm = null; + String nvramSnapshotPath = null; + boolean suspendedByThisWrapper = false; + boolean filesystemsFrozenByThisWrapper = false; + CreateDiskOnlyVmSnapshotAnswer answer = null; + String postSnapshotCleanupIssue = null; try { LibvirtUtilitiesHelper libvirtUtilitiesHelper = resource.getLibvirtUtilitiesHelper(); Connect conn = libvirtUtilitiesHelper.getConnection(); @@ -88,26 +101,44 @@ protected Answer takeDiskOnlyVmSnapshotOfRunningVm(CreateDiskOnlyVmSnapshotComma dm = resource.getDomain(conn, vmName); if (dm == null) { - return new CreateDiskOnlyVmSnapshotAnswer(cmd, false, String.format("Creation of disk-only VM Snapshot failed as we could not find the VM [%s].", vmName), null); + answer = new CreateDiskOnlyVmSnapshotAnswer(cmd, false, + String.format("Creation of disk-only VM Snapshot failed as we could not find the VM [%s].", vmName), null, null); + return answer; } VMSnapshotTO target = cmd.getTarget(); Pair>> snapshotXmlAndVolumeToNewPathMap = createSnapshotXmlAndNewVolumePathMap(volumeObjectTOS, disks, target, resource); + if (shouldFreezeVmFilesystemsForSnapshot(cmd)) { + freezeVmFilesystems(dm, vmName); + filesystemsFrozenByThisWrapper = true; + verifyVmFilesystemsFrozen(dm, vmName); + } + if (shouldSuspendVmForSnapshot(cmd)) { + suspendedByThisWrapper = suspendVmIfNeeded(dm); + } + nvramSnapshotPath = backupNvramIfNeeded(cmd, resource); - dm.snapshotCreateXML(snapshotXmlAndVolumeToNewPathMap.first(), getFlagsToUseForRunningVmSnapshotCreation(target)); + dm.snapshotCreateXML(snapshotXmlAndVolumeToNewPathMap.first(), getFlagsToUseForRunningVmSnapshotCreation(target, filesystemsFrozenByThisWrapper)); - return new CreateDiskOnlyVmSnapshotAnswer(cmd, true, null, snapshotXmlAndVolumeToNewPathMap.second()); - } catch (LibvirtException e) { + postSnapshotCleanupIssue = recoverVmAfterSnapshot(dm, vmName, suspendedByThisWrapper, filesystemsFrozenByThisWrapper, postSnapshotCleanupIssue); + filesystemsFrozenByThisWrapper = false; + suspendedByThisWrapper = false; + + answer = new CreateDiskOnlyVmSnapshotAnswer(cmd, true, null, snapshotXmlAndVolumeToNewPathMap.second(), nvramSnapshotPath); + } catch (LibvirtException | IOException e) { String errorMsg = String.format("Creation of disk-only VM snapshot for VM [%s] failed due to %s.", vmName, e.getMessage()); logger.error(errorMsg, e); - if (e.getMessage().contains("QEMU guest agent is not connected")) { + cleanupNvramSnapshotIfNeeded(cmd, resource, nvramSnapshotPath); + if (StringUtils.contains(e.getMessage(), "QEMU guest agent is not connected")) { errorMsg = "QEMU guest agent is not connected. If the VM has been recently started, it might connect soon. Otherwise the VM does not have the" + " guest agent installed; thus the QuiesceVM parameter is not supported."; - return new CreateDiskOnlyVmSnapshotAnswer(cmd, false, errorMsg, null); + answer = new CreateDiskOnlyVmSnapshotAnswer(cmd, false, errorMsg, null, null); + } else { + answer = new CreateDiskOnlyVmSnapshotAnswer(cmd, false, e.getMessage(), null, null); } - return new CreateDiskOnlyVmSnapshotAnswer(cmd, false, e.getMessage(), null); } finally { if (dm != null) { + postSnapshotCleanupIssue = recoverVmAfterSnapshot(dm, vmName, suspendedByThisWrapper, filesystemsFrozenByThisWrapper, postSnapshotCleanupIssue); try { dm.free(); } catch (LibvirtException l) { @@ -115,6 +146,12 @@ protected Answer takeDiskOnlyVmSnapshotOfRunningVm(CreateDiskOnlyVmSnapshotComma } } } + + if (answer != null && StringUtils.isNotBlank(postSnapshotCleanupIssue)) { + answer = new CreateDiskOnlyVmSnapshotAnswer(cmd, answer.getResult(), + appendSnapshotOperationIssue(answer.getDetails(), postSnapshotCleanupIssue), answer.getMapVolumeToSnapshotSizeAndNewVolumePath(), answer.getNvramSnapshotPath()); + } + return answer; } protected Answer takeDiskOnlyVmSnapshotOfStoppedVm(CreateDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource) { @@ -122,10 +159,12 @@ protected Answer takeDiskOnlyVmSnapshotOfStoppedVm(CreateDiskOnlyVmSnapshotComma logger.info("Taking disk-only VM snapshot of stopped VM [{}].", vmName); Map> mapVolumeToSnapshotSizeAndNewVolumePath = new HashMap<>(); + String nvramSnapshotPath = null; List volumeObjectTos = cmd.getVolumeTOs(); KVMStoragePoolManager storagePoolMgr = resource.getStoragePoolMgr(); try { + nvramSnapshotPath = backupNvramIfNeeded(cmd, resource); for (VolumeObjectTO volumeObjectTO : volumeObjectTos) { PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) volumeObjectTO.getDataStore(); KVMStoragePool kvmStoragePool = storagePoolMgr.getStoragePool(primaryDataStoreTO.getPoolType(), primaryDataStoreTO.getUuid()); @@ -144,7 +183,7 @@ protected Answer takeDiskOnlyVmSnapshotOfStoppedVm(CreateDiskOnlyVmSnapshotComma mapVolumeToSnapshotSizeAndNewVolumePath.put(volumeObjectTO.getUuid(), new Pair<>(getFileSize(currentDeltaFullPath), snapshotPath)); } - } catch (LibvirtException | QemuImgException e) { + } catch (LibvirtException | QemuImgException | IOException e) { logger.error("Exception while creating disk-only VM snapshot for VM [{}]. Deleting leftover deltas.", vmName, e); for (VolumeObjectTO volumeObjectTO : volumeObjectTos) { Pair volSizeAndNewPath = mapVolumeToSnapshotSizeAndNewVolumePath.get(volumeObjectTO.getUuid()); @@ -160,14 +199,15 @@ protected Answer takeDiskOnlyVmSnapshotOfStoppedVm(CreateDiskOnlyVmSnapshotComma logger.warn("Tried to delete leftover snapshot at [{}] failed.", volSizeAndNewPath.second(), ex); } } + cleanupNvramSnapshotIfNeeded(cmd, resource, nvramSnapshotPath); return new Answer(cmd, e); } - return new CreateDiskOnlyVmSnapshotAnswer(cmd, true, null, mapVolumeToSnapshotSizeAndNewVolumePath); + return new CreateDiskOnlyVmSnapshotAnswer(cmd, true, null, mapVolumeToSnapshotSizeAndNewVolumePath, nvramSnapshotPath); } - protected int getFlagsToUseForRunningVmSnapshotCreation(VMSnapshotTO target) { - int flags = target.getQuiescevm() ? Domain.SnapshotCreateFlags.QUIESCE : 0; + protected int getFlagsToUseForRunningVmSnapshotCreation(VMSnapshotTO target, boolean filesystemsFrozenByThisWrapper) { + int flags = target.getQuiescevm() && !filesystemsFrozenByThisWrapper ? Domain.SnapshotCreateFlags.QUIESCE : 0; flags += Domain.SnapshotCreateFlags.DISK_ONLY + Domain.SnapshotCreateFlags.ATOMIC + Domain.SnapshotCreateFlags.NO_METADATA; @@ -195,4 +235,166 @@ protected Pair>> createSnapshotXmlAndNewV protected long getFileSize(String path) { return new File(path).length(); } + + protected String backupNvramIfNeeded(CreateDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource) throws IOException, LibvirtException { + if (!cmd.isUefiEnabled()) { + return null; + } + + String activeNvramPath = resource.getUefiNvramPath(cmd.getVmUuid()); + if (StringUtils.isBlank(activeNvramPath) || !Files.exists(Path.of(activeNvramPath))) { + throw new IOException(String.format("Unable to find the active UEFI NVRAM file for VM [%s].", cmd.getVmName())); + } + + VolumeObjectTO rootVolume = getRootVolume(cmd.getVolumeTOs()); + PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) rootVolume.getDataStore(); + KVMStoragePool storagePool = resource.getStoragePoolMgr().getStoragePool(primaryDataStoreTO.getPoolType(), primaryDataStoreTO.getUuid()); + + String nvramSnapshotPath = getNvramSnapshotRelativePath(cmd.getTarget().getId()); + Path targetPath = Path.of(storagePool.getLocalPathFor(nvramSnapshotPath)); + Files.createDirectories(targetPath.getParent()); + Files.copy(Path.of(activeNvramPath), targetPath, StandardCopyOption.REPLACE_EXISTING); + return nvramSnapshotPath; + } + + protected void cleanupNvramSnapshotIfNeeded(CreateDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource, String nvramSnapshotPath) { + if (StringUtils.isBlank(nvramSnapshotPath)) { + return; + } + + try { + VolumeObjectTO rootVolume = getRootVolume(cmd.getVolumeTOs()); + PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) rootVolume.getDataStore(); + KVMStoragePool storagePool = resource.getStoragePoolMgr().getStoragePool(primaryDataStoreTO.getPoolType(), primaryDataStoreTO.getUuid()); + Files.deleteIfExists(Path.of(storagePool.getLocalPathFor(nvramSnapshotPath))); + } catch (Exception e) { + logger.warn("Failed to clean up temporary NVRAM snapshot [{}] for VM [{}].", nvramSnapshotPath, cmd.getVmName(), e); + } + } + + protected String getNvramSnapshotRelativePath(Long vmSnapshotId) { + return String.format("%s/%s.fd", NVRAM_SNAPSHOT_DIR, vmSnapshotId); + } + + protected VolumeObjectTO getRootVolume(List volumeObjectTos) { + return volumeObjectTos.stream() + .filter(volumeObjectTO -> Volume.Type.ROOT.equals(volumeObjectTO.getVolumeType())) + .findFirst() + .orElseThrow(() -> new IllegalStateException("Unable to locate the root volume while handling the VM snapshot.")); + } + + protected boolean shouldSuspendVmForSnapshot(CreateDiskOnlyVmSnapshotCommand cmd) { + return cmd.isUefiEnabled(); + } + + protected boolean shouldFreezeVmFilesystemsForSnapshot(CreateDiskOnlyVmSnapshotCommand cmd) { + return cmd.isUefiEnabled() && cmd.getTarget().getQuiescevm(); + } + + protected boolean suspendVmIfNeeded(Domain domain) throws LibvirtException { + if (domain.getInfo().state == org.libvirt.DomainInfo.DomainState.VIR_DOMAIN_PAUSED) { + return false; + } + + domain.suspend(); + return true; + } + + protected boolean freezeVmFilesystemsIfNeeded(Domain domain, String vmName) throws LibvirtException, IOException { + freezeVmFilesystems(domain, vmName); + verifyVmFilesystemsFrozen(domain, vmName); + return true; + } + + protected void freezeVmFilesystems(Domain domain, String vmName) throws LibvirtException, IOException { + String result = getResultOfQemuCommand(FreezeThawVMCommand.FREEZE, domain); + if (StringUtils.isBlank(result) || result.startsWith("error")) { + throw new IOException(String.format("Failed to freeze VM [%s] filesystems before taking the disk-only VM snapshot. Result: %s", vmName, result)); + } + } + + protected void verifyVmFilesystemsFrozen(Domain domain, String vmName) throws LibvirtException, IOException { + String status = getResultOfQemuCommand(FreezeThawVMCommand.STATUS, domain); + if (StringUtils.isBlank(status) || !new JsonParser().parse(status).isJsonObject()) { + throw new IOException(String.format("Failed to verify VM [%s] filesystem freeze state before taking the disk-only VM snapshot. Result: %s", vmName, status)); + } + + String statusResult = new JsonParser().parse(status).getAsJsonObject().get("return").getAsString(); + if (!FreezeThawVMCommand.FREEZE.equals(statusResult)) { + throw new IOException(String.format("Failed to freeze VM [%s] filesystems before taking the disk-only VM snapshot. Status: %s", vmName, statusResult)); + } + } + + protected boolean thawVmFilesystemsIfNeeded(Domain domain, String vmName, boolean filesystemsFrozenByThisWrapper) { + if (!filesystemsFrozenByThisWrapper) { + return true; + } + return thawVmFilesystemsIfNeeded(domain, vmName); + } + + protected boolean thawVmFilesystemsIfNeeded(Domain domain, String vmName) { + try { + String result = getResultOfQemuCommand(FreezeThawVMCommand.THAW, domain); + if (StringUtils.isBlank(result) || result.startsWith("error")) { + logger.warn("Failed to thaw VM [{}] filesystems after taking the disk-only VM snapshot. Result: {}", vmName, result); + return false; + } + return true; + } catch (LibvirtException e) { + logger.warn("Failed to thaw VM [{}] filesystems after taking the disk-only VM snapshot.", vmName, e); + return false; + } + } + + protected String getResultOfQemuCommand(String cmd, Domain domain) throws LibvirtException { + if (cmd.equals(FreezeThawVMCommand.FREEZE)) { + return domain.qemuAgentCommand(QemuCommand.buildQemuCommand(QemuCommand.AGENT_FREEZE, null), 10, 0); + } else if (cmd.equals(FreezeThawVMCommand.THAW)) { + return domain.qemuAgentCommand(QemuCommand.buildQemuCommand(QemuCommand.AGENT_THAW, null), 10, 0); + } else if (cmd.equals(FreezeThawVMCommand.STATUS)) { + return domain.qemuAgentCommand(QemuCommand.buildQemuCommand(QemuCommand.AGENT_FREEZE_STATUS, null), 10, 0); + } + return null; + } + + protected boolean resumeVmIfNeeded(Domain domain, String vmName, boolean suspendedByThisWrapper) { + if (!suspendedByThisWrapper) { + return true; + } + return resumeVmIfNeeded(domain, vmName); + } + + protected boolean resumeVmIfNeeded(Domain domain, String vmName) { + try { + if (domain.getInfo().state == org.libvirt.DomainInfo.DomainState.VIR_DOMAIN_PAUSED) { + domain.resume(); + } + return true; + } catch (LibvirtException e) { + logger.warn("Failed to resume VM [{}] after taking the disk-only VM snapshot.", vmName, e); + return false; + } + } + + protected String recoverVmAfterSnapshot(Domain domain, String vmName, boolean suspendedByThisWrapper, boolean filesystemsFrozenByThisWrapper, String currentIssue) { + if (suspendedByThisWrapper && !resumeVmIfNeeded(domain, vmName)) { + currentIssue = appendSnapshotOperationIssue(currentIssue, + String.format("VM [%s] could not be resumed after taking the disk-only snapshot. Guest may still be paused.", vmName)); + } + if (filesystemsFrozenByThisWrapper && !thawVmFilesystemsIfNeeded(domain, vmName)) { + currentIssue = appendSnapshotOperationIssue(currentIssue, + String.format("VM [%s] filesystems could not be thawed after taking the disk-only snapshot. Guest may still be frozen.", vmName)); + } + return currentIssue; + } + + protected String appendSnapshotOperationIssue(String currentIssue, String newIssue) { + if (StringUtils.isBlank(newIssue)) { + return currentIssue; + } + if (StringUtils.isBlank(currentIssue)) { + return newIssue; + } + return currentIssue + " " + newIssue; + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteDiskOnlyVMSnapshotCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteDiskOnlyVMSnapshotCommandWrapper.java index 15df8627a8a7..62c7c5013374 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteDiskOnlyVMSnapshotCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteDiskOnlyVMSnapshotCommandWrapper.java @@ -27,12 +27,16 @@ import com.cloud.resource.CommandWrapper; import com.cloud.resource.ResourceWrapper; import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; +import org.apache.cloudstack.storage.to.SnapshotObjectTO; +import org.apache.commons.lang3.StringUtils; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.List; +import com.cloud.storage.Volume; + @ResourceWrapper(handles = DeleteDiskOnlyVmSnapshotCommand.class) public class LibvirtDeleteDiskOnlyVMSnapshotCommandWrapper extends CommandWrapper { @@ -53,6 +57,40 @@ public Answer execute(DeleteDiskOnlyVmSnapshotCommand command, LibvirtComputingR return new Answer(command, e); } } + + deleteNvramSnapshotIfNeeded(command, resource, storagePoolMgr, snapshotsToDelete); return new Answer(command, true, null); } + + protected void deleteNvramSnapshotIfNeeded(DeleteDiskOnlyVmSnapshotCommand command, LibvirtComputingResource resource, KVMStoragePoolManager storagePoolMgr, + List snapshotsToDelete) { + if (StringUtils.isBlank(command.getNvramSnapshotPath())) { + return; + } + + try { + KVMStoragePool storagePool; + if (command.getPrimaryDataStore() != null) { + PrimaryDataStoreTO dataStore = command.getPrimaryDataStore(); + storagePool = storagePoolMgr.getStoragePool(dataStore.getPoolType(), dataStore.getUuid()); + } else { + SnapshotObjectTO rootVolumeSnapshot = snapshotsToDelete.stream() + .map(SnapshotObjectTO.class::cast) + .filter(snapshotObjectTO -> Volume.Type.ROOT.equals(snapshotObjectTO.getVolume().getVolumeType())) + .findFirst() + .orElse(null); + + if (rootVolumeSnapshot == null) { + logger.warn("Unable to locate the root volume snapshot while deleting NVRAM snapshot [{}].", command.getNvramSnapshotPath()); + return; + } + + storagePool = resource.getLibvirtUtilitiesHelper().getPrimaryPoolFromDataTo(rootVolumeSnapshot, storagePoolMgr); + } + + Files.deleteIfExists(Path.of(storagePool.getLocalPathFor(command.getNvramSnapshotPath()))); + } catch (Exception e) { + logger.warn("Failed to delete the UEFI NVRAM snapshot [{}]. It will be left behind on storage.", command.getNvramSnapshotPath(), e); + } + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java index 5a7d6d2c203a..1ff6d7851f20 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java @@ -43,6 +43,7 @@ public final class LibvirtReadyCommandWrapper extends CommandWrapper hostDetails = new HashMap(); + hostDetails.put(Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM, Boolean.TRUE.toString()); if (hostSupportsUefi(libvirtComputingResource.isUbuntuOrDebianHost()) && libvirtComputingResource.isUefiPropertiesFileLoaded()) { hostDetails.put(Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString()); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertDiskOnlyVMSnapshotCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertDiskOnlyVMSnapshotCommandWrapper.java index 1aa79d48eec2..e4cd527331bd 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertDiskOnlyVMSnapshotCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertDiskOnlyVMSnapshotCommandWrapper.java @@ -31,15 +31,20 @@ import org.apache.cloudstack.utils.qemu.QemuImg; import org.apache.cloudstack.utils.qemu.QemuImgException; import org.apache.cloudstack.utils.qemu.QemuImgFile; +import org.apache.commons.lang3.StringUtils; import org.libvirt.LibvirtException; import java.io.IOException; +import java.nio.file.AtomicMoveNotSupportedException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.util.ArrayList; import java.util.HashMap; import java.util.List; +import com.cloud.storage.Volume; + @ResourceWrapper(handles = RevertDiskOnlyVmSnapshotCommand.class) public class LibvirtRevertDiskOnlyVMSnapshotCommandWrapper extends CommandWrapper { @@ -55,6 +60,8 @@ public Answer execute(RevertDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResou HashMap snapshotToNewDeltaPath = new HashMap<>(); try { + SnapshotObjectTO rootVolumeSnapshot = getRootVolumeSnapshot(snapshotObjectTos); + validateNvramRevertState(cmd, resource, rootVolumeSnapshot, storagePoolMgr); for (SnapshotObjectTO snapshotObjectTo : snapshotObjectTos) { KVMStoragePool kvmStoragePool = libvirtUtilitiesHelper.getPrimaryPoolFromDataTo(snapshotObjectTo, storagePoolMgr); @@ -71,7 +78,8 @@ public Answer execute(RevertDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResou qemuImg.create(newDelta, currentDelta); snapshotToNewDeltaPath.put(snapshotObjectTo, deltaPath); } - } catch (LibvirtException | QemuImgException e) { + restoreNvramIfNeeded(cmd, resource, rootVolumeSnapshot, storagePoolMgr); + } catch (LibvirtException | QemuImgException | IOException e) { logger.error("Exception while reverting disk-only VM snapshot for VM [{}]. Deleting leftover deltas.", vmName, e); for (SnapshotObjectTO snapshotObjectTo : snapshotObjectTos) { String newPath = snapshotToNewDeltaPath.get(snapshotObjectTo); @@ -108,4 +116,88 @@ public Answer execute(RevertDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResou return new RevertDiskOnlyVmSnapshotAnswer(cmd, volumeObjectTos); } + + protected SnapshotObjectTO getRootVolumeSnapshot(List snapshotObjectTos) { + return snapshotObjectTos.stream() + .filter(snapshotObjectTO -> Volume.Type.ROOT.equals(snapshotObjectTO.getVolume().getVolumeType())) + .findFirst() + .orElse(null); + } + + protected void validateNvramRevertState(RevertDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource, SnapshotObjectTO rootVolumeSnapshot, + KVMStoragePoolManager storagePoolMgr) throws IOException, LibvirtException { + String activeNvramPath = resource.getUefiNvramPath(cmd.getVmUuid()); + if (StringUtils.isBlank(cmd.getNvramSnapshotPath())) { + if (cmd.isUefiEnabled()) { + throw new IOException(String.format("Cannot safely revert disk-only VM snapshot for UEFI VM [%s] because the snapshot does not contain NVRAM state.", + cmd.getVmName())); + } + return; + } + + if (StringUtils.isBlank(activeNvramPath)) { + throw new IOException(String.format("Unable to determine the active UEFI NVRAM path for VM [%s].", cmd.getVmName())); + } + + Path snapshotNvramPath = getNvramSnapshotAbsolutePath(cmd.getNvramSnapshotPath(), rootVolumeSnapshot, resource, storagePoolMgr); + if (!Files.exists(snapshotNvramPath)) { + throw new IOException(String.format("Unable to find the UEFI NVRAM snapshot [%s] for VM [%s].", cmd.getNvramSnapshotPath(), cmd.getVmName())); + } + } + + protected void restoreNvramIfNeeded(RevertDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource, SnapshotObjectTO rootVolumeSnapshot, + KVMStoragePoolManager storagePoolMgr) throws IOException, LibvirtException { + if (StringUtils.isBlank(cmd.getNvramSnapshotPath())) { + return; + } + + String activeNvramPath = resource.getUefiNvramPath(cmd.getVmUuid()); + if (StringUtils.isBlank(activeNvramPath)) { + throw new IOException(String.format("Unable to determine the active UEFI NVRAM path for VM [%s].", cmd.getVmName())); + } + + Path snapshotNvramPath = getNvramSnapshotAbsolutePath(cmd.getNvramSnapshotPath(), rootVolumeSnapshot, resource, storagePoolMgr); + if (!Files.exists(snapshotNvramPath)) { + throw new IOException(String.format("Unable to find the UEFI NVRAM snapshot [%s] for VM [%s].", cmd.getNvramSnapshotPath(), cmd.getVmName())); + } + + replaceNvramAtomically(snapshotNvramPath, Path.of(activeNvramPath)); + } + + protected void replaceNvramAtomically(Path snapshotNvramPath, Path activeNvramPath) throws IOException { + Path targetDirectory = activeNvramPath.getParent(); + if (targetDirectory != null) { + Files.createDirectories(targetDirectory); + } + + Path temporaryNvramPath = Files.createTempFile(targetDirectory, activeNvramPath.getFileName().toString(), ".tmp"); + try { + copyNvramSnapshotToTemporaryPath(snapshotNvramPath, temporaryNvramPath); + moveTemporaryNvramIntoPlace(temporaryNvramPath, activeNvramPath); + } finally { + Files.deleteIfExists(temporaryNvramPath); + } + } + + protected void copyNvramSnapshotToTemporaryPath(Path snapshotNvramPath, Path temporaryNvramPath) throws IOException { + Files.copy(snapshotNvramPath, temporaryNvramPath, StandardCopyOption.REPLACE_EXISTING); + } + + protected void moveTemporaryNvramIntoPlace(Path temporaryNvramPath, Path activeNvramPath) throws IOException { + try { + Files.move(temporaryNvramPath, activeNvramPath, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); + } catch (AtomicMoveNotSupportedException e) { + Files.move(temporaryNvramPath, activeNvramPath, StandardCopyOption.REPLACE_EXISTING); + } + } + + protected Path getNvramSnapshotAbsolutePath(String nvramSnapshotPath, SnapshotObjectTO rootVolumeSnapshot, LibvirtComputingResource resource, + KVMStoragePoolManager storagePoolMgr) throws IOException, LibvirtException { + if (rootVolumeSnapshot == null) { + throw new IOException("Unable to locate the root volume snapshot while handling the UEFI NVRAM state."); + } + + KVMStoragePool storagePool = resource.getLibvirtUtilitiesHelper().getPrimaryPoolFromDataTo(rootVolumeSnapshot, storagePoolMgr); + return Path.of(storagePool.getLocalPathFor(nvramSnapshotPath)); + } } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDiskOnlyVMSnapshotCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDiskOnlyVMSnapshotCommandWrapperTest.java new file mode 100644 index 000000000000..361761f28d41 --- /dev/null +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDiskOnlyVMSnapshotCommandWrapperTest.java @@ -0,0 +1,711 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package com.cloud.hypervisor.kvm.resource.wrapper; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.UUID; + +import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; +import org.apache.cloudstack.storage.to.SnapshotObjectTO; +import org.apache.cloudstack.storage.to.VolumeObjectTO; +import org.junit.Test; +import org.libvirt.Domain; +import org.libvirt.DomainInfo; + +import com.cloud.agent.api.VMSnapshotTO; +import com.cloud.agent.api.storage.CreateDiskOnlyVmSnapshotCommand; +import com.cloud.agent.api.storage.CreateDiskOnlyVmSnapshotAnswer; +import com.cloud.agent.api.storage.DeleteDiskOnlyVmSnapshotCommand; +import com.cloud.agent.api.storage.RevertDiskOnlyVmSnapshotCommand; +import com.cloud.agent.api.to.DataTO; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.hypervisor.kvm.storage.KVMStoragePool; +import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager; +import com.cloud.storage.Storage; +import com.cloud.storage.Volume; +import com.cloud.utils.Pair; + +public class LibvirtDiskOnlyVMSnapshotCommandWrapperTest { + + @Test + public void testBackupNvramIfNeededCopiesActiveNvram() throws Exception { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper(); + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + KVMStoragePoolManager storagePoolManager = mock(KVMStoragePoolManager.class); + KVMStoragePool storagePool = mock(KVMStoragePool.class); + CreateDiskOnlyVmSnapshotCommand command = mock(CreateDiskOnlyVmSnapshotCommand.class); + VMSnapshotTO vmSnapshotTO = mock(VMSnapshotTO.class); + PrimaryDataStoreTO dataStoreTO = mock(PrimaryDataStoreTO.class); + VolumeObjectTO rootVolume = mock(VolumeObjectTO.class); + + Path activeNvram = Files.createTempFile("active-", ".fd"); + Files.writeString(activeNvram, "snapshot-nvram"); + Path poolDirectory = Files.createTempDirectory("pool-"); + + when(command.isUefiEnabled()).thenReturn(true); + when(command.getVmUuid()).thenReturn("vm-uuid"); + when(command.getVmName()).thenReturn("vm-name"); + when(command.getTarget()).thenReturn(vmSnapshotTO); + when(vmSnapshotTO.getId()).thenReturn(42L); + when(command.getVolumeTOs()).thenReturn(List.of(rootVolume)); + when(rootVolume.getVolumeType()).thenReturn(Volume.Type.ROOT); + when(rootVolume.getDataStore()).thenReturn(dataStoreTO); + when(dataStoreTO.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + when(dataStoreTO.getUuid()).thenReturn("pool-uuid"); + when(resource.getUefiNvramPath("vm-uuid")).thenReturn(activeNvram.toString()); + when(resource.getStoragePoolMgr()).thenReturn(storagePoolManager); + when(storagePoolManager.getStoragePool(Storage.StoragePoolType.NetworkFilesystem, "pool-uuid")).thenReturn(storagePool); + when(storagePool.getLocalPathFor(anyString())).thenAnswer(invocation -> poolDirectory.resolve(invocation.getArgument(0, String.class)).toString()); + + String nvramSnapshotPath = wrapper.backupNvramIfNeeded(command, resource); + + assertEquals(".cloudstack-vm-snapshot-nvram/42.fd", nvramSnapshotPath); + assertEquals("snapshot-nvram", Files.readString(poolDirectory.resolve(nvramSnapshotPath))); + } + + @Test(expected = IOException.class) + public void testBackupNvramIfNeededFailsWhenUefiNvramIsMissing() throws Exception { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper(); + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + CreateDiskOnlyVmSnapshotCommand command = mock(CreateDiskOnlyVmSnapshotCommand.class); + + when(command.isUefiEnabled()).thenReturn(true); + when(command.getVmUuid()).thenReturn("vm-uuid"); + when(command.getVmName()).thenReturn("vm-name"); + when(resource.getUefiNvramPath("vm-uuid")).thenReturn("/tmp/" + UUID.randomUUID() + ".fd"); + + wrapper.backupNvramIfNeeded(command, resource); + } + + @Test(expected = IOException.class) + public void testValidateNvramRevertStateFailsForLegacySnapshotsOnUefiVms() throws Exception { + LibvirtRevertDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtRevertDiskOnlyVMSnapshotCommandWrapper(); + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + RevertDiskOnlyVmSnapshotCommand command = mock(RevertDiskOnlyVmSnapshotCommand.class); + + Path activeNvram = Files.createTempFile("active-", ".fd"); + when(command.getVmUuid()).thenReturn("vm-uuid"); + when(command.getVmName()).thenReturn("vm-name"); + when(command.isUefiEnabled()).thenReturn(true); + when(command.getNvramSnapshotPath()).thenReturn(null); + when(resource.getUefiNvramPath("vm-uuid")).thenReturn(activeNvram.toString()); + + wrapper.validateNvramRevertState(command, resource, null, mock(KVMStoragePoolManager.class)); + } + + @Test(expected = IOException.class) + public void testValidateNvramRevertStateFailsForLegacySnapshotsOnFallbackHostsForUefiVms() throws Exception { + LibvirtRevertDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtRevertDiskOnlyVMSnapshotCommandWrapper(); + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + RevertDiskOnlyVmSnapshotCommand command = mock(RevertDiskOnlyVmSnapshotCommand.class); + + when(command.getVmUuid()).thenReturn("vm-uuid"); + when(command.getVmName()).thenReturn("vm-name"); + when(command.isUefiEnabled()).thenReturn(true); + when(command.getNvramSnapshotPath()).thenReturn(null); + when(resource.getUefiNvramPath("vm-uuid")).thenReturn(Path.of("/tmp", UUID.randomUUID() + ".fd").toString()); + + wrapper.validateNvramRevertState(command, resource, null, mock(KVMStoragePoolManager.class)); + } + + @Test + public void testValidateNvramRevertStateAllowsFallbackHostsWithoutLocalNvram() throws Exception { + LibvirtRevertDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtRevertDiskOnlyVMSnapshotCommandWrapper(); + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + LibvirtUtilitiesHelper libvirtUtilitiesHelper = mock(LibvirtUtilitiesHelper.class); + KVMStoragePoolManager storagePoolManager = mock(KVMStoragePoolManager.class); + KVMStoragePool storagePool = mock(KVMStoragePool.class); + RevertDiskOnlyVmSnapshotCommand command = mock(RevertDiskOnlyVmSnapshotCommand.class); + SnapshotObjectTO rootSnapshot = mock(SnapshotObjectTO.class); + + Path poolDirectory = Files.createTempDirectory("pool-"); + Path snapshotNvram = poolDirectory.resolve("nvram/42.fd"); + Files.createDirectories(snapshotNvram.getParent()); + Files.writeString(snapshotNvram, "snapshot"); + + when(command.getVmUuid()).thenReturn("vm-uuid"); + when(command.getVmName()).thenReturn("vm-name"); + when(command.isUefiEnabled()).thenReturn(true); + when(command.getNvramSnapshotPath()).thenReturn("nvram/42.fd"); + when(resource.getUefiNvramPath("vm-uuid")).thenReturn(poolDirectory.resolve("missing").resolve("vm-uuid.fd").toString()); + when(resource.getLibvirtUtilitiesHelper()).thenReturn(libvirtUtilitiesHelper); + when(libvirtUtilitiesHelper.getPrimaryPoolFromDataTo(rootSnapshot, storagePoolManager)).thenReturn(storagePool); + when(storagePool.getLocalPathFor("nvram/42.fd")).thenReturn(snapshotNvram.toString()); + + wrapper.validateNvramRevertState(command, resource, rootSnapshot, storagePoolManager); + } + + @Test + public void testRestoreNvramIfNeededRestoresSnapshotBytes() throws Exception { + LibvirtRevertDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtRevertDiskOnlyVMSnapshotCommandWrapper(); + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + LibvirtUtilitiesHelper libvirtUtilitiesHelper = mock(LibvirtUtilitiesHelper.class); + KVMStoragePoolManager storagePoolManager = mock(KVMStoragePoolManager.class); + KVMStoragePool storagePool = mock(KVMStoragePool.class); + RevertDiskOnlyVmSnapshotCommand command = mock(RevertDiskOnlyVmSnapshotCommand.class); + SnapshotObjectTO rootSnapshot = mock(SnapshotObjectTO.class); + + Path activeNvram = Files.createTempFile("active-", ".fd"); + Files.writeString(activeNvram, "current"); + Path poolDirectory = Files.createTempDirectory("pool-"); + Path snapshotNvram = poolDirectory.resolve("nvram/42.fd"); + Files.createDirectories(snapshotNvram.getParent()); + Files.writeString(snapshotNvram, "snapshot"); + + when(command.getVmUuid()).thenReturn("vm-uuid"); + when(command.getVmName()).thenReturn("vm-name"); + when(command.getNvramSnapshotPath()).thenReturn("nvram/42.fd"); + when(resource.getUefiNvramPath("vm-uuid")).thenReturn(activeNvram.toString()); + when(resource.getLibvirtUtilitiesHelper()).thenReturn(libvirtUtilitiesHelper); + when(libvirtUtilitiesHelper.getPrimaryPoolFromDataTo(rootSnapshot, storagePoolManager)).thenReturn(storagePool); + when(storagePool.getLocalPathFor("nvram/42.fd")).thenReturn(snapshotNvram.toString()); + + wrapper.restoreNvramIfNeeded(command, resource, rootSnapshot, storagePoolManager); + + assertEquals("snapshot", Files.readString(activeNvram)); + } + + @Test + public void testRestoreNvramIfNeededCreatesMissingActiveNvramFile() throws Exception { + LibvirtRevertDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtRevertDiskOnlyVMSnapshotCommandWrapper(); + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + LibvirtUtilitiesHelper libvirtUtilitiesHelper = mock(LibvirtUtilitiesHelper.class); + KVMStoragePoolManager storagePoolManager = mock(KVMStoragePoolManager.class); + KVMStoragePool storagePool = mock(KVMStoragePool.class); + RevertDiskOnlyVmSnapshotCommand command = mock(RevertDiskOnlyVmSnapshotCommand.class); + SnapshotObjectTO rootSnapshot = mock(SnapshotObjectTO.class); + + Path poolDirectory = Files.createTempDirectory("pool-"); + Path snapshotNvram = poolDirectory.resolve("nvram/42.fd"); + Path activeNvram = poolDirectory.resolve("target").resolve("vm-uuid.fd"); + Files.createDirectories(snapshotNvram.getParent()); + Files.writeString(snapshotNvram, "snapshot"); + + when(command.getVmUuid()).thenReturn("vm-uuid"); + when(command.getVmName()).thenReturn("vm-name"); + when(command.getNvramSnapshotPath()).thenReturn("nvram/42.fd"); + when(resource.getUefiNvramPath("vm-uuid")).thenReturn(activeNvram.toString()); + when(resource.getLibvirtUtilitiesHelper()).thenReturn(libvirtUtilitiesHelper); + when(libvirtUtilitiesHelper.getPrimaryPoolFromDataTo(rootSnapshot, storagePoolManager)).thenReturn(storagePool); + when(storagePool.getLocalPathFor("nvram/42.fd")).thenReturn(snapshotNvram.toString()); + + wrapper.restoreNvramIfNeeded(command, resource, rootSnapshot, storagePoolManager); + + assertEquals("snapshot", Files.readString(activeNvram)); + } + + @Test + public void testRestoreNvramIfNeededPreservesActiveNvramWhenCopyFails() throws Exception { + LibvirtRevertDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtRevertDiskOnlyVMSnapshotCommandWrapper() { + @Override + protected void copyNvramSnapshotToTemporaryPath(Path snapshotNvramPath, Path temporaryNvramPath) throws IOException { + Files.writeString(temporaryNvramPath, "partial"); + throw new IOException("copy failed"); + } + }; + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + LibvirtUtilitiesHelper libvirtUtilitiesHelper = mock(LibvirtUtilitiesHelper.class); + KVMStoragePoolManager storagePoolManager = mock(KVMStoragePoolManager.class); + KVMStoragePool storagePool = mock(KVMStoragePool.class); + RevertDiskOnlyVmSnapshotCommand command = mock(RevertDiskOnlyVmSnapshotCommand.class); + SnapshotObjectTO rootSnapshot = mock(SnapshotObjectTO.class); + + Path activeNvram = Files.createTempFile("active-", ".fd"); + Files.writeString(activeNvram, "current"); + Path poolDirectory = Files.createTempDirectory("pool-"); + Path snapshotNvram = poolDirectory.resolve("nvram/42.fd"); + Files.createDirectories(snapshotNvram.getParent()); + Files.writeString(snapshotNvram, "snapshot"); + + when(command.getVmUuid()).thenReturn("vm-uuid"); + when(command.getVmName()).thenReturn("vm-name"); + when(command.getNvramSnapshotPath()).thenReturn("nvram/42.fd"); + when(resource.getUefiNvramPath("vm-uuid")).thenReturn(activeNvram.toString()); + when(resource.getLibvirtUtilitiesHelper()).thenReturn(libvirtUtilitiesHelper); + when(libvirtUtilitiesHelper.getPrimaryPoolFromDataTo(rootSnapshot, storagePoolManager)).thenReturn(storagePool); + when(storagePool.getLocalPathFor("nvram/42.fd")).thenReturn(snapshotNvram.toString()); + + try { + wrapper.restoreNvramIfNeeded(command, resource, rootSnapshot, storagePoolManager); + fail("Expected restore to fail when the snapshot copy fails."); + } catch (IOException expected) { + assertEquals("current", Files.readString(activeNvram)); + } + } + + @Test + public void testDeleteNvramSnapshotIfNeededDeletesSidecar() throws Exception { + LibvirtDeleteDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtDeleteDiskOnlyVMSnapshotCommandWrapper(); + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + LibvirtUtilitiesHelper libvirtUtilitiesHelper = mock(LibvirtUtilitiesHelper.class); + KVMStoragePoolManager storagePoolManager = mock(KVMStoragePoolManager.class); + KVMStoragePool storagePool = mock(KVMStoragePool.class); + DeleteDiskOnlyVmSnapshotCommand command = mock(DeleteDiskOnlyVmSnapshotCommand.class); + SnapshotObjectTO rootSnapshot = mock(SnapshotObjectTO.class); + VolumeObjectTO rootVolume = mock(VolumeObjectTO.class); + + Path poolDirectory = Files.createTempDirectory("pool-"); + Path snapshotNvram = poolDirectory.resolve("nvram/42.fd"); + Files.createDirectories(snapshotNvram.getParent()); + Files.writeString(snapshotNvram, "snapshot"); + + when(command.getNvramSnapshotPath()).thenReturn("nvram/42.fd"); + when(rootSnapshot.getVolume()).thenReturn(rootVolume); + when(rootVolume.getVolumeType()).thenReturn(Volume.Type.ROOT); + when(resource.getLibvirtUtilitiesHelper()).thenReturn(libvirtUtilitiesHelper); + when(libvirtUtilitiesHelper.getPrimaryPoolFromDataTo(rootSnapshot, storagePoolManager)).thenReturn(storagePool); + when(storagePool.getLocalPathFor("nvram/42.fd")).thenReturn(snapshotNvram.toString()); + + wrapper.deleteNvramSnapshotIfNeeded(command, resource, storagePoolManager, List.of(rootSnapshot)); + + assertFalse(Files.exists(snapshotNvram)); + assertTrue(Files.exists(poolDirectory)); + } + + @Test + public void testDeleteNvramSnapshotIfNeededDeletesSidecarUsingPrimaryDataStore() throws Exception { + LibvirtDeleteDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtDeleteDiskOnlyVMSnapshotCommandWrapper(); + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + KVMStoragePoolManager storagePoolManager = mock(KVMStoragePoolManager.class); + KVMStoragePool storagePool = mock(KVMStoragePool.class); + DeleteDiskOnlyVmSnapshotCommand command = mock(DeleteDiskOnlyVmSnapshotCommand.class); + PrimaryDataStoreTO primaryDataStoreTO = mock(PrimaryDataStoreTO.class); + + Path poolDirectory = Files.createTempDirectory("pool-"); + Path snapshotNvram = poolDirectory.resolve("nvram/42.fd"); + Files.createDirectories(snapshotNvram.getParent()); + Files.writeString(snapshotNvram, "snapshot"); + + when(command.getNvramSnapshotPath()).thenReturn("nvram/42.fd"); + when(command.getPrimaryDataStore()).thenReturn(primaryDataStoreTO); + when(primaryDataStoreTO.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + when(primaryDataStoreTO.getUuid()).thenReturn("pool-uuid"); + when(storagePoolManager.getStoragePool(Storage.StoragePoolType.NetworkFilesystem, "pool-uuid")).thenReturn(storagePool); + when(storagePool.getLocalPathFor("nvram/42.fd")).thenReturn(snapshotNvram.toString()); + + wrapper.deleteNvramSnapshotIfNeeded(command, resource, storagePoolManager, List.of()); + + assertFalse(Files.exists(snapshotNvram)); + } + + @Test + public void testResumeVmIfNeededOnlyResumesWhenWrapperSuspendedVm() throws Exception { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper(); + Domain domain = mock(Domain.class); + + wrapper.resumeVmIfNeeded(domain, "vm-name", false); + + verify(domain, never()).resume(); + verify(domain, never()).getInfo(); + } + + @Test + public void testSuspendVmIfNeededSkipsAlreadyPausedVm() throws Exception { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper(); + Domain domain = mock(Domain.class); + DomainInfo domainInfo = new DomainInfo(); + domainInfo.state = DomainInfo.DomainState.VIR_DOMAIN_PAUSED; + when(domain.getInfo()).thenReturn(domainInfo); + + assertFalse(wrapper.suspendVmIfNeeded(domain)); + verify(domain, never()).suspend(); + } + + @Test + public void testShouldSuspendVmForSnapshotWhenUefiAndNotQuiesced() { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper(); + CreateDiskOnlyVmSnapshotCommand command = mock(CreateDiskOnlyVmSnapshotCommand.class); + VMSnapshotTO target = mock(VMSnapshotTO.class); + + when(command.isUefiEnabled()).thenReturn(true); + when(command.getTarget()).thenReturn(target); + when(target.getQuiescevm()).thenReturn(false); + + assertTrue(wrapper.shouldSuspendVmForSnapshot(command)); + } + + @Test + public void testShouldSuspendVmForSnapshotWhenQuiesceIsRequested() { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper(); + CreateDiskOnlyVmSnapshotCommand command = mock(CreateDiskOnlyVmSnapshotCommand.class); + VMSnapshotTO target = mock(VMSnapshotTO.class); + + when(command.isUefiEnabled()).thenReturn(true); + when(command.getTarget()).thenReturn(target); + when(target.getQuiescevm()).thenReturn(true); + + assertTrue(wrapper.shouldSuspendVmForSnapshot(command)); + } + + @Test + public void testShouldFreezeVmFilesystemsForSnapshotWhenQuiesceIsRequested() { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper(); + CreateDiskOnlyVmSnapshotCommand command = mock(CreateDiskOnlyVmSnapshotCommand.class); + VMSnapshotTO target = mock(VMSnapshotTO.class); + + when(command.isUefiEnabled()).thenReturn(true); + when(command.getTarget()).thenReturn(target); + when(target.getQuiescevm()).thenReturn(true); + + assertTrue(wrapper.shouldFreezeVmFilesystemsForSnapshot(command)); + } + + @Test + public void testGetFlagsToUseForRunningVmSnapshotCreationOmitsLibvirtQuiesceWhenAlreadyFrozen() { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper(); + VMSnapshotTO target = mock(VMSnapshotTO.class); + + when(target.getQuiescevm()).thenReturn(true); + + int flags = wrapper.getFlagsToUseForRunningVmSnapshotCreation(target, true); + + assertEquals(0, flags & Domain.SnapshotCreateFlags.QUIESCE); + assertTrue((flags & Domain.SnapshotCreateFlags.DISK_ONLY) != 0); + assertTrue((flags & Domain.SnapshotCreateFlags.ATOMIC) != 0); + assertTrue((flags & Domain.SnapshotCreateFlags.NO_METADATA) != 0); + } + + @Test + public void testFreezeVmFilesystemsIfNeededSucceedsWhenGuestAgentReportsFrozen() throws Exception { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper() { + @Override + protected String getResultOfQemuCommand(String cmd, Domain domain) { + if ("status".equals(cmd)) { + return "{\"return\":\"frozen\"}"; + } + return "{\"return\":0}"; + } + }; + + assertTrue(wrapper.freezeVmFilesystemsIfNeeded(mock(Domain.class), "vm-name")); + } + + @Test + public void testTakeDiskOnlyVmSnapshotOfRunningVmThawsWhenFreezeVerificationFails() throws Exception { + final boolean[] thawCalled = {false}; + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper() { + @Override + protected Pair>> createSnapshotXmlAndNewVolumePathMap(List volumeObjectTOS, + List disks, VMSnapshotTO target, LibvirtComputingResource resource) { + return new Pair<>("", Collections.emptyMap()); + } + + @Override + protected boolean shouldSuspendVmForSnapshot(CreateDiskOnlyVmSnapshotCommand cmd) { + return false; + } + + @Override + protected boolean shouldFreezeVmFilesystemsForSnapshot(CreateDiskOnlyVmSnapshotCommand cmd) { + return true; + } + + @Override + protected void freezeVmFilesystems(Domain domain, String vmName) { + } + + @Override + protected void verifyVmFilesystemsFrozen(Domain domain, String vmName) throws IOException { + throw new IOException("status verification failed"); + } + + @Override + protected boolean thawVmFilesystemsIfNeeded(Domain domain, String vmName) { + thawCalled[0] = true; + return true; + } + }; + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + LibvirtUtilitiesHelper libvirtUtilitiesHelper = mock(LibvirtUtilitiesHelper.class); + org.libvirt.Connect connect = mock(org.libvirt.Connect.class); + Domain domain = mock(Domain.class); + CreateDiskOnlyVmSnapshotCommand command = mock(CreateDiskOnlyVmSnapshotCommand.class); + VMSnapshotTO target = mock(VMSnapshotTO.class); + + when(command.getVmName()).thenReturn("vm-name"); + when(command.getVolumeTOs()).thenReturn(List.of()); + when(command.getTarget()).thenReturn(target); + when(resource.getLibvirtUtilitiesHelper()).thenReturn(libvirtUtilitiesHelper); + when(libvirtUtilitiesHelper.getConnection()).thenReturn(connect); + when(resource.getDisks(connect, "vm-name")).thenReturn(List.of()); + when(resource.getDomain(connect, "vm-name")).thenReturn(domain); + + CreateDiskOnlyVmSnapshotAnswer answer = (CreateDiskOnlyVmSnapshotAnswer) wrapper.takeDiskOnlyVmSnapshotOfRunningVm(command, resource); + + assertFalse(answer.getResult()); + assertTrue("Thaw must be attempted after a successful freeze followed by verification failure", thawCalled[0]); + } + + @Test + public void testTakeDiskOnlyVmSnapshotOfRunningVmSuspendsBeforeNvramCopyForQuiescedUefiSnapshots() throws Exception { + List operations = new ArrayList<>(); + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper() { + @Override + protected Pair>> createSnapshotXmlAndNewVolumePathMap(List volumeObjectTOS, + List disks, VMSnapshotTO target, LibvirtComputingResource resource) { + return new Pair<>("", Collections.emptyMap()); + } + + @Override + protected void freezeVmFilesystems(Domain domain, String vmName) { + operations.add("freeze"); + } + + @Override + protected void verifyVmFilesystemsFrozen(Domain domain, String vmName) { + operations.add("verify"); + } + + @Override + protected boolean suspendVmIfNeeded(Domain domain) { + operations.add("suspend"); + return true; + } + + @Override + protected String backupNvramIfNeeded(CreateDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource) { + operations.add("backup"); + return "nvram/42.fd"; + } + + @Override + protected void cleanupNvramSnapshotIfNeeded(CreateDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource, String nvramSnapshotPath) { + } + + @Override + protected boolean resumeVmIfNeeded(Domain domain, String vmName) { + operations.add("resume"); + return true; + } + + @Override + protected boolean thawVmFilesystemsIfNeeded(Domain domain, String vmName) { + operations.add("thaw"); + return true; + } + }; + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + LibvirtUtilitiesHelper libvirtUtilitiesHelper = mock(LibvirtUtilitiesHelper.class); + org.libvirt.Connect connect = mock(org.libvirt.Connect.class); + Domain domain = mock(Domain.class); + CreateDiskOnlyVmSnapshotCommand command = mock(CreateDiskOnlyVmSnapshotCommand.class); + VMSnapshotTO target = mock(VMSnapshotTO.class); + + when(command.getVmName()).thenReturn("vm-name"); + when(command.getVolumeTOs()).thenReturn(List.of()); + when(command.getTarget()).thenReturn(target); + when(command.isUefiEnabled()).thenReturn(true); + when(target.getQuiescevm()).thenReturn(true); + when(resource.getLibvirtUtilitiesHelper()).thenReturn(libvirtUtilitiesHelper); + when(libvirtUtilitiesHelper.getConnection()).thenReturn(connect); + when(resource.getDisks(connect, "vm-name")).thenReturn(List.of()); + when(resource.getDomain(connect, "vm-name")).thenReturn(domain); + doThrow(mock(org.libvirt.LibvirtException.class)).when(domain).snapshotCreateXML(anyString(), anyInt()); + + CreateDiskOnlyVmSnapshotAnswer answer = (CreateDiskOnlyVmSnapshotAnswer) wrapper.takeDiskOnlyVmSnapshotOfRunningVm(command, resource); + + assertFalse(answer.getResult()); + assertEquals(List.of("freeze", "verify", "suspend", "backup", "resume", "thaw"), operations); + } + + @Test + public void testTakeDiskOnlyVmSnapshotOfRunningVmReturnsSuccessWithWarningWhenThawFails() throws Exception { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper() { + @Override + protected Pair>> createSnapshotXmlAndNewVolumePathMap(List volumeObjectTOS, + List disks, VMSnapshotTO target, LibvirtComputingResource resource) { + return new Pair<>("", Collections.emptyMap()); + } + + @Override + protected boolean shouldSuspendVmForSnapshot(CreateDiskOnlyVmSnapshotCommand cmd) { + return false; + } + + @Override + protected boolean shouldFreezeVmFilesystemsForSnapshot(CreateDiskOnlyVmSnapshotCommand cmd) { + return true; + } + + @Override + protected void freezeVmFilesystems(Domain domain, String vmName) { + } + + @Override + protected void verifyVmFilesystemsFrozen(Domain domain, String vmName) { + } + + @Override + protected String backupNvramIfNeeded(CreateDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource) { + return null; + } + + @Override + protected boolean thawVmFilesystemsIfNeeded(Domain domain, String vmName) { + return false; + } + + @Override + protected boolean thawVmFilesystemsIfNeeded(Domain domain, String vmName, boolean frozen) { + return false; + } + }; + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + LibvirtUtilitiesHelper libvirtUtilitiesHelper = mock(LibvirtUtilitiesHelper.class); + org.libvirt.Connect connect = mock(org.libvirt.Connect.class); + Domain domain = mock(Domain.class); + CreateDiskOnlyVmSnapshotCommand command = mock(CreateDiskOnlyVmSnapshotCommand.class); + VMSnapshotTO target = mock(VMSnapshotTO.class); + + when(command.getVmName()).thenReturn("vm-name"); + when(command.getVolumeTOs()).thenReturn(List.of()); + when(command.getTarget()).thenReturn(target); + when(resource.getLibvirtUtilitiesHelper()).thenReturn(libvirtUtilitiesHelper); + when(libvirtUtilitiesHelper.getConnection()).thenReturn(connect); + when(resource.getDisks(connect, "vm-name")).thenReturn(List.of()); + when(resource.getDomain(connect, "vm-name")).thenReturn(domain); + + CreateDiskOnlyVmSnapshotAnswer answer = (CreateDiskOnlyVmSnapshotAnswer) wrapper.takeDiskOnlyVmSnapshotOfRunningVm(command, resource); + + assertTrue("Snapshot metadata must still be returned when thaw fails after snapshot creation", answer.getResult()); + assertTrue(answer.getDetails().contains("could not be thawed")); + } + + @Test + public void testTakeDiskOnlyVmSnapshotOfRunningVmReturnsSuccessWithWarningWhenResumeFails() throws Exception { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper() { + @Override + protected Pair>> createSnapshotXmlAndNewVolumePathMap(List volumeObjectTOS, + List disks, VMSnapshotTO target, LibvirtComputingResource resource) { + return new Pair<>("", Collections.emptyMap()); + } + + @Override + protected boolean shouldSuspendVmForSnapshot(CreateDiskOnlyVmSnapshotCommand cmd) { + return true; + } + + @Override + protected boolean suspendVmIfNeeded(Domain domain) { + return true; + } + + @Override + protected boolean shouldFreezeVmFilesystemsForSnapshot(CreateDiskOnlyVmSnapshotCommand cmd) { + return false; + } + + @Override + protected String backupNvramIfNeeded(CreateDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource) { + return null; + } + + @Override + protected boolean resumeVmIfNeeded(Domain domain, String vmName) { + return false; + } + + @Override + protected boolean resumeVmIfNeeded(Domain domain, String vmName, boolean suspended) { + return false; + } + }; + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + LibvirtUtilitiesHelper libvirtUtilitiesHelper = mock(LibvirtUtilitiesHelper.class); + org.libvirt.Connect connect = mock(org.libvirt.Connect.class); + Domain domain = mock(Domain.class); + CreateDiskOnlyVmSnapshotCommand command = mock(CreateDiskOnlyVmSnapshotCommand.class); + VMSnapshotTO target = mock(VMSnapshotTO.class); + + when(command.getVmName()).thenReturn("vm-name"); + when(command.getVolumeTOs()).thenReturn(List.of()); + when(command.getTarget()).thenReturn(target); + when(resource.getLibvirtUtilitiesHelper()).thenReturn(libvirtUtilitiesHelper); + when(libvirtUtilitiesHelper.getConnection()).thenReturn(connect); + when(resource.getDisks(connect, "vm-name")).thenReturn(List.of()); + when(resource.getDomain(connect, "vm-name")).thenReturn(domain); + + CreateDiskOnlyVmSnapshotAnswer answer = (CreateDiskOnlyVmSnapshotAnswer) wrapper.takeDiskOnlyVmSnapshotOfRunningVm(command, resource); + + assertTrue("Snapshot metadata must still be returned when resume fails after snapshot creation", answer.getResult()); + assertTrue(answer.getDetails().contains("could not be resumed")); + } + + @Test + public void testTakeDiskOnlyVmSnapshotOfRunningVmHandlesNullErrorMessage() throws Exception { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper() { + @Override + protected Pair>> createSnapshotXmlAndNewVolumePathMap(List volumeObjectTOS, + List disks, VMSnapshotTO target, LibvirtComputingResource resource) { + return new Pair<>("", Collections.emptyMap()); + } + + @Override + protected boolean shouldSuspendVmForSnapshot(CreateDiskOnlyVmSnapshotCommand cmd) { + return false; + } + + @Override + protected boolean shouldFreezeVmFilesystemsForSnapshot(CreateDiskOnlyVmSnapshotCommand cmd) { + return false; + } + + @Override + protected String backupNvramIfNeeded(CreateDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource) throws IOException { + throw new IOException(); + } + }; + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + LibvirtUtilitiesHelper libvirtUtilitiesHelper = mock(LibvirtUtilitiesHelper.class); + org.libvirt.Connect connect = mock(org.libvirt.Connect.class); + Domain domain = mock(Domain.class); + CreateDiskOnlyVmSnapshotCommand command = mock(CreateDiskOnlyVmSnapshotCommand.class); + VMSnapshotTO target = mock(VMSnapshotTO.class); + + when(command.getVmName()).thenReturn("vm-name"); + when(command.getVolumeTOs()).thenReturn(List.of()); + when(command.getTarget()).thenReturn(target); + when(command.isUefiEnabled()).thenReturn(true); + when(resource.getLibvirtUtilitiesHelper()).thenReturn(libvirtUtilitiesHelper); + when(libvirtUtilitiesHelper.getConnection()).thenReturn(connect); + when(resource.getDisks(connect, "vm-name")).thenReturn(List.of()); + when(resource.getDomain(connect, "vm-name")).thenReturn(domain); + + CreateDiskOnlyVmSnapshotAnswer answer = (CreateDiskOnlyVmSnapshotAnswer) wrapper.takeDiskOnlyVmSnapshotOfRunningVm(command, resource); + + assertFalse(answer.getResult()); + } +}