shwstppr commented on a change in pull request #5410:
URL: https://github.com/apache/cloudstack/pull/5410#discussion_r703260492
##########
File path:
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -2932,8 +2932,6 @@ protected void
createStoragePoolMappingsForVolumes(VirtualMachineProfile profile
executeManagedStorageChecksWhenTargetStoragePoolNotProvided(targetHost,
currentPool, volume);
if (ScopeType.HOST.equals(currentPool.getScope()) ||
isStorageCrossClusterMigration(plan.getClusterId(), currentPool)) {
createVolumeToStoragePoolMappingIfPossible(profile, plan,
volumeToPoolObjectMap, volume, currentPool);
- } else {
- volumeToPoolObjectMap.put(volume, currentPool);
Review comment:
@GabrielBrascher as you have seen `allVolumes` is a confusing name here.
It is only the volumes that are not mapped using API. I've not tested but
VMware might expect mappings for each of the volumes of the VM.
I feel instead of preventing adding a map here, we should look into the code
that does migration and skip volumes that are already on the destination pool.
For KVM this could be
https://github.com/apache/cloudstack/blob/main/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java#L1978-L1980
--
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]
