orchestration: fix post storage migration (#39)

Detaching volume after cross-cluster storage migration of a stopped was resulting in failure.
This PR fixes it by clearing last_host_id value and setting pod_id(podIdToDeployIn) in the database on cluster change when hypervisor migration strategy is used.

Also, backports worker VM hardware version changes during volume migration from community PR, apache#4385

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

Author: shwstppr

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

@@ -1807,7 +1807,7 @@ private Pair<Long, Long> findClusterAndHostIdForVm(VMInstanceVO vm) {
         if (clusterId == null && hostId != null) {
             HostVO host = _hostDao.findById(hostId);
             if (host != null) {
-                clusterId = host.getId();
+                clusterId = host.getClusterId();
             }
         }
         return new Pair<>(clusterId, hostId);
@@ -1827,7 +1827,7 @@ private void migrateThroughHypervisorOrStorage(VMInstanceVO vm, Map<Volume, Stor
                 s_logger.debug("Storage migration failed");
             }
         } else {
-            postHypervisorMigrationCleanup(vm, volumeToPool, hypervisorMigrationResults);
+            postHypervisorMigrationCleanup(vm, volumeToPool, sourceClusterId, hypervisorMigrationResults);
         }
     }
 
@@ -1931,7 +1931,7 @@ private Answer[] attemptHypervisorMigration(VMInstanceVO vm, Map<Volume, Storage
         return null;
     }
 
-    private void postHypervisorMigrationCleanup(VMInstanceVO vm, Map<Volume, StoragePool> volumeToPool, Answer[] hypervisorMigrationResults) throws InsufficientCapacityException {
+    private void postHypervisorMigrationCleanup(VMInstanceVO vm, Map<Volume, StoragePool> volumeToPool, Long sourceClusterId, Answer[] hypervisorMigrationResults) throws InsufficientCapacityException {
         if(s_logger.isDebugEnabled()) {
             String msg = String.format("Cleaning up after hypervisor pool migration volumes for VM %s(%s)", vm.getInstanceName(), vm.getUuid());
             s_logger.debug(msg);
@@ -1944,19 +1944,14 @@ private void postHypervisorMigrationCleanup(VMInstanceVO vm, Map<Volume, Storage
             }
         }
         setDestinationPoolAndReallocateNetwork(rootVolumePool, 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 = rootVolumePool != null ? rootVolumePool.getPodId() : null;
-        Long vmPodId = vm.getPodIdToDeployIn();
-        if (destPodId == null || ! destPodId.equals(vmPodId)) {
-            if(s_logger.isDebugEnabled()) {
-                String msg = String.format("Resetting lastHost for VM %s(%s) as pod (%s) is no good.", vm.getInstanceName(), vm.getUuid(), destPodId);
-                s_logger.debug(msg);
+        Long destClusterId = rootVolumePool != null ? rootVolumePool.getClusterId() : null;
+        if (destClusterId != null && !destClusterId.equals(sourceClusterId)) {
+            if (s_logger.isDebugEnabled()) {
+                s_logger.debug(String.format("Resetting lastHost for VM %s(%s)", vm.getInstanceName(), vm.getUuid()));
             }
-
             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
+            vm.setPodIdToDeployIn(rootVolumePool.getPodId());
+        }
         markVolumesInPool(vm, hypervisorMigrationResults);
         // OfflineVmwareMigration: deal with answers, if (hypervisorMigrationResults.length > 0)
         // OfflineVmwareMigration: iterate over the volumes for data updates

plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java

@@ -3585,9 +3585,17 @@ private Answer migrateVolume(MigrateVolumeCommand cmd) {
                 throw new CloudRuntimeException(msg);
             }
             targetDsMo = new DatastoreMO(dsHost.getContext(), morTargetDS);
+            String hardwareVersion = null;
+            if (hostInTargetCluster != null) {
+                Integer sourceHardwareVersion = HypervisorHostHelper.getHostHardwareVersion(hyperHost);
+                Integer destinationHardwareVersion = HypervisorHostHelper.getHostHardwareVersion(dsHost);
+                if (sourceHardwareVersion != null && destinationHardwareVersion != null && !sourceHardwareVersion.equals(destinationHardwareVersion)) {
+                    hardwareVersion = String.valueOf(Math.min(sourceHardwareVersion, destinationHardwareVersion));
+                }
+            }
             s_logger.info("Create worker VM " + vmName);
                 // OfflineVmwareMigration: 2. create the worker with access to the data(store)
-            vmMo = HypervisorHostHelper.createWorkerVM(hyperHost, sourceDsMo, vmName, null);
+            vmMo = HypervisorHostHelper.createWorkerVM(hyperHost, sourceDsMo, 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");

vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java

@@ -22,10 +22,10 @@
 import java.security.InvalidParameterException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-import com.cloud.utils.StringUtils;
 import org.apache.log4j.Logger;
 
 import com.cloud.exception.CloudException;
@@ -34,6 +34,7 @@
 import com.cloud.network.Networks.BroadcastDomainType;
 import com.cloud.utils.ActionDelegate;
 import com.cloud.utils.Pair;
+import com.cloud.utils.StringUtils;
 import com.cloud.utils.cisco.n1kv.vsm.NetconfHelper;
 import com.cloud.utils.cisco.n1kv.vsm.PolicyMap;
 import com.cloud.utils.cisco.n1kv.vsm.PortProfile;
@@ -49,12 +50,12 @@
 import com.vmware.vim25.BoolPolicy;
 import com.vmware.vim25.ClusterConfigInfoEx;
 import com.vmware.vim25.CustomFieldStringValue;
-import com.vmware.vim25.DatacenterConfigInfo;
 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.DynamicProperty;
 import com.vmware.vim25.HostNetworkSecurityPolicy;
 import com.vmware.vim25.HostNetworkTrafficShapingPolicy;
@@ -104,6 +105,48 @@ public class HypervisorHostHelper {
     // make vmware-base loosely coupled with cloud-specific stuff, duplicate VLAN.UNTAGGED constant here
     private static final String UNTAGGED_VLAN_NAME = "untagged";
 
+    protected final static Map<String, Integer> apiVersionHardwareVersionMap;
+
+    static {
+        apiVersionHardwareVersionMap = new HashMap<String, Integer>();
+        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) {
@@ -1661,4 +1704,19 @@ public static boolean isIdeController(String controller) {
         return DiskControllerType.getType(controller) == DiskControllerType.ide;
     }
 
+    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;
+    }
+
 }