Skip to content

Commit 6a08bf2

Browse files
JoaoJandreJoaoJandre
committed
Add limits to avoid data loss
1 parent a14aae8 commit 6a08bf2

File tree

6 files changed

+109
-2
lines changed

6 files changed

+109
-2
lines changed

‎engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDao.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,4 +58,6 @@ public interface SnapshotDao extends GenericDao<SnapshotVO, Long>, StateDao<Snap
5858
*/
5959
List<SnapshotVO> listByIds(Object... ids);
6060
List<SnapshotVO> searchByVolumes(List<Long> volumeIds);
61+
62+
List<SnapshotVO> listByVolumeIdAndTypeNotInAndStateNotRemoved(long volumeId, Type... type);
6163
}

‎engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDaoImpl.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.sql.PreparedStatement;
2020
import java.sql.ResultSet;
2121
import java.util.ArrayList;
22+
import java.util.Arrays;
2223
import java.util.List;
2324

2425
import javax.annotation.PostConstruct;
@@ -56,6 +57,10 @@ public class SnapshotDaoImpl extends GenericDaoBase<SnapshotVO, Long> implements
5657
private static final String GET_LAST_SNAPSHOT =
5758
"SELECT snapshots.id FROM snapshot_store_ref, snapshots where snapshots.id = snapshot_store_ref.snapshot_id AND snapshosts.volume_id = ? AND snapshot_store_ref.role = ? ORDER BY created DESC";
5859

60+
private static final String VOLUME_ID = "volumeId";
61+
private static final String NOT_TYPE = "notType";
62+
private static final String STATUS = "status";
63+
5964
private SearchBuilder<SnapshotVO> snapshotIdsSearch;
6065
private SearchBuilder<SnapshotVO> VolumeIdSearch;
6166
private SearchBuilder<SnapshotVO> VolumeIdTypeSearch;
@@ -66,6 +71,8 @@ public class SnapshotDaoImpl extends GenericDaoBase<SnapshotVO, Long> implements
6671
private SearchBuilder<SnapshotVO> StatusSearch;
6772
private SearchBuilder<SnapshotVO> notInStatusSearch;
6873
private GenericSearchBuilder<SnapshotVO, Long> CountSnapshotsByAccount;
74+
75+
private SearchBuilder<SnapshotVO> volumeIdAndTypeNotInSearch;
6976
@Inject
7077
ResourceTagDao _tagsDao;
7178
@Inject
@@ -181,6 +188,12 @@ protected void init() {
181188

182189
InstanceIdSearch.join("instanceSnapshots", volumeSearch, volumeSearch.entity().getId(), InstanceIdSearch.entity().getVolumeId(), JoinType.INNER);
183190
InstanceIdSearch.done();
191+
192+
volumeIdAndTypeNotInSearch = createSearchBuilder();
193+
volumeIdAndTypeNotInSearch.and(VOLUME_ID, volumeIdAndTypeNotInSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
194+
volumeIdAndTypeNotInSearch.and(STATUS, volumeIdAndTypeNotInSearch.entity().getState(), SearchCriteria.Op.NEQ);
195+
volumeIdAndTypeNotInSearch.and(NOT_TYPE, volumeIdAndTypeNotInSearch.entity().getTypeDescription(), SearchCriteria.Op.NOTIN);
196+
volumeIdAndTypeNotInSearch.done();
184197
}
185198

186199
@Override
@@ -299,4 +312,14 @@ public List<SnapshotVO> searchByVolumes(List<Long> volumeIds) {
299312
sc.setParameters("volumeIds", volumeIds.toArray());
300313
return search(sc, null);
301314
}
315+
316+
@Override
317+
public List<SnapshotVO> listByVolumeIdAndTypeNotInAndStateNotRemoved(long volumeId, Type... types) {
318+
SearchCriteria<SnapshotVO> sc = volumeIdAndTypeNotInSearch.create();
319+
sc.setParameters(VOLUME_ID, volumeId);
320+
sc.setParameters(NOT_TYPE, Arrays.stream(types).map(Type::toString).toArray());
321+
sc.setParameters(STATUS, State.Destroyed);
322+
323+
return listBy(sc);
324+
}
302325
}

‎engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,15 @@
2323

2424
import javax.inject.Inject;
2525

