rg9975 commented on code in PR #7889:
URL: https://github.com/apache/cloudstack/pull/7889#discussion_r1374793715
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -6319,6 +6319,16 @@ private VMInstanceVO preVmStorageMigrationCheck(Long
vmId) {
+ " hypervisors: [%s].", hypervisorType,
HYPERVISORS_THAT_CAN_DO_STORAGE_MIGRATION_ON_NON_USER_VMS));
}
+ List<VolumeVO> vols = _volsDao.findByInstance(vm.getId());
+ if (vols.size() > 1) {
+ // OffLineVmwareMigration: data disks are not permitted, here!
+ if (vols.size() > 1 &&
+ // OffLineVmwareMigration: allow multiple disks for vmware
+ !(HypervisorType.VMware.equals(hypervisorType) ||
HypervisorType.KVM.equals(hypervisorType))) {
+ throw new InvalidParameterValueException("Data disks attached
to the vm, can not migrate. Need to detach data disks first");
+ }
+ }
+
Review Comment:
Looking at the logic and the pre-code, I'm not seeing a reason for this
piece of code. It may have been a change made when trying to fully understand
how migration occurred in the Cloudstack framework that was not warranted.
Outside of potentially impacting other hypervisors, its presence doesn't change
anything about KVM. I am removing it in a new commit and if encounter a new
issue on retest will identify and re-introduce a more appropriate change.
--
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]
