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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