26+
import com.cloud.storage.VolumeApiServiceImpl;
2627
import com.cloud.utils.db.TransactionCallback;
28+
import com.cloud.vm.VMInstanceVO;
29+
import com.cloud.vm.dao.VMInstanceDao;
30+
import com.cloud.vm.snapshot.VMSnapshot;
31+
import com.cloud.vm.snapshot.VMSnapshotDetailsVO;
32+
import com.cloud.vm.snapshot.VMSnapshotVO;
33+
import com.cloud.vm.snapshot.dao.VMSnapshotDao;
34+
import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao;
2735
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
2836
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
2937
import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.Event;
@@ -100,6 +108,15 @@ public class DefaultSnapshotStrategy extends SnapshotStrategyBase {
100108
@Inject
101109
SnapshotZoneDao snapshotZoneDao;
102110

111+
@Inject
112+
private VMSnapshotDao vmSnapshotDao;
113+
114+
@Inject
115+
private VMSnapshotDetailsDao vmSnapshotDetailsDao;
116+
117+
@Inject
118+
private VMInstanceDao vmInstanceDao;
119+
103120
private final List<Snapshot.State> snapshotStatesAbleToDeleteSnapshot = Arrays.asList(Snapshot.State.Destroying, Snapshot.State.Destroyed, Snapshot.State.Error, Snapshot.State.Hidden);
104121

105122
public SnapshotDataStoreVO getSnapshotImageStoreRef(long snapshotId, long zoneId) {
@@ -610,6 +627,9 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
610627

611628
@Override
612629
public StrategyPriority canHandle(Snapshot snapshot, Long zoneId, SnapshotOperation op) {
630+
if (SnapshotOperation.TAKE.equals(op)) {
631+
return validateVmSnapshot(snapshot);
632+
}
613633
if (SnapshotOperation.REVERT.equals(op)) {
614634
long volumeId = snapshot.getVolumeId();
615635
VolumeVO volumeVO = volumeDao.findById(volumeId);
@@ -626,6 +646,30 @@ public StrategyPriority canHandle(Snapshot snapshot, Long zoneId, SnapshotOperat
626646
return StrategyPriority.DEFAULT;
627647
}
628648

649+
private StrategyPriority validateVmSnapshot(Snapshot snapshot) {
650+
VolumeVO volumeVO = volumeDao.findById(snapshot.getVolumeId());
651+
Long instanceId = volumeVO.getInstanceId();
652+
if (instanceId == null) {
653+
return StrategyPriority.DEFAULT;
654+
}
655+
656+
VMInstanceVO vm = vmInstanceDao.findById(instanceId);
657+
if (vm == null) {
658+
return StrategyPriority.DEFAULT;
659+
}
660+
661+
for (VMSnapshotVO vmSnapshotVO : vmSnapshotDao.findByVmAndByType(vm.getId(), VMSnapshot.Type.Disk)) {
662+
List<VMSnapshotDetailsVO> vmSnapshotDetails = vmSnapshotDetailsDao.listDetails(vmSnapshotVO.getId());
663+
if (vmSnapshotDetails.stream().anyMatch(vmSnapshotDetailsVO -> vmSnapshotDetailsVO.getName().equals(VolumeApiServiceImpl.KVM_FILE_BASED_STORAGE_SNAPSHOT))) {
664+
logger.warn("VM [{}] already has KVM File-Based storage VM snapshots. These VM snapshots and volume snapshots are not supported " +
665+
"together for KVM. As restoring volume snapshots will erase the VM snapshots and cause data loss.", vm.getUuid());
666+
return StrategyPriority.CANT_HANDLE;
667+
}
668+
}
669+
670+
return StrategyPriority.DEFAULT;
671+
}
672+
629673
protected boolean isSnapshotStoredOnSameZoneStoreForQCOW2Volume(Snapshot snapshot, VolumeVO volumeVO) {
630674
if (volumeVO == null || !ImageFormat.QCOW2.equals(volumeVO.getFormat())) {
631675
return false;

‎engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@
4949
import com.cloud.vm.snapshot.VMSnapshot;
5050
import com.cloud.vm.snapshot.VMSnapshotDetailsVO;
5151
import com.cloud.vm.snapshot.VMSnapshotVO;
52+
import org.apache.cloudstack.backup.BackupOfferingVO;
53+
import org.apache.cloudstack.backup.dao.BackupOfferingDao;
5254
import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
5355
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
5456
import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
@@ -81,6 +83,9 @@ public class KvmFileBasedStorageVmSnapshotStrategy extends StorageVMSnapshotStra
8183
@Inject
8284
protected ResourceLimitService resourceLimitManager;
8385

86+
@Inject
87+
protected BackupOfferingDao backupOfferingDao;
88+
8489
@Override
8590
public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) {
8691
Map<VolumeInfo, SnapshotObject> volumeInfoToSnapshotObjectMap = new HashMap<>();
@@ -316,11 +321,23 @@ public StrategyPriority canHandle(Long vmId, Long rootPoolId, boolean snapshotMe
316321
for (VolumeVO volume : volumes) {
317322
StoragePoolVO storagePoolVO = storagePool.findById(volume.getPoolId());
318323
if (!supportedStoragePoolTypes.contains(storagePoolVO.getPoolType())) {
319-
logger.debug("{} as the VM has a volume that is in a storage with unsupported type [{}].", cantHandleLog, storagePoolVO.getPoolType());
324+
logger.debug(String.format("%s as the VM has a volume that is in a storage with unsupported type [%s].", cantHandleLog, storagePoolVO.getPoolType()));
325+
return StrategyPriority.CANT_HANDLE;
326+
}
327+
List<SnapshotVO> snapshots = snapshotDao.listByVolumeIdAndTypeNotInAndStateNotRemoved(volume.getId(), Snapshot.Type.GROUP);
328+
if (CollectionUtils.isNotEmpty(snapshots)) {
329+
logger.debug("{} as VM has a volume with snapshots {}. Volume snapshots and KvmFileBasedStorageVmSnapshotStrategy are not compatible, as restoring volume snapshots will erase VM " +
330+
"snapshots and cause data loss.", cantHandleLog, snapshots);
320331
return StrategyPriority.CANT_HANDLE;
321332
}
322333
}
323334

335+
BackupOfferingVO backupOffering = backupOfferingDao.findById(vm.getBackupOfferingId());
336+
if (backupOffering != null) {
337+
logger.debug("{} as the VM has a backup offering. This strategy does not support snapshots on VMs with current backup providers.", cantHandleLog);
338+
return StrategyPriority.CANT_HANDLE;
339+
}
340+
324341
return StrategyPriority.HIGHEST;
325342
}
326343

‎plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import com.cloud.storage.ScopeType;
2828
import com.cloud.storage.StoragePoolHostVO;
2929
import com.cloud.storage.Volume;
30+
import com.cloud.storage.VolumeApiServiceImpl;
3031
import com.cloud.storage.VolumeVO;
3132
import com.cloud.storage.dao.StoragePoolHostDao;
3233
import com.cloud.storage.dao.VolumeDao;
@@ -35,6 +36,12 @@
3536
import com.cloud.utils.exception.CloudRuntimeException;
3637
import com.cloud.vm.VirtualMachine;
3738
import com.cloud.vm.dao.VMInstanceDao;
39+
import com.cloud.vm.snapshot.VMSnapshot;
40+
import com.cloud.vm.snapshot.VMSnapshotDetailsVO;
41+
import com.cloud.vm.snapshot.VMSnapshotVO;
42+
import com.cloud.vm.snapshot.dao.VMSnapshotDao;
43+
import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao;
44+
3845

3946
import org.apache.cloudstack.backup.dao.BackupDao;
4047
import org.apache.cloudstack.backup.dao.BackupRepositoryDao;
@@ -87,6 +94,12 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co
8794
@Inject
8895
private AgentManager agentManager;
8996

97+
@Inject
98+
private VMSnapshotDao vmSnapshotDao;
99+
100+
@Inject
101+
private VMSnapshotDetailsDao vmSnapshotDetailsDao;
102+
90103
protected Host getLastVMHypervisorHost(VirtualMachine vm) {
91104
Long hostId = vm.getLastHostId();
92105
if (hostId == null) {
@@ -400,6 +413,14 @@ public Backup createNewBackupEntryForRestorePoint(Backup.RestorePoint restorePoi
400413

401414
@Override
402415
public boolean assignVMToBackupOffering(VirtualMachine vm, BackupOffering backupOffering) {
416+
for (VMSnapshotVO vmSnapshotVO : vmSnapshotDao.findByVmAndByType(vm.getId(), VMSnapshot.Type.Disk)) {
417+
List<VMSnapshotDetailsVO> vmSnapshotDetails = vmSnapshotDetailsDao.listDetails(vmSnapshotVO.getId());
418+
if (vmSnapshotDetails.stream().anyMatch(vmSnapshotDetailsVO -> vmSnapshotDetailsVO.getName().equals(VolumeApiServiceImpl.KVM_FILE_BASED_STORAGE_SNAPSHOT))) {
419+
logger.warn("VM [{}] has VM snapshots using the KvmFileBasedStorageVmSnapshot Strategy; this provider does not support backups on VMs with these snapshots!");
420+
return false;
421+
}
422+
}
423+
403424
return Hypervisor.HypervisorType.KVM.equals(vm.getHypervisorType());
404425
}
405426

‎server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
365365
@Inject
366366
private VMSnapshotDetailsDao vmSnapshotDetailsDao;
367367

368-
protected static final String KVM_FILE_BASED_STORAGE_SNAPSHOT = "kvmFileBasedStorageSnapshot";
368+
public static final String KVM_FILE_BASED_STORAGE_SNAPSHOT = "kvmFileBasedStorageSnapshot";
369369

370370
protected Gson _gson;
371371

0 commit comments

Comments
 (0)