slavkap commented on a change in pull request #4304:
URL: https://github.com/apache/cloudstack/pull/4304#discussion_r575677050
##########
File path: server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
##########
@@ -358,9 +363,33 @@ public VMSnapshot allocVMSnapshot(Long vmId, String
vsDisplayName, String vsDesc
throw new InvalidParameterValueException("Can not snapshot memory
when VM is not in Running state");
}
+ List<VolumeVO> rootVolumes =
_volumeDao.findReadyRootVolumesByInstance(userVmVo.getId());
+ if (rootVolumes == null || rootVolumes.isEmpty()) {
+ throw new CloudRuntimeException("Unable to find root volume for
the user vm:" + userVmVo.getUuid());
+ }
+
+ VolumeVO rootVolume = rootVolumes.get(0);
+ StoragePoolVO rootVolumePool =
_storagePoolDao.findById(rootVolume.getPoolId());
+ if (rootVolumePool == null) {
+ throw new CloudRuntimeException("Unable to find root volume
storage pool for the user vm:" + userVmVo.getUuid());
+ }
+
// for KVM, only allow snapshot with memory when VM is in running state
- if (userVmVo.getHypervisorType() == HypervisorType.KVM &&
userVmVo.getState() == State.Running && !snapshotMemory) {
- throw new InvalidParameterValueException("KVM VM does not allow to
take a disk-only snapshot when VM is in running state");
+ if (userVmVo.getHypervisorType() == HypervisorType.KVM) {
+ if (rootVolumePool.getPoolType() !=
Storage.StoragePoolType.PowerFlex) {
+ if (userVmVo.getState() == State.Running && !snapshotMemory) {
+ throw new InvalidParameterValueException("KVM VM does not
allow to take a disk-only snapshot when VM is in running state");
+ }
+ } else {
+ if (snapshotMemory) {
+ throw new InvalidParameterValueException("Can not snapshot
memory for PowerFlex storage pool");
+ }
+
+ // All volumes should be on the same PowerFlex storage pool
for VM Snapshot
+ if (!isVolumesOfUserVmOnSameStoragePool(userVmVo.getId(),
rootVolumePool.getId())) {
Review comment:
If you consider using VMSnapshotStrategy, this check won't be needed
because you're checking in `ScaleIOVMSnapshotStrategy.canHandle()` that all
volumes are on PowerFlex. Also, this PR #3724 (if it gets in someday) will
handle snapshots of the volumes with different storage pool types
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
