From a30d518e8a62cddeafeff0402bdf36c0cf615b8f Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Sat, 24 Apr 2021 18:55:25 +0530 Subject: [PATCH] vmware: fix stopped VM volume migration (#4758) * prevent other vm disks getting deleted Signed-off-by: Abhishek Kumar * 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 * fix detached volume inter-cluster migration Signed-off-by: Abhishek Kumar * cleanup unused method Signed-off-by: Abhishek Kumar * review changes Signed-off-by: Abhishek Kumar * changes Signed-off-by: Abhishek Kumar * vmware: allow attached volume migration using VmwareStorageMotionStrategy Signed-off-by: Abhishek Kumar * 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 * fix successive storage migration Signed-off-by: Abhishek Kumar * fix intercluster check Signed-off-by: Abhishek Kumar * refactor vm cluster, host method Signed-off-by: Abhishek Kumar * remove inter-pod check Added by mistake, VMware won't have pods Signed-off-by: Abhishek Kumar * address review comment Signed-off-by: Abhishek Kumar --- .../api/storage/MigrateVolumeCommand.java | 4 +- .../com/cloud/vm/VirtualMachineManager.java | 3 + .../cloud/vm/VirtualMachineManagerImpl.java | 86 +++++++------ .../com/cloud/hypervisor/guru/VMwareGuru.java | 47 +------ .../resource/VmwareStorageProcessor.java | 19 ++- .../motion/VmwareStorageMotionStrategy.java | 119 +++++++++++------- .../VmwareStorageMotionStrategyTest.java | 45 ++++--- .../cloud/storage/VolumeApiServiceImpl.java | 20 ++- 8 files changed, 184 insertions(+), 159 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 5443ad12ead..0550e8d010e 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 class MigrateVolumeCommand extends Command { 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/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java b/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java index 49b13e1e698..40777b38b6d 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.storage.StoragePool; 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,6 @@ public interface VirtualMachineManager extends Manager { boolean unmanage(String vmUuid); UserVm restoreVirtualMachine(long vmId, Long newTemplateId) throws ResourceUnavailableException, InsufficientCapacityException; + + 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 6faeeb5f466..765bb907b27 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 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } } - 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,53 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac _jobMgr.marshallResultObject(result)); } + private 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); + } + + private 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 edb3b88c18f..48347d4b41a 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 @@ public class VMwareGuru extends HypervisorGuruBase implements HypervisorGuru, Co @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 @@ public class VMwareGuru extends HypervisorGuruBase implements HypervisorGuru, Co @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 @@ public class VMwareGuru extends HypervisorGuruBase implements HypervisorGuru, Co } 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/com/cloud/storage/resource/VmwareStorageProcessor.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java index 1584bd003dc..a2aee6b53e1 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.ExecutorService; 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.HostMO; 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.VirtualMachine.PowerState; 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.HostUnresolvedVmfsResignatureSpec; 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 class VmwareStorageProcessor implements StorageProcessor { 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(); } 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 2854a7c3abe..04111bc3b6b 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,11 +31,9 @@ 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.commons.collections.CollectionUtils; import org.apache.log4j.Logger; @@ -67,6 +65,8 @@ import com.cloud.storage.dao.VolumeDao; 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.VirtualMachineManager; import com.cloud.vm.dao.VMInstanceDao; @Component @@ -77,13 +77,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) { @@ -91,8 +91,7 @@ public class VmwareStorageMotionStrategy implements DataMotionStrategy { if (isOnVmware(srcData, destData) && isOnPrimary(srcData, destData) && isVolumesOnly(srcData, destData) - && isIntraPodOrZoneWideStoreInvolved(srcData, destData) - && isDettached(srcData)) { + && 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() @@ -107,20 +106,14 @@ public class VmwareStorageMotionStrategy implements DataMotionStrategy { 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 isAttachedToStoppedVM(Volume volume) { + VMInstanceVO vm = instanceDao.findById(volume.getInstanceId()); + return vm != null && VirtualMachine.State.Stopped.equals(vm.getState()); } - private boolean isDettached(DataObject srcData) { + 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) { @@ -138,11 +131,46 @@ public class VmwareStorageMotionStrategy implements DataMotionStrategy { && 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 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 = 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())); + } + 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)) { @@ -151,17 +179,7 @@ public class VmwareStorageMotionStrategy implements DataMotionStrategy { 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) { @@ -171,6 +189,19 @@ public class VmwareStorageMotionStrategy implements DataMotionStrategy { 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) { @@ -205,17 +236,18 @@ public class VmwareStorageMotionStrategy implements DataMotionStrategy { // 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); @@ -255,9 +287,11 @@ public class VmwareStorageMotionStrategy implements DataMotionStrategy { 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; @@ -266,6 +300,7 @@ public class VmwareStorageMotionStrategy implements DataMotionStrategy { 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/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 4cc3a77baaa..e59c7978fdc 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 java.util.Map; 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.ContextConfiguration; 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 class VmwareStorageMotionStrategyTest { 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 { diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 05ce5435fcc..74b65b8ccdc 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 java.util.concurrent.ExecutionException; 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.configuration.Resource.ResourceType; 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; @@ -2227,13 +2227,13 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic } // 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"); @@ -2263,10 +2263,17 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic 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); @@ -2299,7 +2306,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic 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"); }