You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2022/05/18 09:21:09 UTC
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
DaanHoogland commented on code in PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#discussion_r875661080
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java:
##########
@@ -191,41 +194,59 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore)
final VolumeVO volume = volumeDao.findById(dataObject.getId());
LOGGER.debug("Revoking access for PowerFlex volume: " + volume.getPath());
- final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
- final Sdc sdc = client.getConnectedSdcByIp(host.getPrivateIpAddress());
- if (sdc == null) {
+ final String sdcId = getConnectedSdc(dataStore.getId(), host.getId());
+ if (StringUtils.isBlank(sdcId)) {
throw new CloudRuntimeException("Unable to revoke access for volume: " + dataObject.getId() + ", no Sdc connected with host ip: " + host.getPrivateIpAddress());
}
Review Comment:
this bit seems to be repeated quite a few times. can we extract?
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/provider/ScaleIOHostListener.java:
##########
@@ -69,7 +73,41 @@ public boolean hostConnect(long hostId, long poolId) {
return false;
}
- if (!isHostSdcConnected(host.getPrivateIpAddress(), poolId)) {
+ StoragePool storagePool = (StoragePool)_dataStoreMgr.getDataStore(poolId, DataStoreRole.Primary);
+
+ String systemId = _storagePoolDetailsDao.findDetail(poolId, ScaleIOGatewayClient.STORAGE_POOL_SYSTEM_ID).getValue();
+ if (systemId == null) {
+ throw new CloudRuntimeException("Failed to get the system id for PowerFlex storage pool " + storagePool.getName());
+ }
+ Map<String,String> details = new HashMap<>();
+ details.put(ScaleIOGatewayClient.STORAGE_POOL_SYSTEM_ID, systemId);
+
+ ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(true, storagePool, storagePool.getPath(), details);
+ ModifyStoragePoolAnswer answer = sendModifyStoragePoolCommand(cmd, storagePool, hostId);
+ Map<String,String> poolDetails = answer.getPoolInfo().getDetails();
+ if (MapUtils.isEmpty(poolDetails)) {
+ String msg = "SDC details not found on the host: " + hostId + ", (re)install SDC and restart agent";
+ s_logger.warn(msg);
+ _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "SDC not found on host: " + host.getUuid(), msg);
+ return false;
+ }
+
+ String sdcId = null;
+ if (poolDetails.containsKey(ScaleIOGatewayClient.SDC_ID)) {
+ sdcId = poolDetails.get(ScaleIOGatewayClient.SDC_ID);
+ } else if (poolDetails.containsKey(ScaleIOGatewayClient.SDC_GUID)) {
+ String sdcGuid = poolDetails.get(ScaleIOGatewayClient.SDC_GUID);
+ sdcId = getHostSdcId(sdcGuid, poolId);
+ }
+
+ if (StringUtils.isBlank(sdcId)) {
+ String msg = "Couldn't retrieve SDC details from the host: " + hostId + ", (re)install SDC and restart agent";
+ s_logger.warn(msg);
+ _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "SDC details not found on host: " + host.getUuid(), msg);
+ return false;
+ }
+
+ if (!isHostSdcConnected(sdcId, poolId)) {
Review Comment:
ca this bit be extracted in a few bits/modularised a bit more to improve readability?
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java:
##########
@@ -191,41 +194,59 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore)
final VolumeVO volume = volumeDao.findById(dataObject.getId());
LOGGER.debug("Revoking access for PowerFlex volume: " + volume.getPath());
- final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
- final Sdc sdc = client.getConnectedSdcByIp(host.getPrivateIpAddress());
- if (sdc == null) {
+ final String sdcId = getConnectedSdc(dataStore.getId(), host.getId());
+ if (StringUtils.isBlank(sdcId)) {
throw new CloudRuntimeException("Unable to revoke access for volume: " + dataObject.getId() + ", no Sdc connected with host ip: " + host.getPrivateIpAddress());
}
Review Comment:
maybe wit an extra `boolean doAlert`
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/util/ScaleIOUtil.java:
##########
@@ -53,6 +54,16 @@ public class ScaleIOUtil {
// VOL-ID 6c33633100000009 MDM-ID 218ce1797566a00f
// VOL-ID 6c3362a30000000a MDM-ID 218ce1797566a00f
+ private static final String QUERY_GUID_CMD = "drv_cfg --query_guid";
+ // Sample output for cmd: drv_cfg --query_guid:
+ // B0E3BFB8-C20B-43BF-93C8-13339E85AA50
+
+ private static final String QUERY_MDMS_CMD = "drv_cfg --query_mdms";
+ // Sample output for cmd: drv_cfg --query_mdms:
+ // Retrieved 2 mdm(s)
+ // MDM-ID 3ef46cbf2aaf5d0f SDC ID 6b18479c00000003 INSTALLATION ID 68ab55462cbb3ae4 IPs [0]-x.x.x.x [1]-x.x.x.x
+ // MDM-ID 2e706b2740ec200f SDC ID 301b852c00000003 INSTALLATION ID 33f8662e7a5c1e6c IPs [0]-x.x.x.x [1]-x.x.x.x
Review Comment:
javadoc?
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java:
##########
@@ -191,41 +194,59 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore)
final VolumeVO volume = volumeDao.findById(dataObject.getId());
LOGGER.debug("Revoking access for PowerFlex volume: " + volume.getPath());
- final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
- final Sdc sdc = client.getConnectedSdcByIp(host.getPrivateIpAddress());
- if (sdc == null) {
+ final String sdcId = getConnectedSdc(dataStore.getId(), host.getId());
+ if (StringUtils.isBlank(sdcId)) {
throw new CloudRuntimeException("Unable to revoke access for volume: " + dataObject.getId() + ", no Sdc connected with host ip: " + host.getPrivateIpAddress());
}
- client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(volume.getPath()), sdc.getId());
+ final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
+ client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(volume.getPath()), sdcId);
} else if (DataObjectType.TEMPLATE.equals(dataObject.getType())) {
final VMTemplateStoragePoolVO templatePoolRef = vmTemplatePoolDao.findByPoolTemplate(dataStore.getId(), dataObject.getId(), null);
LOGGER.debug("Revoking access for PowerFlex template volume: " + templatePoolRef.getInstallPath());
- final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
- final Sdc sdc = client.getConnectedSdcByIp(host.getPrivateIpAddress());
- if (sdc == null) {
+ final String sdcId = getConnectedSdc(dataStore.getId(), host.getId());
+ if (StringUtils.isBlank(sdcId)) {
throw new CloudRuntimeException("Unable to revoke access for template: " + dataObject.getId() + ", no Sdc connected with host ip: " + host.getPrivateIpAddress());
}
- client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(templatePoolRef.getInstallPath()), sdc.getId());
+ final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
+ client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(templatePoolRef.getInstallPath()), sdcId);
} else if (DataObjectType.SNAPSHOT.equals(dataObject.getType())) {
SnapshotInfo snapshot = (SnapshotInfo) dataObject;
LOGGER.debug("Revoking access for PowerFlex volume snapshot: " + snapshot.getPath());
- final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
- final Sdc sdc = client.getConnectedSdcByIp(host.getPrivateIpAddress());
- if (sdc == null) {
+ final String sdcId = getConnectedSdc(dataStore.getId(), host.getId());
+ if (StringUtils.isBlank(sdcId)) {
throw new CloudRuntimeException("Unable to revoke access for snapshot: " + dataObject.getId() + ", no Sdc connected with host ip: " + host.getPrivateIpAddress());
}
- client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(snapshot.getPath()), sdc.getId());
+ final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
+ client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(snapshot.getPath()), sdcId);
}
} catch (Exception e) {
LOGGER.warn("Failed to revoke access due to: " + e.getMessage(), e);
}
}
+ private String getConnectedSdc(long poolId, long hostId) {
+ try {
+ StoragePoolHostVO poolHostVO = storagePoolHostDao.findByPoolHost(poolId, hostId);
+ if (poolHostVO == null) {
+ return null;
+ }
+
+ final ScaleIOGatewayClient client = getScaleIOClient(poolId);
+ if (client.isSdcConnected(poolHostVO.getLocalPath())) {
+ return poolHostVO.getLocalPath();
+ }
+ } catch (Exception e) {
Review Comment:
storage maybe you want to inform in the message that the problem is with the client and not with the MS-code?
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/util/ScaleIOUtil.java:
##########
@@ -53,6 +54,16 @@ public class ScaleIOUtil {
// VOL-ID 6c33633100000009 MDM-ID 218ce1797566a00f
// VOL-ID 6c3362a30000000a MDM-ID 218ce1797566a00f
+ private static final String QUERY_GUID_CMD = "drv_cfg --query_guid";
+ // Sample output for cmd: drv_cfg --query_guid:
+ // B0E3BFB8-C20B-43BF-93C8-13339E85AA50
Review Comment:
is this javadoc for the above constant?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org