DaanHoogland commented on code in PR #6785:
URL: https://github.com/apache/cloudstack/pull/6785#discussion_r1168539271
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmd.java:
##########
@@ -83,6 +83,12 @@ public class MigrateVirtualMachineWithVolumeCmd extends
BaseAsyncCmd {
"<1b331390-59f2-4796-9993-bf11c6e76225>&migrateto[2].pool=<41fdb564-9d3b-447d-88ed-7628f7640cbc>")
private Map migrateVolumeTo;
+ @Parameter(name = ApiConstants.AUTO_SELECT,
+ since = "4.18.0",
Review Comment:
no longer true
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -6751,8 +6737,23 @@ private boolean isImplicitPlannerUsedByOffering(long
offeringId) {
return implicitPlannerUsed;
}
- private boolean isVmVolumesOnZoneWideStore(VMInstanceVO vm) {
- final List<VolumeVO> volumes =
_volsDao.findCreatedByInstance(vm.getId());
+ private boolean isVmVolumesUsingLocalStorage(final List<VolumeVO> volumes)
{
Review Comment:
The code in this method looks exactly like the bit in
`isVmVolumesOnZoneWideStore`
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -6751,8 +6737,23 @@ private boolean isImplicitPlannerUsedByOffering(long
offeringId) {
return implicitPlannerUsed;
}
- private boolean isVmVolumesOnZoneWideStore(VMInstanceVO vm) {
- final List<VolumeVO> volumes =
_volsDao.findCreatedByInstance(vm.getId());
+ private boolean isVmVolumesUsingLocalStorage(final List<VolumeVO> volumes)
{
+ if (CollectionUtils.isEmpty(volumes)) {
+ return false;
+ }
+ for (Volume volume : volumes) {
+ if (volume == null || volume.getPoolId() == null) {
+ return false;
+ }
+ StoragePoolVO pool = _storagePoolDao.findById(volume.getPoolId());
+ if (pool == null || !ScopeType.ZONE.equals(pool.getScope())) {
+ return false;
+ }
Review Comment:
should this read something like
```suggestion
if (volume == null || volume.getPoolId() == null) {
return false;
}
StoragePoolVO pool =
_storagePoolDao.findById(volume.getPoolId());
if (pool == null || !ScopeType.ZONE.equals(pool.getScope())) {
return false;
}
```
???
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -6952,6 +6958,18 @@ public VirtualMachine
migrateVirtualMachineWithVolume(Long vmId, Host destinatio
Map<Long, Long> volToPoolObjectMap =
getVolumePoolMappingForMigrateVmWithStorage(vm, volumeToPool);
+ if (destinationHost == null) {
+ Long poolId = null;
+ if (MapUtils.isNotEmpty(volToPoolObjectMap)) {
+ poolId = new ArrayList<>(volToPoolObjectMap.values()).get(0);
+ }
+ DeployDestination deployDestination =
chooseVmMigrationDestination(vm, srcHost, poolId);
+ if (deployDestination == null) {
+ throw new CloudRuntimeException("Unable to find suitable
destination to migrate VM " + vm.getInstanceName());
+ }
+ destinationHost = deployDestination.getHost();
+ }
Review Comment:
can this be extracted?
--
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]
