hsato03 commented on code in PR #10632:
URL: https://github.com/apache/cloudstack/pull/10632#discussion_r2190240100
##########
engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java:
##########
@@ -469,16 +479,39 @@ public boolean deleteVMSnapshotFromDB(VMSnapshot
vmSnapshot, boolean unmanage) {
@Override
public StrategyPriority canHandle(Long vmId, Long rootPoolId, boolean
snapshotMemory) {
UserVmVO vm = userVmDao.findById(vmId);
- if (vm.getState() == State.Running && !snapshotMemory) {
+ if (State.Running.equals(vm.getState()) && !snapshotMemory) {
+ logger.debug("Default VM snapshot cannot handle VM snapshot for
[{}] as it is running and its memory will not be affected.", vm);
Review Comment:
```suggestion
logger.debug("Default VM snapshot strategy cannot handle VM
snapshot for [{}] as it is running and its memory will not be affected.", vm);
```
##########
engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java:
##########
@@ -469,16 +479,39 @@ public boolean deleteVMSnapshotFromDB(VMSnapshot
vmSnapshot, boolean unmanage) {
@Override
public StrategyPriority canHandle(Long vmId, Long rootPoolId, boolean
snapshotMemory) {
UserVmVO vm = userVmDao.findById(vmId);
- if (vm.getState() == State.Running && !snapshotMemory) {
+ if (State.Running.equals(vm.getState()) && !snapshotMemory) {
+ logger.debug("Default VM snapshot cannot handle VM snapshot for
[{}] as it is running and its memory will not be affected.", vm);
+ return StrategyPriority.CANT_HANDLE;
+ }
+
+ if (vmHasKvmDiskOnlySnapshot(vm)) {
+ logger.debug("Default VM snapshot 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);
return StrategyPriority.CANT_HANDLE;
}
List<VolumeVO> volumes = volumeDao.findByInstance(vmId);
for (VolumeVO volume : volumes) {
if (volume.getFormat() != ImageFormat.QCOW2) {
+ logger.debug("Default VM snapshot cannot handle VM snapshot
for [{}] as it has a volume [{}] that is not in the QCOW2 format.", vm, volume);
Review Comment:
```suggestion
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);
```
##########
engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java:
##########
@@ -469,16 +479,39 @@ public boolean deleteVMSnapshotFromDB(VMSnapshot
vmSnapshot, boolean unmanage) {
@Override
public StrategyPriority canHandle(Long vmId, Long rootPoolId, boolean
snapshotMemory) {
UserVmVO vm = userVmDao.findById(vmId);
- if (vm.getState() == State.Running && !snapshotMemory) {
+ if (State.Running.equals(vm.getState()) && !snapshotMemory) {
+ logger.debug("Default VM snapshot cannot handle VM snapshot for
[{}] as it is running and its memory will not be affected.", vm);
+ return StrategyPriority.CANT_HANDLE;
+ }
+
+ if (vmHasKvmDiskOnlySnapshot(vm)) {
+ logger.debug("Default VM snapshot cannot handle VM snapshot for
[{}] as it has a disk-only VM snapshot using kvmFileBasedStorageSnapshot
strategy." +
Review Comment:
```suggestion
logger.debug("Default VM snapshot strategy cannot handle VM
snapshot for [{}] as it has a disk-only VM snapshot using
kvmFileBasedStorageSnapshot strategy." +
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
