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 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 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 getBackupStorageStats(Long zoneId) { final List 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;