[CSTACKEX-328] QoS Support Part 2

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java

@@ -205,7 +205,8 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet
                      *      via this path; bootable recovery is createTemplate(snapshotid) → deploy).
                      *    - Backend: iSCSI → POST /api/storage/luns (clone.source in .snapshot/);
                      *      NFS → POST /api/storage/file/clone with snapshot.name.
-                     *    - IOPS: MIN_IOPS/MAX_IOPS may be on snapshot_details; apply is TODO below.
+                     *    - IOPS: the new volume's min/max (snapshot values, or the root offering when one
+                     *      is supplied). Applied here; the temporary template clone does not get a policy.
                      *
                      * 2) cloneOfTemplate — deploy / create from cached template on this pool.
                      *
@@ -219,9 +220,7 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet
                     CloudStackVolume clonedCloudStackVolume;
                     if (cloneOfSnapshotId != null) {
                         clonedCloudStackVolume = cloneCloudStackVolumeFromSnapshot(
-                                storagePool, volInfo, details, cloneOfSnapshotId);
-                        // TODO(CSTACKEX-306): apply persisted MIN_IOPS / MAX_IOPS from snapshot_details
-                        // onto this CloudStack volume (and ONTAP QoS if applicable) after successful clone.
+                                storagePool, volInfo, details, cloneOfSnapshotId, true);
                     } else if (cloneOfTemplateId != null) {
                         clonedCloudStackVolume = cloneCloudStackVolumeFromTemplate(
                                 storagePool, volInfo, details, cloneOfTemplateId);
@@ -525,9 +524,15 @@ private Long getSnapshotIdForCloning(long volumeId) {
      *
      * <p>Optional grow when the disk offering is larger than the snapshot size (same pattern as
      * clone-from-template).</p>
+     *
+     * <p>When {@code applyQos} is true, the new volume's min and max IOPS are applied the same way
+     * as a template clone: iSCSI carries the policy on the clone request, NFS attaches it after
+     * the file exists. The temporary volume used for {@code createTemplate(snapshotid)} passes
+     * false and is left without a policy.</p>
      */
     private CloudStackVolume cloneCloudStackVolumeFromSnapshot(StoragePoolVO storagePool, VolumeInfo volumeInfo,
-                                                               Map<String, String> details, long csSnapshotId) {
+                                                               Map<String, String> details, long csSnapshotId,
+                                                               boolean applyQos) {
         String snapshotName = requireSnapshotDetail(csSnapshotId, OntapStorageConstants.ONTAP_SNAP_NAME);
         String volumePath = requireSnapshotDetail(csSnapshotId, OntapStorageConstants.VOLUME_PATH);
         String primaryPoolId = requireSnapshotDetail(csSnapshotId, OntapStorageConstants.PRIMARY_POOL_ID);
@@ -546,30 +551,56 @@ private CloudStackVolume cloneCloudStackVolumeFromSnapshot(StoragePoolVO storage
         }
 
         StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(details);
+        boolean iscsi = isIscsi(details);
+        VolumeQosPolicy qosPolicy = null;
 
         logger.info("cloneCloudStackVolumeFromSnapshot: Cloning from CS snapshot [{}] (ONTAP snap [{}], path [{}]) "
                         + "for volume [{}] on pool [{}] protocol [{}]",
                 csSnapshotId, snapshotName, volumePath, volumeInfo.getId(), storagePool.getId(), poolProtocol);
 
-        CloudStackVolume request = isIscsi(details)
-                ? createCloneLunFromSnapshotRequest(storagePool, details, volumeInfo, volumePath, snapshotName)
-                : createCloneFileFromSnapshotRequest(storagePool, details, volumeInfo, volumePath, snapshotName);
-        CloudStackVolume cloned = storageStrategy.cloneCloudStackVolume(request);
-        if (cloned == null) {
-            throw new CloudRuntimeException("ONTAP returned nothing when cloning snapshot [" + csSnapshotId
-                    + "] for volume [" + volumeInfo.getId() + "]");
-        }
+        try {
+            if (applyQos) {
+                verifySufficientIopsForStoragePool(storagePool, volumeInfo.getMinIops(), volumeInfo.getId());
+                Volume.Type volumeType = volumeInfo.getVolumeType();
+                if (volumeType == Volume.Type.DATADISK || volumeType == Volume.Type.ROOT) {
+                    qosPolicy = createQosPolicyIfNeeded(storageStrategy, details,
+                            volumeInfo.getMinIops(), volumeInfo.getMaxIops(), storagePool.getId());
+                }
+            }
 
-        long requestedSize = getDataObjectSizeIncludingHypervisorSnapshotReserve(volumeInfo, storagePool);
-        long snapshotSize = resolveSnapshotSizeBytes(csSnapshotId);
-        if (snapshotSize > 0 && requestedSize > snapshotSize) {
-            logger.info("cloneCloudStackVolumeFromSnapshot: Growing clone of snapshot [{}] from {} to {} bytes for volume [{}]",
-                    csSnapshotId, snapshotSize, requestedSize, volumeInfo.getId());
-            resizeClonedVolumeOrCleanup(storageStrategy, cloned, requestedSize,
-                    "cloneCloudStackVolumeFromSnapshot snapshot [" + csSnapshotId + "] volume [" + volumeInfo.getId() + "]");
-        }
+            CloudStackVolume request = iscsi
+                    ? createCloneLunFromSnapshotRequest(storagePool, details, volumeInfo, volumePath, snapshotName, qosPolicy)
+                    : createCloneFileFromSnapshotRequest(storagePool, details, volumeInfo, volumePath, snapshotName);
+            CloudStackVolume cloned = storageStrategy.cloneCloudStackVolume(request);
+            if (cloned == null) {
+                throw new CloudRuntimeException("ONTAP returned nothing when cloning snapshot [" + csSnapshotId
+                        + "] for volume [" + volumeInfo.getId() + "]");
+            }
+
+            long requestedSize = getDataObjectSizeIncludingHypervisorSnapshotReserve(volumeInfo, storagePool);
+            long snapshotSize = resolveSnapshotSizeBytes(csSnapshotId);
+            if (snapshotSize > 0 && requestedSize > snapshotSize) {
+                logger.info("cloneCloudStackVolumeFromSnapshot: Growing clone of snapshot [{}] from {} to {} bytes for volume [{}]",
+                        csSnapshotId, snapshotSize, requestedSize, volumeInfo.getId());
+                resizeClonedVolumeOrCleanup(storageStrategy, cloned, requestedSize,
+                        "cloneCloudStackVolumeFromSnapshot snapshot [" + csSnapshotId + "] volume [" + volumeInfo.getId() + "]");
+            }
 
-        return cloned;
+            if (!iscsi && qosPolicy != null) {
+                CloudStackVolume cloudStackVolume = createCloudStackVolumeRequestByProtocol(
+                        storagePool, details, volumeInfo, qosPolicy);
+                storageStrategy.updateCloudStackVolume(cloudStackVolume);
+            }
+            if (applyQos) {
+                persistQosPolicyDetails(volumeInfo.getId(), qosPolicy);
+            }
+            return cloned;
+        } catch (Exception e) {
+            if (qosPolicy != null) {
+                storageStrategy.deleteVolumeQosPolicy(qosPolicy.getUuid());
+            }
+            throw e;
+        }
     }
 
     /**
@@ -779,10 +810,24 @@ public void deleteAsync(DataStore store, DataObject data, AsyncCompletionCallbac
                 StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(details);
                 logger.info("createCloudStackVolumeForTypeVolume: Connection to Ontap SVM [{}] successful, preparing CloudStackVolumeRequest", details.get(OntapStorageConstants.SVM_NAME));
                 VolumeInfo volumeInfo = (VolumeInfo) data;
-                VolumeDetailVO qosPolicyDetail = volumeDetailsDao.findDetail(
-                        volumeInfo.getId(), OntapStorageConstants.QOS_POLICY_UUID);
-                CloudStackVolume cloudStackVolumeRequest = createDeleteCloudStackVolumeRequest(storagePool, details, volumeInfo);
-                storageStrategy.deleteCloudStackVolume(cloudStackVolumeRequest);
+                VolumeDetailVO qosPolicyDetail = volumeDetailsDao.findDetail(volumeInfo.getId(), OntapStorageConstants.QOS_POLICY_UUID);
+
+                // NFS file delete leaves the QoS assignment in place unless the file is set to none first.
+                // iSCSI LUN delete already drops object_count.
+                // TODO: revisit this NFS none-policy step when the file is deleted from ONTAP instead of KVM.
+                boolean qosCleared = false;
+                if (!isIscsi(details)) {
+                    qosCleared = clearQosPolicyBeforeDelete(storageStrategy, storagePool, details, volumeInfo, qosPolicyDetail);
+                }
+                try {
+                    CloudStackVolume cloudStackVolumeRequest = createDeleteCloudStackVolumeRequest(storagePool, details, volumeInfo);
+                    storageStrategy.deleteCloudStackVolume(cloudStackVolumeRequest);
+                } catch (Exception e) {
+                    if (qosCleared) {
+                        restoreQosPolicyAfterFailedDelete(storageStrategy, storagePool, details, volumeInfo, qosPolicyDetail);
+                    }
+                    throw e;
+                }
                 if (qosPolicyDetail != null) {
                     deleteUnusedQosPolicy(storageStrategy, qosPolicyDetail.getValue());
                 }
@@ -813,6 +858,51 @@ public void deleteAsync(DataStore store, DataObject data, AsyncCompletionCallbac
         }
     }
 
+    /**
+     * Sets the NFS file QoS policy to {@code none} before the file is deleted.
+     * Failure must not block volume delete.
+     *
+     * @return true when the file was moved off its policy
+     */
+    private boolean clearQosPolicyBeforeDelete(StorageStrategy storageStrategy, StoragePoolVO storagePool,
+                                               Map<String, String> details, VolumeInfo volumeInfo,
+                                               VolumeDetailVO qosPolicyDetail) {
+        if (qosPolicyDetail == null || qosPolicyDetail.getValue() == null) {
+            return false;
+        }
+        VolumeQosPolicy nonePolicy = new VolumeQosPolicy();
+        nonePolicy.setName(OntapStorageConstants.QOS_POLICY_NONE);
+        try {
+            CloudStackVolume request = createCloudStackVolumeRequestByProtocol(storagePool, details, volumeInfo, nonePolicy);
+            storageStrategy.updateCloudStackVolume(request);
+            return true;
+        } catch (Exception e) {
+            logger.warn("Could not clear QoS policy [{}] on volume [{}] before delete: {}",
+                    qosPolicyDetail.getValue(), volumeInfo.getId(), e.getMessage());
+            return false;
+        }
+    }
+
+    /**
+     * Puts the previous QoS policy back on an NFS file when delete fails after the policy was cleared.
+     */
+    private void restoreQosPolicyAfterFailedDelete(StorageStrategy storageStrategy, StoragePoolVO storagePool,
+                                                   Map<String, String> details, VolumeInfo volumeInfo,
+                                                   VolumeDetailVO qosPolicyDetail) {
+        if (qosPolicyDetail == null || qosPolicyDetail.getValue() == null) {
+            return;
+        }
+        VolumeQosPolicy previousPolicy = new VolumeQosPolicy();
+        previousPolicy.setUuid(qosPolicyDetail.getValue());
+        try {
+            CloudStackVolume request = createCloudStackVolumeRequestByProtocol(storagePool, details, volumeInfo, previousPolicy);
+            storageStrategy.updateCloudStackVolume(request);
+        } catch (Exception e) {
+            logger.warn("Could not restore QoS policy [{}] on volume [{}] after delete failed: {}",
+                    qosPolicyDetail.getValue(), volumeInfo.getId(), e.getMessage());
+        }
+    }
+
     /**
      * Orchestrates CloudStack volume-snapshot delete on ONTAP.
      *
@@ -984,7 +1074,7 @@ private CopyCommandResult copySnapshotToTemporaryVolume(DataObject srcData, Volu
             renameTemporarySnapshotCopy(volInfo, volumeVO, srcData.getId());
             logger.info("copySnapshotToTemporaryVolume: Cloning CS snapshot [{}] into temporary volume [{}] on pool [{}] for template creation",
                     srcData.getId(), volInfo.getId(), storagePool.getId());
-            CloudStackVolume cloned = cloneCloudStackVolumeFromSnapshot(storagePool, volInfo, details, srcData.getId());
+            CloudStackVolume cloned = cloneCloudStackVolumeFromSnapshot(storagePool, volInfo, details, srcData.getId(), false);
             if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL))) {
                 volumeVO.setPath(volInfo.getUuid());
             }
@@ -1825,7 +1915,8 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback<CreateCm
             snapshotDetails.put(OntapStorageConstants.PROTOCOL, protocol);
             // iSCSI only — needed for LUN restore / identity; null (skipped) for NFS.
             snapshotDetails.put(OntapStorageConstants.LUN_DOT_UUID, lunUuid);
-            // TODO(CSTACKEX-306): on create-volume-from-snapshot, read these and apply to the new volume.
+            // Source-disk IOPS at snapshot time. Create-volume-from-snapshot applies the new volume's
+            // min/max (these values, or a root disk offering) rather than reading this detail back.
             putPositiveIopsDetail(snapshotDetails, OntapStorageConstants.MIN_IOPS, volumeVO.getMinIops());
             putPositiveIopsDetail(snapshotDetails, OntapStorageConstants.MAX_IOPS, volumeVO.getMaxIops());
             updateSnapshotDetails(snapshot.getId(), snapshotDetails);
@@ -2252,7 +2343,7 @@ private CloudStackVolume createCloneFileRequest(StoragePoolVO storagePool, Volum
      */
     private CloudStackVolume createCloneLunFromSnapshotRequest(StoragePoolVO storagePool, Map<String, String> details,
                                                                VolumeInfo volumeInfo, String sourceVolumePath,
-                                                               String snapshotName) {
+                                                               String snapshotName, VolumeQosPolicy qosPolicy) {
         String lunName = volumeInfo.getName().replace(OntapStorageConstants.HYPHEN, OntapStorageConstants.UNDERSCORE);
         if (!OntapStorageUtils.isValidName(lunName)) {
             throw new CloudRuntimeException("Invalid dataObject name [" + lunName
@@ -2276,6 +2367,12 @@ private CloudStackVolume createCloneLunFromSnapshotRequest(StoragePoolVO storage
         lunRequest.setSvm(svm);
         lunRequest.setName(OntapStorageUtils.getLunName(storagePool.getName(), lunName));
         lunRequest.setClone(clone);
+        if (qosPolicy != null) {
+            VolumeQosPolicy qosPolicyReference = new VolumeQosPolicy();
+            qosPolicyReference.setName(qosPolicy.getName());
+            qosPolicyReference.setUuid(qosPolicy.getUuid());
+            lunRequest.setQosPolicy(qosPolicyReference);
+        }
 
         CloudStackVolume request = new CloudStackVolume();
         request.setLun(lunRequest);

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java

@@ -46,6 +46,7 @@
 import org.apache.cloudstack.storage.feign.model.OntapStorage;
 import org.apache.cloudstack.storage.feign.model.Svm;
 import org.apache.cloudstack.storage.feign.model.Volume;
+import org.apache.cloudstack.storage.feign.model.VolumeQosPolicy;
 import org.apache.cloudstack.storage.feign.model.response.JobResponse;
 import org.apache.cloudstack.storage.feign.model.response.OntapResponse;
 import org.apache.cloudstack.storage.service.model.AccessGroup;
@@ -139,19 +140,23 @@ public CloudStackVolume createTemplateCache(StoragePoolVO storagePool, TemplateI
     public CloudStackVolume updateCloudStackVolume(CloudStackVolume cloudstackVolume) {
         if (cloudstackVolume == null || cloudstackVolume.getVolumeInfo() == null
                 || cloudstackVolume.getFlexVolumeUuid() == null || cloudstackVolume.getFile() == null) {
-            throw new CloudRuntimeException("Invalid NFS volume QoS update request");
+            throw new CloudRuntimeException("Invalid NFS volume request");
+        }
+        VolumeQosPolicy qosPolicy = cloudstackVolume.getFile().getQosPolicy();
+        if (qosPolicy == null) {
+            return cloudstackVolume;
         }
         FileInfo fileInfo = new FileInfo();
-        fileInfo.setQosPolicy(cloudstackVolume.getFile().getQosPolicy());
+        fileInfo.setQosPolicy(qosPolicy);
         String filePath = cloudstackVolume.getVolumeInfo().getUuid();
         try {
             nasFeignClient.updateFile(getAuthHeader(), cloudstackVolume.getFlexVolumeUuid(), filePath, fileInfo);
         } catch (FeignException e) {
-            throw new CloudRuntimeException("Failed to apply QoS policy to NFS volume file: " + e.getMessage(), e);
+            throw new CloudRuntimeException("Failed to update NFS volume file: " + e.getMessage(), e);
+        } catch (Exception e) {
+            throw new CloudRuntimeException("Failed to update NFS volume file: " + e.getMessage(), e);
         }
-        logger.info("Applied QoS policy [{}] to NFS volume file [{}]",
-                cloudstackVolume.getFile().getQosPolicy() != null
-                        ? cloudstackVolume.getFile().getQosPolicy().getName() : null, filePath);
+        logger.info("Updated NFS volume file [{}] with QoS policy [{}]", filePath, qosPolicy.getName());
         return cloudstackVolume;
     }
 

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java

@@ -196,7 +196,28 @@ private void bestEffortDeleteTemplateCacheLun(String svmName, String lunName, St
 
     @Override
     public CloudStackVolume updateCloudStackVolume(CloudStackVolume cloudstackVolume) {
-        return null;
+        if (cloudstackVolume == null || cloudstackVolume.getLun() == null
+                || cloudstackVolume.getLun().getUuid() == null) {
+            throw new CloudRuntimeException("Invalid iSCSI volume request");
+        }
+        VolumeQosPolicy qosPolicy = cloudstackVolume.getLun().getQosPolicy();
+        if (qosPolicy == null) {
+            return cloudstackVolume;
+        }
+        Lun lunUpdate = new Lun();
+        lunUpdate.setQosPolicy(qosPolicy);
+        try {
+            JobResponse response = sanFeignClient.updateLun(
+                    getAuthHeader(), cloudstackVolume.getLun().getUuid(), lunUpdate);
+            pollJobIfPresent(response, "update LUN [" + cloudstackVolume.getLun().getUuid() + "]");
+        } catch (FeignException e) {
+            throw new CloudRuntimeException("Failed to update LUN: " + e.getMessage(), e);
+        } catch (Exception e) {
+            throw new CloudRuntimeException("Failed to update LUN: " + e.getMessage(), e);
+        }
+        logger.info("Updated LUN [{}] with QoS policy [{}]",
+                cloudstackVolume.getLun().getUuid(), qosPolicy.getName());
+        return cloudstackVolume;
     }
 
     @Override
@@ -326,7 +347,9 @@ public void resizeCloudStackVolume(CloudStackVolume cloudstackVolume, long sizeI
             lunSpace.setSize(sizeInBytes);
             Lun lunPatch = new Lun();
             lunPatch.setSpace(lunSpace);
-            lunPatch.setQosPolicy(qosPolicy);
+            if (qosPolicy != null) {
+                lunPatch.setQosPolicy(qosPolicy);
+            }
             JobResponse response = sanFeignClient.updateLun(authHeader, lunUuid, lunPatch);
             pollJobIfPresent(response, "resize Lun [" + lunUuid + "]");
             logger.debug("resizeCloudStackVolume: Lun {} resized to {} bytes", lunUuid, sizeInBytes);