From 1489e06f8c6301a1ea893deeaaf83b9671b6c9e1 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Mon, 25 May 2026 12:22:20 +0530 Subject: [PATCH 1/2] Restrict VM snapshot without memory for VM with encrypted volumes using KvmFileBasedStorageVmSnapshotStrategy, and Use file copy for revert to snapshot from encrypted volumes (as earlier) --- .../main/java/com/cloud/storage/Volume.java | 2 ++ .../orchestration/VolumeOrchestrator.java | 10 +++---- .../orchestration/VolumeOrchestratorTest.java | 7 ++--- .../main/java/com/cloud/storage/VolumeVO.java | 5 ++++ .../snapshot/DefaultSnapshotStrategy.java | 2 +- .../snapshot/SnapshotDataFactoryImpl.java | 1 - ...KvmFileBasedStorageVmSnapshotStrategy.java | 6 +++- .../storage/volume/VolumeObject.java | 5 ++++ .../LibvirtRevertSnapshotCommandWrapper.java | 21 +++++++++---- .../kvm/storage/LibvirtStorageAdaptor.java | 2 +- .../kvm/storage/ScaleIOStorageAdaptor.java | 4 +-- ...bvirtRevertSnapshotCommandWrapperTest.java | 30 +++++++++++++++---- .../cloud/storage/VolumeApiServiceImpl.java | 3 +- .../storage/snapshot/SnapshotManagerImpl.java | 8 ++--- .../vm/snapshot/VMSnapshotManagerImpl.java | 3 +- 15 files changed, 73 insertions(+), 36 deletions(-) diff --git a/api/src/main/java/com/cloud/storage/Volume.java b/api/src/main/java/com/cloud/storage/Volume.java index c7fbdb0a5445..57d249cde954 100644 --- a/api/src/main/java/com/cloud/storage/Volume.java +++ b/api/src/main/java/com/cloud/storage/Volume.java @@ -279,5 +279,7 @@ enum Event { void setEncryptFormat(String encryptFormat); + boolean isEncrypted(); + boolean isDeleteProtection(); } diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java index 933accbda524..0d25233f1b8c 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java @@ -317,7 +317,7 @@ public VolumeInfo moveVolume(VolumeInfo volumeInfo, long destPoolDcId, Long dest // Find a destination storage pool with the specified criteria DiskOffering diskOffering = _entityMgr.findById(DiskOffering.class, volumeInfo.getDiskOfferingId()); DiskProfile dskCh = new DiskProfile(volumeInfo.getId(), volumeInfo.getVolumeType(), volumeInfo.getName(), diskOffering.getId(), diskOffering.getDiskSize(), diskOffering.getTagsArray(), - diskOffering.isUseLocalStorage(), diskOffering.isRecreatable(), null, (diskOffering.getEncrypt() || volumeInfo.getPassphraseId() != null)); + diskOffering.isUseLocalStorage(), diskOffering.isRecreatable(), null, (diskOffering.getEncrypt() || volumeInfo.isEncrypted())); dskCh.setHyperType(dataDiskHyperType); storageMgr.setDiskProfileThrottling(dskCh, null, diskOffering); @@ -352,7 +352,7 @@ public VolumeVO allocateDuplicateVolumeVO(Volume oldVol, DiskOffering diskOfferi newVol.setInstanceId(oldVol.getInstanceId()); newVol.setRecreatable(oldVol.isRecreatable()); newVol.setFormat(oldVol.getFormat()); - if ((diskOffering == null || diskOffering.getEncrypt()) && oldVol.getPassphraseId() != null) { + if ((diskOffering == null || diskOffering.getEncrypt()) && oldVol.isEncrypted()) { PassphraseVO passphrase = passphraseDao.persist(new PassphraseVO(true)); newVol.setPassphraseId(passphrase.getId()); } @@ -646,7 +646,7 @@ public VolumeInfo createVolumeFromSnapshot(Volume volume, Snapshot snapshot, Use } protected DiskProfile createDiskCharacteristics(VolumeInfo volumeInfo, VirtualMachineTemplate template, DataCenter dc, DiskOffering diskOffering) { - boolean requiresEncryption = diskOffering.getEncrypt() || volumeInfo.getPassphraseId() != null; + boolean requiresEncryption = diskOffering.getEncrypt() || volumeInfo.isEncrypted(); if (volumeInfo.getVolumeType() == Type.ROOT && Storage.ImageFormat.ISO != template.getFormat()) { String templateToString = getReflectOnlySelectedFields(template); String zoneToString = getReflectOnlySelectedFields(dc); @@ -1905,7 +1905,7 @@ private Pair recreateVolume(VolumeVO vol, VirtualMachinePro } private VolumeVO setPassphraseForVolumeEncryption(VolumeVO volume) { - if (volume.getPassphraseId() != null) { + if (volume.isEncrypted()) { return volume; } logger.debug("Creating passphrase for the volume: " + volume.getName()); @@ -1942,7 +1942,7 @@ protected void updateVolumeSize(DataStore store, VolumeVO vol) throws ResourceAl PrimaryDataStoreDriver driver = (PrimaryDataStoreDriver) store.getDriver(); long newSize = driver.getVolumeSizeRequiredOnPool(vol.getSize(), template == null ? null : template.getSize(), - vol.getPassphraseId() != null); + vol.isEncrypted()); if (newSize == vol.getSize()) { return; diff --git a/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java b/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java index b4a26c17e2e5..a4003a5057c1 100644 --- a/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java +++ b/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java @@ -274,7 +274,7 @@ public void testAllocateDuplicateVolumeVOBasic() { Mockito.when(oldVol.getInstanceId()).thenReturn(6L); Mockito.when(oldVol.isRecreatable()).thenReturn(false); Mockito.when(oldVol.getFormat()).thenReturn(Storage.ImageFormat.QCOW2); - Mockito.when(oldVol.getPassphraseId()).thenReturn(null); // no encryption + Mockito.when(oldVol.isEncrypted()).thenReturn(false); // no encryption VolumeVO persistedVol = Mockito.mock(VolumeVO.class); Mockito.when(volumeDao.persist(Mockito.any(VolumeVO.class))).thenReturn(persistedVol); @@ -303,7 +303,7 @@ public void testAllocateDuplicateVolumeVOWithEncryption() { Mockito.when(oldVol.getInstanceId()).thenReturn(7L); Mockito.when(oldVol.isRecreatable()).thenReturn(true); Mockito.when(oldVol.getFormat()).thenReturn(Storage.ImageFormat.RAW); - Mockito.when(oldVol.getPassphraseId()).thenReturn(42L); + Mockito.when(oldVol.isEncrypted()).thenReturn(true); PassphraseVO passphrase = Mockito.mock(PassphraseVO.class); Mockito.when(passphrase.getId()).thenReturn(999L); @@ -336,9 +336,6 @@ public void testAllocateDuplicateVolumeVOWithTemplateOverride() { VolumeVO persistedVol = Mockito.mock(VolumeVO.class); Mockito.when(volumeDao.persist(Mockito.any())).thenReturn(persistedVol); - PassphraseVO mockPassPhrase = Mockito.mock(PassphraseVO.class); - Mockito.when(passphraseDao.persist(Mockito.any())).thenReturn(mockPassPhrase); - VolumeVO result = volumeOrchestrator.allocateDuplicateVolumeVO(oldVol, null, 222L); assertNotNull(result); } diff --git a/engine/schema/src/main/java/com/cloud/storage/VolumeVO.java b/engine/schema/src/main/java/com/cloud/storage/VolumeVO.java index 653be54a9109..871bdacda9e6 100644 --- a/engine/schema/src/main/java/com/cloud/storage/VolumeVO.java +++ b/engine/schema/src/main/java/com/cloud/storage/VolumeVO.java @@ -687,6 +687,11 @@ public void setExternalUuid(String externalUuid) { public void setEncryptFormat(String encryptFormat) { this.encryptFormat = encryptFormat; } + @Override + public boolean isEncrypted() { + return this.passphraseId != null || this.encryptFormat != null; + } + @Override public boolean isDeleteProtection() { return deleteProtection; diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java index 3c037f05639e..83401e273ce9 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java @@ -646,7 +646,7 @@ public StrategyPriority canHandle(Snapshot snapshot, Long zoneId, SnapshotOperat return StrategyPriority.CANT_HANDLE; } if (zoneId != null && SnapshotOperation.DELETE.equals(op)) { - logger.debug(String.format("canHandle for zone ID: %d, operation: %s - %s", zoneId, op, StrategyPriority.DEFAULT)); + logger.debug("canHandle for zone ID: {}, operation: {} - {}", zoneId, op, StrategyPriority.DEFAULT); } return StrategyPriority.DEFAULT; } diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotDataFactoryImpl.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotDataFactoryImpl.java index 5c0a613d82d6..2f8ecc14a74c 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotDataFactoryImpl.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotDataFactoryImpl.java @@ -205,7 +205,6 @@ public SnapshotInfo getReadySnapshotOnCache(long snapshotId) { } else { return null; } - } @Override 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..ec59a0d14054 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 @@ -324,9 +324,13 @@ public StrategyPriority canHandle(Long vmId, Long rootPoolId, boolean snapshotMe List volumes = volumeDao.findByInstance(vmId); for (VolumeVO volume : volumes) { + if (volume.isEncrypted()) { + logger.debug("{} as the VM has a volume that is encrypted.", cantHandleLog); + return StrategyPriority.CANT_HANDLE; + } StoragePoolVO storagePoolVO = storagePool.findById(volume.getPoolId()); if (!supportedStoragePoolTypes.contains(storagePoolVO.getPoolType())) { - logger.debug(String.format("%s as the VM has a volume that is in a storage with unsupported type [%s].", cantHandleLog, storagePoolVO.getPoolType())); + logger.debug("{} as the VM has a volume that is in a storage with unsupported type [{}].", cantHandleLog, storagePoolVO.getPoolType()); return StrategyPriority.CANT_HANDLE; } List snapshots = snapshotDao.listByVolumeIdAndTypeNotInAndStateNotRemoved(volume.getId(), Snapshot.Type.GROUP); diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java index 43218b3f6a02..68d602adcce1 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java @@ -947,6 +947,11 @@ public void setEncryptFormat(String encryptFormat) { volumeVO.setEncryptFormat(encryptFormat); } + @Override + public boolean isEncrypted() { + return volumeVO.isEncrypted(); + } + @Override public boolean isDeleteProtection() { return volumeVO.isDeleteProtection(); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java index 5d76d140f229..17c72fc5f302 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java @@ -20,8 +20,10 @@ package com.cloud.hypervisor.kvm.resource.wrapper; import java.io.File; +import java.io.IOException; import java.nio.file.Files; import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; import java.util.Set; import java.util.HashSet; import java.util.Arrays; @@ -174,12 +176,13 @@ protected void revertVolumeToSnapshot(KVMStoragePool kvmStoragePoolSecondary, Sn storagePoolSet = resource.connectToAllVolumeSnapshotSecondaryStorages(volumeObjectTo); } - logger.debug(String.format("Reverting volume [%s] to snapshot [%s].", volumeObjectTo, snapshotToPrint)); + logger.debug("Reverting volume [{}] to snapshot [{}].", volumeObjectTo, snapshotToPrint); try { - replaceVolumeWithSnapshot(volumePath, snapshotPath); - logger.debug(String.format("Successfully reverted volume [%s] to snapshot [%s].", volumeObjectTo, snapshotToPrint)); - } catch (LibvirtException | QemuImgException ex) { + boolean isVolumeEncrypted = volumeObjectTo.getPassphrase() != null || volumeObjectTo.getEncryptFormat() != null; + replaceVolumeWithSnapshot(volumePath, snapshotPath, isVolumeEncrypted); + logger.debug("Successfully reverted volume [{}] to snapshot [{}].", volumeObjectTo, snapshotToPrint); + } catch (LibvirtException | QemuImgException | IOException ex) { throw new CloudRuntimeException(String.format("Unable to revert volume [%s] to snapshot [%s] due to [%s].", volumeObjectTo, snapshotToPrint, ex.getMessage()), ex); } finally { if (storagePoolSet != null) { @@ -228,8 +231,14 @@ protected Pair getSnapshot(SnapshotObjectTO snapshotOn * @throws LibvirtException If can't replace the current volume with the snapshot. * @throws QemuImgException If can't replace the current volume with the snapshot. */ - protected void replaceVolumeWithSnapshot(String volumePath, String snapshotPath) throws LibvirtException, QemuImgException { - logger.debug(String.format("Replacing volume at [%s] with snapshot that is at [%s].", volumePath, snapshotPath)); + protected void replaceVolumeWithSnapshot(String volumePath, String snapshotPath, boolean isVolumeEncrypted) throws LibvirtException, QemuImgException, IOException { + if (isVolumeEncrypted) { + logger.debug("Replacing encrypted volume at [{}] with snapshot that is at [{}] using file copy.", volumePath, snapshotPath); + Files.copy(Paths.get(snapshotPath), Paths.get(volumePath), StandardCopyOption.REPLACE_EXISTING); + return; + } + + logger.debug("Replacing volume at [{}] with snapshot that is at [{}] using qemu-img.", volumePath, snapshotPath); QemuImg qemuImg = new QemuImg(AgentPropertiesFileHandler.getPropertyValue(AgentProperties.REVERT_SNAPSHOT_TIMEOUT) * 1000); QemuImgFile volumeImg = new QemuImgFile(volumePath, QemuImg.PhysicalDiskFormat.QCOW2); QemuImgFile snapshotImg = new QemuImgFile(snapshotPath, QemuImg.PhysicalDiskFormat.QCOW2); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java index a03daeb197bf..f3384580b189 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java @@ -1261,7 +1261,7 @@ public KVMPhysicalDisk createDiskFromTemplate(KVMPhysicalDisk template, if (destPool.getType() == StoragePoolType.RBD) { disk = createDiskFromTemplateOnRBD(template, name, format, provisioningType, size, destPool, timeout); } else { - try (KeyFile keyFile = new KeyFile(passphrase)){ + try (KeyFile keyFile = new KeyFile(passphrase)) { String newUuid = name; List passphraseObjects = new ArrayList<>(); disk = destPool.createPhysicalDisk(newUuid, format, provisioningType, template.getVirtualSize(), passphrase); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java index 82bc35f009ee..d140721d4d24 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java @@ -252,7 +252,7 @@ public KVMPhysicalDisk createPhysicalDisk(String name, KVMStoragePool pool, Qemu return null; } - if(!connectPhysicalDisk(name, pool, null, false)) { + if (!connectPhysicalDisk(name, pool, null, false)) { throw new CloudRuntimeException(String.format("Failed to ensure disk %s was present", name)); } @@ -261,7 +261,7 @@ public KVMPhysicalDisk createPhysicalDisk(String name, KVMStoragePool pool, Qemu if (provisioningType.equals(Storage.ProvisioningType.THIN)) { disk.setFormat(QemuImg.PhysicalDiskFormat.QCOW2); disk.setQemuEncryptFormat(QemuObject.EncryptFormat.LUKS); - try (KeyFile keyFile = new KeyFile(passphrase)){ + try (KeyFile keyFile = new KeyFile(passphrase)) { QemuImg qemuImg = new QemuImg(0, true, false); Map options = new HashMap<>(); List qemuObjects = new ArrayList<>(); diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapperTest.java index cfcb2a2f972d..e861f2b65538 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapperTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapperTest.java @@ -18,6 +18,7 @@ package com.cloud.hypervisor.kvm.resource.wrapper; import java.io.File; +import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -144,28 +145,45 @@ public void validateGetSnapshotPathDoesNotExistsOnSecondaryStorageThrows() { } @Test - public void validateRevertVolumeToSnapshotReplaceSuccessfully() throws LibvirtException, QemuImgException { + public void validateRevertVolumeToSnapshotReplaceSuccessfully() throws LibvirtException, QemuImgException, IOException { Mockito.doReturn(volumeObjectToMock).when(snapshotObjectToSecondaryMock).getVolume(); + Mockito.doReturn(null).when(volumeObjectToMock).getPassphrase(); + Mockito.doReturn(null).when(volumeObjectToMock).getEncryptFormat(); Mockito.doReturn(pairStringSnapshotObjectToMock).when(libvirtRevertSnapshotCommandWrapperSpy).getSnapshot(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); - Mockito.doNothing().when(libvirtRevertSnapshotCommandWrapperSpy).replaceVolumeWithSnapshot(Mockito.any(), Mockito.any()); + Mockito.doNothing().when(libvirtRevertSnapshotCommandWrapperSpy).replaceVolumeWithSnapshot(Mockito.any(), Mockito.any(), Mockito.anyBoolean()); libvirtRevertSnapshotCommandWrapperSpy.revertVolumeToSnapshot(kvmStoragePoolSecondaryMock, snapshotObjectToPrimaryMock, snapshotObjectToSecondaryMock, kvmStoragePoolPrimaryMock, resourceMock ); } @Test (expected = CloudRuntimeException.class) - public void validateRevertVolumeToSnapshotReplaceVolumeThrowsQemuImgException() throws LibvirtException, QemuImgException { + public void validateRevertVolumeToSnapshotReplaceVolumeThrowsQemuImgException() throws LibvirtException, QemuImgException, IOException { Mockito.doReturn(volumeObjectToMock).when(snapshotObjectToSecondaryMock).getVolume(); + Mockito.doReturn(null).when(volumeObjectToMock).getPassphrase(); + Mockito.doReturn(null).when(volumeObjectToMock).getEncryptFormat(); Mockito.doReturn(pairStringSnapshotObjectToMock).when(libvirtRevertSnapshotCommandWrapperSpy).getSnapshot(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); - Mockito.doThrow(QemuImgException.class).when(libvirtRevertSnapshotCommandWrapperSpy).replaceVolumeWithSnapshot(Mockito.any(), Mockito.any()); + Mockito.doThrow(QemuImgException.class).when(libvirtRevertSnapshotCommandWrapperSpy).replaceVolumeWithSnapshot(Mockito.any(), Mockito.any(), Mockito.anyBoolean()); libvirtRevertSnapshotCommandWrapperSpy.revertVolumeToSnapshot(kvmStoragePoolSecondaryMock, snapshotObjectToPrimaryMock, snapshotObjectToSecondaryMock, kvmStoragePoolPrimaryMock, resourceMock ); } @Test (expected = CloudRuntimeException.class) - public void validateRevertVolumeToSnapshotReplaceVolumeThrowsLibvirtException() throws LibvirtException, QemuImgException { + public void validateRevertVolumeToSnapshotReplaceVolumeThrowsLibvirtException() throws LibvirtException, QemuImgException, IOException { Mockito.doReturn(volumeObjectToMock).when(snapshotObjectToSecondaryMock).getVolume(); + Mockito.doReturn(null).when(volumeObjectToMock).getPassphrase(); + Mockito.doReturn(null).when(volumeObjectToMock).getEncryptFormat(); Mockito.doReturn(pairStringSnapshotObjectToMock).when(libvirtRevertSnapshotCommandWrapperSpy).getSnapshot(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); - Mockito.doThrow(LibvirtException.class).when(libvirtRevertSnapshotCommandWrapperSpy).replaceVolumeWithSnapshot(Mockito.any(), Mockito.any()); + Mockito.doThrow(LibvirtException.class).when(libvirtRevertSnapshotCommandWrapperSpy).replaceVolumeWithSnapshot(Mockito.any(), Mockito.any(), Mockito.anyBoolean()); + libvirtRevertSnapshotCommandWrapperSpy.revertVolumeToSnapshot(kvmStoragePoolSecondaryMock, snapshotObjectToPrimaryMock, snapshotObjectToSecondaryMock, kvmStoragePoolPrimaryMock, resourceMock + ); + } + + @Test + public void validateRevertEncryptedVolumeToSnapshotReplaceSuccessfully() throws LibvirtException, QemuImgException, IOException { + Mockito.doReturn(volumeObjectToMock).when(snapshotObjectToSecondaryMock).getVolume(); + Mockito.doReturn(null).when(volumeObjectToMock).getPassphrase(); + Mockito.doReturn("luks").when(volumeObjectToMock).getEncryptFormat(); + Mockito.doReturn(pairStringSnapshotObjectToMock).when(libvirtRevertSnapshotCommandWrapperSpy).getSnapshot(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); + Mockito.doNothing().when(libvirtRevertSnapshotCommandWrapperSpy).replaceVolumeWithSnapshot(Mockito.any(), Mockito.any(), Mockito.anyBoolean()); libvirtRevertSnapshotCommandWrapperSpy.revertVolumeToSnapshot(kvmStoragePoolSecondaryMock, snapshotObjectToPrimaryMock, snapshotObjectToSecondaryMock, kvmStoragePoolPrimaryMock, resourceMock ); } diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 687e432f166b..f0f4389dbcd6 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -4045,7 +4045,7 @@ private Snapshot orchestrateTakeVolumeSnapshot(Long volumeId, Long policyId, Lon } boolean isSnapshotOnStorPoolOnly = volume.getStoragePoolType() == StoragePoolType.StorPool && SnapshotInfo.BackupSnapshotAfterTakingSnapshot.value(); - if (volume.getEncryptFormat() != null && volume.getAttachedVM() != null && volume.getAttachedVM().getState() != State.Stopped && !isSnapshotOnStorPoolOnly) { + if (volume.getPassphraseId() != null && volume.getAttachedVM() != null && volume.getAttachedVM().getState() != State.Stopped && !isSnapshotOnStorPoolOnly) { logger.debug(String.format("Refusing to take snapshot of encrypted volume (%s) on running VM (%s)", volume, volume.getAttachedVM())); throw new UnsupportedOperationException("Volume snapshots for encrypted volumes are not supported if VM is running"); } @@ -4160,7 +4160,6 @@ public Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, } } - return snapshotMgr.allocSnapshot(volumeId, policyId, snapshotName, locationType, false, zoneIds); } diff --git a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java index d60bff095406..89fe043fc5ba 100755 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -383,12 +383,12 @@ public Snapshot revertSnapshot(Long snapshotId) { Long instanceId = volume.getInstanceId(); - // If this volume is attached to an VM, then the VM needs to be in the stopped state - // in order to revert the volume + // If this volume is attached to an Instance, then the Instance needs to be in the stopped or shutdown state + // in order to revert volume to the snapshot if (instanceId != null) { UserVmVO vm = _vmDao.findById(instanceId); if (vm.getState() != State.Stopped && vm.getState() != State.Shutdown) { - throw new InvalidParameterValueException("The Instance the specified disk is attached to is not in the shutdown state."); + throw new InvalidParameterValueException("The Instance of the specified disk is attached to, is not in the stopped/shutdown state."); } // If target VM has associated VM snapshots then don't allow to revert from snapshot List vmSnapshots = _vmSnapshotDao.findByVm(instanceId); @@ -1177,7 +1177,7 @@ public SnapshotPolicyVO createPolicy(CreateSnapshotPolicyCmd cmd, Account policy DiskOfferingVO diskOffering = diskOfferingDao.findByIdIncludingRemoved(volume.getDiskOfferingId()); if (diskOffering == null) { throw new InvalidParameterValueException(String.format("Failed to find disk offering for the volume [%s]", volume.getUuid())); - } else if(diskOffering.getEncrypt()) { + } else if (diskOffering.getEncrypt()) { throw new UnsupportedOperationException(String.format("Encrypted volumes don't support snapshot schedules, cannot create snapshot policy for the volume [%s]", volume.getUuid())); } diff --git a/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java b/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java index baa74128ce38..2268dad05c72 100644 --- a/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java @@ -368,7 +368,7 @@ public VMSnapshot allocVMSnapshot(Long vmId, String vsDisplayName, String vsDesc throw new InvalidParameterValueException("Creating Instance Snapshot failed because Instance:" + vmId + " is not in Running or Stopped state"); } - if(snapshotMemory && userVmVo.getState() != VirtualMachine.State.Running){ + if (snapshotMemory && userVmVo.getState() != VirtualMachine.State.Running) { throw new InvalidParameterValueException("Can not Snapshot memory when the Instance is not in Running state"); } @@ -399,7 +399,6 @@ public VMSnapshot allocVMSnapshot(Long vmId, String vsDisplayName, String vsDesc if (rootVolume.getPassphraseId() != null && userVmVo.getState() == VirtualMachine.State.Running && Boolean.TRUE.equals(snapshotMemory)) { throw new UnsupportedOperationException("Cannot create Instance memory Snapshots on KVM from encrypted root volumes"); } - } // check access From 1f6ad6f09c1387032760c9782aa1403f8541f78f Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Mon, 25 May 2026 12:37:22 +0530 Subject: [PATCH 2/2] Updated encrypted volume check --- .../main/java/com/cloud/storage/VolumeApiServiceImpl.java | 6 +++--- .../main/java/com/cloud/template/TemplateManagerImpl.java | 4 ++-- .../java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index f0f4389dbcd6..06c560137402 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -903,7 +903,7 @@ public VolumeVO allocVolume(CreateVolumeCmd cmd) throws ResourceAllocationExcept parentVolume = _volsDao.findByIdIncludingRemoved(snapshotCheck.getVolumeId()); // Don't support creating templates from encrypted volumes (yet) - if (parentVolume.getPassphraseId() != null) { + if (parentVolume.isEncrypted()) { throw new UnsupportedOperationException("Cannot create new volumes from encrypted volume snapshots"); } @@ -4045,7 +4045,7 @@ private Snapshot orchestrateTakeVolumeSnapshot(Long volumeId, Long policyId, Lon } boolean isSnapshotOnStorPoolOnly = volume.getStoragePoolType() == StoragePoolType.StorPool && SnapshotInfo.BackupSnapshotAfterTakingSnapshot.value(); - if (volume.getPassphraseId() != null && volume.getAttachedVM() != null && volume.getAttachedVM().getState() != State.Stopped && !isSnapshotOnStorPoolOnly) { + if (volume.isEncrypted() && volume.getAttachedVM() != null && volume.getAttachedVM().getState() != State.Stopped && !isSnapshotOnStorPoolOnly) { logger.debug(String.format("Refusing to take snapshot of encrypted volume (%s) on running VM (%s)", volume, volume.getAttachedVM())); throw new UnsupportedOperationException("Volume snapshots for encrypted volumes are not supported if VM is running"); } @@ -4285,7 +4285,7 @@ public String extractVolume(ExtractVolumeCmd cmd) { throw ex; } - if (volume.getPassphraseId() != null) { + if (volume.isEncrypted()) { throw new InvalidParameterValueException("Extraction of encrypted volumes is unsupported"); } diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index c0e45a75f810..58e215165d94 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -1970,7 +1970,7 @@ public VMTemplateVO createPrivateTemplateRecord(CreateTemplateCmd cmd, Account t _accountMgr.checkAccess(caller, null, true, volume); // Don't support creating templates from encrypted volumes (yet) - if (volume.getPassphraseId() != null) { + if (volume.isEncrypted()) { throw new UnsupportedOperationException("Cannot create Templates from encrypted volumes"); } @@ -1998,7 +1998,7 @@ public VMTemplateVO createPrivateTemplateRecord(CreateTemplateCmd cmd, Account t volume = _volumeDao.findByIdIncludingRemoved(snapshot.getVolumeId()); // Don't support creating templates from encrypted volumes (yet) - if (volume != null && volume.getPassphraseId() != null) { + if (volume != null && volume.isEncrypted()) { throw new UnsupportedOperationException("Cannot create Templates from Snapshots of encrypted volumes"); } diff --git a/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java b/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java index 2268dad05c72..0b5ca4fcde07 100644 --- a/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java @@ -396,7 +396,7 @@ public VMSnapshot allocVMSnapshot(Long vmId, String vsDisplayName, String vsDesc } // disallow KVM snapshots for VMs if root volume is encrypted (Qemu crash) - if (rootVolume.getPassphraseId() != null && userVmVo.getState() == VirtualMachine.State.Running && Boolean.TRUE.equals(snapshotMemory)) { + if (rootVolume.isEncrypted() && userVmVo.getState() == VirtualMachine.State.Running && Boolean.TRUE.equals(snapshotMemory)) { throw new UnsupportedOperationException("Cannot create Instance memory Snapshots on KVM from encrypted root volumes"); } }