winterhazel commented on code in PR #13053:
URL: https://github.com/apache/cloudstack/pull/13053#discussion_r3188061574
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java:
##########
@@ -157,14 +159,17 @@ public DataStore initialize(Map<String, Object> dsInfos) {
throw new CloudRuntimeException("ONTAP details validation failed,
cannot create primary storage");
}
+ // Determine storage pool type, path and port based on protocol
String path;
int port;
switch (protocol) {
case NFS3:
parameters.setType(Storage.StoragePoolType.NetworkFilesystem);
path = OntapStorageConstants.SLASH + storagePoolName;
port = OntapStorageConstants.NFS3_PORT;
- logger.info("Setting NFS path for storage pool: " + path + ",
port: " + port);
+ // Force NFSv3 for ONTAP managed storage to avoid NFSv4 ID
mapping issues
+ details.put(OntapStorageConstants.NFS_MOUNT_OPTIONS,
OntapStorageConstants.NFS3_MOUNT_OPTIONS_VER_3);
Review Comment:
We can use `ApiConstants.NFS_MOUNT_OPTIONS` instead of
`OntapStorageConstants.NFS_MOUNT_OPTIONS`
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java:
##########
@@ -207,70 +212,67 @@ private long validateInitializeInputs(Long capacityBytes,
Long podId, Long clust
capacityBytes = ONTAP_MIN_VOLUME_SIZE_IN_BYTES;
}
- // Scope (pod/cluster/zone) validation
+ // Validate scope
if (podId == null ^ clusterId == null) {
throw new CloudRuntimeException("Cluster Id or Pod Id is null,
cannot create primary storage");
}
- if (podId == null && clusterId == null) {
+
+ if (podId == null) {
if (zoneId != null) {
logger.info("Both Pod Id and Cluster Id are null, Primary
storage pool will be associated with a Zone");
} else {
throw new CloudRuntimeException("Pod Id, Cluster Id and Zone
Id are all null, cannot create primary storage");
}
}
- // Basic parameter validation
- if (StringUtils.isBlank(storagePoolName)) {
+ if (storagePoolName == null || storagePoolName.isEmpty()) {
Review Comment:
Why was `StringUtils.isBlank` replaced here?
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java:
##########
@@ -207,70 +212,67 @@ private long validateInitializeInputs(Long capacityBytes,
Long podId, Long clust
capacityBytes = ONTAP_MIN_VOLUME_SIZE_IN_BYTES;
}
- // Scope (pod/cluster/zone) validation
+ // Validate scope
if (podId == null ^ clusterId == null) {
throw new CloudRuntimeException("Cluster Id or Pod Id is null,
cannot create primary storage");
}
- if (podId == null && clusterId == null) {
+
+ if (podId == null) {
if (zoneId != null) {
logger.info("Both Pod Id and Cluster Id are null, Primary
storage pool will be associated with a Zone");
} else {
throw new CloudRuntimeException("Pod Id, Cluster Id and Zone
Id are all null, cannot create primary storage");
}
}
- // Basic parameter validation
- if (StringUtils.isBlank(storagePoolName)) {
+ if (storagePoolName == null || storagePoolName.isEmpty()) {
throw new CloudRuntimeException("Storage pool name is null or
empty, cannot create primary storage");
}
- if (StringUtils.isBlank(providerName)) {
+
+ if (providerName == null || providerName.isEmpty()) {
throw new CloudRuntimeException("Provider name is null or empty,
cannot create primary storage");
}
+
+ PrimaryDataStoreParameters parameters = new
PrimaryDataStoreParameters();
+ if (clusterId != null) {
+ ClusterVO clusterVO = _clusterDao.findById(clusterId);
+ Preconditions.checkNotNull(clusterVO, "Unable to locate the
specified cluster");
+ if (clusterVO.getHypervisorType() !=
Hypervisor.HypervisorType.KVM) {
+ throw new CloudRuntimeException("ONTAP primary storage is
supported only for KVM hypervisor");
+ }
+ parameters.setHypervisorType(clusterVO.getHypervisorType());
+ }
+
logger.debug("ONTAP primary storage will be created as " + (managed ?
"managed" : "unmanaged"));
if (!managed) {
throw new CloudRuntimeException("ONTAP primary storage must be
managed");
}
- // Details key validation
+ //Required ONTAP detail keys
Set<String> requiredKeys = Set.of(
OntapStorageConstants.USERNAME,
OntapStorageConstants.PASSWORD,
OntapStorageConstants.SVM_NAME,
OntapStorageConstants.PROTOCOL,
- OntapStorageConstants.MANAGEMENT_LIF
- );
- Set<String> optionalKeys = Set.of(
- OntapStorageConstants.IS_DISAGGREGATED
+ OntapStorageConstants.STORAGE_IP
);
- Set<String> allowedKeys = new java.util.HashSet<>(requiredKeys);
- allowedKeys.addAll(optionalKeys);
-
- if (StringUtils.isNotBlank(url)) {
- for (String segment : url.split(OntapStorageConstants.SEMICOLON)) {
- if (segment.isEmpty()) {
- continue;
- }
- String[] kv = segment.split(OntapStorageConstants.EQUALS, 2);
- if (kv.length == 2) {
- details.put(kv[0].trim(), kv[1].trim());
- }
- }
- }
+ // Validate existing entries (reject unexpected keys, empty values)
for (Map.Entry<String, String> e : details.entrySet()) {
String key = e.getKey();
String val = e.getValue();
- if (!allowedKeys.contains(key)) {
+ if (!requiredKeys.contains(key)) {
throw new CloudRuntimeException("Unexpected ONTAP detail key
in URL: " + key);
}
- if (StringUtils.isBlank(val)) {
+ if (val == null || val.isEmpty()) {
throw new CloudRuntimeException("ONTAP primary storage
creation failed, empty detail: " + key);
}
}
- Set<String> providedKeys = new HashSet<>(details.keySet());
+ // Detect missing required keys
+ Set<String> providedKeys = new java.util.HashSet<>(details.keySet());
Review Comment:
PLease import `HashSet` and use only its classname
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java:
##########
@@ -306,21 +308,24 @@ public boolean attachCluster(DataStore dataStore,
ClusterScope scope) {
logger.error(errMsg);
throw new CloudRuntimeException(errMsg);
}
-
logger.debug("attachCluster: Attaching the pool to each of the host in
the cluster: {}", primaryStore.getClusterId());
- if (hostsIdentifier != null && hostsIdentifier.size() > 0) {
- try {
- AccessGroup accessGroupRequest = new AccessGroup();
- accessGroupRequest.setHostsToConnect(hostsToConnect);
- accessGroupRequest.setScope(scope);
- primaryStore.setDetails(details);
- accessGroupRequest.setPrimaryDataStoreInfo(primaryStore);
- strategy.createAccessGroup(accessGroupRequest);
- } catch (Exception e) {
- logger.error("attachCluster: Failed to create access group on
storage system for cluster: " + primaryStore.getClusterId() + ". Exception: " +
e.getMessage());
- throw new CloudRuntimeException("attachCluster: Failed to
create access group on storage system for cluster: " +
primaryStore.getClusterId() + ". Exception: " + e.getMessage());
+ // We need to create export policy at pool level and igroup at host
level(in grantAccess)
+ if
(ProtocolType.NFS3.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL)))
{
Review Comment:
We can use `ApiConstants.PROTOCOL` instead of
`OntapStorageConstants.PROTOCOL`
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java:
##########
@@ -65,14 +108,235 @@ public DataTO getTO(DataObject data) {
@Override
public DataStoreTO getStoreTO(DataStore store) { return null; }
+ @Override
+ public boolean volumesRequireGrantAccessWhenUsed(){
+ logger.info("OntapPrimaryDatastoreDriver:
volumesRequireGrantAccessWhenUsed: Called");
+ return true;
+ }
+
+ /**
+ * Creates a volume on the ONTAP storage system.
+ */
@Override
public void createAsync(DataStore dataStore, DataObject dataObject,
AsyncCompletionCallback<CreateCmdResult> callback) {
- throw new UnsupportedOperationException("Create operation is not
supported for ONTAP primary storage.");
+ CreateCmdResult createCmdResult = null;
+ String errMsg;
+
+ if (dataObject == null) {
+ throw new InvalidParameterValueException("dataObject should not be
null");
+ }
+ if (dataStore == null) {
+ throw new InvalidParameterValueException("dataStore should not be
null");
+ }
+ if (callback == null) {
+ throw new InvalidParameterValueException("callback should not be
null");
+ }
+
+ try {
+ logger.info("Started for data store name [{}] and data object name
[{}] of type [{}]",
+ dataStore.getName(), dataObject.getName(),
dataObject.getType());
+
+ StoragePoolVO storagePool =
storagePoolDao.findById(dataStore.getId());
+ if (storagePool == null) {
+ logger.error("createAsync: Storage Pool not found for id: " +
dataStore.getId());
+ throw new CloudRuntimeException("Storage Pool not found for
id: " + dataStore.getId());
+ }
+ String storagePoolUuid = dataStore.getUuid();
+
+ Map<String, String> details =
storagePoolDetailsDao.listDetailsKeyPairs(dataStore.getId());
+
+ if (dataObject.getType() == DataObjectType.VOLUME) {
+ VolumeInfo volInfo = (VolumeInfo) dataObject;
+
+ // Create the backend storage object (LUN for iSCSI, no-op for
NFS)
+ CloudStackVolume created = createCloudStackVolume(dataStore,
volInfo, details);
+
+ // Update CloudStack volume record with storage pool
association and protocol-specific details
+ VolumeVO volumeVO = volumeDao.findById(volInfo.getId());
+ if (volumeVO != null) {
+ volumeVO.setPoolType(storagePool.getPoolType());
+ volumeVO.setPoolId(storagePool.getId());
+
+ if
(ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL)))
{
+ String lunName = created != null && created.getLun()
!= null ? created.getLun().getName() : null;
+ if (lunName == null) {
+ throw new CloudRuntimeException("Missing LUN name
for volume " + volInfo.getId());
+ }
+
+ // Persist LUN details for future operations (delete,
grant/revoke access)
+ volumeDetailsDao.addDetail(volInfo.getId(),
OntapStorageConstants.LUN_DOT_UUID, created.getLun().getUuid(), false);
+ volumeDetailsDao.addDetail(volInfo.getId(),
OntapStorageConstants.LUN_DOT_NAME, lunName, false);
+ if (created.getLun().getUuid() != null) {
+ volumeVO.setFolder(created.getLun().getUuid());
+ }
+
+ logger.info("createAsync: Created LUN [{}] for volume
[{}]. LUN mapping will occur during grantAccess() to per-host igroup.",
+ lunName, volumeVO.getId());
+ createCmdResult = new CreateCmdResult(lunName, new
Answer(null, true, null));
+ } else if
(ProtocolType.NFS3.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL)))
{
+ createCmdResult = new
CreateCmdResult(volInfo.getUuid(), new Answer(null, true, null));
+ logger.info("createAsync: Managed NFS volume [{}] with
path [{}] associated with pool {}",
+ volumeVO.getId(), volInfo.getUuid(),
storagePool.getId());
+ }
+ volumeDao.update(volumeVO.getId(), volumeVO);
+ }
+ } else {
+ errMsg = "Invalid DataObjectType (" + dataObject.getType() +
") passed to createAsync";
+ logger.error(errMsg);
+ throw new CloudRuntimeException(errMsg);
+ }
+ } catch (Exception e) {
+ errMsg = e.getMessage();
+ logger.error("createAsync: Failed for dataObject name [{}]: {}",
dataObject.getName(), errMsg);
+ createCmdResult = new CreateCmdResult(null, new Answer(null,
false, errMsg));
+ createCmdResult.setResult(e.toString());
+ } finally {
+ if (createCmdResult != null && createCmdResult.isSuccess()) {
+ logger.info("createAsync: Operation completed successfully for
{}", dataObject.getType());
+ }
+ callback.complete(createCmdResult);
+ }
+ }
+
+ /**
+ * Creates a volume on the ONTAP backend.
+ */
+ private CloudStackVolume createCloudStackVolume(DataStore dataStore,
DataObject dataObject, Map<String, String> details) {
Review Comment:
This method could receive (1) the `StoragePoolVO` from the calling method
instead of a `dataStore` to avoid the extra call to the DB, and (2) a
`VolumeInfo` instead of a `DataObject`, as it was already casted in the calling
method
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java:
##########
@@ -65,14 +108,235 @@ public DataTO getTO(DataObject data) {
@Override
public DataStoreTO getStoreTO(DataStore store) { return null; }
+ @Override
+ public boolean volumesRequireGrantAccessWhenUsed(){
+ logger.info("OntapPrimaryDatastoreDriver:
volumesRequireGrantAccessWhenUsed: Called");
+ return true;
+ }
+
+ /**
+ * Creates a volume on the ONTAP storage system.
+ */
@Override
public void createAsync(DataStore dataStore, DataObject dataObject,
AsyncCompletionCallback<CreateCmdResult> callback) {
- throw new UnsupportedOperationException("Create operation is not
supported for ONTAP primary storage.");
+ CreateCmdResult createCmdResult = null;
+ String errMsg;
+
+ if (dataObject == null) {
+ throw new InvalidParameterValueException("dataObject should not be
null");
+ }
+ if (dataStore == null) {
+ throw new InvalidParameterValueException("dataStore should not be
null");
+ }
+ if (callback == null) {
+ throw new InvalidParameterValueException("callback should not be
null");
+ }
+
+ try {
+ logger.info("Started for data store name [{}] and data object name
[{}] of type [{}]",
+ dataStore.getName(), dataObject.getName(),
dataObject.getType());
+
+ StoragePoolVO storagePool =
storagePoolDao.findById(dataStore.getId());
+ if (storagePool == null) {
+ logger.error("createAsync: Storage Pool not found for id: " +
dataStore.getId());
+ throw new CloudRuntimeException("Storage Pool not found for
id: " + dataStore.getId());
+ }
+ String storagePoolUuid = dataStore.getUuid();
+
+ Map<String, String> details =
storagePoolDetailsDao.listDetailsKeyPairs(dataStore.getId());
+
+ if (dataObject.getType() == DataObjectType.VOLUME) {
+ VolumeInfo volInfo = (VolumeInfo) dataObject;
+
+ // Create the backend storage object (LUN for iSCSI, no-op for
NFS)
+ CloudStackVolume created = createCloudStackVolume(dataStore,
volInfo, details);
+
+ // Update CloudStack volume record with storage pool
association and protocol-specific details
+ VolumeVO volumeVO = volumeDao.findById(volInfo.getId());
+ if (volumeVO != null) {
Review Comment:
This `if` could be inverted to return early and reduce indentation
Also, does it make sense to throw a runtime exception if volumeVO is null?
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java:
##########
@@ -65,14 +108,235 @@ public DataTO getTO(DataObject data) {
@Override
public DataStoreTO getStoreTO(DataStore store) { return null; }
+ @Override
+ public boolean volumesRequireGrantAccessWhenUsed(){
+ logger.info("OntapPrimaryDatastoreDriver:
volumesRequireGrantAccessWhenUsed: Called");
+ return true;
+ }
+
+ /**
+ * Creates a volume on the ONTAP storage system.
+ */
@Override
public void createAsync(DataStore dataStore, DataObject dataObject,
AsyncCompletionCallback<CreateCmdResult> callback) {
- throw new UnsupportedOperationException("Create operation is not
supported for ONTAP primary storage.");
+ CreateCmdResult createCmdResult = null;
+ String errMsg;
+
+ if (dataObject == null) {
+ throw new InvalidParameterValueException("dataObject should not be
null");
+ }
+ if (dataStore == null) {
+ throw new InvalidParameterValueException("dataStore should not be
null");
+ }
+ if (callback == null) {
+ throw new InvalidParameterValueException("callback should not be
null");
+ }
+
+ try {
+ logger.info("Started for data store name [{}] and data object name
[{}] of type [{}]",
+ dataStore.getName(), dataObject.getName(),
dataObject.getType());
+
+ StoragePoolVO storagePool =
storagePoolDao.findById(dataStore.getId());
+ if (storagePool == null) {
+ logger.error("createAsync: Storage Pool not found for id: " +
dataStore.getId());
+ throw new CloudRuntimeException("Storage Pool not found for
id: " + dataStore.getId());
+ }
+ String storagePoolUuid = dataStore.getUuid();
+
+ Map<String, String> details =
storagePoolDetailsDao.listDetailsKeyPairs(dataStore.getId());
+
+ if (dataObject.getType() == DataObjectType.VOLUME) {
+ VolumeInfo volInfo = (VolumeInfo) dataObject;
+
+ // Create the backend storage object (LUN for iSCSI, no-op for
NFS)
+ CloudStackVolume created = createCloudStackVolume(dataStore,
volInfo, details);
+
+ // Update CloudStack volume record with storage pool
association and protocol-specific details
+ VolumeVO volumeVO = volumeDao.findById(volInfo.getId());
+ if (volumeVO != null) {
+ volumeVO.setPoolType(storagePool.getPoolType());
+ volumeVO.setPoolId(storagePool.getId());
+
+ if
(ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL)))
{
+ String lunName = created != null && created.getLun()
!= null ? created.getLun().getName() : null;
+ if (lunName == null) {
+ throw new CloudRuntimeException("Missing LUN name
for volume " + volInfo.getId());
+ }
+
+ // Persist LUN details for future operations (delete,
grant/revoke access)
+ volumeDetailsDao.addDetail(volInfo.getId(),
OntapStorageConstants.LUN_DOT_UUID, created.getLun().getUuid(), false);
+ volumeDetailsDao.addDetail(volInfo.getId(),
OntapStorageConstants.LUN_DOT_NAME, lunName, false);
+ if (created.getLun().getUuid() != null) {
+ volumeVO.setFolder(created.getLun().getUuid());
+ }
+
+ logger.info("createAsync: Created LUN [{}] for volume
[{}]. LUN mapping will occur during grantAccess() to per-host igroup.",
+ lunName, volumeVO.getId());
+ createCmdResult = new CreateCmdResult(lunName, new
Answer(null, true, null));
+ } else if
(ProtocolType.NFS3.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL)))
{
+ createCmdResult = new
CreateCmdResult(volInfo.getUuid(), new Answer(null, true, null));
+ logger.info("createAsync: Managed NFS volume [{}] with
path [{}] associated with pool {}",
+ volumeVO.getId(), volInfo.getUuid(),
storagePool.getId());
+ }
+ volumeDao.update(volumeVO.getId(), volumeVO);
+ }
+ } else {
+ errMsg = "Invalid DataObjectType (" + dataObject.getType() +
") passed to createAsync";
+ logger.error(errMsg);
+ throw new CloudRuntimeException(errMsg);
+ }
+ } catch (Exception e) {
+ errMsg = e.getMessage();
+ logger.error("createAsync: Failed for dataObject name [{}]: {}",
dataObject.getName(), errMsg);
+ createCmdResult = new CreateCmdResult(null, new Answer(null,
false, errMsg));
+ createCmdResult.setResult(e.toString());
+ } finally {
+ if (createCmdResult != null && createCmdResult.isSuccess()) {
+ logger.info("createAsync: Operation completed successfully for
{}", dataObject.getType());
+ }
+ callback.complete(createCmdResult);
+ }
+ }
+
+ /**
+ * Creates a volume on the ONTAP backend.
+ */
+ private CloudStackVolume createCloudStackVolume(DataStore dataStore,
DataObject dataObject, Map<String, String> details) {
+ StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId());
+ if (storagePool == null) {
+ logger.error("createCloudStackVolume: Storage Pool not found for
id: {}", dataStore.getId());
+ throw new CloudRuntimeException("Storage Pool not found for id: "
+ dataStore.getId());
+ }
+
+ StorageStrategy storageStrategy =
OntapStorageUtils.getStrategyByStoragePoolDetails(details);
+
+ if (dataObject.getType() == DataObjectType.VOLUME) {
+ VolumeInfo volumeObject = (VolumeInfo) dataObject;
+ CloudStackVolume cloudStackVolumeRequest =
OntapStorageUtils.createCloudStackVolumeRequestByProtocol(storagePool, details,
volumeObject);
+ return
storageStrategy.createCloudStackVolume(cloudStackVolumeRequest);
+ } else {
+ throw new CloudRuntimeException("Unsupported DataObjectType: " +
dataObject.getType());
+ }
}
+ /**
+ * Deletes a volume or snapshot from the ONTAP storage system.
+ *
+ * <p>For volumes, deletes the backend storage object (LUN for iSCSI,
no-op for NFS).
+ * For snapshots, deletes the FlexVolume snapshot from ONTAP that was
created by takeSnapshot.</p>
+ */
@Override
public void deleteAsync(DataStore store, DataObject data,
AsyncCompletionCallback<CommandResult> callback) {
- throw new UnsupportedOperationException("Delete operation is not
supported for ONTAP primary storage.");
+ CommandResult commandResult = new CommandResult();
+ try {
+ if (store == null || data == null) {
+ throw new CloudRuntimeException("store or data is null");
+ }
+
+ if (data.getType() == DataObjectType.VOLUME) {
+ StoragePoolVO storagePool =
storagePoolDao.findById(store.getId());
+ if (storagePool == null) {
+ logger.error("deleteAsync: Storage Pool not found for id:
" + store.getId());
+ throw new CloudRuntimeException("Storage Pool not found
for id: " + store.getId());
+ }
+ Map<String, String> details =
storagePoolDetailsDao.listDetailsKeyPairs(store.getId());
+ StorageStrategy storageStrategy =
OntapStorageUtils.getStrategyByStoragePoolDetails(details);
+ logger.info("createCloudStackVolumeForTypeVolume: Connection
to Ontap SVM [{}] successful, preparing CloudStackVolumeRequest",
details.get(OntapStorageConstants.SVM_NAME));
+ VolumeInfo volumeInfo = (VolumeInfo) data;
+ CloudStackVolume cloudStackVolumeRequest =
createDeleteCloudStackVolumeRequest(storagePool,details,volumeInfo);
+
storageStrategy.deleteCloudStackVolume(cloudStackVolumeRequest);
+ logger.info("deleteAsync: Volume deleted: " +
volumeInfo.getId());
+ commandResult.setResult(null);
+ commandResult.setSuccess(true);
+ } else if (data.getType() == DataObjectType.SNAPSHOT) {
+ // Delete the ONTAP FlexVolume snapshot that was created by
takeSnapshot
+ deleteOntapSnapshot((SnapshotInfo) data, commandResult);
+ } else {
+ throw new CloudRuntimeException("Unsupported data object type:
" + data.getType());
+ }
+ } catch (Exception e) {
+ logger.error("deleteAsync: Failed for data object [{}]: {}", data,
e.getMessage());
+ commandResult.setSuccess(false);
+ commandResult.setResult(e.getMessage());
+ } finally {
+ callback.complete(commandResult);
+ }
+ }
+
+ /**
+ * Deletes an ONTAP FlexVolume snapshot.
+ *
+ * <p>Retrieves the snapshot details stored during takeSnapshot and calls
the ONTAP
+ * REST API to delete the FlexVolume snapshot.</p>
+ *
+ * @param snapshotInfo The CloudStack snapshot to delete
+ * @param commandResult Result object to populate with success/failure
+ */
+ private void deleteOntapSnapshot(SnapshotInfo snapshotInfo, CommandResult
commandResult) {
+ long snapshotId = snapshotInfo.getId();
+ logger.info("deleteOntapSnapshot: Deleting ONTAP FlexVolume snapshot
for CloudStack snapshot [{}]", snapshotId);
+
+ try {
+ // Retrieve snapshot details stored during takeSnapshot
+ String flexVolUuid = getSnapshotDetail(snapshotId,
OntapStorageConstants.BASE_ONTAP_FV_ID);
+ String ontapSnapshotUuid = getSnapshotDetail(snapshotId,
OntapStorageConstants.ONTAP_SNAP_ID);
+ String snapshotName = getSnapshotDetail(snapshotId,
OntapStorageConstants.ONTAP_SNAP_NAME);
+ String poolIdStr = getSnapshotDetail(snapshotId,
OntapStorageConstants.PRIMARY_POOL_ID);
+
+ if (flexVolUuid == null || ontapSnapshotUuid == null) {
+ logger.warn("deleteOntapSnapshot: Missing ONTAP snapshot
details for snapshot [{}]. " +
+ "flexVolUuid={}, ontapSnapshotUuid={}. Snapshot may
have been created by a different method or already deleted.",
+ snapshotId, flexVolUuid, ontapSnapshotUuid);
+ // Consider this a success since there's nothing to delete on
ONTAP
+ commandResult.setSuccess(true);
+ commandResult.setResult(null);
+ return;
+ }
+
+ long poolId = Long.parseLong(poolIdStr);
+ Map<String, String> poolDetails =
storagePoolDetailsDao.listDetailsKeyPairs(poolId);
+
+ StorageStrategy storageStrategy =
OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails);
+ SnapshotFeignClient snapshotClient =
storageStrategy.getSnapshotFeignClient();
+ String authHeader = storageStrategy.getAuthHeader();
+
+ logger.info("deleteOntapSnapshot: Deleting ONTAP snapshot [{}]
(uuid={}) from FlexVol [{}]",
+ snapshotName, ontapSnapshotUuid, flexVolUuid);
+
+ // Call ONTAP REST API to delete the snapshot
+ JobResponse jobResponse =
snapshotClient.deleteSnapshot(authHeader, flexVolUuid, ontapSnapshotUuid);
+
+ if (jobResponse != null && jobResponse.getJob() != null) {
+ // Poll for job completion
+ Boolean jobSucceeded =
storageStrategy.jobPollForSuccess(jobResponse.getJob().getUuid(), 30, 2);
Review Comment:
Not related to snapshot deletion, but `sleepTimeInSecs` is not being used in
`jobPollForSuccess`. Could you check if something is missing, and remove the
parameter if not?
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java:
##########
@@ -98,14 +362,234 @@ public ChapInfo getChapInfo(DataObject dataObject) {
return null;
}
+ /**
+ * Grants a host access to a volume.
+ */
@Override
public boolean grantAccess(DataObject dataObject, Host host, DataStore
dataStore) {
- return false;
+ try {
+ if (dataStore == null) {
+ throw new InvalidParameterValueException("dataStore should not
be null");
+ }
+ if (dataObject == null) {
+ throw new InvalidParameterValueException("dataObject should
not be null");
+ }
+ if (host == null) {
+ throw new InvalidParameterValueException("host should not be
null");
+ }
+
+ StoragePoolVO storagePool =
storagePoolDao.findById(dataStore.getId());
+ if (storagePool == null) {
+ logger.error("grantAccess: Storage Pool not found for id: " +
dataStore.getId());
+ throw new CloudRuntimeException("Storage Pool not found for
id: " + dataStore.getId());
+ }
+ String storagePoolUuid = dataStore.getUuid();
+
+ // ONTAP managed storage only supports cluster and zone scoped
pools
+ if (storagePool.getScope() != ScopeType.CLUSTER &&
storagePool.getScope() != ScopeType.ZONE) {
+ logger.error("grantAccess: Only Cluster and Zone scoped
primary storage is supported for storage Pool: " + storagePool.getName());
+ throw new CloudRuntimeException("Only Cluster and Zone scoped
primary storage is supported for Storage Pool: " + storagePool.getName());
+ }
+
+ if (dataObject.getType() == DataObjectType.VOLUME) {
+ VolumeVO volumeVO = volumeDao.findById(dataObject.getId());
+ if (volumeVO == null) {
+ logger.error("grantAccess: CloudStack Volume not found for
id: " + dataObject.getId());
+ throw new CloudRuntimeException("CloudStack Volume not
found for id: " + dataObject.getId());
+ }
+
+ Map<String, String> details =
storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId());
+ String svmName = details.get(OntapStorageConstants.SVM_NAME);
+
+ if
(ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL)))
{
+ // Only retrieve LUN name for iSCSI volumes
+ String cloudStackVolumeName =
volumeDetailsDao.findDetail(volumeVO.getId(),
OntapStorageConstants.LUN_DOT_NAME).getValue();
+ UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy)
OntapStorageUtils.getStrategyByStoragePoolDetails(details);
+ String accessGroupName =
OntapStorageUtils.getIgroupName(svmName, host.getName());
+
+ // Validate if Igroup exist ONTAP for this host as we may
be using delete_on_unmap= true and igroup may be deleted by ONTAP automatically
+ Map<String, String> getAccessGroupMap = Map.of(
+ OntapStorageConstants.NAME, accessGroupName,
+ OntapStorageConstants.SVM_DOT_NAME, svmName
+ );
+ Igroup igroup = new Igroup();
Review Comment:
`igroup` is not used here
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java:
##########
@@ -281,16 +316,32 @@ public Volume createStorageVolume(String volumeName, Long
size) {
}
}
+ /**
+ * Updates ONTAP Flex-Volume
+ * Eligible only for Unified ONTAP storage
+ * throw exception in case of disaggregated ONTAP storage
+ *
+ * @param volume the volume to update
+ * @return the updated Volume object
+ */
public Volume updateStorageVolume(Volume volume) {
return null;
}
+ /**
+ * Delete ONTAP Flex-Volume
+ * Eligible only for Unified ONTAP storage
+ * throw exception in case of disaggregated ONTAP storage
+ *
+ * @param volume the volume to delete
+ */
public void deleteStorageVolume(Volume volume) {
logger.info("Deleting ONTAP volume by name: " + volume.getName() + "
and uuid: " + volume.getUuid());
String authHeader =
OntapStorageUtils.generateAuthHeader(storage.getUsername(),
storage.getPassword());
try {
+ // TODO: Implement lun and file deletion, if any, before deleting
the volume
JobResponse jobResponse =
volumeFeignClient.deleteVolume(authHeader, volume.getUuid());
- Boolean jobSucceeded =
jobPollForSuccess(jobResponse.getJob().getUuid());
+ Boolean jobSucceeded =
jobPollForSuccess(jobResponse.getJob().getUuid(),10, 1);
Review Comment:
```suggestion
Boolean jobSucceeded =
jobPollForSuccess(jobResponse.getJob().getUuid(), 10, 1);
```
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java:
##########
@@ -207,70 +212,67 @@ private long validateInitializeInputs(Long capacityBytes,
Long podId, Long clust
capacityBytes = ONTAP_MIN_VOLUME_SIZE_IN_BYTES;
}
- // Scope (pod/cluster/zone) validation
+ // Validate scope
if (podId == null ^ clusterId == null) {
throw new CloudRuntimeException("Cluster Id or Pod Id is null,
cannot create primary storage");
}
- if (podId == null && clusterId == null) {
+
+ if (podId == null) {
if (zoneId != null) {
logger.info("Both Pod Id and Cluster Id are null, Primary
storage pool will be associated with a Zone");
} else {
throw new CloudRuntimeException("Pod Id, Cluster Id and Zone
Id are all null, cannot create primary storage");
}
}
Review Comment:
These two ifs can be reduced to a single `if (podId == null && zoneId !=
null) {`
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java:
##########
@@ -282,16 +284,16 @@ private long validateInitializeInputs(Long capacityBytes,
Long podId, Long clust
public boolean attachCluster(DataStore dataStore, ClusterScope scope) {
logger.debug("In attachCluster for ONTAP primary storage");
if (dataStore == null) {
- throw new InvalidParameterValueException("attachCluster: dataStore
should not be null");
+ throw new InvalidParameterValueException(" dataStore should not be
null");
}
if (scope == null) {
- throw new InvalidParameterValueException("attachCluster: scope
should not be null");
+ throw new InvalidParameterValueException(" scope should not be
null");
}
List<String> hostsIdentifier = new ArrayList<>();
StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId());
if (storagePool == null) {
logger.error("attachCluster : Storage Pool not found for id: " +
dataStore.getId());
- throw new CloudRuntimeException("attachCluster : Storage Pool not
found for id: " + dataStore.getId());
+ throw new CloudRuntimeException(" Storage Pool not found for id: "
+ dataStore.getId());
Review Comment:
```suggestion
throw new CloudRuntimeException("Storage Pool not found for id:
" + dataStore.getId());
```
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java:
##########
@@ -282,16 +284,16 @@ private long validateInitializeInputs(Long capacityBytes,
Long podId, Long clust
public boolean attachCluster(DataStore dataStore, ClusterScope scope) {
logger.debug("In attachCluster for ONTAP primary storage");
if (dataStore == null) {
- throw new InvalidParameterValueException("attachCluster: dataStore
should not be null");
+ throw new InvalidParameterValueException(" dataStore should not be
null");
}
if (scope == null) {
- throw new InvalidParameterValueException("attachCluster: scope
should not be null");
+ throw new InvalidParameterValueException(" scope should not be
null");
Review Comment:
```suggestion
throw new InvalidParameterValueException("scope should not be
null");
```
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java:
##########
@@ -207,70 +212,67 @@ private long validateInitializeInputs(Long capacityBytes,
Long podId, Long clust
capacityBytes = ONTAP_MIN_VOLUME_SIZE_IN_BYTES;
}
- // Scope (pod/cluster/zone) validation
+ // Validate scope
if (podId == null ^ clusterId == null) {
throw new CloudRuntimeException("Cluster Id or Pod Id is null,
cannot create primary storage");
}
- if (podId == null && clusterId == null) {
+
+ if (podId == null) {
if (zoneId != null) {
logger.info("Both Pod Id and Cluster Id are null, Primary
storage pool will be associated with a Zone");
} else {
throw new CloudRuntimeException("Pod Id, Cluster Id and Zone
Id are all null, cannot create primary storage");
}
}
- // Basic parameter validation
- if (StringUtils.isBlank(storagePoolName)) {
+ if (storagePoolName == null || storagePoolName.isEmpty()) {
throw new CloudRuntimeException("Storage pool name is null or
empty, cannot create primary storage");
}
- if (StringUtils.isBlank(providerName)) {
+
+ if (providerName == null || providerName.isEmpty()) {
Review Comment:
Why was `StringUtils.isBlank` replaced here?
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java:
##########
@@ -207,70 +212,67 @@ private long validateInitializeInputs(Long capacityBytes,
Long podId, Long clust
capacityBytes = ONTAP_MIN_VOLUME_SIZE_IN_BYTES;
}
- // Scope (pod/cluster/zone) validation
+ // Validate scope
if (podId == null ^ clusterId == null) {
throw new CloudRuntimeException("Cluster Id or Pod Id is null,
cannot create primary storage");
}
- if (podId == null && clusterId == null) {
+
+ if (podId == null) {
if (zoneId != null) {
logger.info("Both Pod Id and Cluster Id are null, Primary
storage pool will be associated with a Zone");
} else {
throw new CloudRuntimeException("Pod Id, Cluster Id and Zone
Id are all null, cannot create primary storage");
}
}
- // Basic parameter validation
- if (StringUtils.isBlank(storagePoolName)) {
+ if (storagePoolName == null || storagePoolName.isEmpty()) {
throw new CloudRuntimeException("Storage pool name is null or
empty, cannot create primary storage");
}
- if (StringUtils.isBlank(providerName)) {
+
+ if (providerName == null || providerName.isEmpty()) {
throw new CloudRuntimeException("Provider name is null or empty,
cannot create primary storage");
}
+
+ PrimaryDataStoreParameters parameters = new
PrimaryDataStoreParameters();
+ if (clusterId != null) {
+ ClusterVO clusterVO = _clusterDao.findById(clusterId);
+ Preconditions.checkNotNull(clusterVO, "Unable to locate the
specified cluster");
+ if (clusterVO.getHypervisorType() !=
Hypervisor.HypervisorType.KVM) {
+ throw new CloudRuntimeException("ONTAP primary storage is
supported only for KVM hypervisor");
+ }
+ parameters.setHypervisorType(clusterVO.getHypervisorType());
+ }
+
logger.debug("ONTAP primary storage will be created as " + (managed ?
"managed" : "unmanaged"));
if (!managed) {
throw new CloudRuntimeException("ONTAP primary storage must be
managed");
}
- // Details key validation
+ //Required ONTAP detail keys
Set<String> requiredKeys = Set.of(
OntapStorageConstants.USERNAME,
OntapStorageConstants.PASSWORD,
OntapStorageConstants.SVM_NAME,
OntapStorageConstants.PROTOCOL,
- OntapStorageConstants.MANAGEMENT_LIF
- );
- Set<String> optionalKeys = Set.of(
- OntapStorageConstants.IS_DISAGGREGATED
+ OntapStorageConstants.STORAGE_IP
);
- Set<String> allowedKeys = new java.util.HashSet<>(requiredKeys);
- allowedKeys.addAll(optionalKeys);
-
- if (StringUtils.isNotBlank(url)) {
- for (String segment : url.split(OntapStorageConstants.SEMICOLON)) {
- if (segment.isEmpty()) {
- continue;
- }
- String[] kv = segment.split(OntapStorageConstants.EQUALS, 2);
- if (kv.length == 2) {
- details.put(kv[0].trim(), kv[1].trim());
- }
- }
- }
+ // Validate existing entries (reject unexpected keys, empty values)
for (Map.Entry<String, String> e : details.entrySet()) {
String key = e.getKey();
String val = e.getValue();
- if (!allowedKeys.contains(key)) {
+ if (!requiredKeys.contains(key)) {
throw new CloudRuntimeException("Unexpected ONTAP detail key
in URL: " + key);
}
- if (StringUtils.isBlank(val)) {
+ if (val == null || val.isEmpty()) {
Review Comment:
Why was `StringUtils.isBlank` replaced here?
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java:
##########
@@ -306,21 +308,24 @@ public boolean attachCluster(DataStore dataStore,
ClusterScope scope) {
logger.error(errMsg);
throw new CloudRuntimeException(errMsg);
}
-
logger.debug("attachCluster: Attaching the pool to each of the host in
the cluster: {}", primaryStore.getClusterId());
- if (hostsIdentifier != null && hostsIdentifier.size() > 0) {
- try {
- AccessGroup accessGroupRequest = new AccessGroup();
- accessGroupRequest.setHostsToConnect(hostsToConnect);
- accessGroupRequest.setScope(scope);
- primaryStore.setDetails(details);
- accessGroupRequest.setPrimaryDataStoreInfo(primaryStore);
- strategy.createAccessGroup(accessGroupRequest);
- } catch (Exception e) {
- logger.error("attachCluster: Failed to create access group on
storage system for cluster: " + primaryStore.getClusterId() + ". Exception: " +
e.getMessage());
- throw new CloudRuntimeException("attachCluster: Failed to
create access group on storage system for cluster: " +
primaryStore.getClusterId() + ". Exception: " + e.getMessage());
+ // We need to create export policy at pool level and igroup at host
level(in grantAccess)
Review Comment:
Lines 312-322 are duplicated in `attachZone`. Can we turn it into a single
method?
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java:
##########
@@ -282,16 +284,16 @@ private long validateInitializeInputs(Long capacityBytes,
Long podId, Long clust
public boolean attachCluster(DataStore dataStore, ClusterScope scope) {
logger.debug("In attachCluster for ONTAP primary storage");
if (dataStore == null) {
- throw new InvalidParameterValueException("attachCluster: dataStore
should not be null");
+ throw new InvalidParameterValueException(" dataStore should not be
null");
Review Comment:
```suggestion
throw new InvalidParameterValueException("dataStore should not
be null");
```
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java:
##########
@@ -65,14 +108,235 @@ public DataTO getTO(DataObject data) {
@Override
public DataStoreTO getStoreTO(DataStore store) { return null; }
+ @Override
+ public boolean volumesRequireGrantAccessWhenUsed(){
Review Comment:
```suggestion
public boolean volumesRequireGrantAccessWhenUsed() {
```
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java:
##########
@@ -411,18 +421,18 @@ private boolean
validateProtocolSupportAndFetchHostsIdentifier(List<HostVO> host
for (HostVO host : hosts) {
if (host != null) {
ip = host.getStorageIpAddress() != null ?
host.getStorageIpAddress().trim() : "";
- if (ip.isEmpty()) {
- if (host.getPrivateIpAddress() == null ||
host.getPrivateIpAddress().trim().isEmpty()) {
- return false;
- }
- ip = host.getPrivateIpAddress().trim();
+ if (ip.isEmpty() &&
StringUtils.isBlank(host.getPrivateIpAddress() )) {
+ // TODO we will inform customer through alert for
excluded host because of protocol enabled on host
Review Comment:
Is this going to be addressed in this PR?
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java:
##########
@@ -401,7 +410,8 @@ private boolean
validateProtocolSupportAndFetchHostsIdentifier(List<HostVO> host
for (HostVO host : hosts) {
if (host == null || host.getStorageUrl() == null ||
host.getStorageUrl().trim().isEmpty()
||
!host.getStorageUrl().startsWith(protocolPrefix)) {
- return false;
+ // TODO we will inform customer through alert for
excluded host because of protocol enabled on host
Review Comment:
Is this going to be addressed in this PR?
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java:
##########
@@ -65,14 +108,235 @@ public DataTO getTO(DataObject data) {
@Override
public DataStoreTO getStoreTO(DataStore store) { return null; }
+ @Override
+ public boolean volumesRequireGrantAccessWhenUsed(){
+ logger.info("OntapPrimaryDatastoreDriver:
volumesRequireGrantAccessWhenUsed: Called");
+ return true;
+ }
+
+ /**
+ * Creates a volume on the ONTAP storage system.
+ */
@Override
public void createAsync(DataStore dataStore, DataObject dataObject,
AsyncCompletionCallback<CreateCmdResult> callback) {
- throw new UnsupportedOperationException("Create operation is not
supported for ONTAP primary storage.");
+ CreateCmdResult createCmdResult = null;
+ String errMsg;
+
+ if (dataObject == null) {
+ throw new InvalidParameterValueException("dataObject should not be
null");
+ }
+ if (dataStore == null) {
+ throw new InvalidParameterValueException("dataStore should not be
null");
+ }
+ if (callback == null) {
+ throw new InvalidParameterValueException("callback should not be
null");
+ }
+
+ try {
+ logger.info("Started for data store name [{}] and data object name
[{}] of type [{}]",
+ dataStore.getName(), dataObject.getName(),
dataObject.getType());
+
+ StoragePoolVO storagePool =
storagePoolDao.findById(dataStore.getId());
+ if (storagePool == null) {
+ logger.error("createAsync: Storage Pool not found for id: " +
dataStore.getId());
+ throw new CloudRuntimeException("Storage Pool not found for
id: " + dataStore.getId());
+ }
+ String storagePoolUuid = dataStore.getUuid();
Review Comment:
`storagePoolUuid` is not used
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java:
##########
@@ -65,14 +108,235 @@ public DataTO getTO(DataObject data) {
@Override
public DataStoreTO getStoreTO(DataStore store) { return null; }
+ @Override
+ public boolean volumesRequireGrantAccessWhenUsed(){
+ logger.info("OntapPrimaryDatastoreDriver:
volumesRequireGrantAccessWhenUsed: Called");
+ return true;
+ }
+
+ /**
+ * Creates a volume on the ONTAP storage system.
+ */
@Override
public void createAsync(DataStore dataStore, DataObject dataObject,
AsyncCompletionCallback<CreateCmdResult> callback) {
- throw new UnsupportedOperationException("Create operation is not
supported for ONTAP primary storage.");
+ CreateCmdResult createCmdResult = null;
+ String errMsg;
+
+ if (dataObject == null) {
+ throw new InvalidParameterValueException("dataObject should not be
null");
+ }
+ if (dataStore == null) {
+ throw new InvalidParameterValueException("dataStore should not be
null");
+ }
+ if (callback == null) {
+ throw new InvalidParameterValueException("callback should not be
null");
+ }
+
+ try {
+ logger.info("Started for data store name [{}] and data object name
[{}] of type [{}]",
+ dataStore.getName(), dataObject.getName(),
dataObject.getType());
+
+ StoragePoolVO storagePool =
storagePoolDao.findById(dataStore.getId());
+ if (storagePool == null) {
+ logger.error("createAsync: Storage Pool not found for id: " +
dataStore.getId());
+ throw new CloudRuntimeException("Storage Pool not found for
id: " + dataStore.getId());
+ }
+ String storagePoolUuid = dataStore.getUuid();
+
+ Map<String, String> details =
storagePoolDetailsDao.listDetailsKeyPairs(dataStore.getId());
+
+ if (dataObject.getType() == DataObjectType.VOLUME) {
Review Comment:
This `if` can be inverted to throw early and reduce the indentation
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java:
##########
@@ -65,14 +108,235 @@ public DataTO getTO(DataObject data) {
@Override
public DataStoreTO getStoreTO(DataStore store) { return null; }
+ @Override
+ public boolean volumesRequireGrantAccessWhenUsed(){
+ logger.info("OntapPrimaryDatastoreDriver:
volumesRequireGrantAccessWhenUsed: Called");
+ return true;
+ }
+
+ /**
+ * Creates a volume on the ONTAP storage system.
+ */
@Override
public void createAsync(DataStore dataStore, DataObject dataObject,
AsyncCompletionCallback<CreateCmdResult> callback) {
- throw new UnsupportedOperationException("Create operation is not
supported for ONTAP primary storage.");
+ CreateCmdResult createCmdResult = null;
+ String errMsg;
+
+ if (dataObject == null) {
+ throw new InvalidParameterValueException("dataObject should not be
null");
+ }
+ if (dataStore == null) {
+ throw new InvalidParameterValueException("dataStore should not be
null");
+ }
+ if (callback == null) {
+ throw new InvalidParameterValueException("callback should not be
null");
+ }
+
+ try {
+ logger.info("Started for data store name [{}] and data object name
[{}] of type [{}]",
+ dataStore.getName(), dataObject.getName(),
dataObject.getType());
+
+ StoragePoolVO storagePool =
storagePoolDao.findById(dataStore.getId());
+ if (storagePool == null) {
+ logger.error("createAsync: Storage Pool not found for id: " +
dataStore.getId());
+ throw new CloudRuntimeException("Storage Pool not found for
id: " + dataStore.getId());
+ }
+ String storagePoolUuid = dataStore.getUuid();
+
+ Map<String, String> details =
storagePoolDetailsDao.listDetailsKeyPairs(dataStore.getId());
+
+ if (dataObject.getType() == DataObjectType.VOLUME) {
+ VolumeInfo volInfo = (VolumeInfo) dataObject;
+
+ // Create the backend storage object (LUN for iSCSI, no-op for
NFS)
+ CloudStackVolume created = createCloudStackVolume(dataStore,
volInfo, details);
+
+ // Update CloudStack volume record with storage pool
association and protocol-specific details
+ VolumeVO volumeVO = volumeDao.findById(volInfo.getId());
+ if (volumeVO != null) {
+ volumeVO.setPoolType(storagePool.getPoolType());
+ volumeVO.setPoolId(storagePool.getId());
+
+ if
(ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL)))
{
+ String lunName = created != null && created.getLun()
!= null ? created.getLun().getName() : null;
+ if (lunName == null) {
+ throw new CloudRuntimeException("Missing LUN name
for volume " + volInfo.getId());
+ }
+
+ // Persist LUN details for future operations (delete,
grant/revoke access)
+ volumeDetailsDao.addDetail(volInfo.getId(),
OntapStorageConstants.LUN_DOT_UUID, created.getLun().getUuid(), false);
+ volumeDetailsDao.addDetail(volInfo.getId(),
OntapStorageConstants.LUN_DOT_NAME, lunName, false);
+ if (created.getLun().getUuid() != null) {
+ volumeVO.setFolder(created.getLun().getUuid());
+ }
+
+ logger.info("createAsync: Created LUN [{}] for volume
[{}]. LUN mapping will occur during grantAccess() to per-host igroup.",
+ lunName, volumeVO.getId());
+ createCmdResult = new CreateCmdResult(lunName, new
Answer(null, true, null));
+ } else if
(ProtocolType.NFS3.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL)))
{
+ createCmdResult = new
CreateCmdResult(volInfo.getUuid(), new Answer(null, true, null));
+ logger.info("createAsync: Managed NFS volume [{}] with
path [{}] associated with pool {}",
+ volumeVO.getId(), volInfo.getUuid(),
storagePool.getId());
+ }
+ volumeDao.update(volumeVO.getId(), volumeVO);
+ }
+ } else {
+ errMsg = "Invalid DataObjectType (" + dataObject.getType() +
") passed to createAsync";
+ logger.error(errMsg);
+ throw new CloudRuntimeException(errMsg);
+ }
+ } catch (Exception e) {
+ errMsg = e.getMessage();
+ logger.error("createAsync: Failed for dataObject name [{}]: {}",
dataObject.getName(), errMsg);
+ createCmdResult = new CreateCmdResult(null, new Answer(null,
false, errMsg));
+ createCmdResult.setResult(e.toString());
+ } finally {
+ if (createCmdResult != null && createCmdResult.isSuccess()) {
+ logger.info("createAsync: Operation completed successfully for
{}", dataObject.getType());
+ }
+ callback.complete(createCmdResult);
+ }
+ }
+
+ /**
+ * Creates a volume on the ONTAP backend.
+ */
+ private CloudStackVolume createCloudStackVolume(DataStore dataStore,
DataObject dataObject, Map<String, String> details) {
+ StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId());
+ if (storagePool == null) {
+ logger.error("createCloudStackVolume: Storage Pool not found for
id: {}", dataStore.getId());
+ throw new CloudRuntimeException("Storage Pool not found for id: "
+ dataStore.getId());
+ }
+
+ StorageStrategy storageStrategy =
OntapStorageUtils.getStrategyByStoragePoolDetails(details);
+
+ if (dataObject.getType() == DataObjectType.VOLUME) {
+ VolumeInfo volumeObject = (VolumeInfo) dataObject;
+ CloudStackVolume cloudStackVolumeRequest =
OntapStorageUtils.createCloudStackVolumeRequestByProtocol(storagePool, details,
volumeObject);
+ return
storageStrategy.createCloudStackVolume(cloudStackVolumeRequest);
+ } else {
+ throw new CloudRuntimeException("Unsupported DataObjectType: " +
dataObject.getType());
+ }
}
+ /**
+ * Deletes a volume or snapshot from the ONTAP storage system.
+ *
+ * <p>For volumes, deletes the backend storage object (LUN for iSCSI,
no-op for NFS).
+ * For snapshots, deletes the FlexVolume snapshot from ONTAP that was
created by takeSnapshot.</p>
+ */
@Override
public void deleteAsync(DataStore store, DataObject data,
AsyncCompletionCallback<CommandResult> callback) {
- throw new UnsupportedOperationException("Delete operation is not
supported for ONTAP primary storage.");
+ CommandResult commandResult = new CommandResult();
+ try {
+ if (store == null || data == null) {
+ throw new CloudRuntimeException("store or data is null");
+ }
+
+ if (data.getType() == DataObjectType.VOLUME) {
+ StoragePoolVO storagePool =
storagePoolDao.findById(store.getId());
+ if (storagePool == null) {
+ logger.error("deleteAsync: Storage Pool not found for id:
" + store.getId());
+ throw new CloudRuntimeException("Storage Pool not found
for id: " + store.getId());
+ }
+ Map<String, String> details =
storagePoolDetailsDao.listDetailsKeyPairs(store.getId());
+ StorageStrategy storageStrategy =
OntapStorageUtils.getStrategyByStoragePoolDetails(details);
+ logger.info("createCloudStackVolumeForTypeVolume: Connection
to Ontap SVM [{}] successful, preparing CloudStackVolumeRequest",
details.get(OntapStorageConstants.SVM_NAME));
+ VolumeInfo volumeInfo = (VolumeInfo) data;
+ CloudStackVolume cloudStackVolumeRequest =
createDeleteCloudStackVolumeRequest(storagePool,details,volumeInfo);
Review Comment:
```suggestion
CloudStackVolume cloudStackVolumeRequest =
createDeleteCloudStackVolumeRequest(storagePool, details, volumeInfo);
```
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java:
##########
@@ -87,6 +97,19 @@ CloudStackVolume updateCloudStackVolume(CloudStackVolume
cloudstackVolume) {
@Override
public void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) {
+ logger.info("deleteCloudStackVolume: Delete cloudstack volume " +
cloudstackVolume);
+ try {
+ // Step 1: Send command to KVM host to delete qcow2 file using
qemu-img
+ Answer answer =
deleteVolumeOnKVMHost(cloudstackVolume.getVolumeInfo());
+ if (answer == null || !answer.getResult()) {
+ String errMsg = answer != null ? answer.getDetails() : "Failed
to delete qcow2 on KVM host";
+ logger.error("deleteCloudStackVolume: " + errMsg);
+ throw new CloudRuntimeException(errMsg);
+ }
+ }catch (Exception e) {
Review Comment:
```suggestion
} catch (Exception e) {
```
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java:
##########
@@ -281,16 +316,32 @@ public Volume createStorageVolume(String volumeName, Long
size) {
}
}
+ /**
+ * Updates ONTAP Flex-Volume
+ * Eligible only for Unified ONTAP storage
+ * throw exception in case of disaggregated ONTAP storage
+ *
+ * @param volume the volume to update
+ * @return the updated Volume object
+ */
public Volume updateStorageVolume(Volume volume) {
return null;
}
+ /**
+ * Delete ONTAP Flex-Volume
+ * Eligible only for Unified ONTAP storage
+ * throw exception in case of disaggregated ONTAP storage
+ *
+ * @param volume the volume to delete
+ */
public void deleteStorageVolume(Volume volume) {
logger.info("Deleting ONTAP volume by name: " + volume.getName() + "
and uuid: " + volume.getUuid());
String authHeader =
OntapStorageUtils.generateAuthHeader(storage.getUsername(),
storage.getPassword());
try {
+ // TODO: Implement lun and file deletion, if any, before deleting
the volume
Review Comment:
Is this going to be addressed in this PR, or separately?
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java:
##########
@@ -224,7 +257,7 @@ public Volume createStorageVolume(String volumeName, Long
size) {
}
String jobUUID = jobResponse.getJob().getUuid();
- Boolean jobSucceeded = jobPollForSuccess(jobUUID);
+ Boolean jobSucceeded = jobPollForSuccess(jobUUID,10, 1);
Review Comment:
```suggestion
Boolean jobSucceeded = jobPollForSuccess(jobUUID, 10, 1);
```
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java:
##########
@@ -98,14 +362,234 @@ public ChapInfo getChapInfo(DataObject dataObject) {
return null;
}
+ /**
+ * Grants a host access to a volume.
+ */
@Override
public boolean grantAccess(DataObject dataObject, Host host, DataStore
dataStore) {
- return false;
+ try {
+ if (dataStore == null) {
+ throw new InvalidParameterValueException("dataStore should not
be null");
+ }
+ if (dataObject == null) {
+ throw new InvalidParameterValueException("dataObject should
not be null");
+ }
+ if (host == null) {
+ throw new InvalidParameterValueException("host should not be
null");
+ }
+
+ StoragePoolVO storagePool =
storagePoolDao.findById(dataStore.getId());
+ if (storagePool == null) {
+ logger.error("grantAccess: Storage Pool not found for id: " +
dataStore.getId());
+ throw new CloudRuntimeException("Storage Pool not found for
id: " + dataStore.getId());
+ }
+ String storagePoolUuid = dataStore.getUuid();
+
+ // ONTAP managed storage only supports cluster and zone scoped
pools
+ if (storagePool.getScope() != ScopeType.CLUSTER &&
storagePool.getScope() != ScopeType.ZONE) {
+ logger.error("grantAccess: Only Cluster and Zone scoped
primary storage is supported for storage Pool: " + storagePool.getName());
+ throw new CloudRuntimeException("Only Cluster and Zone scoped
primary storage is supported for Storage Pool: " + storagePool.getName());
+ }
+
+ if (dataObject.getType() == DataObjectType.VOLUME) {
Review Comment:
This `if` can be inverted to throw early and reduce indentation
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java:
##########
@@ -98,14 +362,234 @@ public ChapInfo getChapInfo(DataObject dataObject) {
return null;
}
+ /**
+ * Grants a host access to a volume.
+ */
@Override
public boolean grantAccess(DataObject dataObject, Host host, DataStore
dataStore) {
- return false;
+ try {
+ if (dataStore == null) {
+ throw new InvalidParameterValueException("dataStore should not
be null");
+ }
+ if (dataObject == null) {
+ throw new InvalidParameterValueException("dataObject should
not be null");
+ }
+ if (host == null) {
+ throw new InvalidParameterValueException("host should not be
null");
+ }
+
+ StoragePoolVO storagePool =
storagePoolDao.findById(dataStore.getId());
+ if (storagePool == null) {
+ logger.error("grantAccess: Storage Pool not found for id: " +
dataStore.getId());
+ throw new CloudRuntimeException("Storage Pool not found for
id: " + dataStore.getId());
+ }
+ String storagePoolUuid = dataStore.getUuid();
Review Comment:
`storagePoolUuid` is not used here
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java:
##########
@@ -128,11 +612,268 @@ public long getUsedIops(StoragePool storagePool) {
return 0;
}
+ /**
+ * Takes a snapshot by creating an ONTAP FlexVolume-level snapshot.
+ *
+ * <p>This method creates a point-in-time, space-efficient snapshot of the
entire
+ * FlexVolume containing the CloudStack volume. FlexVolume snapshots are
atomic
+ * and capture all files/LUNs within the volume at the moment of
creation.</p>
+ *
+ * <p>Both NFS and iSCSI protocols use the same FlexVolume snapshot
approach:
+ * <ul>
+ * <li>NFS: The QCOW2 file is captured within the FlexVolume
snapshot</li>
+ * <li>iSCSI: The LUN is captured within the FlexVolume snapshot</li>
+ * </ul>
+ * </p>
+ *
+ * <p>With {@code STORAGE_SYSTEM_SNAPSHOT=true}, {@code
StorageSystemSnapshotStrategy}
+ * handles the workflow.</p>
+ */
@Override
- public void takeSnapshot(SnapshotInfo snapshot,
AsyncCompletionCallback<CreateCmdResult> callback) {}
+ public void takeSnapshot(SnapshotInfo snapshot,
AsyncCompletionCallback<CreateCmdResult> callback) {
+ logger.info("OntapPrimaryDatastoreDriver.takeSnapshot: Creating
FlexVolume snapshot for snapshot [{}]", snapshot.getId());
+ CreateCmdResult result;
+
+ try {
+ VolumeInfo volumeInfo = snapshot.getBaseVolume();
+
+ VolumeVO volumeVO = volumeDao.findById(volumeInfo.getId());
+ if (volumeVO == null) {
+ throw new CloudRuntimeException("VolumeVO not found for id: "
+ volumeInfo.getId());
+ }
+
+ StoragePoolVO storagePool =
storagePoolDao.findById(volumeVO.getPoolId());
+ if (storagePool == null) {
+ logger.error("takeSnapshot: Storage Pool not found for id:
{}", volumeVO.getPoolId());
+ throw new CloudRuntimeException("Storage Pool not found for
id: " + volumeVO.getPoolId());
+ }
+
+ Map<String, String> poolDetails =
storagePoolDetailsDao.listDetailsKeyPairs(volumeVO.getPoolId());
+ String protocol = poolDetails.get(OntapStorageConstants.PROTOCOL);
+ String flexVolUuid =
poolDetails.get(OntapStorageConstants.VOLUME_UUID);
+ if (flexVolUuid == null || flexVolUuid.isEmpty()) {
+ throw new CloudRuntimeException("FlexVolume UUID not found in
pool details for pool " + volumeVO.getPoolId());
+ }
+
+ StorageStrategy storageStrategy =
OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails);
+ SnapshotFeignClient snapshotClient =
storageStrategy.getSnapshotFeignClient();
+ String authHeader = storageStrategy.getAuthHeader();
+
+ SnapshotObjectTO snapshotObjectTo = (SnapshotObjectTO)
snapshot.getTO();
+
+ // Build snapshot name using volume name and snapshot UUID
+ String snapshotName = buildSnapshotName(volumeInfo.getName(),
snapshot.getUuid());
+
+ // Resolve the volume path for storing in snapshot details (for
revert operation)
+ String volumePath = resolveVolumePathOnOntap(volumeVO, protocol,
poolDetails);
+
+ // For iSCSI, retrieve LUN UUID for restore operations
+ String lunUuid = null;
+ if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol)) {
+ lunUuid = volumeDetailsDao.findDetail(volumeVO.getId(),
OntapStorageConstants.LUN_DOT_UUID) != null
Review Comment:
Extract `volumeDetailsDao.findDetail(volumeVO.getId(),
OntapStorageConstants.LUN_DOT_UUID)` into a variable to avoid callign it twice
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java:
##########
@@ -98,14 +362,234 @@ public ChapInfo getChapInfo(DataObject dataObject) {
return null;
}
+ /**
+ * Grants a host access to a volume.
+ */
@Override
public boolean grantAccess(DataObject dataObject, Host host, DataStore
dataStore) {
- return false;
+ try {
+ if (dataStore == null) {
+ throw new InvalidParameterValueException("dataStore should not
be null");
+ }
+ if (dataObject == null) {
+ throw new InvalidParameterValueException("dataObject should
not be null");
+ }
+ if (host == null) {
+ throw new InvalidParameterValueException("host should not be
null");
+ }
+
+ StoragePoolVO storagePool =
storagePoolDao.findById(dataStore.getId());
+ if (storagePool == null) {
+ logger.error("grantAccess: Storage Pool not found for id: " +
dataStore.getId());
+ throw new CloudRuntimeException("Storage Pool not found for
id: " + dataStore.getId());
+ }
+ String storagePoolUuid = dataStore.getUuid();
+
+ // ONTAP managed storage only supports cluster and zone scoped
pools
+ if (storagePool.getScope() != ScopeType.CLUSTER &&
storagePool.getScope() != ScopeType.ZONE) {
+ logger.error("grantAccess: Only Cluster and Zone scoped
primary storage is supported for storage Pool: " + storagePool.getName());
+ throw new CloudRuntimeException("Only Cluster and Zone scoped
primary storage is supported for Storage Pool: " + storagePool.getName());
+ }
+
+ if (dataObject.getType() == DataObjectType.VOLUME) {
+ VolumeVO volumeVO = volumeDao.findById(dataObject.getId());
+ if (volumeVO == null) {
+ logger.error("grantAccess: CloudStack Volume not found for
id: " + dataObject.getId());
+ throw new CloudRuntimeException("CloudStack Volume not
found for id: " + dataObject.getId());
+ }
+
+ Map<String, String> details =
storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId());
+ String svmName = details.get(OntapStorageConstants.SVM_NAME);
+
+ if
(ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL)))
{
+ // Only retrieve LUN name for iSCSI volumes
+ String cloudStackVolumeName =
volumeDetailsDao.findDetail(volumeVO.getId(),
OntapStorageConstants.LUN_DOT_NAME).getValue();
+ UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy)
OntapStorageUtils.getStrategyByStoragePoolDetails(details);
+ String accessGroupName =
OntapStorageUtils.getIgroupName(svmName, host.getName());
+
+ // Validate if Igroup exist ONTAP for this host as we may
be using delete_on_unmap= true and igroup may be deleted by ONTAP automatically
+ Map<String, String> getAccessGroupMap = Map.of(
+ OntapStorageConstants.NAME, accessGroupName,
+ OntapStorageConstants.SVM_DOT_NAME, svmName
+ );
+ Igroup igroup = new Igroup();
+ AccessGroup accessGroup =
sanStrategy.getAccessGroup(getAccessGroupMap);
+ if(accessGroup == null || accessGroup.getIgroup() == null)
{
+ logger.info("grantAccess: Igroup {} does not exist for
the host {} : Need to create Igroup for the host ", accessGroupName,
host.getName());
+ // create the igroup for the host and perform
lun-mapping
+ accessGroup = new AccessGroup();
+ List<HostVO> hosts = new ArrayList<>();
+ hosts.add((HostVO) host);
+ accessGroup.setHostsToConnect(hosts);
+ accessGroup.setStoragePoolId(storagePool.getId());
+ accessGroup =
sanStrategy.createAccessGroup(accessGroup);
+ }else{
+ logger.info("grantAccess: Igroup {} already exist for
the host {}: ", accessGroup.getIgroup().getName() ,host.getName());
+ igroup = accessGroup.getIgroup();
+ /* TODO Below cases will be covered later, for now
they will be a pre-requisite on customer side
+ 1. Igroup exist with the same name but host
initiator has been rempved
+ 2. Igroup exist with the same name but host
initiator has been changed may be due to new NIC or new adapter
+ In both cases we need to verify current host
initiator is registered in the igroup before allowing access
+ Incase it is not , add it and proceed for lun-mapping
+ */
+ }
+ logger.info("grantAccess: Igroup {} is present now with
initiators {} ", accessGroup.getIgroup().getName(),
accessGroup.getIgroup().getInitiators());
+ // Create or retrieve existing LUN mapping
+ String lunNumber = sanStrategy.ensureLunMapped(svmName,
cloudStackVolumeName, accessGroupName);
+
+ // Update volume path if changed (e.g., after migration or
re-mapping)
+ String iscsiPath = OntapStorageConstants.SLASH +
storagePool.getPath() + OntapStorageConstants.SLASH + lunNumber;
+ if (volumeVO.getPath() == null ||
!volumeVO.getPath().equals(iscsiPath)) {
+ volumeVO.set_iScsiName(iscsiPath);
+ volumeVO.setPath(iscsiPath);
+ }
+ } else if
(ProtocolType.NFS3.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL)))
{
+ // For NFS, no access grant needed - file is accessible
via mount
+ logger.debug("grantAccess: NFS volume [{}], no igroup
mapping required", volumeVO.getUuid());
+ return true;
+ }
+ volumeVO.setPoolType(storagePool.getPoolType());
+ volumeVO.setPoolId(storagePool.getId());
+ volumeDao.update(volumeVO.getId(), volumeVO);
+ } else {
+ logger.error("Invalid DataObjectType (" + dataObject.getType()
+ ") passed to grantAccess");
+ throw new CloudRuntimeException("Invalid DataObjectType (" +
dataObject.getType() + ") passed to grantAccess");
+ }
+ return true;
+ } catch (Exception e) {
+ logger.error("grantAccess: Failed for dataObject [{}]: {}",
dataObject, e.getMessage());
+ throw new CloudRuntimeException("Failed with error: " +
e.getMessage(), e);
+ }
}
+ /**
+ * Revokes a host's access to a volume.
+ */
@Override
public void revokeAccess(DataObject dataObject, Host host, DataStore
dataStore) {
- throw new UnsupportedOperationException("Revoke access operation is
not supported for ONTAP primary storage.");
+ try {
+ if (dataStore == null) {
+ throw new InvalidParameterValueException("dataStore should not
be null");
+ }
+ if (dataObject == null) {
+ throw new InvalidParameterValueException("dataObject should
not be null");
+ }
+ if (host == null) {
+ throw new InvalidParameterValueException("host should not be
null");
+ }
+
+ StoragePoolVO storagePool =
storagePoolDao.findById(dataStore.getId());
+ if (storagePool == null) {
+ logger.error("revokeAccess: Storage Pool not found for id: " +
dataStore.getId());
+ throw new CloudRuntimeException("Storage Pool not found for
id: " + dataStore.getId());
+ }
+
+ if (storagePool.getScope() != ScopeType.CLUSTER &&
storagePool.getScope() != ScopeType.ZONE) {
+ logger.error("revokeAccess: Only Cluster and Zone scoped
primary storage is supported for storage Pool: " + storagePool.getName());
+ throw new CloudRuntimeException("Only Cluster and Zone scoped
primary storage is supported for Storage Pool: " + storagePool.getName());
+ }
+
+ if (dataObject.getType() == DataObjectType.VOLUME) {
+ VolumeVO volumeVO = volumeDao.findById(dataObject.getId());
+ if (volumeVO == null) {
+ logger.error("revokeAccess: CloudStack Volume not found
for id: " + dataObject.getId());
+ throw new CloudRuntimeException("CloudStack Volume not
found for id: " + dataObject.getId());
+ }
+ revokeAccessForVolume(storagePool, volumeVO, host);
+ } else {
+ logger.error("revokeAccess: Invalid DataObjectType (" +
dataObject.getType() + ") passed to revokeAccess");
+ throw new CloudRuntimeException("Invalid DataObjectType (" +
dataObject.getType() + ") passed to revokeAccess");
+ }
+ } catch (Exception e) {
+ logger.error("revokeAccess: Failed for dataObject [{}]: {}",
dataObject, e.getMessage());
+ throw new CloudRuntimeException("Failed with error: " +
e.getMessage(), e);
+ }
+ }
+
+ /**
+ * Revokes volume access for the specified host.
+ */
+ private void revokeAccessForVolume(StoragePoolVO storagePool, VolumeVO
volumeVO, Host host) {
+ logger.info("revokeAccessForVolume: Revoking access to volume [{}] for
host [{}]", volumeVO.getName(), host.getName());
+
+ Map<String, String> details =
storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId());
+ StorageStrategy storageStrategy =
OntapStorageUtils.getStrategyByStoragePoolDetails(details);
+ String svmName = details.get(OntapStorageConstants.SVM_NAME);
+
+ if
(ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL)))
{
+ String accessGroupName = OntapStorageUtils.getIgroupName(svmName,
host.getName());
+
+ // Retrieve LUN name from volume details; if missing, volume may
not have been fully created
+ String lunName = volumeDetailsDao.findDetail(volumeVO.getId(),
OntapStorageConstants.LUN_DOT_NAME) != null ?
Review Comment:
Extract `volumeDetailsDao.findDetail(volumeVO.getId(),
OntapStorageConstants.LUN_DOT_NAME)` into a variable to avoid calling it twice
--
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]