vmware: fix stopped VM volume migration (#4758)

* prevent other vm disks getting deleted

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

* 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 <abhishek.mrt22@gmail.com>

* fix detached volume inter-cluster migration

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

* cleanup unused method

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

* review changes

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

* changes

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

* vmware: allow attached volume migration using VmwareStorageMotionStrategy

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

* 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 <abhishek.mrt22@gmail.com>

* fix successive storage migration

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

* fix intercluster check

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

* refactor vm cluster, host method

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

* remove inter-pod check

Added by mistake, VMware won't have pods

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

* address review comment

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

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) {
+    public MigrateVolumeCommand(long volumeId, String volumePath, String attachedVmName, StoragePool sourcePool, StoragePool targetPool, String hostGuidInTargetCluster) {
-        this(volumeId,volumePath,targetPool, null, Volume.Type.UNKNOWN, -1);
+        this(volumeId,volumePath,targetPool, attachedVmName, Volume.Type.UNKNOWN, -1);
         this.sourcePool = new StorageFilerTO(sourcePool);
         this.hostGuidInTargetCluster = hostGuidInTargetCluster;
     }

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<Long, Long> findClusterAndHostIdForVm(long vmId);
 }

engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

@@ -2294,43 +2294,6 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
         }
     }
 
-    private Pair<Long, Long> 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<VolumeVO> 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<HostVO> 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<Long, Long> vmClusterAndHost = findClusterAndHostIdForVm(vm);
@@ -5913,4 +5876,53 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
                 _jobMgr.marshallResultObject(result));
     }
 
+    private Pair<Long, Long> findClusterAndHostIdForVmFromVolumes(long vmId) {
+        Long clusterId = null;
+        Long hostId = null;
+        List<VolumeVO> 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<HostVO> hosts = _hostDao.findHypervisorHostInCluster(pool.getClusterId());
+                    if (CollectionUtils.isNotEmpty(hosts)) {
+                        hostId = hosts.get(0).getId();
+                        break;
+                    }
+                }
+            }
+        }
+        return new Pair<>(clusterId, hostId);
+    }
+
+    private Pair<Long, Long> 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<Long, Long> findClusterAndHostIdForVm(long vmId) {
+        VMInstanceVO vm = _vmDao.findById(vmId);
+        if (vm == null) {
+            return new Pair<>(null, null);
+        }
+        return findClusterAndHostIdForVm(vm);
+    }
 }

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()));
+        return vmwareVmImplementer.implement(vm, toVirtualMachineTO(vm), vmManager.findClusterAndHostIdForVm(vm.getId()).first());
-    }
-
-    private Long getClusterIdFromVmVolume(long vmId) {
-        Long clusterId = null;
-        List<VolumeVO> 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;
     }
 
     @Override @DB public Pair<Boolean, Long> getCommandHostDelegation(long hostId, Command cmd) {
@@ -445,7 +404,7 @@ public class VMwareGuru extends HypervisorGuruBase implements HypervisorGuru, Co
 
     @Override public Map<String, String> getClusterSettings(long vmId) {
         Map<String, String> details = new HashMap<String, String>();
-        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);

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<VirtualDisk> virtualDisks = vmMo.getVirtualDisks();
                         List<String> managedDatastoreNames = getManagedDatastoreNamesFromVirtualDisks(virtualDisks);
 
+                        // Preserve other disks of the VM
+                        List<String> 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<String> detachedDisks = vmMo.detachAllDisksExcept(vol.getPath(), diskInfo != null ? diskInfo.getDiskDeviceBusName() : null);
-                            VmwareStorageLayoutHelper.moveVolumeToRootFolder(new DatacenterMO(context, morDc), detachedDisks);
                             vmMo.unregisterVm();
-                        }
+                        } else {
-                        else {
                             vmMo.destroy();
                         }
 

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)
+                && isDetachedOrAttachedToStoppedVM(srcData)) {
-                && isDettached(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) {
+    private boolean isAttachedToStoppedVM(Volume volume) {
-        DataStore srcStore = srcData.getDataStore();
+        VMInstanceVO vm = instanceDao.findById(volume.getInstanceId());
-        StoragePoolVO srcPool = storagePoolDao.findById(srcStore.getId());
+        return vm != null && VirtualMachine.State.Stopped.equals(vm.getState());
-        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) {
+    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<Long, String> getHostIdForVmAndHostGuidInTargetCluster(DataObject srcData, DataObject destData) {
+    private String getHostGuidInTargetCluster (Long sourceClusterId,
-        StoragePool sourcePool = (StoragePool) srcData.getDataStore();
+                                               StoragePool targetPool,
-        ScopeType sourceScopeType = srcData.getDataStore().getScope().getScopeType();
+                                               ScopeType targetScopeType) {
-        StoragePool targetPool = (StoragePool) destData.getDataStore();
+        String hostGuidInTargetCluster = null;
-        ScopeType targetScopeType = destData.getDataStore().getScope().getScopeType();
+        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<HostVO> 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<Long, String> getHostIdForVmAndHostGuidInTargetClusterForAttachedVm(VirtualMachine vm,
+                                                                                     StoragePool targetPool,
+                                                                                     ScopeType targetScopeType) {
+        Pair<Long, Long> 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<Long, String> 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())) {
+            hostGuidInTargetCluster = getHostGuidInTargetCluster(sourcePool.getClusterId(), targetPool, sourceScopeType);
-                // Without host vMotion might fail between non-shared storages with error similar to,
-                // https://kb.vmware.com/s/article/1003795
-                List<HostVO> 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) {
@@ -171,6 +189,19 @@ public class VmwareStorageMotionStrategy implements DataMotionStrategy {
         return new Pair<>(hostId, hostGuidInTargetCluster);
     }
 
+    private Pair<Long, String> 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<VolumeInfo, DataStore> 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<Long, String> hostIdForVmAndHostGuidInTargetCluster = getHostIdForVmAndHostGuidInTargetCluster(srcData, destData);
+        VirtualMachine vm = getVolumeVm(srcData);
-        Long hostId = hostIdForVmAndHostGuidInTargetCluster.first();
-        String hostGuidInTargetCluster = hostIdForVmAndHostGuidInTargetCluster.second();
         StoragePool sourcePool = (StoragePool) srcData.getDataStore();
         StoragePool targetPool = (StoragePool) destData.getDataStore();
+        Pair<Long, String> 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);
+                , hostIdForVmAndHostGuidInTargetCluster.second());
-        // OfflineVmwareMigration: should be ((StoragePool)srcData.getDataStore()).getHypervisor() but that is NULL, so hardcoding
         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);
+            if (destinationVO.getId() != sourceVO.getId()) {
-            destinationVO.setRemoved(new Date());
+                destinationVO.setState(Volume.State.Expunged);
-            volDao.update(destinationVO.getId(), destinationVO);
+                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);
     }

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 com.cloud.agent.AgentManager;
-import static org.junit.Assert.assertTrue;
+import com.cloud.agent.api.MigrateWithStorageAnswer;
-import static org.mockito.Matchers.anyLong;
+import com.cloud.agent.api.MigrateWithStorageCommand;
-import static org.mockito.Matchers.isA;
+import com.cloud.agent.api.to.VirtualMachineTO;
-import static org.mockito.Mockito.mock;
+import com.cloud.host.Host;
-import static org.mockito.Mockito.when;
+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 {

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()) {
+        if (vm != null &&
-            throw new InvalidParameterValueException("The volume " + vol + "is attached to a vm and for migrating it " + "the parameter livemigrate should be specified");
+                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");
             }