This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new 8171d9568c9 Block use of internal and external snapshots on KVM
(#11039)
8171d9568c9 is described below
commit 8171d9568c941877e0290f77051ed45f31a73544
Author: João Jandre <[email protected]>
AuthorDate: Mon Nov 24 07:39:19 2025 -0300
Block use of internal and external snapshots on KVM (#11039)
---
.../apache/cloudstack/backup/BackupProvider.java | 4 ++
.../apache/cloudstack/backup/BackupService.java | 7 ++++
.../storage/snapshot/DefaultSnapshotStrategy.java | 6 +++
.../vmsnapshot/DefaultVMSnapshotStrategy.java | 48 +++++++++++++++++++---
.../vmsnapshot/VMSnapshotStrategyKVMTest.java | 12 ++++++
.../storage/vmsnapshot/VMSnapshotStrategyTest.java | 23 +++++++++++
.../cloudstack/backup/NASBackupProvider.java | 12 ++++++
.../cloudstack/backup/NASBackupProviderTest.java | 4 ++
8 files changed, 110 insertions(+), 6 deletions(-)
diff --git a/api/src/main/java/org/apache/cloudstack/backup/BackupProvider.java
b/api/src/main/java/org/apache/cloudstack/backup/BackupProvider.java
index 32a714370df..23b8092425d 100644
--- a/api/src/main/java/org/apache/cloudstack/backup/BackupProvider.java
+++ b/api/src/main/java/org/apache/cloudstack/backup/BackupProvider.java
@@ -124,6 +124,10 @@ public interface BackupProvider {
*/
boolean supportsInstanceFromBackup();
+ default boolean supportsMemoryVmSnapshot() {
+ return true;
+ }
+
/**
* Returns the backup storage usage (Used, Total) for a backup provider
* @param zoneId the zone for which to return metrics
diff --git a/api/src/main/java/org/apache/cloudstack/backup/BackupService.java
b/api/src/main/java/org/apache/cloudstack/backup/BackupService.java
index d4beb629fe0..3ba2978c0fa 100644
--- a/api/src/main/java/org/apache/cloudstack/backup/BackupService.java
+++ b/api/src/main/java/org/apache/cloudstack/backup/BackupService.java
@@ -34,4 +34,11 @@ public interface BackupService {
* @return backup provider
*/
BackupProvider getBackupProvider(final Long zoneId);
+
+ /**
+ * Find backup provider by name
+ * @param name backup provider name
+ * @return backup provider
+ */
+ BackupProvider getBackupProvider(final String name);
}
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 c1981941ac0..c6d89a87014 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
@@ -672,6 +672,12 @@ public class DefaultSnapshotStrategy extends
SnapshotStrategyBase {
}
}
+ if
(CollectionUtils.isNotEmpty(vmSnapshotDao.findByVmAndByType(volumeVO.getInstanceId(),
VMSnapshot.Type.DiskAndMemory))) {
+ logger.debug("DefaultSnapshotStrategy cannot handle snapshot [{}]
for volume [{}] as the volume is attached to a VM with disk-and-memory VM
snapshots." +
+ "Restoring the volume snapshot will corrupt any newer
disk-and-memory VM snapshots.", snapshot);
+ return StrategyPriority.CANT_HANDLE;
+ }
+
return StrategyPriority.DEFAULT;
}
diff --git
a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
index 6e933396871..527b7fbeda1 100644
---
a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
+++
b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
@@ -26,14 +26,21 @@ import javax.inject.Inject;
import javax.naming.ConfigurationException;
import com.cloud.hypervisor.Hypervisor;
+import com.cloud.storage.Snapshot;
+import com.cloud.storage.dao.SnapshotDao;
import com.cloud.vm.snapshot.VMSnapshotDetailsVO;
import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao;
+import org.apache.cloudstack.backup.BackupManager;
+import org.apache.cloudstack.backup.BackupOfferingVO;
+import org.apache.cloudstack.backup.BackupProvider;
+import org.apache.cloudstack.backup.dao.BackupOfferingDao;
import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotOptions;
import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotStrategy;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
import org.apache.cloudstack.storage.to.VolumeObjectTO;
+import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.lang3.StringUtils;
import com.cloud.agent.AgentManager;
@@ -104,7 +111,16 @@ public class DefaultVMSnapshotStrategy extends ManagerBase
implements VMSnapshot
PrimaryDataStoreDao primaryDataStoreDao;
@Inject
- VMSnapshotDetailsDao vmSnapshotDetailsDao;
+ private VMSnapshotDetailsDao vmSnapshotDetailsDao;
+
+ @Inject
+ private BackupManager backupManager;
+
+ @Inject
+ private BackupOfferingDao backupOfferingDao;
+
+ @Inject
+ private SnapshotDao snapshotDao;
protected static final String KVM_FILE_BASED_STORAGE_SNAPSHOT =
"kvmFileBasedStorageSnapshot";
@@ -480,24 +496,44 @@ public class DefaultVMSnapshotStrategy extends
ManagerBase implements VMSnapshot
@Override
public StrategyPriority canHandle(Long vmId, Long rootPoolId, boolean
snapshotMemory) {
UserVmVO vm = userVmDao.findById(vmId);
+ String cantHandleLog = String.format("Default VM snapshot cannot
handle VM snapshot for [%s]", vm);
if (State.Running.equals(vm.getState()) && !snapshotMemory) {
- logger.debug("Default VM snapshot strategy cannot handle VM
snapshot for [{}] as it is running and its memory will not be affected.", vm);
+ logger.debug("{} as it is running and its memory will not be
affected.", cantHandleLog, vm);
return StrategyPriority.CANT_HANDLE;
}
if (vmHasKvmDiskOnlySnapshot(vm)) {
- logger.debug("Default VM snapshot strategy cannot handle VM
snapshot for [{}] as it has a disk-only VM snapshot using
kvmFileBasedStorageSnapshot strategy." +
- "These two strategies are not compatible, as reverting a
disk-only VM snapshot will erase newer disk-and-memory VM snapshots.", vm);
+ logger.debug("{} as it is not compatible with disk-only VM
snapshot on KVM. As disk-and-memory snapshots use internal snapshots and
disk-only VM snapshots use" +
+ " external snapshots. When restoring external snapshots,
any newer internal snapshots are lost.", cantHandleLog);
return StrategyPriority.CANT_HANDLE;
}
List<VolumeVO> volumes = volumeDao.findByInstance(vmId);
for (VolumeVO volume : volumes) {
if (volume.getFormat() != ImageFormat.QCOW2) {
- logger.debug("Default VM snapshot strategy cannot handle VM
snapshot for [{}] as it has a volume [{}] that is not in the QCOW2 format.",
vm, volume);
+ logger.debug("{} as it has a volume [{}] that is not in the
QCOW2 format.", cantHandleLog, vm, volume);
+ return StrategyPriority.CANT_HANDLE;
+ }
+
+ if
(CollectionUtils.isNotEmpty(snapshotDao.listByVolumeIdAndTypeNotInAndStateNotRemoved(volume.getId(),
Snapshot.Type.GROUP))) {
+ logger.debug("{} as it has a volume [{}] with volume
snapshots. As disk-and-memory snapshots use internal snapshots and volume
snapshots use external" +
+ " snapshots. When restoring external snapshots, any
newer internal snapshots are lost.", cantHandleLog, volume);
return StrategyPriority.CANT_HANDLE;
}
}
+
+
+ BackupOfferingVO backupOfferingVO =
backupOfferingDao.findById(vm.getBackupOfferingId());
+ if (backupOfferingVO == null) {
+ return StrategyPriority.DEFAULT;
+ }
+
+ BackupProvider provider =
backupManager.getBackupProvider(backupOfferingVO.getProvider());
+ if (!provider.supportsMemoryVmSnapshot()) {
+ logger.debug("{} as the VM has a backup offering for a provider
that is not supported.", cantHandleLog);
+ return StrategyPriority.CANT_HANDLE;
+ }
+
return StrategyPriority.DEFAULT;
}
@@ -508,7 +544,7 @@ public class DefaultVMSnapshotStrategy extends ManagerBase
implements VMSnapshot
for (VMSnapshotVO vmSnapshotVO :
vmSnapshotDao.findByVmAndByType(vm.getId(), VMSnapshot.Type.Disk)) {
List<VMSnapshotDetailsVO> vmSnapshotDetails =
vmSnapshotDetailsDao.listDetails(vmSnapshotVO.getId());
- if (vmSnapshotDetails.stream().anyMatch(vmSnapshotDetailsVO ->
vmSnapshotDetailsVO.getName().equals(KVM_FILE_BASED_STORAGE_SNAPSHOT))) {
+ if (vmSnapshotDetails.stream().anyMatch(detailsVO ->
KVM_FILE_BASED_STORAGE_SNAPSHOT.equals(detailsVO.getName()) ||
STORAGE_SNAPSHOT.equals(detailsVO.getName()))) {
return true;
}
}
diff --git
a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyKVMTest.java
b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyKVMTest.java
index 609a1225118..02c17f8f3bd 100644
---
a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyKVMTest.java
+++
b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyKVMTest.java
@@ -29,6 +29,8 @@ import java.util.UUID;
import javax.inject.Inject;
+import org.apache.cloudstack.backup.BackupManager;
+import org.apache.cloudstack.backup.dao.BackupOfferingDao;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
import
org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProviderManager;
@@ -431,5 +433,15 @@ public class VMSnapshotStrategyKVMTest extends TestCase{
public VMSnapshotDetailsDao vmSnapshotDetailsDao () {
return Mockito.mock(VMSnapshotDetailsDao.class);
}
+
+ @Bean
+ public BackupOfferingDao backupOfferingDao() {
+ return Mockito.mock(BackupOfferingDao.class);
+ }
+
+ @Bean
+ public BackupManager backupManager() {
+ return Mockito.mock(BackupManager.class);
+ }
}
}
diff --git
a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyTest.java
b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyTest.java
index 829ca5ade39..a20b52fac2d 100644
---
a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyTest.java
+++
b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyTest.java
@@ -25,7 +25,10 @@ import java.util.List;
import javax.inject.Inject;
+import com.cloud.storage.dao.SnapshotDao;
import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao;
+import org.apache.cloudstack.backup.BackupManager;
+import org.apache.cloudstack.backup.dao.BackupOfferingDao;
import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotStrategy;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
@@ -318,5 +321,25 @@ public class VMSnapshotStrategyTest extends TestCase {
public PrimaryDataStoreDao primaryDataStoreDao() {
return Mockito.mock(PrimaryDataStoreDao.class);
}
+
+ @Bean
+ public BackupOfferingDao backupOfferingDao() {
+ return Mockito.mock(BackupOfferingDao.class);
+ }
+
+ @Bean
+ public VMSnapshotDetailsDao VMSnapshotDetailsDao() {
+ return Mockito.mock(VMSnapshotDetailsDao.class);
+ }
+
+ @Bean
+ public SnapshotDao snapshotDao() {
+ return Mockito.mock(SnapshotDao.class);
+ }
+
+ @Bean
+ public BackupManager backupManager() {
+ return Mockito.mock(BackupManager.class);
+ }
}
}
diff --git
a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java
b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java
index 3b2c2692b0d..a350d80d27f 100644
---
a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java
+++
b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java
@@ -56,6 +56,7 @@ import org.apache.cloudstack.framework.config.Configurable;
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
+import org.apache.commons.collections4.CollectionUtils;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.LogManager;
@@ -195,6 +196,12 @@ public class NASBackupProvider extends AdapterBase
implements BackupProvider, Co
throw new CloudRuntimeException("No valid backup repository found
for the VM, please check the attached backup offering");
}
+ if
(CollectionUtils.isNotEmpty(vmSnapshotDao.findByVmAndByType(vm.getId(),
VMSnapshot.Type.DiskAndMemory))) {
+ logger.debug("NAS backup provider cannot take backups of a VM [{}]
with disk-and-memory VM snapshots. Restoring the backup will corrupt any newer
disk-and-memory " +
+ "VM snapshots.", vm);
+ throw new CloudRuntimeException(String.format("Cannot take backup
of VM [%s] as it has disk-and-memory VM snapshots.", vm.getUuid()));
+ }
+
final Date creationDate = new Date();
final String backupPath = String.format("%s/%s", vm.getInstanceName(),
new
SimpleDateFormat("yyyy.MM.dd.HH.mm.ss").format(creationDate));
@@ -517,6 +524,11 @@ public class NASBackupProvider extends AdapterBase
implements BackupProvider, Co
return true;
}
+ @Override
+ public boolean supportsMemoryVmSnapshot() {
+ return false;
+ }
+
@Override
public Pair<Long, Long> getBackupStorageStats(Long zoneId) {
final List<BackupRepository> repositories =
backupRepositoryDao.listByZoneAndProvider(zoneId, getName());
diff --git
a/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java
b/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java
index 7540cabbbf5..a512292cd28 100644
---
a/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java
+++
b/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java
@@ -24,6 +24,7 @@ import java.util.List;
import java.util.Objects;
import java.util.Optional;
+import com.cloud.vm.snapshot.dao.VMSnapshotDao;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -92,6 +93,9 @@ public class NASBackupProviderTest {
@Mock
private PrimaryDataStoreDao storagePoolDao;
+ @Mock
+ private VMSnapshotDao vmSnapshotDaoMock;
+
@Test
public void testDeleteBackup() throws OperationTimedoutException,
AgentUnavailableException {
Long hostId = 1L;
