DaanHoogland commented on code in PR #6661:
URL: https://github.com/apache/cloudstack/pull/6661#discussion_r992365929
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java:
##########
@@ -709,24 +709,34 @@ public void copyAsync(DataObject srcData, DataObject
dstData, AsyncCompletionCal
} else {
// download volume - first copies to secondary
VolumeObjectTO srcTO = (VolumeObjectTO)srcData.getTO();
-
StorPoolUtil.spLog("StorpoolPrimaryDataStoreDriverImpl.copyAsnc SRC path=%s ",
srcTO.getPath());
-
StorPoolUtil.spLog("StorpoolPrimaryDataStoreDriverImpl.copyAsnc DST
canonicalName=%s ", dstData.getDataStore().getClass().getCanonicalName());
+
StorPoolUtil.spLog("StorpoolPrimaryDataStoreDriverImpl.copyAsnc SRC path=%s DST
canonicalName=%s ", srcTO.getPath(),
dstData.getDataStore().getClass().getCanonicalName());
Review Comment:
this `copyAsync` method is huge, can you at leasrt extract any block you
touched into a private method, please?
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -2905,9 +2905,13 @@ public Volume migrateVolume(MigrateVolumeCmd cmd) {
}
if (liveMigrateVolume &&
HypervisorType.KVM.equals(host.getHypervisorType())) {
- throw new InvalidParameterValueException("KVM does not
support volume live migration due to the limited possibility to refresh VM XML
domain. " +
- "Therefore, to live migrate a volume between
storage pools, one must migrate the VM to a different host as well to force the
VM XML domain update. " +
- "Use 'migrateVirtualMachineWithVolumes' instead.");
+ StoragePoolVO destinationStoragePoolVo =
_storagePoolDao.findById(storagePoolId);
+
+ if (isSourceOrDestNotOnStorPool(storagePoolVO,
destinationStoragePoolVo)) {
+ throw new InvalidParameterValueException("KVM does not
support volume live migration due to the limited possibility to refresh VM XML
domain. " +
+ "Therefore, to live migrate a volume between
storage pools, one must migrate the VM to a different host as well to force the
VM XML domain update. " +
+ "Use 'migrateVirtualMachineWithVolumes'
instead.");
+ }
Review Comment:
can you extract this in an extra method?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
