From 80f9b4cbafd6be0858329c06d84deaabe0b64fb6 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 5 Mar 2021 14:25:19 +0530 Subject: [PATCH 01/13] prevent other vm disks getting deleted Signed-off-by: Abhishek Kumar --- .../resource/VmwareStorageProcessor.java | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java index 1584bd003dc4..a2aee6b53e1e 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java @@ -34,11 +34,6 @@ import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; -import com.cloud.hypervisor.vmware.mo.VirtualStorageObjectManagerMO; -import com.cloud.hypervisor.vmware.mo.VirtualMachineDiskInfoBuilder; -import com.vmware.vim25.BaseConfigInfoDiskFileBackingInfo; -import com.vmware.vim25.VStorageObject; -import com.vmware.vim25.VirtualDiskType; import org.apache.cloudstack.agent.directdownload.DirectDownloadCommand; import org.apache.cloudstack.storage.command.AttachAnswer; import org.apache.cloudstack.storage.command.AttachCommand; @@ -84,7 +79,9 @@ import com.cloud.hypervisor.vmware.mo.HostStorageSystemMO; import com.cloud.hypervisor.vmware.mo.HypervisorHostHelper; import com.cloud.hypervisor.vmware.mo.NetworkDetails; +import com.cloud.hypervisor.vmware.mo.VirtualMachineDiskInfoBuilder; import com.cloud.hypervisor.vmware.mo.VirtualMachineMO; +import com.cloud.hypervisor.vmware.mo.VirtualStorageObjectManagerMO; import com.cloud.hypervisor.vmware.mo.VmwareHypervisorHost; import com.cloud.hypervisor.vmware.resource.VmwareResource; import com.cloud.hypervisor.vmware.util.VmwareContext; @@ -104,6 +101,7 @@ import com.cloud.vm.VmDetailConstants; import com.google.common.base.Strings; import com.google.gson.Gson; +import com.vmware.vim25.BaseConfigInfoDiskFileBackingInfo; import com.vmware.vim25.DatastoreHostMount; import com.vmware.vim25.HostHostBusAdapter; import com.vmware.vim25.HostInternetScsiHba; @@ -122,11 +120,13 @@ import com.vmware.vim25.HostUnresolvedVmfsVolume; import com.vmware.vim25.InvalidStateFaultMsg; import com.vmware.vim25.ManagedObjectReference; +import com.vmware.vim25.VStorageObject; import com.vmware.vim25.VirtualDeviceBackingInfo; import com.vmware.vim25.VirtualDeviceConfigSpec; import com.vmware.vim25.VirtualDeviceConfigSpecOperation; import com.vmware.vim25.VirtualDisk; import com.vmware.vim25.VirtualDiskFlatVer2BackingInfo; +import com.vmware.vim25.VirtualDiskType; import com.vmware.vim25.VirtualMachineConfigSpec; import com.vmware.vim25.VmConfigInfo; import com.vmware.vim25.VmfsDatastoreExpandSpec; @@ -2683,15 +2683,14 @@ public Answer deleteVolume(DeleteCommand cmd) { List virtualDisks = vmMo.getVirtualDisks(); List managedDatastoreNames = getManagedDatastoreNamesFromVirtualDisks(virtualDisks); + // Preserve other disks of the VM + List detachedDisks = vmMo.detachAllDisksExcept(vol.getPath(), diskInfo != null ? diskInfo.getDiskDeviceBusName() : null); + VmwareStorageLayoutHelper.moveVolumeToRootFolder(new DatacenterMO(context, morDc), detachedDisks); // let vmMo.destroy to delete volume for us // vmMo.tearDownDevices(new Class[] { VirtualDisk.class, VirtualEthernetCard.class }); - if (isManaged) { - List detachedDisks = vmMo.detachAllDisksExcept(vol.getPath(), diskInfo != null ? diskInfo.getDiskDeviceBusName() : null); - VmwareStorageLayoutHelper.moveVolumeToRootFolder(new DatacenterMO(context, morDc), detachedDisks); vmMo.unregisterVm(); - } - else { + } else { vmMo.destroy(); } From 54ef6e789aa2d42b4ce2f9cb289f9b703afbbacf Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 31 Mar 2021 12:49:09 +0530 Subject: [PATCH 02/13] vmware: fix inter-cluster stopped vm migration Fixes #4838 For inter-cluster migration without shared storage, VMware needs a host to be specified. Fix is to specify an appropriate host in the target cluster. Signed-off-by: Abhishek Kumar --- .../agent/api/MigrateVmToPoolCommand.java | 18 ++++- .../com/cloud/hypervisor/guru/VMwareGuru.java | 68 ++++++++++++++----- .../vmware/resource/VmwareResource.java | 43 ++++++++---- .../vmware/mo/VirtualMachineMO.java | 29 ++++---- .../hypervisor/vmware/util/VmwareHelper.java | 14 ++++ 5 files changed, 125 insertions(+), 47 deletions(-) diff --git a/core/src/main/java/com/cloud/agent/api/MigrateVmToPoolCommand.java b/core/src/main/java/com/cloud/agent/api/MigrateVmToPoolCommand.java index 91a911d7c181..f5afd484ebcc 100644 --- a/core/src/main/java/com/cloud/agent/api/MigrateVmToPoolCommand.java +++ b/core/src/main/java/com/cloud/agent/api/MigrateVmToPoolCommand.java @@ -18,10 +18,10 @@ // package com.cloud.agent.api; -import com.cloud.agent.api.to.VolumeTO; - import java.util.Collection; +import com.cloud.agent.api.to.VolumeTO; + /** * used to tell the agent to migrate a vm to a different primary storage pool. * It is for now only implemented on Vmware and is supposed to work irrespective of whether the VM is started or not. @@ -32,6 +32,7 @@ public class MigrateVmToPoolCommand extends Command { private String vmName; private String destinationPool; private boolean executeInSequence = false; + private String hostGuidInTargetCluster; protected MigrateVmToPoolCommand() { } @@ -41,15 +42,22 @@ protected MigrateVmToPoolCommand() { * @param vmName the name of the VM to migrate * @param volumes used to supply feedback on vmware generated names * @param destinationPool the primary storage pool to migrate the VM to + * @param hostGuidInTargetCluster GUID of host in target cluster when migrating across clusters * @param executeInSequence */ - public MigrateVmToPoolCommand(String vmName, Collection volumes, String destinationPool, boolean executeInSequence) { + public MigrateVmToPoolCommand(String vmName, Collection volumes, String destinationPool, + String hostGuidInTargetCluster, boolean executeInSequence) { this.vmName = vmName; this.volumes = volumes; this.destinationPool = destinationPool; + this.hostGuidInTargetCluster = hostGuidInTargetCluster; this.executeInSequence = executeInSequence; } + public MigrateVmToPoolCommand(String vmName, Collection volumes, String destinationPool, boolean executeInSequence) { + this(vmName, volumes, destinationPool, null, executeInSequence); + } + public Collection getVolumes() { return volumes; } @@ -62,6 +70,10 @@ public String getVmName() { return vmName; } + public String getHostGuidInTargetCluster() { + return hostGuidInTargetCluster; + } + @Override public boolean executeInSequence() { return executeInSequence; diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java index d48a5d9b1019..ec4ff0da3ccf 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java @@ -16,6 +16,8 @@ // under the License. package com.cloud.hypervisor.guru; +import static com.cloud.utils.NumbersUtil.toHumanReadableSize; + import java.util.ArrayList; import java.util.Date; import java.util.HashMap; @@ -149,8 +151,6 @@ import com.vmware.vim25.VirtualMachineConfigSummary; import com.vmware.vim25.VirtualMachineRuntimeInfo; -import static com.cloud.utils.NumbersUtil.toHumanReadableSize; - public class VMwareGuru extends HypervisorGuruBase implements HypervisorGuru, Configurable { private static final Logger s_logger = Logger.getLogger(VMwareGuru.class); @@ -209,16 +209,35 @@ protected VMwareGuru() { return vmwareVmImplementer.implement(vm, toVirtualMachineTO(vm), getClusterId(vm.getId())); } - long getClusterId(long vmId) { - long clusterId; - Long hostId; - - hostId = _vmDao.findById(vmId).getHostId(); - if (hostId == null) { + Long getClusterId(long vmId) { + Long clusterId = null; + Long hostId = null; + VMInstanceVO vm = _vmDao.findById(vmId); + if (vm != null) { + hostId = _vmDao.findById(vmId).getHostId(); + } + if (vm != null && hostId == null) { // If VM is in stopped state then hostId would be undefined. Hence read last host's Id instead. hostId = _vmDao.findById(vmId).getLastHostId(); } - clusterId = _hostDao.findById(hostId).getClusterId(); + HostVO host = null; + if (hostId != null) { + host = _hostDao.findById(hostId); + } + if (host != null) { + clusterId = host.getClusterId(); + } else { + List volumes = _volumeDao.findByInstanceAndType(vmId, Volume.Type.ROOT); + if (CollectionUtils.isNotEmpty(volumes)) { + VolumeVO rootVolume = volumes.get(0); + if (rootVolume.getPoolId() != null) { + StoragePoolVO pool = _storagePoolDao.findById(rootVolume.getPoolId()); + if (pool != null && pool.getClusterId() != null) { + clusterId = pool.getClusterId(); + } + } + } + } return clusterId; } @@ -418,9 +437,11 @@ private static String resolveNameInGuid(String guid) { @Override public Map getClusterSettings(long vmId) { Map details = new HashMap(); - long clusterId = getClusterId(vmId); - details.put(VmwareReserveCpu.key(), VmwareReserveCpu.valueIn(clusterId).toString()); - details.put(VmwareReserveMemory.key(), VmwareReserveMemory.valueIn(clusterId).toString()); + Long clusterId = getClusterId(vmId); + if (clusterId != null) { + details.put(VmwareReserveCpu.key(), VmwareReserveCpu.valueIn(clusterId).toString()); + details.put(VmwareReserveMemory.key(), VmwareReserveMemory.valueIn(clusterId).toString()); + } return details; } @@ -1066,14 +1087,29 @@ private VirtualDisk getAttachedDisk(VirtualMachineMO vmMo, String diskPath) thro VolumeTO vol = new VolumeTO(volume, destination); vols.add(vol); } - MigrateVmToPoolCommand migrateVmToPoolCommand = new MigrateVmToPoolCommand(vm.getInstanceName(), vols, destination.getUuid(), true); - commands.add(migrateVmToPoolCommand); - // OfflineVmwareMigration: cleanup if needed final Long destClusterId = destination.getClusterId(); final Long srcClusterId = getClusterId(vm.getId()); + final boolean isInterClusterMigration = srcClusterId != null && destClusterId != null && ! srcClusterId.equals(destClusterId); + Host hostInTargetCluster = null; + if (isInterClusterMigration) { + // Without host vMotion might fail between non-shared storages with error similar to, + // https://kb.vmware.com/s/article/1003795 + // As this is offline migration VM won't be started on this host + List hosts = _hostDao.findHypervisorHostInCluster(destClusterId); + if (CollectionUtils.isNotEmpty(hosts)) { + hostInTargetCluster = hosts.get(0); + } + if (hostInTargetCluster == null) { + throw new CloudRuntimeException("Migration failed, unable to find suitable target host for VM placement while migrating between storage pools of different clusters without shared storages"); + } + } + MigrateVmToPoolCommand migrateVmToPoolCommand = new MigrateVmToPoolCommand(vm.getInstanceName(), + vols, destination.getUuid(), hostInTargetCluster == null ? null : hostInTargetCluster.getGuid(), true); + commands.add(migrateVmToPoolCommand); - if (srcClusterId != null && destClusterId != null && !srcClusterId.equals(destClusterId)) { + // OfflineVmwareMigration: cleanup if needed + if (isInterClusterMigration) { final String srcDcName = _clusterDetailsDao.getVmwareDcName(srcClusterId); final String destDcName = _clusterDetailsDao.getVmwareDcName(destClusterId); if (srcDcName != null && destDcName != null && !srcDcName.equals(destDcName)) { diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index e59c7578b061..449ffe483e31 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -16,6 +16,9 @@ // under the License. package com.cloud.hypervisor.vmware.resource; +import static com.cloud.utils.HumanReadableJson.getHumanReadableBytesJson; +import static com.cloud.utils.NumbersUtil.toHumanReadableSize; + import java.io.File; import java.io.IOException; import java.io.UnsupportedEncodingException; @@ -45,11 +48,12 @@ import javax.naming.ConfigurationException; import javax.xml.datatype.XMLGregorianCalendar; -import com.cloud.agent.api.to.DataTO; -import com.cloud.agent.api.to.DeployAsIsInfoTO; -import com.cloud.agent.api.ValidateVcenterDetailsCommand; import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.storage.command.CopyCommand; +import org.apache.cloudstack.storage.command.StorageSubSystemCommand; import org.apache.cloudstack.storage.configdrive.ConfigDrive; +import org.apache.cloudstack.storage.resource.NfsSecondaryStorageResource; +import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; import org.apache.cloudstack.storage.to.TemplateObjectTO; import org.apache.cloudstack.storage.to.VolumeObjectTO; import org.apache.cloudstack.utils.volume.VirtualMachineDiskInfo; @@ -162,6 +166,7 @@ import com.cloud.agent.api.UpgradeSnapshotCommand; import com.cloud.agent.api.ValidateSnapshotAnswer; import com.cloud.agent.api.ValidateSnapshotCommand; +import com.cloud.agent.api.ValidateVcenterDetailsCommand; import com.cloud.agent.api.VmDiskStatsEntry; import com.cloud.agent.api.VmStatsEntry; import com.cloud.agent.api.VolumeStatsEntry; @@ -178,12 +183,13 @@ import com.cloud.agent.api.storage.DestroyCommand; import com.cloud.agent.api.storage.MigrateVolumeAnswer; import com.cloud.agent.api.storage.MigrateVolumeCommand; -import com.cloud.agent.api.to.deployasis.OVFPropertyTO; import com.cloud.agent.api.storage.PrimaryStorageDownloadAnswer; import com.cloud.agent.api.storage.PrimaryStorageDownloadCommand; import com.cloud.agent.api.storage.ResizeVolumeAnswer; import com.cloud.agent.api.storage.ResizeVolumeCommand; import com.cloud.agent.api.to.DataStoreTO; +import com.cloud.agent.api.to.DataTO; +import com.cloud.agent.api.to.DeployAsIsInfoTO; import com.cloud.agent.api.to.DiskTO; import com.cloud.agent.api.to.IpAddressTO; import com.cloud.agent.api.to.NfsTO; @@ -191,6 +197,7 @@ import com.cloud.agent.api.to.StorageFilerTO; import com.cloud.agent.api.to.VirtualMachineTO; import com.cloud.agent.api.to.VolumeTO; +import com.cloud.agent.api.to.deployasis.OVFPropertyTO; import com.cloud.agent.resource.virtualnetwork.VRScripts; import com.cloud.agent.resource.virtualnetwork.VirtualRouterDeployer; import com.cloud.agent.resource.virtualnetwork.VirtualRoutingResource; @@ -219,8 +226,8 @@ import com.cloud.hypervisor.vmware.mo.HypervisorHostHelper; import com.cloud.hypervisor.vmware.mo.NetworkDetails; import com.cloud.hypervisor.vmware.mo.PbmProfileManagerMO; -import com.cloud.hypervisor.vmware.mo.TaskMO; import com.cloud.hypervisor.vmware.mo.StoragepodMO; +import com.cloud.hypervisor.vmware.mo.TaskMO; import com.cloud.hypervisor.vmware.mo.VirtualEthernetCardType; import com.cloud.hypervisor.vmware.mo.VirtualMachineDiskInfoBuilder; import com.cloud.hypervisor.vmware.mo.VirtualMachineMO; @@ -289,7 +296,6 @@ import com.vmware.vim25.HostPortGroupSpec; import com.vmware.vim25.ManagedObjectReference; import com.vmware.vim25.NasDatastoreInfo; -import com.vmware.vim25.VirtualMachineDefinedProfileSpec; import com.vmware.vim25.ObjectContent; import com.vmware.vim25.OptionValue; import com.vmware.vim25.PerfCounterInfo; @@ -324,6 +330,7 @@ import com.vmware.vim25.VirtualIDEController; import com.vmware.vim25.VirtualMachineBootOptions; import com.vmware.vim25.VirtualMachineConfigSpec; +import com.vmware.vim25.VirtualMachineDefinedProfileSpec; import com.vmware.vim25.VirtualMachineFileInfo; import com.vmware.vim25.VirtualMachineFileLayoutEx; import com.vmware.vim25.VirtualMachineFileLayoutExFileInfo; @@ -343,13 +350,6 @@ import com.vmware.vim25.VmConfigSpec; import com.vmware.vim25.VmwareDistributedVirtualSwitchPvlanSpec; import com.vmware.vim25.VmwareDistributedVirtualSwitchVlanIdSpec; -import org.apache.cloudstack.storage.command.CopyCommand; -import org.apache.cloudstack.storage.command.StorageSubSystemCommand; -import org.apache.cloudstack.storage.resource.NfsSecondaryStorageResource; -import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; - -import static com.cloud.utils.HumanReadableJson.getHumanReadableBytesJson; -import static com.cloud.utils.NumbersUtil.toHumanReadableSize; public class VmwareResource implements StoragePoolResource, ServerResource, VmwareHostService, VirtualRouterDeployer { private static final Logger s_logger = Logger.getLogger(VmwareResource.class); @@ -4415,7 +4415,20 @@ protected Answer execute(MigrateVmToPoolCommand cmd) { private Answer migrateAndAnswer(VirtualMachineMO vmMo, String poolUuid, VmwareHypervisorHost hyperHost, Command cmd) throws Exception { ManagedObjectReference morDs = getTargetDatastoreMOReference(poolUuid, hyperHost); - + VmwareHypervisorHost targetHyperHost = null; + if (cmd instanceof MigrateVmToPoolCommand) { + MigrateVmToPoolCommand vmToPoolCommand = (MigrateVmToPoolCommand) cmd; + targetHyperHost = VmwareHelper.getHostMOFromHostName(getServiceContext(), + vmToPoolCommand.getHostGuidInTargetCluster()); + } + if (morDs == null && targetHyperHost != null) { + morDs = getTargetDatastoreMOReference(poolUuid, targetHyperHost); + } + if (morDs == null) { + String msg = String.format("Failed to find datastore for pool UUID: %s", poolUuid); + s_logger.error(msg); + throw new CloudRuntimeException(msg); + } try { // OfflineVmwareMigration: getVolumesFromCommand(cmd); Map volumeDeviceKey = getVolumesFromCommand(vmMo, cmd); @@ -4424,7 +4437,7 @@ private Answer migrateAndAnswer(VirtualMachineMO vmMo, String poolUuid, VmwareHy s_logger.trace(String.format("disk to migrate has disk id %d and volumeId %d", diskId, volumeDeviceKey.get(diskId))); } } - if (vmMo.changeDatastore(morDs)) { + if (vmMo.changeDatastore(morDs, targetHyperHost)) { // OfflineVmwareMigration: create target specification to include in answer // Consolidate VM disks after successful VM migration // In case of a linked clone VM, if VM's disks are not consolidated, further VM operations such as volume snapshot, VM snapshot etc. will result in DB inconsistencies. diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java index 5f61b31171ca..0a97f94e1d89 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java @@ -16,6 +16,8 @@ // under the License. package com.cloud.hypervisor.vmware.mo; +import static com.cloud.utils.NumbersUtil.toHumanReadableSize; + import java.io.BufferedReader; import java.io.BufferedWriter; import java.io.ByteArrayInputStream; @@ -38,6 +40,14 @@ import org.apache.commons.lang.StringUtils; import org.apache.log4j.Logger; +import com.cloud.hypervisor.vmware.mo.SnapshotDescriptor.SnapshotInfo; +import com.cloud.hypervisor.vmware.util.VmwareContext; +import com.cloud.hypervisor.vmware.util.VmwareHelper; +import com.cloud.utils.ActionDelegate; +import com.cloud.utils.Pair; +import com.cloud.utils.Ternary; +import com.cloud.utils.concurrency.NamedThreadFactory; +import com.cloud.utils.script.Script; import com.google.gson.Gson; import com.vmware.vim25.ArrayOfManagedObjectReference; import com.vmware.vim25.ChoiceOption; @@ -91,6 +101,7 @@ import com.vmware.vim25.VirtualMachineConfigOption; import com.vmware.vim25.VirtualMachineConfigSpec; import com.vmware.vim25.VirtualMachineConfigSummary; +import com.vmware.vim25.VirtualMachineDefinedProfileSpec; import com.vmware.vim25.VirtualMachineFileInfo; import com.vmware.vim25.VirtualMachineFileLayoutEx; import com.vmware.vim25.VirtualMachineMessage; @@ -105,18 +116,6 @@ import com.vmware.vim25.VirtualMachineSnapshotTree; import com.vmware.vim25.VirtualSCSIController; import com.vmware.vim25.VirtualSCSISharing; -import com.vmware.vim25.VirtualMachineDefinedProfileSpec; - -import com.cloud.hypervisor.vmware.mo.SnapshotDescriptor.SnapshotInfo; -import com.cloud.hypervisor.vmware.util.VmwareContext; -import com.cloud.hypervisor.vmware.util.VmwareHelper; -import com.cloud.utils.ActionDelegate; -import com.cloud.utils.Pair; -import com.cloud.utils.Ternary; -import com.cloud.utils.concurrency.NamedThreadFactory; -import com.cloud.utils.script.Script; - -import static com.cloud.utils.NumbersUtil.toHumanReadableSize; public class VirtualMachineMO extends BaseMO { private static final Logger s_logger = Logger.getLogger(VirtualMachineMO.class); @@ -459,9 +458,13 @@ public boolean changeHost(VirtualMachineRelocateSpec relocateSpec) throws Except return false; } - public boolean changeDatastore(ManagedObjectReference morDataStore) throws Exception { + public boolean changeDatastore(ManagedObjectReference morDataStore, VmwareHypervisorHost targetHost) throws Exception { VirtualMachineRelocateSpec relocateSpec = new VirtualMachineRelocateSpec(); relocateSpec.setDatastore(morDataStore); + if (targetHost != null) { + relocateSpec.setHost(targetHost.getMor()); + relocateSpec.setPool(targetHost.getHyperHostOwnerResourcePool()); + } ManagedObjectReference morTask = _context.getService().relocateVMTask(_mor, relocateSpec, null); diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java index 424027ba7121..ff0120c9b8b0 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java @@ -744,4 +744,18 @@ public static XMLGregorianCalendar getXMLGregorianCalendar(final Date date, fina return DatatypeFactory.newInstance().newXMLGregorianCalendar(gregorianCalendar); } + public static HostMO getHostMOFromHostName(final VmwareContext context, final String hostName) { + HostMO host = null; + if (com.cloud.utils.StringUtils.isNotBlank(hostName) && hostName.contains("@")) { + String hostMorInfo = hostName.split("@")[0]; + if (hostMorInfo.contains(":")) { + ManagedObjectReference morHost = new ManagedObjectReference(); + morHost.setType(hostMorInfo.split(":")[0]); + morHost.setValue(hostMorInfo.split(":")[1]); + host = new HostMO(context, morHost); + } + } + return host; + } + } From 7cc6b6e1d359ebf8a49344f96dd73796b4be2342 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 6 Apr 2021 12:49:34 +0530 Subject: [PATCH 03/13] fix detached volume inter-cluster migration Signed-off-by: Abhishek Kumar --- .../api/storage/MigrateVolumeCommand.java | 8 +- .../vmware/resource/VmwareResource.java | 48 +++++---- .../motion/VmwareStorageMotionStrategy.java | 68 ++++++++----- .../vmware/mo/HypervisorHostHelper.java | 97 +++++++++++++++---- .../hypervisor/vmware/util/VmwareHelper.java | 25 +++-- 5 files changed, 172 insertions(+), 74 deletions(-) diff --git a/core/src/main/java/com/cloud/agent/api/storage/MigrateVolumeCommand.java b/core/src/main/java/com/cloud/agent/api/storage/MigrateVolumeCommand.java index 9902a86fb893..5443ad12eadd 100644 --- a/core/src/main/java/com/cloud/agent/api/storage/MigrateVolumeCommand.java +++ b/core/src/main/java/com/cloud/agent/api/storage/MigrateVolumeCommand.java @@ -34,6 +34,7 @@ public class MigrateVolumeCommand extends Command { StorageFilerTO sourcePool; String attachedVmName; Volume.Type volumeType; + private String hostGuidInTargetCluster; private DataTO srcData; private DataTO destData; @@ -54,9 +55,10 @@ public MigrateVolumeCommand(long volumeId, String volumePath, StoragePool pool, this.setWait(timeout); } - public MigrateVolumeCommand(long volumeId, String volumePath, StoragePool sourcePool, StoragePool targetPool) { + public MigrateVolumeCommand(long volumeId, String volumePath, StoragePool sourcePool, StoragePool targetPool, String hostGuidInTargetCluster) { this(volumeId,volumePath,targetPool, null, Volume.Type.UNKNOWN, -1); this.sourcePool = new StorageFilerTO(sourcePool); + this.hostGuidInTargetCluster = hostGuidInTargetCluster; } public MigrateVolumeCommand(DataTO srcData, DataTO destData, Map srcDetails, Map destDetails, int timeout) { @@ -101,6 +103,10 @@ public Volume.Type getVolumeType() { return volumeType; } + public String getHostGuidInTargetCluster() { + return hostGuidInTargetCluster; + } + public DataTO getSrcData() { return srcData; } diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index 449ffe483e31..49750b767a0f 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -4391,6 +4391,11 @@ protected Answer execute(MigrateVmToPoolCommand cmd) { final String vmName = cmd.getVmName(); VmwareHypervisorHost hyperHost = getHyperHost(getServiceContext()); + VmwareHypervisorHost hyperHostInTargetCluster = null; + if (cmd.getHostGuidInTargetCluster() != null) { + hyperHostInTargetCluster = VmwareHelper.getHostMOFromHostName(getServiceContext(), + cmd.getHostGuidInTargetCluster()); + } try { VirtualMachineMO vmMo = getVirtualMachineMO(vmName, hyperHost); if (vmMo == null) { @@ -4400,7 +4405,7 @@ protected Answer execute(MigrateVmToPoolCommand cmd) { } String poolUuid = cmd.getDestinationPool(); - return migrateAndAnswer(vmMo, poolUuid, hyperHost, cmd); + return migrateAndAnswer(vmMo, poolUuid, hyperHost, hyperHostInTargetCluster, cmd); } catch (Throwable e) { // hopefully only CloudRuntimeException :/ if (e instanceof Exception) { return new Answer(cmd, (Exception) e); @@ -4413,14 +4418,9 @@ protected Answer execute(MigrateVmToPoolCommand cmd) { } } - private Answer migrateAndAnswer(VirtualMachineMO vmMo, String poolUuid, VmwareHypervisorHost hyperHost, Command cmd) throws Exception { - ManagedObjectReference morDs = getTargetDatastoreMOReference(poolUuid, hyperHost); - VmwareHypervisorHost targetHyperHost = null; - if (cmd instanceof MigrateVmToPoolCommand) { - MigrateVmToPoolCommand vmToPoolCommand = (MigrateVmToPoolCommand) cmd; - targetHyperHost = VmwareHelper.getHostMOFromHostName(getServiceContext(), - vmToPoolCommand.getHostGuidInTargetCluster()); - } + private Answer migrateAndAnswer(VirtualMachineMO vmMo, String poolUuid, VmwareHypervisorHost sourceHyperHost, + VmwareHypervisorHost targetHyperHost, Command cmd) throws Exception { + ManagedObjectReference morDs = getTargetDatastoreMOReference(poolUuid, sourceHyperHost); if (morDs == null && targetHyperHost != null) { morDs = getTargetDatastoreMOReference(poolUuid, targetHyperHost); } @@ -4640,7 +4640,7 @@ protected Answer execute(MigrateWithStorageCommand cmd) { morDc = srcHyperHost.getHyperHostDatacenter(); morDcOfTargetHost = tgtHyperHost.getHyperHostDatacenter(); if (!morDc.getValue().equalsIgnoreCase(morDcOfTargetHost.getValue())) { - String msg = "Source host & target host are in different datacentesr"; + String msg = "Source host & target host are in different datacenter"; throw new CloudRuntimeException(msg); } VmwareManager mgr = tgtHyperHost.getContext().getStockObject(VmwareManager.CONTEXT_STOCK_NAME); @@ -4852,6 +4852,11 @@ private Answer migrateVolume(MigrateVolumeCommand cmd) { String path = cmd.getVolumePath(); VmwareHypervisorHost hyperHost = getHyperHost(getServiceContext()); + VmwareHypervisorHost hyperHostInTargetCluster = null; + if (cmd.getHostGuidInTargetCluster() != null) { + hyperHostInTargetCluster = VmwareHelper.getHostMOFromHostName(getServiceContext(), cmd.getHostGuidInTargetCluster()); + } + VmwareHypervisorHost targetDSHost = hyperHostInTargetCluster != null ? hyperHostInTargetCluster : hyperHost; VirtualMachineMO vmMo = null; DatastoreMO dsMo = null; DatastoreMO destinationDsMo = null; @@ -4867,8 +4872,8 @@ private Answer migrateVolume(MigrateVolumeCommand cmd) { // we need to spawn a worker VM to attach the volume to and move it morSourceDS = HypervisorHostHelper.findDatastoreWithBackwardsCompatibility(hyperHost, cmd.getSourcePool().getUuid()); dsMo = new DatastoreMO(hyperHost.getContext(), morSourceDS); - morDestintionDS = HypervisorHostHelper.findDatastoreWithBackwardsCompatibility(hyperHost, cmd.getTargetPool().getUuid()); - destinationDsMo = new DatastoreMO(hyperHost.getContext(), morDestintionDS); + morDestintionDS = HypervisorHostHelper.findDatastoreWithBackwardsCompatibility(targetDSHost, cmd.getTargetPool().getUuid()); + destinationDsMo = new DatastoreMO(targetDSHost.getContext(), morDestintionDS); vmName = getWorkerName(getServiceContext(), cmd, 0, dsMo); if (destinationDsMo.getDatastoreType().equalsIgnoreCase("VVOL")) { @@ -4876,6 +4881,15 @@ private Answer migrateVolume(MigrateVolumeCommand cmd) { vmName = getWorkerName(getServiceContext(), cmd, 0, destinationDsMo); } + String hardwareVersion = null; + if (hyperHostInTargetCluster != null) { + Integer sourceHardwareVersion = HypervisorHostHelper.getHostHardwareVersion(hyperHost); + Integer destinationHardwareVersion = HypervisorHostHelper.getHostHardwareVersion(hyperHostInTargetCluster); + if (sourceHardwareVersion != null && destinationHardwareVersion != null && !sourceHardwareVersion.equals(destinationHardwareVersion)) { + hardwareVersion = String.valueOf(Math.min(sourceHardwareVersion, destinationHardwareVersion)); + } + } + // OfflineVmwareMigration: refactor for re-use // OfflineVmwareMigration: 1. find data(store) // OfflineVmwareMigration: more robust would be to find the store given the volume as it might have been moved out of band or due to error @@ -4883,7 +4897,7 @@ private Answer migrateVolume(MigrateVolumeCommand cmd) { s_logger.info("Create worker VM " + vmName); // OfflineVmwareMigration: 2. create the worker with access to the data(store) - vmMo = HypervisorHostHelper.createWorkerVM(hyperHost, dsMo, vmName, null); + vmMo = HypervisorHostHelper.createWorkerVM(hyperHost, dsMo, vmName, hardwareVersion); if (vmMo == null) { // OfflineVmwareMigration: don't throw a general Exception but think of a specific one throw new CloudRuntimeException("Unable to create a worker VM for volume operation"); @@ -4947,7 +4961,7 @@ private Answer migrateVolume(MigrateVolumeCommand cmd) { } // OfflineVmwareMigration: this may have to be disected and executed in separate steps - answer = migrateAndAnswer(vmMo, cmd.getTargetPool().getUuid(), hyperHost, cmd); + answer = migrateAndAnswer(vmMo, cmd.getTargetPool().getUuid(), hyperHost, hyperHostInTargetCluster, cmd); } catch (Exception e) { String msg = String.format("Migration of volume '%s' failed due to %s", cmd.getVolumePath(), e.getLocalizedMessage()); s_logger.error(msg, e); @@ -4956,9 +4970,9 @@ private Answer migrateVolume(MigrateVolumeCommand cmd) { try { // OfflineVmwareMigration: worker *may* have been renamed vmName = vmMo.getVmName(); - morSourceDS = HypervisorHostHelper.findDatastoreWithBackwardsCompatibility(hyperHost, cmd.getTargetPool().getUuid()); - dsMo = new DatastoreMO(hyperHost.getContext(), morSourceDS); - s_logger.info("Dettaching disks before destroying worker VM '" + vmName + "' after volume migration"); + morSourceDS = HypervisorHostHelper.findDatastoreWithBackwardsCompatibility(targetDSHost, cmd.getTargetPool().getUuid()); + dsMo = new DatastoreMO(targetDSHost.getContext(), morSourceDS); + s_logger.info("Detaching disks before destroying worker VM '" + vmName + "' after volume migration"); VirtualDisk[] disks = vmMo.getAllDiskDevice(); String format = "disk %d was migrated to %s"; for (VirtualDisk disk : disks) { diff --git a/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java b/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java index 5a7b4c4ca677..8d08c388823f 100644 --- a/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java +++ b/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java @@ -26,6 +26,20 @@ import javax.inject.Inject; +import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; +import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionStrategy; +import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; +import org.apache.cloudstack.framework.async.AsyncCompletionCallback; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.to.VolumeObjectTO; +import org.apache.commons.collections.CollectionUtils; +import org.apache.log4j.Logger; +import org.springframework.stereotype.Component; + import com.cloud.agent.AgentManager; import com.cloud.agent.api.Answer; import com.cloud.agent.api.MigrateWithStorageAnswer; @@ -53,18 +67,6 @@ import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.VMInstanceVO; import com.cloud.vm.dao.VMInstanceDao; -import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; -import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionStrategy; -import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; -import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; -import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority; -import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; -import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; -import org.apache.cloudstack.framework.async.AsyncCompletionCallback; -import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; -import org.apache.cloudstack.storage.to.VolumeObjectTO; -import org.apache.log4j.Logger; -import org.springframework.stereotype.Component; @Component public class VmwareStorageMotionStrategy implements DataMotionStrategy { @@ -88,9 +90,7 @@ public StrategyPriority canHandle(DataObject srcData, DataObject destData) { if (isOnVmware(srcData, destData) && isOnPrimary(srcData, destData) && isVolumesOnly(srcData, destData) - && isDettached(srcData) - && isIntraCluster(srcData, destData) - && isStoreScopeEqual(srcData, destData)) { + && isDettached(srcData)) { if (s_logger.isDebugEnabled()) { String msg = String.format("%s can handle the request because %d(%s) and %d(%s) share the VMware cluster %s (== %s)" , this.getClass() @@ -188,20 +188,42 @@ public void copyAsync(DataObject srcData, DataObject destData, Host destHost, As throw new UnsupportedOperationException(); } StoragePool sourcePool = (StoragePool) srcData.getDataStore(); + ScopeType sourceScopeType = srcData.getDataStore().getScope().getScopeType(); StoragePool targetPool = (StoragePool) destData.getDataStore(); + ScopeType targetScopeType = destData.getDataStore().getScope().getScopeType(); + Long hostId = null; + String hostGuidInTargetCluster = null; + if (ScopeType.CLUSTER.equals(sourceScopeType)) { + // Find Volume source cluster and select any Vmware hypervisor host to attach worker VM + hostId = findSuitableHostIdForWorkerVmPlacement(sourcePool.getClusterId()); + if (hostId == null) { + throw new CloudRuntimeException("Offline Migration failed, unable to find suitable host for worker VM placement in the cluster of storage pool: " + sourcePool.getName()); + } + if (ScopeType.CLUSTER.equals(targetScopeType) && !sourcePool.getClusterId().equals(targetPool.getClusterId())) { + // Without host vMotion might fail between non-shared storages with error similar to, + // https://kb.vmware.com/s/article/1003795 + List hosts = hostDao.findHypervisorHostInCluster(targetPool.getClusterId()); + if (CollectionUtils.isNotEmpty(hosts)) { + hostGuidInTargetCluster = hosts.get(0).getGuid(); + } + if (hostGuidInTargetCluster == null) { + throw new CloudRuntimeException("Offline Migration failed, unable to find suitable target host for worker VM placement while migrating between storage pools of different cluster without shared storages"); + } + } + } else if (ScopeType.CLUSTER.equals(targetScopeType)) { + hostId = findSuitableHostIdForWorkerVmPlacement(targetPool.getClusterId()); + if (hostId == null) { + throw new CloudRuntimeException("Offline Migration failed, unable to find suitable host for worker VM placement in the cluster of storage pool: " + targetPool.getName()); + } + } MigrateVolumeCommand cmd = new MigrateVolumeCommand(srcData.getId() , srcData.getTO().getPath() , sourcePool - , targetPool); + , targetPool + , hostGuidInTargetCluster); // OfflineVmwareMigration: should be ((StoragePool)srcData.getDataStore()).getHypervisor() but that is NULL, so hardcoding Answer answer; - ScopeType scopeType = srcData.getDataStore().getScope().getScopeType(); - if (ScopeType.CLUSTER == scopeType) { - // Find Volume source cluster and select any Vmware hypervisor host to attach worker VM - Long hostId = findSuitableHostIdForWorkerVmPlacement(sourcePool.getClusterId()); - if (hostId == null) { - throw new CloudRuntimeException("Offline Migration failed, unable to find suitable host for worker VM placement in cluster: " + sourcePool.getName()); - } + if (hostId != null) { answer = agentMgr.easySend(hostId, cmd); } else { answer = agentMgr.sendTo(sourcePool.getDataCenterId(), HypervisorType.VMware, cmd); diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java index 1b94ca881b4a..fec4763baa37 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java @@ -18,6 +18,7 @@ import java.io.ByteArrayInputStream; import java.io.File; +import java.io.FileWriter; import java.io.IOException; import java.io.StringWriter; import java.net.URI; @@ -28,6 +29,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.UUID; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; @@ -37,17 +39,6 @@ import javax.xml.transform.dom.DOMSource; import javax.xml.transform.stream.StreamResult; -import com.vmware.vim25.ConcurrentAccessFaultMsg; -import com.vmware.vim25.DuplicateNameFaultMsg; -import com.vmware.vim25.FileFaultFaultMsg; -import com.vmware.vim25.InsufficientResourcesFaultFaultMsg; -import com.vmware.vim25.InvalidDatastoreFaultMsg; -import com.vmware.vim25.InvalidNameFaultMsg; -import com.vmware.vim25.InvalidStateFaultMsg; -import com.vmware.vim25.OutOfBoundsFaultMsg; -import com.vmware.vim25.RuntimeFaultFaultMsg; -import com.vmware.vim25.TaskInProgressFaultMsg; -import com.vmware.vim25.VmConfigFaultFaultMsg; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; import org.apache.commons.collections.MapUtils; import org.apache.commons.lang.StringUtils; @@ -80,19 +71,20 @@ import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.net.NetUtils; import com.cloud.utils.nicira.nvp.plugin.NiciraNvpApiVersion; -import com.vmware.vim25.OvfCreateDescriptorParams; -import com.vmware.vim25.OvfCreateDescriptorResult; import com.vmware.vim25.AlreadyExistsFaultMsg; import com.vmware.vim25.BoolPolicy; -import com.vmware.vim25.CustomFieldStringValue; import com.vmware.vim25.ClusterConfigInfoEx; -import com.vmware.vim25.DatacenterConfigInfo; +import com.vmware.vim25.ConcurrentAccessFaultMsg; +import com.vmware.vim25.CustomFieldStringValue; import com.vmware.vim25.DVPortSetting; import com.vmware.vim25.DVPortgroupConfigInfo; import com.vmware.vim25.DVPortgroupConfigSpec; import com.vmware.vim25.DVSSecurityPolicy; import com.vmware.vim25.DVSTrafficShapingPolicy; +import com.vmware.vim25.DatacenterConfigInfo; +import com.vmware.vim25.DuplicateNameFaultMsg; import com.vmware.vim25.DynamicProperty; +import com.vmware.vim25.FileFaultFaultMsg; import com.vmware.vim25.HostNetworkSecurityPolicy; import com.vmware.vim25.HostNetworkTrafficShapingPolicy; import com.vmware.vim25.HostPortGroup; @@ -101,6 +93,10 @@ import com.vmware.vim25.HttpNfcLeaseDeviceUrl; import com.vmware.vim25.HttpNfcLeaseInfo; import com.vmware.vim25.HttpNfcLeaseState; +import com.vmware.vim25.InsufficientResourcesFaultFaultMsg; +import com.vmware.vim25.InvalidDatastoreFaultMsg; +import com.vmware.vim25.InvalidNameFaultMsg; +import com.vmware.vim25.InvalidStateFaultMsg; import com.vmware.vim25.LocalizedMethodFault; import com.vmware.vim25.LongPolicy; import com.vmware.vim25.ManagedObjectReference; @@ -108,11 +104,16 @@ import com.vmware.vim25.NumericRange; import com.vmware.vim25.ObjectContent; import com.vmware.vim25.OptionValue; +import com.vmware.vim25.OutOfBoundsFaultMsg; +import com.vmware.vim25.OvfCreateDescriptorParams; +import com.vmware.vim25.OvfCreateDescriptorResult; import com.vmware.vim25.OvfCreateImportSpecParams; import com.vmware.vim25.OvfCreateImportSpecResult; -import com.vmware.vim25.OvfFileItem; import com.vmware.vim25.OvfFile; +import com.vmware.vim25.OvfFileItem; import com.vmware.vim25.ParaVirtualSCSIController; +import com.vmware.vim25.RuntimeFaultFaultMsg; +import com.vmware.vim25.TaskInProgressFaultMsg; import com.vmware.vim25.VMwareDVSConfigSpec; import com.vmware.vim25.VMwareDVSPortSetting; import com.vmware.vim25.VMwareDVSPortgroupPolicy; @@ -121,25 +122,24 @@ import com.vmware.vim25.VirtualBusLogicController; import com.vmware.vim25.VirtualController; import com.vmware.vim25.VirtualDevice; -import com.vmware.vim25.VirtualDisk; import com.vmware.vim25.VirtualDeviceConfigSpec; import com.vmware.vim25.VirtualDeviceConfigSpecOperation; +import com.vmware.vim25.VirtualDisk; import com.vmware.vim25.VirtualIDEController; import com.vmware.vim25.VirtualLsiLogicController; import com.vmware.vim25.VirtualLsiLogicSASController; import com.vmware.vim25.VirtualMachineConfigSpec; import com.vmware.vim25.VirtualMachineFileInfo; import com.vmware.vim25.VirtualMachineGuestOsIdentifier; +import com.vmware.vim25.VirtualMachineImportSpec; import com.vmware.vim25.VirtualMachineVideoCard; import com.vmware.vim25.VirtualSCSIController; import com.vmware.vim25.VirtualSCSISharing; -import com.vmware.vim25.VirtualMachineImportSpec; +import com.vmware.vim25.VmConfigFaultFaultMsg; import com.vmware.vim25.VmwareDistributedVirtualSwitchPvlanSpec; import com.vmware.vim25.VmwareDistributedVirtualSwitchTrunkVlanSpec; import com.vmware.vim25.VmwareDistributedVirtualSwitchVlanIdSpec; import com.vmware.vim25.VmwareDistributedVirtualSwitchVlanSpec; -import java.io.FileWriter; -import java.util.UUID; public class HypervisorHostHelper { private static final Logger s_logger = Logger.getLogger(HypervisorHostHelper.class); @@ -153,6 +153,48 @@ public class HypervisorHostHelper { public static final String VSPHERE_DATASTORE_BASE_FOLDER = "fcd"; public static final String VSPHERE_DATASTORE_HIDDEN_FOLDER = ".hidden"; + protected final static Map apiVersionHardwareVersionMap; + + static { + apiVersionHardwareVersionMap = new HashMap(); + apiVersionHardwareVersionMap.put("3.5", 4); + apiVersionHardwareVersionMap.put("3.6", 4); + apiVersionHardwareVersionMap.put("3.7", 4); + apiVersionHardwareVersionMap.put("3.8", 4); + apiVersionHardwareVersionMap.put("3.9", 4); + apiVersionHardwareVersionMap.put("4.0", 7); + apiVersionHardwareVersionMap.put("4.1", 7); + apiVersionHardwareVersionMap.put("4.2", 7); + apiVersionHardwareVersionMap.put("4.3", 7); + apiVersionHardwareVersionMap.put("4.4", 7); + apiVersionHardwareVersionMap.put("4.5", 7); + apiVersionHardwareVersionMap.put("4.6", 7); + apiVersionHardwareVersionMap.put("4.7", 7); + apiVersionHardwareVersionMap.put("4.8", 7); + apiVersionHardwareVersionMap.put("4.9", 7); + apiVersionHardwareVersionMap.put("5.0", 8); + apiVersionHardwareVersionMap.put("5.1", 9); + apiVersionHardwareVersionMap.put("5.2", 9); + apiVersionHardwareVersionMap.put("5.3", 9); + apiVersionHardwareVersionMap.put("5.4", 9); + apiVersionHardwareVersionMap.put("5.5", 10); + apiVersionHardwareVersionMap.put("5.6", 10); + apiVersionHardwareVersionMap.put("5.7", 10); + apiVersionHardwareVersionMap.put("5.8", 10); + apiVersionHardwareVersionMap.put("5.9", 10); + apiVersionHardwareVersionMap.put("6.0", 11); + apiVersionHardwareVersionMap.put("6.1", 11); + apiVersionHardwareVersionMap.put("6.2", 11); + apiVersionHardwareVersionMap.put("6.3", 11); + apiVersionHardwareVersionMap.put("6.4", 11); + apiVersionHardwareVersionMap.put("6.5", 13); + apiVersionHardwareVersionMap.put("6.6", 13); + apiVersionHardwareVersionMap.put("6.7", 14); + apiVersionHardwareVersionMap.put("6.8", 14); + apiVersionHardwareVersionMap.put("6.9", 14); + apiVersionHardwareVersionMap.put("7.0", 17); + } + public static VirtualMachineMO findVmFromObjectContent(VmwareContext context, ObjectContent[] ocs, String name, String instanceNameCustomField) { if (ocs != null && ocs.length > 0) { @@ -2211,4 +2253,19 @@ public static void createBaseFolderInDatastore(DatastoreMO dsMo, VmwareHyperviso dsMo.makeDirectory(hiddenFolderPath, hyperHost.getHyperHostDatacenter()); } } + + public static Integer getHostHardwareVersion(VmwareHypervisorHost host) { + Integer version = null; + HostMO hostMo = new HostMO(host.getContext(), host.getMor()); + String hostApiVersion = ""; + try { + hostApiVersion = hostMo.getHostAboutInfo().getApiVersion(); + } catch (Exception ignored) { + } + if (hostApiVersion == null) { + hostApiVersion = ""; + } + version = apiVersionHardwareVersionMap.get(hostApiVersion); + return version; + } } diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java index ff0120c9b8b0..a9620414fd58 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java @@ -40,6 +40,17 @@ import org.apache.commons.lang.StringUtils; import org.apache.log4j.Logger; +import com.cloud.hypervisor.vmware.mo.CustomFieldConstants; +import com.cloud.hypervisor.vmware.mo.DatastoreMO; +import com.cloud.hypervisor.vmware.mo.DiskControllerType; +import com.cloud.hypervisor.vmware.mo.HostMO; +import com.cloud.hypervisor.vmware.mo.LicenseAssignmentManagerMO; +import com.cloud.hypervisor.vmware.mo.VirtualEthernetCardType; +import com.cloud.hypervisor.vmware.mo.VirtualMachineMO; +import com.cloud.hypervisor.vmware.mo.VmwareHypervisorHost; +import com.cloud.utils.Pair; +import com.cloud.utils.Ternary; +import com.cloud.utils.exception.ExceptionUtil; import com.vmware.vim25.DistributedVirtualSwitchPortConnection; import com.vmware.vim25.DynamicProperty; import com.vmware.vim25.GuestOsDescriptor; @@ -56,7 +67,6 @@ import com.vmware.vim25.VirtualDevice; import com.vmware.vim25.VirtualDeviceBackingInfo; import com.vmware.vim25.VirtualDeviceConnectInfo; -import com.vmware.vim25.VirtualUSBController; import com.vmware.vim25.VirtualDisk; import com.vmware.vim25.VirtualDiskFlatVer1BackingInfo; import com.vmware.vim25.VirtualDiskFlatVer2BackingInfo; @@ -72,21 +82,10 @@ import com.vmware.vim25.VirtualMachineConfigSpec; import com.vmware.vim25.VirtualMachineSnapshotTree; import com.vmware.vim25.VirtualPCNet32; +import com.vmware.vim25.VirtualUSBController; import com.vmware.vim25.VirtualVmxnet2; import com.vmware.vim25.VirtualVmxnet3; -import com.cloud.hypervisor.vmware.mo.DiskControllerType; -import com.cloud.hypervisor.vmware.mo.DatastoreMO; -import com.cloud.hypervisor.vmware.mo.HostMO; -import com.cloud.hypervisor.vmware.mo.CustomFieldConstants; -import com.cloud.hypervisor.vmware.mo.LicenseAssignmentManagerMO; -import com.cloud.hypervisor.vmware.mo.VirtualEthernetCardType; -import com.cloud.hypervisor.vmware.mo.VirtualMachineMO; -import com.cloud.hypervisor.vmware.mo.VmwareHypervisorHost; -import com.cloud.utils.Pair; -import com.cloud.utils.Ternary; -import com.cloud.utils.exception.ExceptionUtil; - public class VmwareHelper { @SuppressWarnings("unused") private static final Logger s_logger = Logger.getLogger(VmwareHelper.class); From ee6aacd129e558f74e50896a7efe32e2164c0ba4 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 6 Apr 2021 14:16:49 +0530 Subject: [PATCH 04/13] cleanup unused method Signed-off-by: Abhishek Kumar --- .../storage/motion/VmwareStorageMotionStrategy.java | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java b/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java index 8d08c388823f..a0db8f079896 100644 --- a/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java +++ b/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java @@ -127,17 +127,6 @@ private boolean isOnVmware(DataObject srcData, DataObject destData) { && HypervisorType.VMware.equals(destData.getTO().getHypervisorType()); } - private boolean isIntraCluster(DataObject srcData, DataObject destData) { - DataStore srcStore = srcData.getDataStore(); - StoragePool srcPool = storagePoolDao.findById(srcStore.getId()); - DataStore destStore = destData.getDataStore(); - StoragePool destPool = storagePoolDao.findById(destStore.getId()); - if (srcPool.getClusterId() != null && destPool.getClusterId() != null) { - return srcPool.getClusterId().equals(destPool.getClusterId()); - } - return false; - } - /** * Ensure that the scope of source and destination storage pools match * From db2363dc6275a2c5bdb65481161632f30989f16f Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 7 Apr 2021 11:54:48 +0530 Subject: [PATCH 05/13] review changes Signed-off-by: Abhishek Kumar --- .../com/cloud/hypervisor/guru/VMwareGuru.java | 28 +++--- .../vmware/resource/VmwareResource.java | 40 ++++---- .../motion/VmwareStorageMotionStrategy.java | 95 ++++++++++--------- .../vmware/mo/HypervisorHostHelper.java | 17 ++++ 4 files changed, 101 insertions(+), 79 deletions(-) diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java index ec4ff0da3ccf..c1f4a4a867e7 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java @@ -209,7 +209,22 @@ protected VMwareGuru() { return vmwareVmImplementer.implement(vm, toVirtualMachineTO(vm), getClusterId(vm.getId())); } - Long getClusterId(long vmId) { + private Long getClusterIdFromVmVolume(long vmId) { + Long clusterId = null; + List volumes = _volumeDao.findByInstanceAndType(vmId, Volume.Type.ROOT); + if (CollectionUtils.isNotEmpty(volumes)) { + VolumeVO rootVolume = volumes.get(0); + if (rootVolume.getPoolId() != null) { + StoragePoolVO pool = _storagePoolDao.findById(rootVolume.getPoolId()); + if (pool != null && pool.getClusterId() != null) { + clusterId = pool.getClusterId(); + } + } + } + return clusterId; + } + + private Long getClusterId(long vmId) { Long clusterId = null; Long hostId = null; VMInstanceVO vm = _vmDao.findById(vmId); @@ -227,16 +242,7 @@ Long getClusterId(long vmId) { if (host != null) { clusterId = host.getClusterId(); } else { - List volumes = _volumeDao.findByInstanceAndType(vmId, Volume.Type.ROOT); - if (CollectionUtils.isNotEmpty(volumes)) { - VolumeVO rootVolume = volumes.get(0); - if (rootVolume.getPoolId() != null) { - StoragePoolVO pool = _storagePoolDao.findById(rootVolume.getPoolId()); - if (pool != null && pool.getClusterId() != null) { - clusterId = pool.getClusterId(); - } - } - } + clusterId = getClusterIdFromVmVolume(vmId); } return clusterId; diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index 49750b767a0f..6dfdeb5dd818 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -4417,18 +4417,10 @@ protected Answer execute(MigrateVmToPoolCommand cmd) { return new Answer(cmd, false, "unknown problem: " + e.getLocalizedMessage()); } } - - private Answer migrateAndAnswer(VirtualMachineMO vmMo, String poolUuid, VmwareHypervisorHost sourceHyperHost, - VmwareHypervisorHost targetHyperHost, Command cmd) throws Exception { - ManagedObjectReference morDs = getTargetDatastoreMOReference(poolUuid, sourceHyperHost); - if (morDs == null && targetHyperHost != null) { - morDs = getTargetDatastoreMOReference(poolUuid, targetHyperHost); - } - if (morDs == null) { - String msg = String.format("Failed to find datastore for pool UUID: %s", poolUuid); - s_logger.error(msg); - throw new CloudRuntimeException(msg); - } + private Answer migrateAndAnswer(VirtualMachineMO vmMo, String poolUuid, + VmwareHypervisorHost sourceHyperHost, VmwareHypervisorHost targetHyperHost, + Command cmd) throws Exception { + ManagedObjectReference morDs = getTargetDatastoreMOReference(poolUuid, sourceHyperHost, targetHyperHost); try { // OfflineVmwareMigration: getVolumesFromCommand(cmd); Map volumeDeviceKey = getVolumesFromCommand(vmMo, cmd); @@ -4513,18 +4505,28 @@ private void addVolumeDiskmapping(VirtualMachineMO vmMo, Map volu volumeDeviceKey.put(diskId, volumeId); } - private ManagedObjectReference getTargetDatastoreMOReference(String destinationPool, VmwareHypervisorHost hyperHost) { + private ManagedObjectReference getTargetDatastoreMOReference(String destinationPool, + VmwareHypervisorHost hyperHost, + VmwareHypervisorHost targetHyperHost) { ManagedObjectReference morDs; try { if (s_logger.isDebugEnabled()) { s_logger.debug(String.format("finding datastore %s", destinationPool)); } morDs = HypervisorHostHelper.findDatastoreWithBackwardsCompatibility(hyperHost, destinationPool); + if (morDs == null && targetHyperHost != null) { + morDs = HypervisorHostHelper.findDatastoreWithBackwardsCompatibility(targetHyperHost, destinationPool); + } } catch (Exception e) { String msg = "exception while finding data store " + destinationPool; s_logger.error(msg); throw new CloudRuntimeException(msg + ": " + e.getLocalizedMessage()); } + if (morDs == null) { + String msg = String.format("Failed to find datastore for pool UUID: %s", destinationPool); + s_logger.error(msg); + throw new CloudRuntimeException(msg); + } return morDs; } @@ -4881,15 +4883,6 @@ private Answer migrateVolume(MigrateVolumeCommand cmd) { vmName = getWorkerName(getServiceContext(), cmd, 0, destinationDsMo); } - String hardwareVersion = null; - if (hyperHostInTargetCluster != null) { - Integer sourceHardwareVersion = HypervisorHostHelper.getHostHardwareVersion(hyperHost); - Integer destinationHardwareVersion = HypervisorHostHelper.getHostHardwareVersion(hyperHostInTargetCluster); - if (sourceHardwareVersion != null && destinationHardwareVersion != null && !sourceHardwareVersion.equals(destinationHardwareVersion)) { - hardwareVersion = String.valueOf(Math.min(sourceHardwareVersion, destinationHardwareVersion)); - } - } - // OfflineVmwareMigration: refactor for re-use // OfflineVmwareMigration: 1. find data(store) // OfflineVmwareMigration: more robust would be to find the store given the volume as it might have been moved out of band or due to error @@ -4897,7 +4890,8 @@ private Answer migrateVolume(MigrateVolumeCommand cmd) { s_logger.info("Create worker VM " + vmName); // OfflineVmwareMigration: 2. create the worker with access to the data(store) - vmMo = HypervisorHostHelper.createWorkerVM(hyperHost, dsMo, vmName, hardwareVersion); + vmMo = HypervisorHostHelper.createWorkerVM(hyperHost, dsMo, vmName, + HypervisorHostHelper.getMinimumHostHardwareVersion(hyperHost, hyperHostInTargetCluster)); if (vmMo == null) { // OfflineVmwareMigration: don't throw a general Exception but think of a specific one throw new CloudRuntimeException("Unable to create a worker VM for volume operation"); diff --git a/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java b/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java index a0db8f079896..2854a7c3abe1 100644 --- a/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java +++ b/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java @@ -35,6 +35,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.framework.async.AsyncCompletionCallback; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.to.VolumeObjectTO; import org.apache.commons.collections.CollectionUtils; import org.apache.log4j.Logger; @@ -86,20 +87,19 @@ public class VmwareStorageMotionStrategy implements DataMotionStrategy { @Override public StrategyPriority canHandle(DataObject srcData, DataObject destData) { - // OfflineVmwareMigration: return StrategyPriority.HYPERVISOR when destData is in a storage pool in the same vmware-cluster and both are volumes + // OfflineVmwareMigration: return StrategyPriority.HYPERVISOR when destData is in a storage pool in the same pod or one of srcData & destData is in a zone-wide pool and both are volumes if (isOnVmware(srcData, destData) && isOnPrimary(srcData, destData) && isVolumesOnly(srcData, destData) + && isIntraPodOrZoneWideStoreInvolved(srcData, destData) && isDettached(srcData)) { if (s_logger.isDebugEnabled()) { - String msg = String.format("%s can handle the request because %d(%s) and %d(%s) share the VMware cluster %s (== %s)" + String msg = String.format("%s can handle the request because %d(%s) and %d(%s) share the pod" , this.getClass() , srcData.getId() , srcData.getUuid() , destData.getId() - , destData.getUuid() - , storagePoolDao.findById(srcData.getDataStore().getId()).getClusterId() - , storagePoolDao.findById(destData.getDataStore().getId()).getClusterId()); + , destData.getUuid()); s_logger.debug(msg); } return StrategyPriority.HYPERVISOR; @@ -107,6 +107,17 @@ && isDettached(srcData)) { return StrategyPriority.CANT_HANDLE; } + private boolean isIntraPodOrZoneWideStoreInvolved(DataObject srcData, DataObject destData) { + DataStore srcStore = srcData.getDataStore(); + StoragePoolVO srcPool = storagePoolDao.findById(srcStore.getId()); + DataStore destStore = destData.getDataStore(); + StoragePoolVO destPool = storagePoolDao.findById(destStore.getId()); + if (srcPool.getPodId() != null && destPool.getPodId() != null) { + return srcPool.getPodId().equals(destPool.getPodId()); + } + return (ScopeType.ZONE.equals(srcPool.getScope()) || ScopeType.ZONE.equals(destPool.getScope())); + } + private boolean isDettached(DataObject srcData) { VolumeVO volume = volDao.findById(srcData.getId()); return volume.getInstanceId() == null; @@ -127,19 +138,37 @@ private boolean isOnVmware(DataObject srcData, DataObject destData) { && HypervisorType.VMware.equals(destData.getTO().getHypervisorType()); } - /** - * Ensure that the scope of source and destination storage pools match - * - * @param srcData - * @param destData - * @return - */ - private boolean isStoreScopeEqual(DataObject srcData, DataObject destData) { - DataStore srcStore = srcData.getDataStore(); - DataStore destStore = destData.getDataStore(); - String msg = String.format("Storage scope of source pool is %s and of destination pool is %s", srcStore.getScope().toString(), destStore.getScope().toString()); - s_logger.debug(msg); - return srcStore.getScope().getScopeType() == (destStore.getScope().getScopeType()); + private Pair getHostIdForVmAndHostGuidInTargetCluster(DataObject srcData, DataObject destData) { + StoragePool sourcePool = (StoragePool) srcData.getDataStore(); + ScopeType sourceScopeType = srcData.getDataStore().getScope().getScopeType(); + StoragePool targetPool = (StoragePool) destData.getDataStore(); + ScopeType targetScopeType = destData.getDataStore().getScope().getScopeType(); + Long hostId = null; + String hostGuidInTargetCluster = null; + if (ScopeType.CLUSTER.equals(sourceScopeType)) { + // Find Volume source cluster and select any Vmware hypervisor host to attach worker VM + hostId = findSuitableHostIdForWorkerVmPlacement(sourcePool.getClusterId()); + if (hostId == null) { + throw new CloudRuntimeException("Offline Migration failed, unable to find suitable host for worker VM placement in the cluster of storage pool: " + sourcePool.getName()); + } + if (ScopeType.CLUSTER.equals(targetScopeType) && !sourcePool.getClusterId().equals(targetPool.getClusterId())) { + // Without host vMotion might fail between non-shared storages with error similar to, + // https://kb.vmware.com/s/article/1003795 + List hosts = hostDao.findHypervisorHostInCluster(targetPool.getClusterId()); + if (CollectionUtils.isNotEmpty(hosts)) { + hostGuidInTargetCluster = hosts.get(0).getGuid(); + } + if (hostGuidInTargetCluster == null) { + throw new CloudRuntimeException("Offline Migration failed, unable to find suitable target host for worker VM placement while migrating between storage pools of different cluster without shared storages"); + } + } + } else if (ScopeType.CLUSTER.equals(targetScopeType)) { + hostId = findSuitableHostIdForWorkerVmPlacement(targetPool.getClusterId()); + if (hostId == null) { + throw new CloudRuntimeException("Offline Migration failed, unable to find suitable host for worker VM placement in the cluster of storage pool: " + targetPool.getName()); + } + } + return new Pair<>(hostId, hostGuidInTargetCluster); } @Override @@ -176,35 +205,11 @@ public void copyAsync(DataObject srcData, DataObject destData, Host destHost, As // OfflineVmwareMigration: we shouldn't be here as we would have refused in the canHandle call throw new UnsupportedOperationException(); } + Pair hostIdForVmAndHostGuidInTargetCluster = getHostIdForVmAndHostGuidInTargetCluster(srcData, destData); + Long hostId = hostIdForVmAndHostGuidInTargetCluster.first(); + String hostGuidInTargetCluster = hostIdForVmAndHostGuidInTargetCluster.second(); StoragePool sourcePool = (StoragePool) srcData.getDataStore(); - ScopeType sourceScopeType = srcData.getDataStore().getScope().getScopeType(); StoragePool targetPool = (StoragePool) destData.getDataStore(); - ScopeType targetScopeType = destData.getDataStore().getScope().getScopeType(); - Long hostId = null; - String hostGuidInTargetCluster = null; - if (ScopeType.CLUSTER.equals(sourceScopeType)) { - // Find Volume source cluster and select any Vmware hypervisor host to attach worker VM - hostId = findSuitableHostIdForWorkerVmPlacement(sourcePool.getClusterId()); - if (hostId == null) { - throw new CloudRuntimeException("Offline Migration failed, unable to find suitable host for worker VM placement in the cluster of storage pool: " + sourcePool.getName()); - } - if (ScopeType.CLUSTER.equals(targetScopeType) && !sourcePool.getClusterId().equals(targetPool.getClusterId())) { - // Without host vMotion might fail between non-shared storages with error similar to, - // https://kb.vmware.com/s/article/1003795 - List hosts = hostDao.findHypervisorHostInCluster(targetPool.getClusterId()); - if (CollectionUtils.isNotEmpty(hosts)) { - hostGuidInTargetCluster = hosts.get(0).getGuid(); - } - if (hostGuidInTargetCluster == null) { - throw new CloudRuntimeException("Offline Migration failed, unable to find suitable target host for worker VM placement while migrating between storage pools of different cluster without shared storages"); - } - } - } else if (ScopeType.CLUSTER.equals(targetScopeType)) { - hostId = findSuitableHostIdForWorkerVmPlacement(targetPool.getClusterId()); - if (hostId == null) { - throw new CloudRuntimeException("Offline Migration failed, unable to find suitable host for worker VM placement in the cluster of storage pool: " + targetPool.getName()); - } - } MigrateVolumeCommand cmd = new MigrateVolumeCommand(srcData.getId() , srcData.getTO().getPath() , sourcePool diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java index fec4763baa37..3dac56622911 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java @@ -2268,4 +2268,21 @@ public static Integer getHostHardwareVersion(VmwareHypervisorHost host) { version = apiVersionHardwareVersionMap.get(hostApiVersion); return version; } + + /* + Finds minimum host hardware version as String, of two hosts when both of them are not null + and hardware version of both hosts is different. + Return null otherwise + */ + public static String getMinimumHostHardwareVersion(VmwareHypervisorHost host1, VmwareHypervisorHost host2) { + String hardwareVersion = null; + if (host1 != null & host2 != null) { + Integer host1Version = getHostHardwareVersion(host1); + Integer host2Version = getHostHardwareVersion(host2); + if (host1Version != null && host2Version != null && !host1Version.equals(host2Version)) { + hardwareVersion = String.valueOf(Math.min(host1Version, host2Version)); + } + } + return hardwareVersion; + } } From e172623d8777179af999140c75dc5dcabaf6c855 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 7 Apr 2021 12:13:33 +0530 Subject: [PATCH 06/13] changes Signed-off-by: Abhishek Kumar --- .../com/cloud/hypervisor/guru/VMwareGuru.java | 40 ++++++++++++------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java index c1f4a4a867e7..66fd18681d70 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java @@ -1083,22 +1083,14 @@ private VirtualDisk getAttachedDisk(VirtualMachineMO vmMo, String diskPath) thro return null; } - @Override public List finalizeMigrate(VirtualMachine vm, StoragePool destination) { - List commands = new ArrayList(); - - // OfflineVmwareMigration: specialised migration command - List volumes = _volumeDao.findByInstance(vm.getId()); - List vols = new ArrayList<>(); - for (Volume volume : volumes) { - VolumeTO vol = new VolumeTO(volume, destination); - vols.add(vol); - } + private boolean isInterClusterMigration(Long srcClusterId, Long destClusterId) { + return srcClusterId != null && destClusterId != null && ! srcClusterId.equals(destClusterId); + } - final Long destClusterId = destination.getClusterId(); - final Long srcClusterId = getClusterId(vm.getId()); - final boolean isInterClusterMigration = srcClusterId != null && destClusterId != null && ! srcClusterId.equals(destClusterId); - Host hostInTargetCluster = null; + private String getHostGuidInTargetCluster(boolean isInterClusterMigration, Long destClusterId) { + String hostGuidInTargetCluster = null; if (isInterClusterMigration) { + Host hostInTargetCluster = null; // Without host vMotion might fail between non-shared storages with error similar to, // https://kb.vmware.com/s/article/1003795 // As this is offline migration VM won't be started on this host @@ -1109,9 +1101,27 @@ private VirtualDisk getAttachedDisk(VirtualMachineMO vmMo, String diskPath) thro if (hostInTargetCluster == null) { throw new CloudRuntimeException("Migration failed, unable to find suitable target host for VM placement while migrating between storage pools of different clusters without shared storages"); } + hostGuidInTargetCluster = hostInTargetCluster.getGuid(); + } + return hostGuidInTargetCluster; + } + + @Override public List finalizeMigrate(VirtualMachine vm, StoragePool destination) { + List commands = new ArrayList(); + + // OfflineVmwareMigration: specialised migration command + List volumes = _volumeDao.findByInstance(vm.getId()); + List vols = new ArrayList<>(); + for (Volume volume : volumes) { + VolumeTO vol = new VolumeTO(volume, destination); + vols.add(vol); } + + final Long destClusterId = destination.getClusterId(); + final Long srcClusterId = getClusterId(vm.getId()); + final boolean isInterClusterMigration = isInterClusterMigration(destClusterId, srcClusterId); MigrateVmToPoolCommand migrateVmToPoolCommand = new MigrateVmToPoolCommand(vm.getInstanceName(), - vols, destination.getUuid(), hostInTargetCluster == null ? null : hostInTargetCluster.getGuid(), true); + vols, destination.getUuid(), getHostGuidInTargetCluster(isInterClusterMigration, destClusterId), true); commands.add(migrateVmToPoolCommand); // OfflineVmwareMigration: cleanup if needed From 691ca7020bbde37219a69f13b2efdcaa41ff16ff Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 5 Apr 2021 11:36:53 +0530 Subject: [PATCH 07/13] vmware: allow attached volume migration using VmwareStorageMotionStrategy Signed-off-by: Abhishek Kumar --- .../motion/VmwareStorageMotionStrategy.java | 68 +++++++++---------- .../cloud/storage/VolumeApiServiceImpl.java | 17 +++-- 2 files changed, 44 insertions(+), 41 deletions(-) diff --git a/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java b/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java index 5a7b4c4ca677..39ee62f14a67 100644 --- a/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java +++ b/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java @@ -26,6 +26,20 @@ import javax.inject.Inject; +import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; +import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionStrategy; +import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; +import org.apache.cloudstack.framework.async.AsyncCompletionCallback; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.cloudstack.storage.to.VolumeObjectTO; +import org.apache.log4j.Logger; +import org.springframework.stereotype.Component; + import com.cloud.agent.AgentManager; import com.cloud.agent.api.Answer; import com.cloud.agent.api.MigrateWithStorageAnswer; @@ -52,19 +66,8 @@ import com.cloud.utils.Pair; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.VMInstanceVO; +import com.cloud.vm.VirtualMachine; import com.cloud.vm.dao.VMInstanceDao; -import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; -import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionStrategy; -import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; -import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; -import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority; -import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; -import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; -import org.apache.cloudstack.framework.async.AsyncCompletionCallback; -import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; -import org.apache.cloudstack.storage.to.VolumeObjectTO; -import org.apache.log4j.Logger; -import org.springframework.stereotype.Component; @Component public class VmwareStorageMotionStrategy implements DataMotionStrategy { @@ -88,9 +91,8 @@ public StrategyPriority canHandle(DataObject srcData, DataObject destData) { if (isOnVmware(srcData, destData) && isOnPrimary(srcData, destData) && isVolumesOnly(srcData, destData) - && isDettached(srcData) - && isIntraCluster(srcData, destData) - && isStoreScopeEqual(srcData, destData)) { + && isDetachedOrAttachedToStoppedVM(srcData) + && isIntraClusterOrZoneWideStoreInvolved(srcData, destData)) { if (s_logger.isDebugEnabled()) { String msg = String.format("%s can handle the request because %d(%s) and %d(%s) share the VMware cluster %s (== %s)" , this.getClass() @@ -107,9 +109,18 @@ && isStoreScopeEqual(srcData, destData)) { return StrategyPriority.CANT_HANDLE; } - private boolean isDettached(DataObject srcData) { + private boolean isAttachedToStoppedVM(Volume volume) { + VMInstanceVO vm = instanceDao.findById(volume.getInstanceId()); + if (vm != null && VirtualMachine.State.Stopped.equals(vm.getState())) { + List volumes = volDao.findByInstance(vm.getId()); + return volumes.size() == 1; + } + return false; + } + + private boolean isDetachedOrAttachedToStoppedVM(DataObject srcData) { VolumeVO volume = volDao.findById(srcData.getId()); - return volume.getInstanceId() == null; + return volume.getInstanceId() == null || isAttachedToStoppedVM(volume); } private boolean isVolumesOnly(DataObject srcData, DataObject destData) { @@ -127,30 +138,15 @@ private boolean isOnVmware(DataObject srcData, DataObject destData) { && HypervisorType.VMware.equals(destData.getTO().getHypervisorType()); } - private boolean isIntraCluster(DataObject srcData, DataObject destData) { + private boolean isIntraClusterOrZoneWideStoreInvolved(DataObject srcData, DataObject destData) { DataStore srcStore = srcData.getDataStore(); - StoragePool srcPool = storagePoolDao.findById(srcStore.getId()); + StoragePoolVO srcPool = storagePoolDao.findById(srcStore.getId()); DataStore destStore = destData.getDataStore(); - StoragePool destPool = storagePoolDao.findById(destStore.getId()); + StoragePoolVO destPool = storagePoolDao.findById(destStore.getId()); if (srcPool.getClusterId() != null && destPool.getClusterId() != null) { return srcPool.getClusterId().equals(destPool.getClusterId()); } - return false; - } - - /** - * Ensure that the scope of source and destination storage pools match - * - * @param srcData - * @param destData - * @return - */ - private boolean isStoreScopeEqual(DataObject srcData, DataObject destData) { - DataStore srcStore = srcData.getDataStore(); - DataStore destStore = destData.getDataStore(); - String msg = String.format("Storage scope of source pool is %s and of destination pool is %s", srcStore.getScope().toString(), destStore.getScope().toString()); - s_logger.debug(msg); - return srcStore.getScope().getScopeType() == (destStore.getScope().getScopeType()); + return ScopeType.ZONE.equals(srcPool.getScope()) || ScopeType.ZONE.equals(destPool.getScope()); } @Override diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 0adeb836c7c6..213d0f1e9db4 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -31,7 +31,6 @@ import javax.inject.Inject; -import com.cloud.dc.Pod; import org.apache.cloudstack.api.command.user.volume.AttachVolumeCmd; import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd; import org.apache.cloudstack.api.command.user.volume.DetachVolumeCmd; @@ -100,6 +99,7 @@ import com.cloud.dc.ClusterDetailsDao; import com.cloud.dc.DataCenter; import com.cloud.dc.DataCenterVO; +import com.cloud.dc.Pod; import com.cloud.dc.dao.DataCenterDao; import com.cloud.domain.Domain; import com.cloud.event.ActionEvent; @@ -2208,13 +2208,13 @@ public Volume migrateVolume(MigrateVolumeCmd cmd) { } // Check that Vm to which this volume is attached does not have VM Snapshots - // OfflineVmwareMigration: considder if this is needed and desirable + // OfflineVmwareMigration: consider if this is needed and desirable if (vm != null && _vmSnapshotDao.findByVm(vm.getId()).size() > 0) { throw new InvalidParameterValueException("Volume cannot be migrated, please remove all VM snapshots for VM to which this volume is attached"); } // OfflineVmwareMigration: extract this block as method and check if it is subject to regression - if (vm != null && vm.getState() == State.Running) { + if (vm != null && State.Running.equals(vm.getState())) { // Check if the VM is GPU enabled. if (_serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString()) != null) { throw new InvalidParameterValueException("Live Migration of GPU enabled VM is not supported"); @@ -2244,10 +2244,17 @@ public Volume migrateVolume(MigrateVolumeCmd cmd) { if (!liveMigrateVolume) { throw new InvalidParameterValueException("Volume needs to be detached from VM"); } + + if (!cmd.isLiveMigrate()) { + throw new InvalidParameterValueException("The volume " + vol + "is attached to a vm and for migrating it " + "the parameter livemigrate should be specified"); + } } - if (liveMigrateVolume && !cmd.isLiveMigrate()) { - throw new InvalidParameterValueException("The volume " + vol + "is attached to a vm and for migrating it " + "the parameter livemigrate should be specified"); + if (vm != null && + HypervisorType.VMware.equals(vm.getHypervisorType()) && + State.Stopped.equals(vm.getState())) { + // For VMware, use liveMigrateVolume=true so that it follows VmwareStorageMotionStrategy + liveMigrateVolume = true; } StoragePool destPool = (StoragePool)dataStoreMgr.getDataStore(storagePoolId, DataStoreRole.Primary); From 9973e57197a48ed3444cb793b412a0163da6063d Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 8 Apr 2021 10:28:06 +0530 Subject: [PATCH 08/13] find vm clusterid with multiple ROOT volumes VM can have multiple ROOT volumes and some can be on zone-wide store therefore iterate over all of them till a cluster ID is found. Signed-off-by: Abhishek Kumar --- .../java/com/cloud/hypervisor/guru/VMwareGuru.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java index 66fd18681d70..edb3b88c18fd 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java @@ -213,11 +213,13 @@ private Long getClusterIdFromVmVolume(long vmId) { Long clusterId = null; List volumes = _volumeDao.findByInstanceAndType(vmId, Volume.Type.ROOT); if (CollectionUtils.isNotEmpty(volumes)) { - VolumeVO rootVolume = volumes.get(0); - if (rootVolume.getPoolId() != null) { - StoragePoolVO pool = _storagePoolDao.findById(rootVolume.getPoolId()); - if (pool != null && pool.getClusterId() != null) { - clusterId = pool.getClusterId(); + for (VolumeVO rootVolume : volumes) { + if (rootVolume.getPoolId() != null) { + StoragePoolVO pool = _storagePoolDao.findById(rootVolume.getPoolId()); + if (pool != null && pool.getClusterId() != null) { + clusterId = pool.getClusterId(); + break; + } } } } From 23568943389b454ecd34f225bfa117dedf418e57 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 8 Apr 2021 15:37:42 +0530 Subject: [PATCH 09/13] fix successive storage migration Signed-off-by: Abhishek Kumar --- .../cloud/vm/VirtualMachineManagerImpl.java | 92 +++++++++++++------ 1 file changed, 64 insertions(+), 28 deletions(-) diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index f1ab9cd1ff95..6faeeb5f4666 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -40,7 +40,6 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; -import com.cloud.deployasis.dao.UserVmDeployAsIsDetailsDao; import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.command.admin.vm.MigrateVMCmd; @@ -142,6 +141,7 @@ import com.cloud.deploy.DeploymentPlanner; import com.cloud.deploy.DeploymentPlanner.ExcludeList; import com.cloud.deploy.DeploymentPlanningManager; +import com.cloud.deployasis.dao.UserVmDeployAsIsDetailsDao; import com.cloud.event.EventTypes; import com.cloud.event.UsageEventUtils; import com.cloud.event.UsageEventVO; @@ -2210,7 +2210,10 @@ private void orchestrateStorageMigration(final String vmUuid, final StoragePool } } - private Answer[] attemptHypervisorMigration(StoragePool destPool, VMInstanceVO vm) { + private Answer[] attemptHypervisorMigration(StoragePool destPool, VMInstanceVO vm, Long hostId) { + if (hostId == null) { + return null; + } final HypervisorGuru hvGuru = _hvGuruMgr.getGuru(vm.getHypervisorType()); // OfflineVmwareMigration: in case of vmware call vcenter to do it for us. // OfflineVmwareMigration: should we check the proximity of source and destination @@ -2218,15 +2221,6 @@ private Answer[] attemptHypervisorMigration(StoragePool destPool, VMInstanceVO v // OfflineVmwareMigration: we are checking on success to optionally delete an old vm if we are not List commandsToSend = hvGuru.finalizeMigrate(vm, destPool); - Long hostId = vm.getHostId(); - // OfflineVmwareMigration: probably this is null when vm is stopped - if(hostId == null) { - hostId = vm.getLastHostId(); - if (s_logger.isDebugEnabled()) { - s_logger.debug(String.format("host id is null, using last host id %d", hostId) ); - } - } - if(CollectionUtils.isNotEmpty(commandsToSend)) { Commands commandsContainer = new Commands(Command.OnError.Stop); commandsContainer.addCommands(commandsToSend); @@ -2241,7 +2235,7 @@ private Answer[] attemptHypervisorMigration(StoragePool destPool, VMInstanceVO v return null; } - private void afterHypervisorMigrationCleanup(StoragePool destPool, VMInstanceVO vm, HostVO srcHost, Long srcClusterId, Answer[] hypervisorMigrationResults) throws InsufficientCapacityException { + private void afterHypervisorMigrationCleanup(StoragePool destPool, VMInstanceVO vm, Long srcClusterId, Answer[] hypervisorMigrationResults) throws InsufficientCapacityException { boolean isDebugEnabled = s_logger.isDebugEnabled(); if(isDebugEnabled) { String msg = String.format("cleaning up after hypervisor pool migration volumes for VM %s(%s) to pool %s(%s)", vm.getInstanceName(), vm.getUuid(), destPool.getName(), destPool.getUuid()); @@ -2250,18 +2244,23 @@ private void afterHypervisorMigrationCleanup(StoragePool destPool, VMInstanceVO setDestinationPoolAndReallocateNetwork(destPool, vm); // OfflineVmwareMigration: don't set this to null or have another way to address the command; twice migrating will lead to an NPE Long destPodId = destPool.getPodId(); - Long vmPodId = vm.getPodIdToDeployIn(); - if (destPodId == null || ! destPodId.equals(vmPodId)) { + + if (destPodId == null || !destPodId.equals(vm.getPodIdToDeployIn())) { if(isDebugEnabled) { String msg = String.format("resetting lasHost for VM %s(%s) as pod (%s) is no good.", vm.getInstanceName(), vm.getUuid(), destPodId); s_logger.debug(msg); } - vm.setLastHostId(null); vm.setPodIdToDeployIn(destPodId); // OfflineVmwareMigration: a consecutive migration will fail probably (no host not pod) - }// else keep last host set for this vm - markVolumesInPool(vm,destPool, hypervisorMigrationResults); + } else if (srcClusterId != null && destPool.getClusterId() != null && !srcClusterId.equals(destPool.getClusterId())) { + if(isDebugEnabled) { + String msg = String.format("resetting lasHost for VM %s(%s) as cluster changed", vm.getInstanceName(), vm.getUuid()); + s_logger.debug(msg); + } + vm.setLastHostId(null); + } // else keep last host set for this vm + markVolumesInPool(vm, destPool, hypervisorMigrationResults); // OfflineVmwareMigration: deal with answers, if (hypervisorMigrationResults.length > 0) // OfflineVmwareMigration: iterate over the volumes for data updates } @@ -2295,23 +2294,60 @@ private void markVolumesInPool(VMInstanceVO vm, StoragePool destPool, Answer[] h } } + private Pair findClusterAndHostIdForVm(VMInstanceVO vm) { + Long hostId = vm.getHostId(); + Long clusterId = null; + // OfflineVmwareMigration: probably this is null when vm is stopped + if(hostId == null) { + hostId = vm.getLastHostId(); + if (s_logger.isDebugEnabled()) { + s_logger.debug(String.format("host id is null, using last host id %d", hostId) ); + } + } + if (hostId == null) { + List volumes = _volsDao.findByInstanceAndType(vm.getId(), Type.ROOT); + if (CollectionUtils.isNotEmpty(volumes)) { + for (VolumeVO rootVolume : volumes) { + if (rootVolume.getPoolId() != null) { + StoragePoolVO pool = _storagePoolDao.findById(rootVolume.getPoolId()); + if (pool != null && pool.getClusterId() != null) { + clusterId = pool.getClusterId(); + List hosts = _hostDao.findHypervisorHostInCluster(pool.getClusterId()); + if (CollectionUtils.isNotEmpty(hosts)) { + hostId = hosts.get(0).getId(); + break; + } + } + } + } + } + } + if (clusterId == null && hostId != null) { + HostVO host = _hostDao.findById(hostId); + if (host != null) { + clusterId = host.getClusterId(); + } + } + return new Pair<>(clusterId, hostId); + } + private void migrateThroughHypervisorOrStorage(StoragePool destPool, VMInstanceVO vm) throws StorageUnavailableException, InsufficientCapacityException { final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm); - final Long srchostId = vm.getHostId() != null ? vm.getHostId() : vm.getLastHostId(); - final HostVO srcHost = _hostDao.findById(srchostId); - final Long srcClusterId = srcHost.getClusterId(); - Answer[] hypervisorMigrationResults = attemptHypervisorMigration(destPool, vm); + Pair vmClusterAndHost = findClusterAndHostIdForVm(vm); + final Long sourceClusterId = vmClusterAndHost.first(); + final Long sourceHostId = vmClusterAndHost.second(); + Answer[] hypervisorMigrationResults = attemptHypervisorMigration(destPool, vm, sourceHostId); boolean migrationResult = false; if (hypervisorMigrationResults == null) { // OfflineVmwareMigration: if the HypervisorGuru can't do it, let the volume manager take care of it. migrationResult = volumeMgr.storageMigration(profile, destPool); if (migrationResult) { - afterStorageMigrationCleanup(destPool, vm, srcHost, srcClusterId); + afterStorageMigrationCleanup(destPool, vm, sourceHostId, sourceClusterId); } else { s_logger.debug("Storage migration failed"); } } else { - afterHypervisorMigrationCleanup(destPool, vm, srcHost, srcClusterId, hypervisorMigrationResults); + afterHypervisorMigrationCleanup(destPool, vm, sourceClusterId, hypervisorMigrationResults); } } @@ -2366,7 +2402,7 @@ static boolean matches(List volumeTags, List storagePoolTags) { } - private void afterStorageMigrationCleanup(StoragePool destPool, VMInstanceVO vm, HostVO srcHost, Long srcClusterId) throws InsufficientCapacityException { + private void afterStorageMigrationCleanup(StoragePool destPool, VMInstanceVO vm, Long srcHostId, Long srcClusterId) throws InsufficientCapacityException { setDestinationPoolAndReallocateNetwork(destPool, vm); //when start the vm next time, don;'t look at last_host_id, only choose the host based on volume/storage pool @@ -2376,7 +2412,7 @@ private void afterStorageMigrationCleanup(StoragePool destPool, VMInstanceVO vm, // If VM was cold migrated between clusters belonging to two different VMware DCs, // unregister the VM from the source host and cleanup the associated VM files. if (vm.getHypervisorType().equals(HypervisorType.VMware)) { - afterStorageMigrationVmwareVMcleanup(destPool, vm, srcHost, srcClusterId); + afterStorageMigrationVmwareVMCleanup(destPool, vm, srcHostId, srcClusterId); } } @@ -2394,14 +2430,14 @@ private void setDestinationPoolAndReallocateNetwork(StoragePool destPool, VMInst } } - private void afterStorageMigrationVmwareVMcleanup(StoragePool destPool, VMInstanceVO vm, HostVO srcHost, Long srcClusterId) { + private void afterStorageMigrationVmwareVMCleanup(StoragePool destPool, VMInstanceVO vm, Long srcHostId, Long srcClusterId) { // OfflineVmwareMigration: this should only happen on storage migration, else the guru would already have issued the command final Long destClusterId = destPool.getClusterId(); - if (srcClusterId != null && destClusterId != null && ! srcClusterId.equals(destClusterId)) { + if (srcHostId != null && srcClusterId != null && destClusterId != null && ! srcClusterId.equals(destClusterId)) { final String srcDcName = _clusterDetailsDao.getVmwareDcName(srcClusterId); final String destDcName = _clusterDetailsDao.getVmwareDcName(destClusterId); if (srcDcName != null && destDcName != null && !srcDcName.equals(destDcName)) { - removeStaleVmFromSource(vm, srcHost); + removeStaleVmFromSource(vm, _hostDao.findById(srcHostId)); } } } From 79bb82cb081ad33db796d001491ca69cf5f0456e Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Sun, 11 Apr 2021 22:41:42 +0530 Subject: [PATCH 10/13] fix intercluster check Signed-off-by: Abhishek Kumar --- .../api/storage/MigrateVolumeCommand.java | 4 +- .../motion/VmwareStorageMotionStrategy.java | 134 ++++++++++++++---- .../cloud/storage/VolumeApiServiceImpl.java | 3 +- 3 files changed, 109 insertions(+), 32 deletions(-) diff --git a/core/src/main/java/com/cloud/agent/api/storage/MigrateVolumeCommand.java b/core/src/main/java/com/cloud/agent/api/storage/MigrateVolumeCommand.java index 5443ad12eadd..0550e8d010e9 100644 --- a/core/src/main/java/com/cloud/agent/api/storage/MigrateVolumeCommand.java +++ b/core/src/main/java/com/cloud/agent/api/storage/MigrateVolumeCommand.java @@ -55,8 +55,8 @@ public MigrateVolumeCommand(long volumeId, String volumePath, StoragePool pool, this.setWait(timeout); } - public MigrateVolumeCommand(long volumeId, String volumePath, StoragePool sourcePool, StoragePool targetPool, String hostGuidInTargetCluster) { - this(volumeId,volumePath,targetPool, null, Volume.Type.UNKNOWN, -1); + public MigrateVolumeCommand(long volumeId, String volumePath, String attachedVmName, StoragePool sourcePool, StoragePool targetPool, String hostGuidInTargetCluster) { + this(volumeId,volumePath,targetPool, attachedVmName, Volume.Type.UNKNOWN, -1); this.sourcePool = new StorageFilerTO(sourcePool); this.hostGuidInTargetCluster = hostGuidInTargetCluster; } diff --git a/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java b/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java index fd687dd4748e..ab57fdfbd4ed 100644 --- a/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java +++ b/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java @@ -110,11 +110,7 @@ && isIntraPodOrZoneWideStoreInvolved(srcData, destData)) { private boolean isAttachedToStoppedVM(Volume volume) { VMInstanceVO vm = instanceDao.findById(volume.getInstanceId()); - if (vm != null && VirtualMachine.State.Stopped.equals(vm.getState())) { - List volumes = volDao.findByInstance(vm.getId()); - return volumes.size() == 1; - } - return false; + return vm != null && VirtualMachine.State.Stopped.equals(vm.getState()); } private boolean isDetachedOrAttachedToStoppedVM(DataObject srcData) { @@ -148,11 +144,83 @@ private boolean isOnVmware(DataObject srcData, DataObject destData) { && HypervisorType.VMware.equals(destData.getTO().getHypervisorType()); } - private Pair getHostIdForVmAndHostGuidInTargetCluster(DataObject srcData, DataObject destData) { - StoragePool sourcePool = (StoragePool) srcData.getDataStore(); - ScopeType sourceScopeType = srcData.getDataStore().getScope().getScopeType(); - StoragePool targetPool = (StoragePool) destData.getDataStore(); - ScopeType targetScopeType = destData.getDataStore().getScope().getScopeType(); + private Pair findClusterAndHostIdForVm(VirtualMachine vm) { + Long hostId = vm.getHostId(); + Long clusterId = null; + // OfflineVmwareMigration: probably this is null when vm is stopped + if(hostId == null) { + hostId = vm.getLastHostId(); + if (s_logger.isDebugEnabled()) { + s_logger.debug(String.format("host id is null, using last host id %d", hostId) ); + } + } + if (hostId == null) { + List volumes = volDao.findByInstanceAndType(vm.getId(), Volume.Type.ROOT); + if (CollectionUtils.isNotEmpty(volumes)) { + for (VolumeVO rootVolume : volumes) { + if (rootVolume.getPoolId() != null) { + StoragePoolVO pool = storagePoolDao.findById(rootVolume.getPoolId()); + if (pool != null && pool.getClusterId() != null) { + clusterId = pool.getClusterId(); + List hosts = hostDao.findHypervisorHostInCluster(pool.getClusterId()); + if (CollectionUtils.isNotEmpty(hosts)) { + hostId = hosts.get(0).getId(); + break; + } + } + } + } + } + } + if (clusterId == null && hostId != null) { + HostVO host = hostDao.findById(hostId); + if (host != null) { + clusterId = host.getClusterId(); + } + } + return new Pair<>(clusterId, hostId); + } + + private String getHostGuidInTargetCluster (Long sourceClusterId, + StoragePool targetPool, + ScopeType targetScopeType) { + String hostGuidInTargetCluster = null; + if (ScopeType.CLUSTER.equals(targetScopeType) && !sourceClusterId.equals(targetPool.getClusterId())) { + // Without host vMotion might fail between non-shared storages with error similar to, + // https://kb.vmware.com/s/article/1003795 + List hosts = hostDao.findHypervisorHostInCluster(targetPool.getClusterId()); + if (CollectionUtils.isNotEmpty(hosts)) { + hostGuidInTargetCluster = hosts.get(0).getGuid(); + } + if (hostGuidInTargetCluster == null) { + throw new CloudRuntimeException("Offline Migration failed, unable to find suitable target host for VM placement while migrating between storage pools of different cluster without shared storages"); + } + } + return hostGuidInTargetCluster; + } + + private VirtualMachine getVolumeVm(DataObject srcData) { + if (srcData instanceof VolumeInfo) { + return ((VolumeInfo)srcData).getAttachedVM(); + } + VolumeVO volume = volDao.findById(srcData.getId()); + return volume.getInstanceId() == null ? null : instanceDao.findById(volume.getInstanceId()); + } + + private Pair getHostIdForVmAndHostGuidInTargetClusterForAttachedVm(VirtualMachine vm, + StoragePool targetPool, + ScopeType targetScopeType) { + Pair clusterAndHostId = findClusterAndHostIdForVm(vm); + if (clusterAndHostId.second() == null) { + throw new CloudRuntimeException(String.format("Offline Migration failed, unable to find host for VM: %s", vm.getUuid())); + } + return new Pair<>(clusterAndHostId.second(), getHostGuidInTargetCluster(clusterAndHostId.first(), targetPool, targetScopeType)); + } + + private Pair getHostIdForVmAndHostGuidInTargetClusterForWorkerVm(StoragePool sourcePool, + ScopeType sourceScopeType, + StoragePool targetPool, + ScopeType targetScopeType) { Long hostId = null; String hostGuidInTargetCluster = null; if (ScopeType.CLUSTER.equals(sourceScopeType)) { @@ -161,17 +229,7 @@ private Pair getHostIdForVmAndHostGuidInTargetCluster(DataObject s if (hostId == null) { throw new CloudRuntimeException("Offline Migration failed, unable to find suitable host for worker VM placement in the cluster of storage pool: " + sourcePool.getName()); } - if (ScopeType.CLUSTER.equals(targetScopeType) && !sourcePool.getClusterId().equals(targetPool.getClusterId())) { - // Without host vMotion might fail between non-shared storages with error similar to, - // https://kb.vmware.com/s/article/1003795 - List hosts = hostDao.findHypervisorHostInCluster(targetPool.getClusterId()); - if (CollectionUtils.isNotEmpty(hosts)) { - hostGuidInTargetCluster = hosts.get(0).getGuid(); - } - if (hostGuidInTargetCluster == null) { - throw new CloudRuntimeException("Offline Migration failed, unable to find suitable target host for worker VM placement while migrating between storage pools of different cluster without shared storages"); - } - } + hostGuidInTargetCluster = getHostGuidInTargetCluster(sourcePool.getClusterId(), targetPool, sourceScopeType); } else if (ScopeType.CLUSTER.equals(targetScopeType)) { hostId = findSuitableHostIdForWorkerVmPlacement(targetPool.getClusterId()); if (hostId == null) { @@ -181,6 +239,20 @@ private Pair getHostIdForVmAndHostGuidInTargetCluster(DataObject s return new Pair<>(hostId, hostGuidInTargetCluster); } + private Pair getHostIdForVmAndHostGuidInTargetCluster(VirtualMachine vm, + DataObject srcData, + StoragePool sourcePool, + DataObject destData, + StoragePool targetPool) { + ScopeType sourceScopeType = srcData.getDataStore().getScope().getScopeType(); + ScopeType targetScopeType = destData.getDataStore().getScope().getScopeType(); + if (vm != null) { + return getHostIdForVmAndHostGuidInTargetClusterForAttachedVm(vm, targetPool, targetScopeType); + } + return getHostIdForVmAndHostGuidInTargetClusterForWorkerVm(sourcePool, sourceScopeType, targetPool, targetScopeType); + + } + @Override public StrategyPriority canHandle(Map volumeMap, Host srcHost, Host destHost) { if (srcHost.getHypervisorType() == HypervisorType.VMware && destHost.getHypervisorType() == HypervisorType.VMware) { @@ -215,17 +287,18 @@ public void copyAsync(DataObject srcData, DataObject destData, Host destHost, As // OfflineVmwareMigration: we shouldn't be here as we would have refused in the canHandle call throw new UnsupportedOperationException(); } - Pair hostIdForVmAndHostGuidInTargetCluster = getHostIdForVmAndHostGuidInTargetCluster(srcData, destData); - Long hostId = hostIdForVmAndHostGuidInTargetCluster.first(); - String hostGuidInTargetCluster = hostIdForVmAndHostGuidInTargetCluster.second(); + VirtualMachine vm = getVolumeVm(srcData); StoragePool sourcePool = (StoragePool) srcData.getDataStore(); StoragePool targetPool = (StoragePool) destData.getDataStore(); + Pair hostIdForVmAndHostGuidInTargetCluster = + getHostIdForVmAndHostGuidInTargetCluster(vm, srcData, sourcePool, destData, targetPool); + Long hostId = hostIdForVmAndHostGuidInTargetCluster.first(); MigrateVolumeCommand cmd = new MigrateVolumeCommand(srcData.getId() , srcData.getTO().getPath() + , vm != null ? vm.getInstanceName() : null , sourcePool , targetPool - , hostGuidInTargetCluster); - // OfflineVmwareMigration: should be ((StoragePool)srcData.getDataStore()).getHypervisor() but that is NULL, so hardcoding + , hostIdForVmAndHostGuidInTargetCluster.second()); Answer answer; if (hostId != null) { answer = agentMgr.easySend(hostId, cmd); @@ -265,9 +338,11 @@ private void updateVolumeAfterMigration(Answer answer, DataObject srcData, DataO VolumeVO sourceVO = volDao.findById(srcData.getId()); sourceVO.setState(Volume.State.Ready); volDao.update(sourceVO.getId(), sourceVO); - destinationVO.setState(Volume.State.Expunged); - destinationVO.setRemoved(new Date()); - volDao.update(destinationVO.getId(), destinationVO); + if (destinationVO.getId() != sourceVO.getId()) { + destinationVO.setState(Volume.State.Expunged); + destinationVO.setRemoved(new Date()); + volDao.update(destinationVO.getId(), destinationVO); + } throw new CloudRuntimeException("unexpected answer from hypervisor agent: " + answer.getDetails()); } MigrateVolumeAnswer ans = (MigrateVolumeAnswer) answer; @@ -276,6 +351,7 @@ private void updateVolumeAfterMigration(Answer answer, DataObject srcData, DataO s_logger.debug(String.format(format, ans.getVolumePath(), destData.getId())); } // OfflineVmwareMigration: update the volume with new pool/volume path + destinationVO.setPoolId(destData.getDataStore().getId()); destinationVO.setPath(ans.getVolumePath()); volDao.update(destinationVO.getId(), destinationVO); } diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index f34d5779fa94..80501f90d7b4 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -2302,7 +2302,8 @@ public Volume migrateVolume(MigrateVolumeCmd cmd) { getStoragePoolTags(destPool), vol.getName(), vol.getUuid(), diskOffering.getTags())); } - if (liveMigrateVolume && destPool.getClusterId() != null && srcClusterId != null) { + if (liveMigrateVolume && State.Running.equals(vm.getState()) && + destPool.getClusterId() != null && srcClusterId != null) { if (!srcClusterId.equals(destPool.getClusterId())) { throw new InvalidParameterValueException("Cannot migrate a volume of a virtual machine to a storage pool in a different cluster"); } From 5d685a957680fe5af0fa7a0b70321fd3351d9eac Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 12 Apr 2021 13:08:12 +0530 Subject: [PATCH 11/13] refactor vm cluster, host method Signed-off-by: Abhishek Kumar --- .../com/cloud/vm/VirtualMachineManager.java | 7 ++ .../cloud/vm/VirtualMachineManagerImpl.java | 88 +++++++++++-------- .../com/cloud/hypervisor/guru/VMwareGuru.java | 47 +--------- .../motion/VmwareStorageMotionStrategy.java | 47 ++-------- .../VmwareStorageMotionStrategyTest.java | 45 ++++++---- 5 files changed, 93 insertions(+), 141 deletions(-) diff --git a/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java b/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java index 49b13e1e698b..247e9e2c9b34 100644 --- a/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java +++ b/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java @@ -44,6 +44,7 @@ import com.cloud.template.VirtualMachineTemplate; import com.cloud.user.Account; import com.cloud.uservm.UserVm; +import com.cloud.utils.Pair; import com.cloud.utils.component.Manager; import com.cloud.utils.fsm.NoTransitionException; @@ -255,4 +256,10 @@ static String getHypervisorHostname(String name) { boolean unmanage(String vmUuid); UserVm restoreVirtualMachine(long vmId, Long newTemplateId) throws ResourceUnavailableException, InsufficientCapacityException; + + Pair findClusterAndHostIdForVmFromVolumes(long vmId); + + Pair findClusterAndHostIdForVm(VirtualMachine vm); + + Pair findClusterAndHostIdForVm(long vmId); } diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index 6faeeb5f4666..14e9764297d5 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -2294,43 +2294,6 @@ private void markVolumesInPool(VMInstanceVO vm, StoragePool destPool, Answer[] h } } - private Pair findClusterAndHostIdForVm(VMInstanceVO vm) { - Long hostId = vm.getHostId(); - Long clusterId = null; - // OfflineVmwareMigration: probably this is null when vm is stopped - if(hostId == null) { - hostId = vm.getLastHostId(); - if (s_logger.isDebugEnabled()) { - s_logger.debug(String.format("host id is null, using last host id %d", hostId) ); - } - } - if (hostId == null) { - List volumes = _volsDao.findByInstanceAndType(vm.getId(), Type.ROOT); - if (CollectionUtils.isNotEmpty(volumes)) { - for (VolumeVO rootVolume : volumes) { - if (rootVolume.getPoolId() != null) { - StoragePoolVO pool = _storagePoolDao.findById(rootVolume.getPoolId()); - if (pool != null && pool.getClusterId() != null) { - clusterId = pool.getClusterId(); - List hosts = _hostDao.findHypervisorHostInCluster(pool.getClusterId()); - if (CollectionUtils.isNotEmpty(hosts)) { - hostId = hosts.get(0).getId(); - break; - } - } - } - } - } - } - if (clusterId == null && hostId != null) { - HostVO host = _hostDao.findById(hostId); - if (host != null) { - clusterId = host.getClusterId(); - } - } - return new Pair<>(clusterId, hostId); - } - private void migrateThroughHypervisorOrStorage(StoragePool destPool, VMInstanceVO vm) throws StorageUnavailableException, InsufficientCapacityException { final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm); Pair vmClusterAndHost = findClusterAndHostIdForVm(vm); @@ -5913,4 +5876,55 @@ private Pair orchestrateUpdateDefaultNic(final VmWorkUpd _jobMgr.marshallResultObject(result)); } + @Override + public Pair findClusterAndHostIdForVmFromVolumes(long vmId) { + Long clusterId = null; + Long hostId = null; + List volumes = _volsDao.findByInstance(vmId); + for (VolumeVO volume : volumes) { + if (Volume.State.Ready.equals(volume.getState()) && + volume.getPoolId() != null) { + StoragePoolVO pool = _storagePoolDao.findById(volume.getPoolId()); + if (pool != null && pool.getClusterId() != null) { + clusterId = pool.getClusterId(); + // hostId to be used only for sending commands, capacity check skipped + List hosts = _hostDao.findHypervisorHostInCluster(pool.getClusterId()); + if (CollectionUtils.isNotEmpty(hosts)) { + hostId = hosts.get(0).getId(); + break; + } + } + } + } + return new Pair<>(clusterId, hostId); + } + + @Override + public Pair findClusterAndHostIdForVm(VirtualMachine vm) { + Long hostId = vm.getHostId(); + Long clusterId = null; + if(hostId == null) { + hostId = vm.getLastHostId(); + if (s_logger.isDebugEnabled()) { + s_logger.debug(String.format("host id is null, using last host id %d", hostId) ); + } + } + if (hostId == null) { + return findClusterAndHostIdForVmFromVolumes(vm.getId()); + } + HostVO host = _hostDao.findById(hostId); + if (host != null) { + clusterId = host.getClusterId(); + } + return new Pair<>(clusterId, hostId); + } + + @Override + public Pair findClusterAndHostIdForVm(long vmId) { + VMInstanceVO vm = _vmDao.findById(vmId); + if (vm == null) { + return new Pair<>(null, null); + } + return findClusterAndHostIdForVm(vm); + } } diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java index edb3b88c18fd..48347d4b41ae 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java @@ -206,48 +206,7 @@ protected VMwareGuru() { @Override public VirtualMachineTO implement(VirtualMachineProfile vm) { vmwareVmImplementer.setGlobalNestedVirtualisationEnabled(VmwareEnableNestedVirtualization.value()); vmwareVmImplementer.setGlobalNestedVPerVMEnabled(VmwareEnableNestedVirtualizationPerVM.value()); - return vmwareVmImplementer.implement(vm, toVirtualMachineTO(vm), getClusterId(vm.getId())); - } - - private Long getClusterIdFromVmVolume(long vmId) { - Long clusterId = null; - List volumes = _volumeDao.findByInstanceAndType(vmId, Volume.Type.ROOT); - if (CollectionUtils.isNotEmpty(volumes)) { - for (VolumeVO rootVolume : volumes) { - if (rootVolume.getPoolId() != null) { - StoragePoolVO pool = _storagePoolDao.findById(rootVolume.getPoolId()); - if (pool != null && pool.getClusterId() != null) { - clusterId = pool.getClusterId(); - break; - } - } - } - } - return clusterId; - } - - private Long getClusterId(long vmId) { - Long clusterId = null; - Long hostId = null; - VMInstanceVO vm = _vmDao.findById(vmId); - if (vm != null) { - hostId = _vmDao.findById(vmId).getHostId(); - } - if (vm != null && hostId == null) { - // If VM is in stopped state then hostId would be undefined. Hence read last host's Id instead. - hostId = _vmDao.findById(vmId).getLastHostId(); - } - HostVO host = null; - if (hostId != null) { - host = _hostDao.findById(hostId); - } - if (host != null) { - clusterId = host.getClusterId(); - } else { - clusterId = getClusterIdFromVmVolume(vmId); - } - - return clusterId; + return vmwareVmImplementer.implement(vm, toVirtualMachineTO(vm), vmManager.findClusterAndHostIdForVm(vm.getId()).first()); } @Override @DB public Pair getCommandHostDelegation(long hostId, Command cmd) { @@ -445,7 +404,7 @@ private static String resolveNameInGuid(String guid) { @Override public Map getClusterSettings(long vmId) { Map details = new HashMap(); - Long clusterId = getClusterId(vmId); + Long clusterId = vmManager.findClusterAndHostIdForVm(vmId).first(); if (clusterId != null) { details.put(VmwareReserveCpu.key(), VmwareReserveCpu.valueIn(clusterId).toString()); details.put(VmwareReserveMemory.key(), VmwareReserveMemory.valueIn(clusterId).toString()); @@ -1120,7 +1079,7 @@ private String getHostGuidInTargetCluster(boolean isInterClusterMigration, Long } final Long destClusterId = destination.getClusterId(); - final Long srcClusterId = getClusterId(vm.getId()); + final Long srcClusterId = vmManager.findClusterAndHostIdForVm(vm.getId()).first(); final boolean isInterClusterMigration = isInterClusterMigration(destClusterId, srcClusterId); MigrateVmToPoolCommand migrateVmToPoolCommand = new MigrateVmToPoolCommand(vm.getInstanceName(), vols, destination.getUuid(), getHostGuidInTargetCluster(isInterClusterMigration, destClusterId), true); diff --git a/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java b/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java index 9604dade8e4a..57fd64d446d7 100644 --- a/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java +++ b/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java @@ -31,7 +31,6 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority; -import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.framework.async.AsyncCompletionCallback; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; @@ -68,6 +67,7 @@ import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.VMInstanceVO; import com.cloud.vm.VirtualMachine; +import com.cloud.vm.VirtualMachineManager; import com.cloud.vm.dao.VMInstanceDao; @Component @@ -78,13 +78,13 @@ public class VmwareStorageMotionStrategy implements DataMotionStrategy { @Inject VolumeDao volDao; @Inject - VolumeDataFactory volFactory; - @Inject PrimaryDataStoreDao storagePoolDao; @Inject VMInstanceDao instanceDao; @Inject - private HostDao hostDao; + HostDao hostDao; + @Inject + VirtualMachineManager vmManager; @Override public StrategyPriority canHandle(DataObject srcData, DataObject destData) { @@ -144,43 +144,6 @@ private boolean isOnVmware(DataObject srcData, DataObject destData) { && HypervisorType.VMware.equals(destData.getTO().getHypervisorType()); } - private Pair findClusterAndHostIdForVm(VirtualMachine vm) { - Long hostId = vm.getHostId(); - Long clusterId = null; - // OfflineVmwareMigration: probably this is null when vm is stopped - if(hostId == null) { - hostId = vm.getLastHostId(); - if (s_logger.isDebugEnabled()) { - s_logger.debug(String.format("host id is null, using last host id %d", hostId) ); - } - } - if (hostId == null) { - List volumes = volDao.findByInstanceAndType(vm.getId(), Volume.Type.ROOT); - if (CollectionUtils.isNotEmpty(volumes)) { - for (VolumeVO rootVolume : volumes) { - if (rootVolume.getPoolId() != null) { - StoragePoolVO pool = storagePoolDao.findById(rootVolume.getPoolId()); - if (pool != null && pool.getClusterId() != null) { - clusterId = pool.getClusterId(); - List hosts = hostDao.findHypervisorHostInCluster(pool.getClusterId()); - if (CollectionUtils.isNotEmpty(hosts)) { - hostId = hosts.get(0).getId(); - break; - } - } - } - } - } - } - if (clusterId == null && hostId != null) { - HostVO host = hostDao.findById(hostId); - if (host != null) { - clusterId = host.getClusterId(); - } - } - return new Pair<>(clusterId, hostId); - } - private String getHostGuidInTargetCluster (Long sourceClusterId, StoragePool targetPool, ScopeType targetScopeType) { @@ -210,7 +173,7 @@ private VirtualMachine getVolumeVm(DataObject srcData) { private Pair getHostIdForVmAndHostGuidInTargetClusterForAttachedVm(VirtualMachine vm, StoragePool targetPool, ScopeType targetScopeType) { - Pair clusterAndHostId = findClusterAndHostIdForVm(vm); + Pair clusterAndHostId = vmManager.findClusterAndHostIdForVm(vm); if (clusterAndHostId.second() == null) { throw new CloudRuntimeException(String.format("Offline Migration failed, unable to find host for VM: %s", vm.getUuid())); } diff --git a/plugins/hypervisors/vmware/src/test/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategyTest.java b/plugins/hypervisors/vmware/src/test/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategyTest.java index 4cc3a77baaaf..e59c7978fdc8 100644 --- a/plugins/hypervisors/vmware/src/test/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategyTest.java +++ b/plugins/hypervisors/vmware/src/test/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategyTest.java @@ -16,6 +16,13 @@ // under the License. package org.apache.cloudstack.storage.motion; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.anyLong; +import static org.mockito.Matchers.isA; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + import java.io.IOException; import java.util.HashMap; import java.util.Map; @@ -23,17 +30,6 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; -import com.cloud.agent.AgentManager; -import com.cloud.agent.api.MigrateWithStorageAnswer; -import com.cloud.agent.api.MigrateWithStorageCommand; -import com.cloud.agent.api.to.VirtualMachineTO; -import com.cloud.host.Host; -import com.cloud.host.dao.HostDao; -import com.cloud.hypervisor.Hypervisor.HypervisorType; -import com.cloud.storage.dao.VolumeDao; -import com.cloud.utils.component.ComponentContext; -import com.cloud.vm.VMInstanceVO; -import com.cloud.vm.dao.VMInstanceDao; import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority; @@ -63,12 +59,18 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.test.context.support.AnnotationConfigContextLoader; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.mockito.Matchers.anyLong; -import static org.mockito.Matchers.isA; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import com.cloud.agent.AgentManager; +import com.cloud.agent.api.MigrateWithStorageAnswer; +import com.cloud.agent.api.MigrateWithStorageCommand; +import com.cloud.agent.api.to.VirtualMachineTO; +import com.cloud.host.Host; +import com.cloud.host.dao.HostDao; +import com.cloud.hypervisor.Hypervisor.HypervisorType; +import com.cloud.storage.dao.VolumeDao; +import com.cloud.utils.component.ComponentContext; +import com.cloud.vm.VMInstanceVO; +import com.cloud.vm.VirtualMachineManager; +import com.cloud.vm.dao.VMInstanceDao; @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration(loader = AnnotationConfigContextLoader.class) @@ -87,7 +89,9 @@ public class VmwareStorageMotionStrategyTest { @Inject VMInstanceDao instanceDao; @Inject - private HostDao hostDao; + HostDao hostDao; + @Inject + VirtualMachineManager vmManager; CopyCommandResult result; @@ -268,6 +272,11 @@ public HostDao hostDao() { return Mockito.mock(HostDao.class); } + @Bean + public VirtualMachineManager vmManager() { + return Mockito.mock(VirtualMachineManager.class); + } + public static class Library implements TypeFilter { @Override public boolean match(MetadataReader mdr, MetadataReaderFactory arg1) throws IOException { From 1fdc8258e0d65fdd143f1de994cd45a7411f69ec Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 13 Apr 2021 23:57:27 +0530 Subject: [PATCH 12/13] remove inter-pod check Added by mistake, VMware won't have pods Signed-off-by: Abhishek Kumar --- .../motion/VmwareStorageMotionStrategy.java | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java b/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java index 57fd64d446d7..ab919c7cdb97 100644 --- a/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java +++ b/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java @@ -34,7 +34,6 @@ import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.framework.async.AsyncCompletionCallback; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; -import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.to.VolumeObjectTO; import org.apache.commons.collections.CollectionUtils; import org.apache.log4j.Logger; @@ -92,8 +91,7 @@ public StrategyPriority canHandle(DataObject srcData, DataObject destData) { if (isOnVmware(srcData, destData) && isOnPrimary(srcData, destData) && isVolumesOnly(srcData, destData) - && isDetachedOrAttachedToStoppedVM(srcData) - && isIntraPodOrZoneWideStoreInvolved(srcData, destData)) { + && isDetachedOrAttachedToStoppedVM(srcData)) { if (s_logger.isDebugEnabled()) { String msg = String.format("%s can handle the request because %d(%s) and %d(%s) share the pod" , this.getClass() @@ -118,17 +116,6 @@ private boolean isDetachedOrAttachedToStoppedVM(DataObject srcData) { return volume.getInstanceId() == null || isAttachedToStoppedVM(volume); } - private boolean isIntraPodOrZoneWideStoreInvolved(DataObject srcData, DataObject destData) { - DataStore srcStore = srcData.getDataStore(); - StoragePoolVO srcPool = storagePoolDao.findById(srcStore.getId()); - DataStore destStore = destData.getDataStore(); - StoragePoolVO destPool = storagePoolDao.findById(destStore.getId()); - if (srcPool.getPodId() != null && destPool.getPodId() != null) { - return srcPool.getPodId().equals(destPool.getPodId()); - } - return (ScopeType.ZONE.equals(srcPool.getScope()) || ScopeType.ZONE.equals(destPool.getScope())); - } - private boolean isVolumesOnly(DataObject srcData, DataObject destData) { return DataObjectType.VOLUME.equals(srcData.getType()) && DataObjectType.VOLUME.equals(destData.getType()); From 5e89cb042507380e76163d27c4b9ebab58aefbff Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 15 Apr 2021 12:16:34 +0530 Subject: [PATCH 13/13] address review comment Signed-off-by: Abhishek Kumar --- .../src/main/java/com/cloud/vm/VirtualMachineManager.java | 4 ---- .../main/java/com/cloud/vm/VirtualMachineManagerImpl.java | 6 ++---- .../storage/motion/VmwareStorageMotionStrategy.java | 2 +- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java b/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java index 247e9e2c9b34..40777b38b6d2 100644 --- a/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java +++ b/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java @@ -257,9 +257,5 @@ static String getHypervisorHostname(String name) { UserVm restoreVirtualMachine(long vmId, Long newTemplateId) throws ResourceUnavailableException, InsufficientCapacityException; - Pair findClusterAndHostIdForVmFromVolumes(long vmId); - - Pair findClusterAndHostIdForVm(VirtualMachine vm); - Pair findClusterAndHostIdForVm(long vmId); } diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index 14e9764297d5..765bb907b27c 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -5876,8 +5876,7 @@ private Pair orchestrateUpdateDefaultNic(final VmWorkUpd _jobMgr.marshallResultObject(result)); } - @Override - public Pair findClusterAndHostIdForVmFromVolumes(long vmId) { + private Pair findClusterAndHostIdForVmFromVolumes(long vmId) { Long clusterId = null; Long hostId = null; List volumes = _volsDao.findByInstance(vmId); @@ -5899,8 +5898,7 @@ public Pair findClusterAndHostIdForVmFromVolumes(long vmId) { return new Pair<>(clusterId, hostId); } - @Override - public Pair findClusterAndHostIdForVm(VirtualMachine vm) { + private Pair findClusterAndHostIdForVm(VirtualMachine vm) { Long hostId = vm.getHostId(); Long clusterId = null; if(hostId == null) { diff --git a/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java b/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java index ab919c7cdb97..04111bc3b6be 100644 --- a/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java +++ b/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java @@ -160,7 +160,7 @@ private VirtualMachine getVolumeVm(DataObject srcData) { private Pair getHostIdForVmAndHostGuidInTargetClusterForAttachedVm(VirtualMachine vm, StoragePool targetPool, ScopeType targetScopeType) { - Pair clusterAndHostId = vmManager.findClusterAndHostIdForVm(vm); + Pair clusterAndHostId = vmManager.findClusterAndHostIdForVm(vm.getId()); if (clusterAndHostId.second() == null) { throw new CloudRuntimeException(String.format("Offline Migration failed, unable to find host for VM: %s", vm.getUuid())); }