From 4addc222605c7141f7f704fc9b334cb0203b814f Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 19 May 2021 16:04:59 +0530 Subject: [PATCH 1/6] refactor: migrate with storage host capability check Refactors Boolean HypervisorCapabilitiesDao::isStorageMotionSupported to boolean HypervisorCapabilitiesDao::isStorageMotionSupported for simplifying callers. Refactors log messages. Signed-off-by: Abhishek Kumar --- .../cloud/hypervisor/dao/HypervisorCapabilitiesDao.java | 2 +- .../hypervisor/dao/HypervisorCapabilitiesDaoImpl.java | 2 +- .../main/java/com/cloud/server/ManagementServerImpl.java | 4 ++-- server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | 8 ++++---- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/hypervisor/dao/HypervisorCapabilitiesDao.java b/engine/schema/src/main/java/com/cloud/hypervisor/dao/HypervisorCapabilitiesDao.java index 45c88060496b..9255fae0c9ea 100644 --- a/engine/schema/src/main/java/com/cloud/hypervisor/dao/HypervisorCapabilitiesDao.java +++ b/engine/schema/src/main/java/com/cloud/hypervisor/dao/HypervisorCapabilitiesDao.java @@ -38,5 +38,5 @@ public interface HypervisorCapabilitiesDao extends GenericDao getHypervisorsWithDefaultEntries(); - Boolean isStorageMotionSupported(HypervisorType hypervisorType, String hypervisorVersion); + boolean isStorageMotionSupported(HypervisorType hypervisorType, String hypervisorVersion); } diff --git a/engine/schema/src/main/java/com/cloud/hypervisor/dao/HypervisorCapabilitiesDaoImpl.java b/engine/schema/src/main/java/com/cloud/hypervisor/dao/HypervisorCapabilitiesDaoImpl.java index 09b39749ec29..35f765b554ac 100644 --- a/engine/schema/src/main/java/com/cloud/hypervisor/dao/HypervisorCapabilitiesDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/hypervisor/dao/HypervisorCapabilitiesDaoImpl.java @@ -121,7 +121,7 @@ public List getHypervisorsWithDefaultEntries() { } @Override - public Boolean isStorageMotionSupported(HypervisorType hypervisorType, String hypervisorVersion) { + public boolean isStorageMotionSupported(HypervisorType hypervisorType, String hypervisorVersion) { HypervisorCapabilitiesVO hostCapabilities = findByHypervisorTypeAndVersion(hypervisorType, hypervisorVersion); if (hostCapabilities == null && HypervisorType.KVM.equals(hypervisorType)) { List hypervisorCapabilitiesList = listAllByHypervisorType(HypervisorType.KVM); diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index 2e7c8fe49a18..824e5e007432 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -1291,7 +1291,7 @@ public Ternary, Integer>, List, Map, Integer>, List, Map Date: Wed, 19 May 2021 16:46:01 +0530 Subject: [PATCH 2/6] simplify Signed-off-by: Abhishek Kumar --- .../main/java/com/cloud/server/ManagementServerImpl.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index 824e5e007432..5494e13a900e 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -1345,11 +1345,8 @@ public Ternary, Integer>, List, Map Date: Wed, 19 May 2021 17:42:39 +0530 Subject: [PATCH 3/6] refactor Signed-off-by: Abhishek Kumar --- .../com/cloud/storage/StorageManager.java | 4 +- .../AbstractStoragePoolAllocator.java | 2 +- .../deploy/DeploymentPlanningManagerImpl.java | 4 +- .../com/cloud/storage/StorageManagerImpl.java | 2 +- .../cloud/storage/VolumeApiServiceImpl.java | 2 +- .../java/com/cloud/vm/UserVmManagerImpl.java | 157 ++++++++++-------- 6 files changed, 92 insertions(+), 79 deletions(-) diff --git a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java index db41a2f099ac..c19c623b51ef 100644 --- a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java +++ b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java @@ -19,7 +19,6 @@ import java.math.BigDecimal; import java.util.List; -import com.cloud.agent.api.ModifyStoragePoolAnswer; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.HypervisorHostListener; import org.apache.cloudstack.framework.config.ConfigKey; @@ -27,6 +26,7 @@ import com.cloud.agent.api.Answer; import com.cloud.agent.api.Command; +import com.cloud.agent.api.ModifyStoragePoolAnswer; import com.cloud.agent.api.StoragePoolInfo; import com.cloud.agent.api.to.DataTO; import com.cloud.agent.api.to.DiskTO; @@ -243,7 +243,7 @@ public interface StorageManager extends StorageService { boolean storagePoolCompatibleWithVolumePool(StoragePool pool, Volume volume); - boolean isStoragePoolComplaintWithStoragePolicy(List volumes, StoragePool pool) throws StorageUnavailableException; + boolean isStoragePoolCompliantWithStoragePolicy(List volumes, StoragePool pool) throws StorageUnavailableException; boolean registerHostListener(String providerUuid, HypervisorHostListener listener); diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java index 7de40e924de8..10d39ee88fa9 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java @@ -241,7 +241,7 @@ protected boolean filter(ExcludeList avoid, StoragePool pool, DiskProfile dskCh, } try { - boolean isStoragePoolStoragepolicyComplaince = storageMgr.isStoragePoolComplaintWithStoragePolicy(requestVolumes, pool); + boolean isStoragePoolStoragepolicyComplaince = storageMgr.isStoragePoolCompliantWithStoragePolicy(requestVolumes, pool); if (!isStoragePoolStoragepolicyComplaince) { return false; } diff --git a/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java b/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java index fed29aa5a50d..9a2c02f34ac2 100644 --- a/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java +++ b/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java @@ -1330,7 +1330,7 @@ public int compare(Volume v1, Volume v2) { hostCanAccessPool = true; if (potentialHost.getHypervisorType() == HypervisorType.VMware) { try { - boolean isStoragePoolStoragepolicyComplaince = _storageMgr.isStoragePoolComplaintWithStoragePolicy(allVolumes, storagePool); + boolean isStoragePoolStoragepolicyComplaince = _storageMgr.isStoragePoolCompliantWithStoragePolicy(allVolumes, storagePool); if (!isStoragePoolStoragepolicyComplaince) { continue; } @@ -1369,7 +1369,7 @@ public int compare(Volume v1, Volume v2) { if (potentialHost.getHypervisorType() == HypervisorType.VMware) { try { - boolean isStoragePoolStoragepolicyComplaince = _storageMgr.isStoragePoolComplaintWithStoragePolicy(requestVolumes, potentialSPool); + boolean isStoragePoolStoragepolicyComplaince = _storageMgr.isStoragePoolCompliantWithStoragePolicy(requestVolumes, potentialSPool); if (!isStoragePoolStoragepolicyComplaince) { continue; } diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 5e7be3083239..b37f03ce5418 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -2393,7 +2393,7 @@ public boolean storagePoolHasEnoughSpaceForResize(StoragePool pool, long current } @Override - public boolean isStoragePoolComplaintWithStoragePolicy(List volumes, StoragePool pool) throws StorageUnavailableException { + public boolean isStoragePoolCompliantWithStoragePolicy(List volumes, StoragePool pool) throws StorageUnavailableException { if (volumes == null || volumes.isEmpty()) { return false; } diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index cda3e15399df..eb999c84f45b 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -2406,7 +2406,7 @@ public Volume migrateVolume(MigrateVolumeCmd cmd) { HypervisorType hypervisorType = _volsDao.getHypervisorType(volumeId); if (hypervisorType.equals(HypervisorType.VMware)) { try { - boolean isStoragePoolStoragepolicyComplaince = storageMgr.isStoragePoolComplaintWithStoragePolicy(Arrays.asList(vol), destPool); + boolean isStoragePoolStoragepolicyComplaince = storageMgr.isStoragePoolCompliantWithStoragePolicy(Arrays.asList(vol), destPool); if (!isStoragePoolStoragepolicyComplaince) { throw new CloudRuntimeException(String.format("Storage pool %s is not storage policy compliance with the volume %s", poolUuid, vol.getUuid())); } diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index e449303290a7..fa66dec063b8 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -6220,56 +6220,18 @@ private boolean isVmVolumesOnZoneWideStore(VMInstanceVO vm) { return true; } - @Override - @ActionEvent(eventType = EventTypes.EVENT_VM_MIGRATE, eventDescription = "migrating VM", async = true) - public VirtualMachine migrateVirtualMachineWithVolume(Long vmId, Host destinationHost, Map volumeToPool) throws ResourceUnavailableException, - ConcurrentOperationException, ManagementServerException, VirtualMachineMigrationException { - // Access check - only root administrator can migrate VM. - Account caller = CallContext.current().getCallingAccount(); - if (!_accountMgr.isRootAdmin(caller.getId())) { - if (s_logger.isDebugEnabled()) { - s_logger.debug("Caller is not a root admin, permission denied to migrate the VM"); - } - throw new PermissionDeniedException("No permission to migrate VM, Only Root Admin can migrate a VM!"); - } - - VMInstanceVO vm = _vmInstanceDao.findById(vmId); - if (vm == null) { - throw new InvalidParameterValueException("Unable to find the vm by id " + vmId); - } - - // OfflineVmwareMigration: this would be it ;) if multiple paths exist: unify - if (vm.getState() != State.Running) { - // OfflineVmwareMigration: and not vmware - if (s_logger.isDebugEnabled()) { - s_logger.debug("VM is not Running, unable to migrate the vm " + vm); - } - CloudRuntimeException ex = new CloudRuntimeException("VM is not Running, unable to migrate the vm with" + " specified id"); - ex.addProxyObject(vm.getUuid(), "vmId"); - throw ex; - } - - if(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString()) != null) { - throw new InvalidParameterValueException("Live Migration of GPU enabled VM is not supported"); - } - - // OfflineVmwareMigration: this condition is to complicated. (already a method somewhere) - if (!vm.getHypervisorType().equals(HypervisorType.XenServer) && !vm.getHypervisorType().equals(HypervisorType.VMware) && !vm.getHypervisorType().equals(HypervisorType.KVM) - && !vm.getHypervisorType().equals(HypervisorType.Ovm) && !vm.getHypervisorType().equals(HypervisorType.Hyperv) - && !vm.getHypervisorType().equals(HypervisorType.Simulator)) { - throw new InvalidParameterValueException("Unsupported hypervisor type for vm migration, we support" + " XenServer/VMware/KVM only"); - } - + private Pair getHostsForMigrateVmWithStorage(VMInstanceVO vm, Host destinationHost) throws VirtualMachineMigrationException { long srcHostId = vm.getHostId(); Host srcHost = _resourceMgr.getHost(srcHostId); if (srcHost == null) { - throw new InvalidParameterValueException("Cannot migrate VM, host with id: " + srcHostId + " for VM not found"); + throw new InvalidParameterValueException("Cannot migrate VM, host with ID: " + srcHostId + " for VM not found"); } // Check if source and destination hosts are valid and migrating to same host if (destinationHost.getId() == srcHostId) { - throw new InvalidParameterValueException("Cannot migrate VM, VM is already present on this host, please" + " specify valid destination host to migrate the VM"); + throw new InvalidParameterValueException(String.format("Cannot migrate VM as it is already present on host %s (ID: %s), please specify valid destination host to migrate the VM", + destinationHost.getName(), destinationHost.getUuid())); } String srcHostVersion = srcHost.getHypervisorVersion(); @@ -6300,37 +6262,39 @@ public VirtualMachine migrateVirtualMachineWithVolume(Long vmId, Host destinatio } if (!_hypervisorCapabilitiesDao.isStorageMotionSupported(srcHost.getHypervisorType(), srcHostVersion)) { - throw new CloudRuntimeException(String.format("Migration with storage isn't supported for source host %s [ID: %s] on hypervisor %s with version %s", srcHost.getName(), srcHost.getUuid(), srcHost.getHypervisorType(), srcHost.getHypervisorVersion())); + throw new CloudRuntimeException(String.format("Migration with storage isn't supported for source host %s (ID: %s) on hypervisor %s with version %s", srcHost.getName(), srcHost.getUuid(), srcHost.getHypervisorType(), srcHost.getHypervisorVersion())); } if (srcHostVersion == null || !srcHostVersion.equals(destHostVersion)) { if (!_hypervisorCapabilitiesDao.isStorageMotionSupported(destinationHost.getHypervisorType(), destHostVersion)) { - throw new CloudRuntimeException(String.format("Migration with storage isn't supported for target host %s [ID: %s] on hypervisor %s with version %s", destinationHost.getName(), destinationHost.getUuid(), destinationHost.getHypervisorType(), destinationHost.getHypervisorVersion())); + throw new CloudRuntimeException(String.format("Migration with storage isn't supported for target host %s (ID: %s) on hypervisor %s with version %s", destinationHost.getName(), destinationHost.getUuid(), destinationHost.getHypervisorType(), destinationHost.getHypervisorVersion())); } } // Check if destination host is up. if (destinationHost.getState() != com.cloud.host.Status.Up || destinationHost.getResourceState() != ResourceState.Enabled) { - throw new CloudRuntimeException("Cannot migrate VM, destination host is not in correct state, has " + "status: " + destinationHost.getState() + ", state: " - + destinationHost.getResourceState()); + throw new CloudRuntimeException(String.format("Cannot migrate VM, destination host %s (ID: %s) is not in correct state, has status: %s, state: %s", + destinationHost.getName(), destinationHost.getUuid(), destinationHost.getState(), destinationHost.getResourceState())); } - // Check that Vm does not have VM Snapshots - if (_vmSnapshotDao.findByVm(vmId).size() > 0) { - throw new InvalidParameterValueException("VM with VM Snapshots cannot be migrated with storage, please remove all VM snapshots"); + // Check max guest vm limit for the destinationHost. + if (_capacityMgr.checkIfHostReachMaxGuestLimit(destinationHost)) { + throw new VirtualMachineMigrationException(String.format("Cannot migrate VM as destination host %s (ID: %s) already has max running vms (count includes system VMs)", + destinationHost.getName(), destinationHost.getUuid())); } - List vmVolumes = _volsDao.findUsableVolumesForInstance(vm.getId()); + return new Pair<>(srcHost, destinationHost); + } + + private Map getVolumePoolMappingForMigrateVmWithStorage(VMInstanceVO vm, Map volumeToPool) { Map volToPoolObjectMap = new HashMap(); - if (!isVMUsingLocalStorage(vm) && MapUtils.isEmpty(volumeToPool) - && (destinationHost.getClusterId().equals(srcHost.getClusterId()) || isVmVolumesOnZoneWideStore(vm))){ - // If volumes do not have to be migrated - // call migrateVirtualMachine for non-user VMs else throw exception - if (!VirtualMachine.Type.User.equals(vm.getType())) { - return migrateVirtualMachine(vmId, destinationHost); + + List vmVolumes = _volsDao.findUsableVolumesForInstance(vm.getId()); + // Check if all the volumes are in the correct state. + for (VolumeVO volume : vmVolumes) { + if (volume.getState() != Volume.State.Ready) { + throw new CloudRuntimeException("Volume " + volume + " of the VM is not in Ready state. Cannot " + "migrate the vm with its volumes."); } - throw new InvalidParameterValueException("Migration of the vm " + vm + "from host " + srcHost + " to destination host " + destinationHost - + " doesn't involve migrating the volumes."); } if (MapUtils.isNotEmpty(volumeToPool)) { @@ -6350,13 +6314,13 @@ public VirtualMachine migrateVirtualMachineWithVolume(Long vmId, Host destinatio if (!vmVolumes.contains(volume)) { throw new InvalidParameterValueException("There volume " + volume + " doesn't belong to " + "the virtual machine " + vm + " that has to be migrated"); } - volToPoolObjectMap.put(Long.valueOf(volume.getId()), Long.valueOf(pool.getId())); + volToPoolObjectMap.put(volume.getId(), pool.getId()); } HypervisorType hypervisorType = _volsDao.getHypervisorType(volume.getId()); if (hypervisorType.equals(HypervisorType.VMware)) { try { - boolean isStoragePoolStoragepolicyComplaince = storageManager.isStoragePoolComplaintWithStoragePolicy(Arrays.asList(volume), pool); - if (!isStoragePoolStoragepolicyComplaince) { + boolean isStoragePoolStoragepolicyCompliance = storageManager.isStoragePoolCompliantWithStoragePolicy(Arrays.asList(volume), pool); + if (!isStoragePoolStoragepolicyCompliance) { throw new CloudRuntimeException(String.format("Storage pool %s is not storage policy compliance with the volume %s", pool.getUuid(), volume.getUuid())); } } catch (StorageUnavailableException e) { @@ -6365,24 +6329,73 @@ public VirtualMachine migrateVirtualMachineWithVolume(Long vmId, Host destinatio } } } + return volToPoolObjectMap; + } - // Check if all the volumes are in the correct state. - for (VolumeVO volume : vmVolumes) { - if (volume.getState() != Volume.State.Ready) { - throw new CloudRuntimeException("Volume " + volume + " of the VM is not in Ready state. Cannot " + "migrate the vm with its volumes."); + @Override + @ActionEvent(eventType = EventTypes.EVENT_VM_MIGRATE, eventDescription = "migrating VM", async = true) + public VirtualMachine migrateVirtualMachineWithVolume(Long vmId, Host destinationHost, Map volumeToPool) throws ResourceUnavailableException, + ConcurrentOperationException, ManagementServerException, VirtualMachineMigrationException { + // Access check - only root administrator can migrate VM. + Account caller = CallContext.current().getCallingAccount(); + if (!_accountMgr.isRootAdmin(caller.getId())) { + if (s_logger.isDebugEnabled()) { + s_logger.debug("Caller is not a root admin, permission denied to migrate the VM"); } + throw new PermissionDeniedException("No permission to migrate VM, Only Root Admin can migrate a VM!"); } - // Check max guest vm limit for the destinationHost. - HostVO destinationHostVO = _hostDao.findById(destinationHost.getId()); - if (_capacityMgr.checkIfHostReachMaxGuestLimit(destinationHostVO)) { - throw new VirtualMachineMigrationException("Host name: " + destinationHost.getName() + ", hostId: " + destinationHost.getId() - + " already has max running vms (count includes system VMs). Cannot" + " migrate to this host"); + VMInstanceVO vm = _vmInstanceDao.findById(vmId); + if (vm == null) { + throw new InvalidParameterValueException("Unable to find the vm by id " + vmId); } - checkHostsDedication(vm, srcHostId, destinationHost.getId()); + // OfflineVmwareMigration: this would be it ;) if multiple paths exist: unify + if (vm.getState() != State.Running) { + // OfflineVmwareMigration: and not vmware + if (s_logger.isDebugEnabled()) { + s_logger.debug("VM is not Running, unable to migrate the vm " + vm); + } + CloudRuntimeException ex = new CloudRuntimeException("VM is not Running, unable to migrate the vm with" + " specified id"); + ex.addProxyObject(vm.getUuid(), "vmId"); + throw ex; + } + + if(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString()) != null) { + throw new InvalidParameterValueException("Live Migration of GPU enabled VM is not supported"); + } + + // OfflineVmwareMigration: this condition is to complicated. (already a method somewhere) + if (!vm.getHypervisorType().equals(HypervisorType.XenServer) && !vm.getHypervisorType().equals(HypervisorType.VMware) && !vm.getHypervisorType().equals(HypervisorType.KVM) + && !vm.getHypervisorType().equals(HypervisorType.Ovm) && !vm.getHypervisorType().equals(HypervisorType.Hyperv) + && !vm.getHypervisorType().equals(HypervisorType.Simulator)) { + throw new InvalidParameterValueException("Unsupported hypervisor type for vm migration, we support" + " XenServer/VMware/KVM only"); + } + + Pair sourceDestinationHosts = getHostsForMigrateVmWithStorage(vm, destinationHost); + Host srcHost = sourceDestinationHosts.first(); + + // Check that Vm does not have VM Snapshots + if (_vmSnapshotDao.findByVm(vmId).size() > 0) { + throw new InvalidParameterValueException("VM with VM Snapshots cannot be migrated with storage, please remove all VM snapshots"); + } + + if (!isVMUsingLocalStorage(vm) && MapUtils.isEmpty(volumeToPool) + && (destinationHost.getClusterId().equals(srcHost.getClusterId()) || isVmVolumesOnZoneWideStore(vm))){ + // If volumes do not have to be migrated + // call migrateVirtualMachine for non-user VMs else throw exception + if (!VirtualMachine.Type.User.equals(vm.getType())) { + return migrateVirtualMachine(vmId, destinationHost); + } + throw new InvalidParameterValueException("Migration of the vm " + vm + "from host " + srcHost + " to destination host " + destinationHost + + " doesn't involve migrating the volumes."); + } + + Map volToPoolObjectMap = getVolumePoolMappingForMigrateVmWithStorage(vm, volumeToPool); + + checkHostsDedication(vm, srcHost.getId(), destinationHost.getId()); - _itMgr.migrateWithStorage(vm.getUuid(), srcHostId, destinationHost.getId(), volToPoolObjectMap); + _itMgr.migrateWithStorage(vm.getUuid(), srcHost.getId(), destinationHost.getId(), volToPoolObjectMap); return findMigratedVm(vm.getId(), vm.getType()); } From b266b58c905cf40755f77051597f067e27421400 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 20 May 2021 13:01:58 +0530 Subject: [PATCH 4/6] changes Signed-off-by: Abhishek Kumar --- .../java/com/cloud/vm/UserVmManagerImpl.java | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index fa66dec063b8..209ea224fa6e 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -6312,7 +6312,7 @@ private Map getVolumePoolMappingForMigrateVmWithStorage(VMInstanceVO } else { // Verify the volume given belongs to the vm. if (!vmVolumes.contains(volume)) { - throw new InvalidParameterValueException("There volume " + volume + " doesn't belong to " + "the virtual machine " + vm + " that has to be migrated"); + throw new InvalidParameterValueException(String.format("Volume " + volume + " doesn't belong to the VM %s (ID: %s) that has to be migrated", vm.getInstanceName(), vm.getUuid())); } volToPoolObjectMap.put(volume.getId(), pool.getId()); } @@ -6347,7 +6347,7 @@ public VirtualMachine migrateVirtualMachineWithVolume(Long vmId, Host destinatio VMInstanceVO vm = _vmInstanceDao.findById(vmId); if (vm == null) { - throw new InvalidParameterValueException("Unable to find the vm by id " + vmId); + throw new InvalidParameterValueException("Unable to find the VM by ID " + vmId); } // OfflineVmwareMigration: this would be it ;) if multiple paths exist: unify @@ -6356,7 +6356,7 @@ public VirtualMachine migrateVirtualMachineWithVolume(Long vmId, Host destinatio if (s_logger.isDebugEnabled()) { s_logger.debug("VM is not Running, unable to migrate the vm " + vm); } - CloudRuntimeException ex = new CloudRuntimeException("VM is not Running, unable to migrate the vm with" + " specified id"); + CloudRuntimeException ex = new CloudRuntimeException(String.format("Unable to migrate the VM %s (ID: %s) as it is not in Running state", vm.getInstanceName(), vm.getUuid())); ex.addProxyObject(vm.getUuid(), "vmId"); throw ex; } @@ -6366,20 +6366,22 @@ public VirtualMachine migrateVirtualMachineWithVolume(Long vmId, Host destinatio } // OfflineVmwareMigration: this condition is to complicated. (already a method somewhere) - if (!vm.getHypervisorType().equals(HypervisorType.XenServer) && !vm.getHypervisorType().equals(HypervisorType.VMware) && !vm.getHypervisorType().equals(HypervisorType.KVM) - && !vm.getHypervisorType().equals(HypervisorType.Ovm) && !vm.getHypervisorType().equals(HypervisorType.Hyperv) - && !vm.getHypervisorType().equals(HypervisorType.Simulator)) { - throw new InvalidParameterValueException("Unsupported hypervisor type for vm migration, we support" + " XenServer/VMware/KVM only"); + if (!Arrays.asList(new HypervisorType[]{ + HypervisorType.XenServer, + HypervisorType.VMware, + HypervisorType.KVM, + HypervisorType.Simulator}).contains(vm.getHypervisorType())) { + throw new InvalidParameterValueException(String.format("Unsupported hypervisor: %s for VM migration, we support XenServer/VMware/KVM only", vm.getHypervisorType())); } - Pair sourceDestinationHosts = getHostsForMigrateVmWithStorage(vm, destinationHost); - Host srcHost = sourceDestinationHosts.first(); - // Check that Vm does not have VM Snapshots if (_vmSnapshotDao.findByVm(vmId).size() > 0) { throw new InvalidParameterValueException("VM with VM Snapshots cannot be migrated with storage, please remove all VM snapshots"); } + Pair sourceDestinationHosts = getHostsForMigrateVmWithStorage(vm, destinationHost); + Host srcHost = sourceDestinationHosts.first(); + if (!isVMUsingLocalStorage(vm) && MapUtils.isEmpty(volumeToPool) && (destinationHost.getClusterId().equals(srcHost.getClusterId()) || isVmVolumesOnZoneWideStore(vm))){ // If volumes do not have to be migrated @@ -6387,8 +6389,8 @@ public VirtualMachine migrateVirtualMachineWithVolume(Long vmId, Host destinatio if (!VirtualMachine.Type.User.equals(vm.getType())) { return migrateVirtualMachine(vmId, destinationHost); } - throw new InvalidParameterValueException("Migration of the vm " + vm + "from host " + srcHost + " to destination host " + destinationHost - + " doesn't involve migrating the volumes."); + throw new InvalidParameterValueException(String.format("Migration of the VM: %s (ID: %s) from host %s (ID: %s) to destination host %s (ID: %s) doesn't involve migrating the volumes", + vm.getInstanceName(), vm.getUuid(), srcHost.getName(), srcHost.getUuid(), destinationHost.getName(), destinationHost.getUuid())); } Map volToPoolObjectMap = getVolumePoolMappingForMigrateVmWithStorage(vm, volumeToPool); From 0d26afabe8c5a170876d5f753c0c73371ed8f532 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 25 May 2021 13:43:38 +0530 Subject: [PATCH 5/6] review comments addressed Signed-off-by: Abhishek Kumar --- .../com/cloud/storage/StorageManagerImpl.java | 24 +++++++-------- .../java/com/cloud/vm/UserVmManagerImpl.java | 29 +++++++++++-------- 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index b37f03ce5418..a6ad9ef9ce16 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -16,6 +16,8 @@ // under the License. package com.cloud.storage; +import static com.cloud.utils.NumbersUtil.toHumanReadableSize; + import java.math.BigDecimal; import java.net.URI; import java.net.URISyntaxException; @@ -29,11 +31,11 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Random; import java.util.Set; -import java.util.LinkedHashSet; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; @@ -41,11 +43,6 @@ import javax.inject.Inject; -import com.cloud.agent.api.to.StorageFilerTO; -import com.cloud.dc.VsphereStoragePolicyVO; -import com.cloud.dc.dao.VsphereStoragePolicyDao; -import com.cloud.service.dao.ServiceOfferingDetailsDao; -import com.cloud.utils.StringUtils; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.command.admin.storage.CancelPrimaryStorageMaintenanceCmd; import org.apache.cloudstack.api.command.admin.storage.CreateSecondaryStagingStoreCmd; @@ -53,8 +50,8 @@ import org.apache.cloudstack.api.command.admin.storage.DeleteImageStoreCmd; import org.apache.cloudstack.api.command.admin.storage.DeletePoolCmd; import org.apache.cloudstack.api.command.admin.storage.DeleteSecondaryStagingStoreCmd; -import org.apache.cloudstack.api.command.admin.storage.UpdateStoragePoolCmd; import org.apache.cloudstack.api.command.admin.storage.SyncStoragePoolCmd; +import org.apache.cloudstack.api.command.admin.storage.UpdateStoragePoolCmd; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; @@ -121,12 +118,13 @@ import com.cloud.agent.api.GetStorageStatsCommand; import com.cloud.agent.api.GetVolumeStatsAnswer; import com.cloud.agent.api.GetVolumeStatsCommand; +import com.cloud.agent.api.ModifyStoragePoolAnswer; +import com.cloud.agent.api.ModifyStoragePoolCommand; import com.cloud.agent.api.StoragePoolInfo; import com.cloud.agent.api.VolumeStatsEntry; import com.cloud.agent.api.to.DataTO; import com.cloud.agent.api.to.DiskTO; -import com.cloud.agent.api.ModifyStoragePoolCommand; -import com.cloud.agent.api.ModifyStoragePoolAnswer; +import com.cloud.agent.api.to.StorageFilerTO; import com.cloud.agent.manager.Commands; import com.cloud.api.ApiDBUtils; import com.cloud.api.query.dao.TemplateJoinDao; @@ -143,8 +141,10 @@ import com.cloud.configuration.Resource.ResourceType; import com.cloud.dc.ClusterVO; import com.cloud.dc.DataCenterVO; +import com.cloud.dc.VsphereStoragePolicyVO; import com.cloud.dc.dao.ClusterDao; import com.cloud.dc.dao.DataCenterDao; +import com.cloud.dc.dao.VsphereStoragePolicyDao; import com.cloud.event.ActionEvent; import com.cloud.event.EventTypes; import com.cloud.exception.AgentUnavailableException; @@ -172,6 +172,7 @@ import com.cloud.resource.ResourceState; import com.cloud.server.ConfigurationServer; import com.cloud.server.ManagementServer; +import com.cloud.service.dao.ServiceOfferingDetailsDao; import com.cloud.storage.Storage.ImageFormat; import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.Volume.Type; @@ -195,6 +196,7 @@ import com.cloud.utils.DateUtil; import com.cloud.utils.NumbersUtil; import com.cloud.utils.Pair; +import com.cloud.utils.StringUtils; import com.cloud.utils.UriUtils; import com.cloud.utils.component.ComponentContext; import com.cloud.utils.component.ManagerBase; @@ -218,8 +220,6 @@ import com.cloud.vm.VirtualMachine.State; import com.cloud.vm.dao.VMInstanceDao; -import static com.cloud.utils.NumbersUtil.toHumanReadableSize; - @Component public class StorageManagerImpl extends ManagerBase implements StorageManager, ClusterManagerListener, Configurable { private static final Logger s_logger = Logger.getLogger(StorageManagerImpl.class); @@ -2394,7 +2394,7 @@ public boolean storagePoolHasEnoughSpaceForResize(StoragePool pool, long current @Override public boolean isStoragePoolCompliantWithStoragePolicy(List volumes, StoragePool pool) throws StorageUnavailableException { - if (volumes == null || volumes.isEmpty()) { + if (CollectionUtils.isEmpty(volumes)) { return false; } List> answers = new ArrayList>(); diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 209ea224fa6e..865de4d71688 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -575,6 +575,13 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir private static final ConfigKey VmDestroyForcestop = new ConfigKey("Advanced", Boolean.class, "vm.destroy.forcestop", "false", "On destroy, force-stop takes this value ", true); + public static final List VM_STORAGEMIGRATION_SUPPORTING_HYPERVISORS = new ArrayList<>(Arrays.asList( + HypervisorType.KVM, + HypervisorType.VMware, + HypervisorType.XenServer, + HypervisorType.Simulator + )); + @Override public UserVmVO getVirtualMachine(long vmId) { return _vmDao.findById(vmId); @@ -6286,16 +6293,20 @@ private Pair getHostsForMigrateVmWithStorage(VMInstanceVO vm, Host d return new Pair<>(srcHost, destinationHost); } - private Map getVolumePoolMappingForMigrateVmWithStorage(VMInstanceVO vm, Map volumeToPool) { - Map volToPoolObjectMap = new HashMap(); - + private List getVmVolumesForMigrateVmWithStorage(VMInstanceVO vm) { List vmVolumes = _volsDao.findUsableVolumesForInstance(vm.getId()); - // Check if all the volumes are in the correct state. for (VolumeVO volume : vmVolumes) { if (volume.getState() != Volume.State.Ready) { - throw new CloudRuntimeException("Volume " + volume + " of the VM is not in Ready state. Cannot " + "migrate the vm with its volumes."); + throw new CloudRuntimeException(String.format("Volume %s (ID: %s) of the VM is not in Ready state. Cannot migrate the VM %s (ID: %s) with its volumes", volume.getName(), volume.getUuid(), vm.getInstanceName(), vm.getUuid())); } } + return vmVolumes; + } + + private Map getVolumePoolMappingForMigrateVmWithStorage(VMInstanceVO vm, Map volumeToPool) { + Map volToPoolObjectMap = new HashMap(); + + List vmVolumes = getVmVolumesForMigrateVmWithStorage(vm); if (MapUtils.isNotEmpty(volumeToPool)) { // Check if all the volumes and pools passed as parameters are valid. @@ -6365,16 +6376,10 @@ public VirtualMachine migrateVirtualMachineWithVolume(Long vmId, Host destinatio throw new InvalidParameterValueException("Live Migration of GPU enabled VM is not supported"); } - // OfflineVmwareMigration: this condition is to complicated. (already a method somewhere) - if (!Arrays.asList(new HypervisorType[]{ - HypervisorType.XenServer, - HypervisorType.VMware, - HypervisorType.KVM, - HypervisorType.Simulator}).contains(vm.getHypervisorType())) { + if (VM_STORAGEMIGRATION_SUPPORTING_HYPERVISORS.contains(vm.getHypervisorType())) { throw new InvalidParameterValueException(String.format("Unsupported hypervisor: %s for VM migration, we support XenServer/VMware/KVM only", vm.getHypervisorType())); } - // Check that Vm does not have VM Snapshots if (_vmSnapshotDao.findByVm(vmId).size() > 0) { throw new InvalidParameterValueException("VM with VM Snapshots cannot be migrated with storage, please remove all VM snapshots"); } From 2c6e1266f96292b023c4c5d7a52ee7f5f93cb64f Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 23 Jun 2021 11:22:22 +0530 Subject: [PATCH 6/6] var rename Signed-off-by: Abhishek Kumar --- server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 865de4d71688..c55a729a6606 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -575,7 +575,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir private static final ConfigKey VmDestroyForcestop = new ConfigKey("Advanced", Boolean.class, "vm.destroy.forcestop", "false", "On destroy, force-stop takes this value ", true); - public static final List VM_STORAGEMIGRATION_SUPPORTING_HYPERVISORS = new ArrayList<>(Arrays.asList( + public static final List VM_STORAGE_MIGRATION_SUPPORTING_HYPERVISORS = new ArrayList<>(Arrays.asList( HypervisorType.KVM, HypervisorType.VMware, HypervisorType.XenServer, @@ -6376,7 +6376,7 @@ public VirtualMachine migrateVirtualMachineWithVolume(Long vmId, Host destinatio throw new InvalidParameterValueException("Live Migration of GPU enabled VM is not supported"); } - if (VM_STORAGEMIGRATION_SUPPORTING_HYPERVISORS.contains(vm.getHypervisorType())) { + if (VM_STORAGE_MIGRATION_SUPPORTING_HYPERVISORS.contains(vm.getHypervisorType())) { throw new InvalidParameterValueException(String.format("Unsupported hypervisor: %s for VM migration, we support XenServer/VMware/KVM only", vm.getHypervisorType())); }