vmware: improve solidfire storage plugin integration and fix cases

engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java

@@ -1906,8 +1906,8 @@ public void prepare(VirtualMachineProfile vm, DeployDestination dest) throws Sto
                             throw new StorageAccessException(String.format("Unable to grant access to volume [%s] on host [%s].", volToString, host));
                         }
                     } else {
-                        // This might impact other managed storages, grant access for PowerFlex storage pool only
-                        if (pool.getPoolType() == Storage.StoragePoolType.PowerFlex) {
+                        // This might impact other managed storages, grant access for PowerFlex and Iscsi/Solidfire storage pool only
+                        if (pool.getPoolType() == Storage.StoragePoolType.PowerFlex || pool.getPoolType() == Storage.StoragePoolType.Iscsi) {
                             try {
                                 volService.grantAccess(volFactory.getVolume(vol.getId()), host, (DataStore)pool);
                             } catch (Exception e) {

plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java

@@ -1107,12 +1107,25 @@ private static String getSnapshotRelativeDirInSecStorage(long accountId, long vo
         return "snapshots/" + accountId + "/" + volumeId;
     }
 
+    protected boolean isManagedStorageDatastorePath(final String datastorePath) {
+        // ex. [-iqn.2010-01.com.solidfire:3p53.data-9999.97-0] i-2-9999-VM
+        return datastorePath != null && datastorePath.startsWith("[-iqn.");
+    }
+
+    protected String getManagedDatastoreName(final String datastorePath) {
+        // ex. [-iqn.2010-01.com.solidfire:3p53.data-9999.97-0]
+        return datastorePath == null ? datastorePath : datastorePath.split(" ")[0];
+    }
+
     private long getVMSnapshotChainSize(VmwareContext context, VmwareHypervisorHost hyperHost, String fileName, ManagedObjectReference morDs,
                                         String exceptFileName, String vmName) throws Exception {
         long size = 0;
         DatastoreMO dsMo = new DatastoreMO(context, morDs);
         HostDatastoreBrowserMO browserMo = dsMo.getHostDatastoreBrowserMO();
         String datastorePath = (new DatastoreFile(dsMo.getName(), vmName)).getPath();
+        if (isManagedStorageDatastorePath(datastorePath)) {
+            datastorePath = getManagedDatastoreName(datastorePath);
+        }
         HostDatastoreBrowserSearchSpec searchSpec = new HostDatastoreBrowserSearchSpec();
         FileQueryFlags fqf = new FileQueryFlags();
         fqf.setFileSize(true);
@@ -1241,11 +1254,9 @@ private Map<String, String> getNewDiskMap(VirtualMachineMO vmMo) throws Exceptio
                 String vmdkName = null;
 
                 // if this is managed storage
-                if (fullPath.startsWith("[-iqn.")) { // ex. [-iqn.2010-01.com.company:3y8w.vol-10.64-0] -iqn.2010-01.com.company:3y8w.vol-10.64-0-000001.vmdk
-                    baseName = fullPath.split(" ")[0]; // ex. [-iqn.2010-01.com.company:3y8w.vol-10.64-0]
-
-                    // remove '[' and ']'
-                    baseName = baseName.substring(1, baseName.length() - 1);
+                if (isManagedStorageDatastorePath(fullPath)) {
+                    baseName = getManagedDatastoreName(fullPath);
+                    baseName = baseName.substring(1, baseName.length() - 1); // remove '[' and ']'
 
                     vmdkName = fullPath; // for managed storage, vmdkName == fullPath
                 } else {
@@ -1288,12 +1299,9 @@ protected void setVolumeToPathAndSize(List<VolumeObjectTO> volumeTOs, VirtualMac
                 }
             } else {
                 Map<String, String> mapNewDisk = getNewDiskMap(vmMo);
-                // if this is managed storage
-                if (path.startsWith("[-iqn.")) { // ex. [-iqn.2010-01.com.company:3y8w.vol-10.64-0] -iqn.2010-01.com.company:3y8w.vol-10.64-0-000001.vmdk
-                    path = path.split(" ")[0]; // ex. [-iqn.2010-01.com.company:3y8w.vol-10.64-0]
-
-                    // remove '[' and ']'
-                    baseName = path.substring(1, path.length() - 1);
+                if (isManagedStorageDatastorePath(path)) {
+                    path = getManagedDatastoreName(path);
+                    baseName = path.substring(1, path.length() - 1); // remove '[' and ']'
                 } else {
                     baseName = VmwareHelper.trimSnapshotDeltaPostfix(path);
                 }

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

@@ -940,6 +940,11 @@ private Answer execute(ResizeVolumeCommand cmd) {
                 ManagedObjectReference morDS = HypervisorHostHelper.findDatastoreWithBackwardsCompatibility(hyperHost, VmwareResource.getDatastoreName(iScsiName));
                 DatastoreMO dsMo = new DatastoreMO(hyperHost.getContext(), morDS);
 
+                if (path.startsWith("[-iqn.")) {
+                    // Rescan 1:1 LUN that VMware may not know the LUN was recently resized
+                    _storageProcessor.rescanAllHosts(context, lstHosts, true, true);
+                }
+
                 _storageProcessor.expandDatastore(hostDatastoreSystem, dsMo);
             }
 

plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java

@@ -2825,7 +2825,15 @@ private ManagedObjectReference getVmfsDatastore(VmwareContext context, VmwareHyp
 
         morDs = firstHostDatastoreSystemMO.findDatastoreByName(datastoreName);
         if (morDs == null) {
-            morDs = firstHostDatastoreSystemMO.createVmfsDatastore(datastoreName, hostScsiDisk);
+            final String hostVersion = firstHostMO.getProductVersion();
+            if (hostVersion.compareTo(VmwareHelper.MIN_VERSION_VMFS6) >= 0) {
+                morDs = firstHostDatastoreSystemMO.createVmfs6Datastore(datastoreName, hostScsiDisk);
+            } else {
+                morDs = firstHostDatastoreSystemMO.createVmfs5Datastore(datastoreName, hostScsiDisk);
+            }
+        } else {
+            // in case of iSCSI/solidfire 1:1 VMFS datastore could be inaccessible
+            mountVmfsDatastore(new DatastoreMO(context, morDs), lstHosts);
         }
 
         if (morDs != null) {
@@ -3364,7 +3372,7 @@ private void addRemoveInternetScsiTargetsToAllHosts(boolean add, List<HostIntern
         }
     }
 
-    private void rescanAllHosts(VmwareContext context, List<Pair<ManagedObjectReference, String>> lstHostPairs, boolean rescanHba, boolean rescanVmfs) throws Exception {
+    public void rescanAllHosts(VmwareContext context, List<Pair<ManagedObjectReference, String>> lstHostPairs, boolean rescanHba, boolean rescanVmfs) throws Exception {
         List<HostMO> hosts = new ArrayList<>(lstHostPairs.size());
 
         for (Pair<ManagedObjectReference, String> hostPair : lstHostPairs) {

plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImplTest.java

@@ -116,4 +116,15 @@ public void testSetVolumeToPathAndSizeDatastoreClusterSameChildStore() {
     public void testSetVolumeToPathAndSizeDatastoreClusterDifferentChildStore() {
         testCommon(Storage.StoragePoolType.PreSetup, Storage.StoragePoolType.DatastoreCluster, true);
     }
+
+    @Test
+    public void testIsManagedStorageDatastorePath() {
+        Assert.assertTrue("Test if [-iqn... is a managed storage", storageManager.isManagedStorageDatastorePath("[-iqn.2010-01.com.solidfire:3p53.data-9999.97-0] i-2-9999-VM.vmdk"));
+        Assert.assertFalse("Test if [SomeDS] is not a managed storage", storageManager.isManagedStorageDatastorePath("[SomeDS] i-2-9999-VM/disk.vmdk"));
+    }
+
+    @Test
+    public void testGetManagedDatastoreName() {
+        Assert.assertEquals("[-iqn.2010-01.com.solidfire:3p53.data-9999.97-0]", storageManager.getManagedDatastoreName("[-iqn.2010-01.com.solidfire:3p53.data-9999.97-0] i-2-9999-VM.vmdk"));
+    }
 }

plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/driver/SolidFirePrimaryDataStoreDriver.java

@@ -17,6 +17,7 @@
 package org.apache.cloudstack.storage.datastore.driver;
 
 import java.text.NumberFormat;
+import java.util.Arrays;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -85,6 +86,8 @@
 import com.cloud.utils.Pair;
 import com.cloud.utils.db.GlobalLock;
 import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.vm.dao.VMInstanceDao;
+import com.cloud.vm.VirtualMachine;
 import com.google.common.base.Preconditions;
 
 public class SolidFirePrimaryDataStoreDriver implements PrimaryDataStoreDriver {
@@ -111,6 +114,7 @@ public class SolidFirePrimaryDataStoreDriver implements PrimaryDataStoreDriver {
     @Inject private PrimaryDataStoreDao storagePoolDao;
     @Inject private StoragePoolDetailsDao storagePoolDetailsDao;
     @Inject private VMTemplatePoolDao vmTemplatePoolDao;
+    @Inject private VMInstanceDao vmDao;
     @Inject private VolumeDao volumeDao;
     @Inject private VolumeDetailsDao volumeDetailsDao;
     @Inject private VolumeDataFactory volumeFactory;
@@ -187,13 +191,33 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore
         }
     }
 
+    private boolean isRevokeAccessNotNeeded(DataObject dataObject) {
+        // Workaround: don't unplug iscsi lun when volume is attached to a VM
+        // This is regression workaround from upper layers which are calling
+        // a releaseVmResources() method that calls the revoke on an attached disk
+        if (dataObject.getType() == DataObjectType.VOLUME) {
+            Volume volume = volumeDao.findById(dataObject.getId());
+            if (volume.getInstanceId() != null) {
+                VirtualMachine vm = vmDao.findById(volume.getInstanceId());
+                if (vm != null && !Arrays.asList(VirtualMachine.State.Destroyed, VirtualMachine.State.Expunging, VirtualMachine.State.Error).contains(vm.getState())) {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+
     @Override
-    public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore)
-    {
+    public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) {
         if (dataObject == null || host == null || dataStore == null) {
             return;
         }
 
+        if (isRevokeAccessNotNeeded(dataObject)) {
+            LOGGER.debug("Skipping revoke access for Solidfire data object type:" + dataObject.getType() + " id:" + dataObject.getId());
+            return;
+        }
+
         long sfVolumeId = getSolidFireVolumeId(dataObject, false);
         long clusterId = host.getClusterId();
         long storagePoolId = dataStore.getId();
@@ -210,6 +234,8 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore)
             throw new CloudRuntimeException(errMsg);
         }
 
+        LOGGER.debug("Revoking access for Solidfire data object type:" + dataObject.getType() + " id:" + dataObject.getId());
+
         try {
             SolidFireUtil.SolidFireConnection sfConnection = SolidFireUtil.getSolidFireConnection(storagePoolId, storagePoolDetailsDao);
 

vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HostDatastoreSystemMO.java

@@ -199,18 +199,21 @@ public List<HostScsiDisk> queryAvailableDisksForVmfs() throws Exception {
         return _context.getService().queryAvailableDisksForVmfs(_mor, null);
     }
 
-    public ManagedObjectReference createVmfsDatastore(String datastoreName, HostScsiDisk hostScsiDisk) throws Exception {
-        // just grab the first instance of VmfsDatastoreOption
-        VmfsDatastoreOption vmfsDatastoreOption = _context.getService().queryVmfsDatastoreCreateOptions(_mor, hostScsiDisk.getDevicePath(), 5).get(0);
-
+    public ManagedObjectReference createVmfsDatastore(String datastoreName, HostScsiDisk hostScsiDisk, Integer vmfsVersion) throws Exception {
+        VmfsDatastoreOption vmfsDatastoreOption = _context.getService().queryVmfsDatastoreCreateOptions(_mor, hostScsiDisk.getDevicePath(), vmfsVersion).get(0);
         VmfsDatastoreCreateSpec vmfsDatastoreCreateSpec = (VmfsDatastoreCreateSpec)vmfsDatastoreOption.getSpec();
-
-        // set the name of the datastore to be created
         vmfsDatastoreCreateSpec.getVmfs().setVolumeName(datastoreName);
-
         return _context.getService().createVmfsDatastore(_mor, vmfsDatastoreCreateSpec);
     }
 
+    public ManagedObjectReference createVmfs5Datastore(String datastoreName, HostScsiDisk hostScsiDisk) throws Exception {
+        return createVmfsDatastore(datastoreName, hostScsiDisk, 5);
+    }
+
+    public ManagedObjectReference createVmfs6Datastore(String datastoreName, HostScsiDisk hostScsiDisk) throws Exception {
+        return createVmfsDatastore(datastoreName, hostScsiDisk, 6);
+    }
+
     public boolean deleteDatastore(String name) throws Exception {
         ManagedObjectReference morDatastore = findDatastore(name);
         if (morDatastore != null) {

vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java

@@ -98,6 +98,7 @@ public class VmwareHelper {
     public static final int MAX_SUPPORTED_DEVICES_SCSI_CONTROLLER = MAX_ALLOWED_DEVICES_SCSI_CONTROLLER - 1; // One device node is unavailable for hard disks or SCSI devices
     public static final int MAX_USABLE_SCSI_CONTROLLERS = 2;
     public static final String MIN_VERSION_UEFI_LEGACY = "5.5";
+    public static final String MIN_VERSION_VMFS6 = "6.5";
 
     public static boolean isReservedScsiDeviceNumber(int deviceNumber) {
         // The SCSI controller is assigned to virtual device node (z:7), so that device node is unavailable for hard disks or SCSI devices.