sureshanaparti commented on a change in pull request #4304: URL: https://github.com/apache/cloudstack/pull/4304#discussion_r579090404
########## 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: The check here does't allow to create a VMSnapshot object when all the volumes are not on the PowerFlex/ScaleIO pool. ScaleIOVMSnapshotStrategy.canHandle() re-confirms and allows the VM Snapshot operations that are supported by ScaleIOVMSnapshotStrategy. ---------------------------------------------------------------- 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