You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ro...@apache.org on 2023/08/25 06:06:10 UTC
[cloudstack] branch 4.18 updated: vmware: improve solidfire storage plugin integration and fix cases (#3) (#7761)
This is an automated email from the ASF dual-hosted git repository.
rohit pushed a commit to branch 4.18
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.18 by this push:
new e964395bd45 vmware: improve solidfire storage plugin integration and fix cases (#3) (#7761)
e964395bd45 is described below
commit e964395bd45c390e648f7c9cd567943c45276d69
Author: Rohit Yadav <ro...@shapeblue.com>
AuthorDate: Fri Aug 25 11:36:03 2023 +0530
vmware: improve solidfire storage plugin integration and fix cases (#3) (#7761)
This fixes the following cases in which Solidfire storage integration
caused issues when using Solidfire datadisks with VMware:
1. Take Volume Snapshot of Solidfire data disk
2. Delete an active Instance with Solidfire data disk attached
3. Attach used existing Solidfire data disk to a running/stopped VM
4. Stop and Start an instance with Solidfire data disks attached
5. Expand disk by resizing Solidfire data disk by providing size
6. Expand disk by changing disk offering for the Solidfire data disk
Additional changes:
- Use VMFS6 as managed datastore type if the host supports
- Refactor detection and splitting of managed storage ds name in storage
processor
- Restrict storage rescanning for managed datastore when resizing
Signed-off-by: Rohit Yadav <ro...@shapeblue.com>
---
.../engine/orchestration/VolumeOrchestrator.java | 4 +--
.../vmware/manager/VmwareStorageManagerImpl.java | 30 ++++++++++++++--------
.../hypervisor/vmware/resource/VmwareResource.java | 5 ++++
.../storage/resource/VmwareStorageProcessor.java | 12 +++++++--
.../manager/VmwareStorageManagerImplTest.java | 11 ++++++++
.../driver/SolidFirePrimaryDataStoreDriver.java | 30 ++++++++++++++++++++--
.../vmware/mo/HostDatastoreSystemMO.java | 17 +++++++-----
.../cloud/hypervisor/vmware/util/VmwareHelper.java | 1 +
8 files changed, 86 insertions(+), 24 deletions(-)
diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java
index 52bcf5033af..c3908795f7c 100644
--- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java
+++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java
@@ -1906,8 +1906,8 @@ public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrati
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) {
diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java
index 5b94858b403..d7a736e80e4 100644
--- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java
+++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java
@@ -1107,12 +1107,25 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
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 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
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 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
}
} 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);
}
diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
index 01d6f6816e0..e76b34954c6 100644
--- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
+++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
@@ -940,6 +940,11 @@ public class VmwareResource extends ServerResourceBase implements StoragePoolRes
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);
}
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 7b4d5f118b4..ba9d16f8186 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
@@ -2825,7 +2825,15 @@ public class VmwareStorageProcessor implements StorageProcessor {
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 @@ public class VmwareStorageProcessor implements StorageProcessor {
}
}
- 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) {
diff --git a/plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImplTest.java b/plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImplTest.java
index 1d207e37bca..c72f23c628e 100644
--- a/plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImplTest.java
+++ b/plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImplTest.java
@@ -116,4 +116,15 @@ public class VmwareStorageManagerImplTest {
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"));
+ }
}
diff --git a/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/driver/SolidFirePrimaryDataStoreDriver.java b/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/driver/SolidFirePrimaryDataStoreDriver.java
index 702bdc3669a..4478dc98ca4 100644
--- a/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/driver/SolidFirePrimaryDataStoreDriver.java
+++ b/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.user.dao.AccountDao;
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 class SolidFirePrimaryDataStoreDriver implements PrimaryDataStoreDriver {
}
}
+ 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 class SolidFirePrimaryDataStoreDriver implements PrimaryDataStoreDriver {
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);
diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HostDatastoreSystemMO.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HostDatastoreSystemMO.java
index 30798e31e19..c2fe3f4e54c 100644
--- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HostDatastoreSystemMO.java
+++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HostDatastoreSystemMO.java
@@ -199,18 +199,21 @@ public class HostDatastoreSystemMO extends BaseMO {
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) {
diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java
index 4a81beeff98..841d914af32 100644
--- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java
+++ b/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.