nvazquez commented on a change in pull request #4895:
URL: https://github.com/apache/cloudstack/pull/4895#discussion_r609187099



##########
File path: 
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java
##########
@@ -209,16 +209,41 @@ protected VMwareGuru() {
         return vmwareVmImplementer.implement(vm, toVirtualMachineTO(vm), 
getClusterId(vm.getId()));
     }
 
-    long getClusterId(long vmId) {
-        long clusterId;
-        Long hostId;
+    private Long getClusterIdFromVmVolume(long vmId) {
+        Long clusterId = null;
+        List<VolumeVO> 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();

Review comment:
       Maybe add a `break` here? No need to continue iterating

##########
File path: 
vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java
##########
@@ -2211,4 +2253,36 @@ 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;
+    }
+
+    /*
+      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));

Review comment:
       What about `host1.compareTo(host2)` ? That would give us -1, 0 or 1 and 
won't need the mapping above

##########
File path: 
vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java
##########
@@ -153,6 +153,48 @@
     public static final String VSPHERE_DATASTORE_BASE_FOLDER = "fcd";
     public static final String VSPHERE_DATASTORE_HIDDEN_FOLDER = ".hidden";
 
+    protected final static Map<String, Integer> apiVersionHardwareVersionMap;
+
+    static {
+        apiVersionHardwareVersionMap = new HashMap<String, Integer>();

Review comment:
       Is this mapping used only for comparing hosts versions? Maybe we can 
remove it and do a string comparission?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to