Block use of internal and external snapshots on KVM (#11039)

This commit is contained in:
João Jandre 2025-11-24 07:39:19 -03:00 committed by GitHub
parent 6dc259c7da
commit 8171d9568c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 110 additions and 6 deletions

View File

@ -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

View File

@ -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);
}

View File

@ -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;
}

View File

@ -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;
}
}

View File

@ -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);
}
}
}

View File

@ -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);
}
}
}

View File

@ -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());

View File

@ -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;