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]

Reply via email to