Skip to content

Commit be02af2

Browse files
sandeeplocharlasandeeplocharla
committed
Changes to support migration of volumes with QoS
1 parent 200aef3 commit be02af2

5 files changed

Lines changed: 222 additions & 82 deletions

File tree

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

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,90 @@ private void persistQosPolicyDetails(long volumeId, VolumeQosPolicy qosPolicy) {
376376
volumeDetailsDao.addDetail(volumeId, OntapStorageConstants.QOS_POLICY_UUID, qosPolicy.getUuid(), false);
377377
}
378378

379+
/**
380+
* Creates (or reuses) the SVM-scoped QoS policy for the volume's min/max IOPS and attaches it
381+
* to the ONTAP file or LUN. Called after same-SVM iSCSI LUN copy (no policy on create) and
382+
* after NFS content rewrite (qemu-img / live migrate clears file QoS).
383+
*/
384+
public void applyQosPolicyToVolume(VolumeInfo volumeInfo) {
385+
if (volumeInfo == null) {
386+
return;
387+
}
388+
StoragePoolVO storagePool = storagePoolDao.findById(volumeInfo.getPoolId());
389+
if (storagePool == null
390+
|| !DataStoreProvider.ONTAP_PLUGIN_NAME.equals(storagePool.getStorageProviderName())) {
391+
return;
392+
}
393+
Volume.Type volumeType = volumeInfo.getVolumeType();
394+
if (volumeType != Volume.Type.DATADISK && volumeType != Volume.Type.ROOT) {
395+
return;
396+
}
397+
Map<String, String> details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId());
398+
StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(details);
399+
Long minIops = volumeInfo.getMinIops();
400+
Long maxIops = volumeInfo.getMaxIops();
401+
if (!hasQosIops(minIops, maxIops)) {
402+
// findById(id) can return the same cached object the volume already holds, with empty IOPS.
403+
VolumeVO storedVolume = volumeDao.findById(volumeInfo.getId(), true);
404+
if (storedVolume != null) {
405+
minIops = storedVolume.getMinIops();
406+
maxIops = storedVolume.getMaxIops();
407+
if (hasQosIops(minIops, maxIops)) {
408+
logger.info("applyQosPolicyToVolume: volume [{}] reported no IOPS; using stored min [{}] max [{}]",
409+
volumeInfo.getId(), minIops, maxIops);
410+
}
411+
}
412+
}
413+
VolumeQosPolicy qosPolicy = createQosPolicyIfNeeded(storageStrategy, details,
414+
minIops, maxIops, storagePool.getId());
415+
if (qosPolicy == null) {
416+
logger.warn("applyQosPolicyToVolume: volume [{}] has no IOPS (min [{}], max [{}]); ONTAP object keeps its current QoS",
417+
volumeInfo.getId(), minIops, maxIops);
418+
return;
419+
}
420+
CloudStackVolume request = createCloudStackVolumeRequestByProtocol(
421+
storagePool, details, volumeInfo, qosPolicy);
422+
request.setVolumeInfo(volumeInfo);
423+
if (isIscsi(details) && request.getLun() != null && request.getLun().getUuid() == null) {
424+
VolumeDetailVO lunUuidDetail = volumeDetailsDao.findDetail(volumeInfo.getId(),
425+
OntapStorageConstants.LUN_DOT_UUID);
426+
if (lunUuidDetail != null && StringUtils.isNotBlank(lunUuidDetail.getValue())) {
427+
request.getLun().setUuid(lunUuidDetail.getValue());
428+
}
429+
}
430+
storageStrategy.updateCloudStackVolume(request);
431+
persistQosPolicyDetails(volumeInfo.getId(), qosPolicy);
432+
}
433+
434+
/**
435+
* The destination row is duplicated before the LUN copy. If that object has no IOPS, use the
436+
* source volume's IOPS so the policy attached after {@code copy.source} matches the disk.
437+
*/
438+
private void copyQosIopsFromSourceWhenDestinationHasNone(VolumeInfo srcVolumeInfo, VolumeInfo destVolumeInfo) {
439+
if (srcVolumeInfo == null || destVolumeInfo == null
440+
|| hasQosIops(destVolumeInfo.getMinIops(), destVolumeInfo.getMaxIops())) {
441+
return;
442+
}
443+
VolumeVO destVolume = volumeDao.findById(destVolumeInfo.getId());
444+
if (destVolume != null && hasQosIops(destVolume.getMinIops(), destVolume.getMaxIops())) {
445+
return;
446+
}
447+
if (!hasQosIops(srcVolumeInfo.getMinIops(), srcVolumeInfo.getMaxIops()) || destVolume == null) {
448+
return;
449+
}
450+
destVolume.setMinIops(srcVolumeInfo.getMinIops());
451+
destVolume.setMaxIops(srcVolumeInfo.getMaxIops());
452+
volumeDao.update(destVolume.getId(), destVolume);
453+
logger.info("copyAsync: destination volume [{}] had no IOPS; using source volume [{}] min [{}] max [{}] for QoS",
454+
destVolumeInfo.getId(), srcVolumeInfo.getId(), srcVolumeInfo.getMinIops(), srcVolumeInfo.getMaxIops());
455+
}
456+
457+
private boolean hasQosIops(Long minIops, Long maxIops) {
458+
long min = minIops == null ? 0 : minIops;
459+
long max = maxIops == null ? 0 : maxIops;
460+
return min > 0 || max > 0;
461+
}
462+
379463
/**
380464
* Creates the backend object that caches a template on this pool's FlexVolume.
381465
*
@@ -945,6 +1029,10 @@ public void copyAsync(DataObject srcData, DataObject destData, Host destHost, As
9451029
destDetails, StorageManager.KvmStorageOfflineMigrationWait.value());
9461030

9471031
String copiedPath = persistCopiedVolume(destVolumeInfo, destStoragePool, copiedVolume);
1032+
// Same-SVM iSCSI copy.source creates the LUN without a QoS policy. The destination
1033+
// volume object can report no IOPS even when the row (or the source volume) has them.
1034+
copyQosIopsFromSourceWhenDestinationHasNone(srcVolumeInfo, destVolumeInfo);
1035+
applyQosPolicyToVolume(destVolumeInfo);
9481036
copiedVolumeTO = (VolumeObjectTO)destVolumeInfo.getTO();
9491037
copiedVolumeTO.setPath(copiedPath);
9501038
copiedVolumeTO.setFormat(getImageFormat(destStoragePool));
@@ -2307,6 +2395,7 @@ private CloudStackVolume createCloudStackVolumeRequestByProtocol(StoragePoolVO s
23072395
Svm svm = new Svm();
23082396
svm.setName(details.get(OntapStorageConstants.SVM_NAME));
23092397
CloudStackVolume iscsiRequest = new CloudStackVolume();
2398+
iscsiRequest.setVolumeInfo(volumeObject);
23102399
Lun lunRequest = new Lun();
23112400
lunRequest.setSvm(svm);
23122401

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,9 @@ public interface SANFeignClient {
7979
@Headers({"Authorization: {authHeader}"})
8080
void deleteIgroup(@Param("authHeader") String authHeader, @Param("uuid") String uuid);
8181

82-
// LUN Maps Operation APIs
83-
@RequestLine("POST /api/protocols/san/lun-maps")
84-
@Headers({"Authorization: {authHeader}", "return_records: {returnRecords}"})
82+
// LUN Maps Operation APIs. Callers pass return_records false; the host path uses the LUN serial number.
83+
@RequestLine("POST /api/protocols/san/lun-maps?return_records={returnRecords}")
84+
@Headers({"Authorization: {authHeader}"})
8585
OntapResponse<LunMap> createLunMap(@Param("authHeader") String authHeader, @Param("returnRecords") boolean returnRecords, LunMap lunMap);
8686

8787

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/motion/OntapDataMotionStrategy.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import org.apache.cloudstack.storage.command.CopyCommand;
4545
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
4646
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
47+
import org.apache.cloudstack.storage.driver.OntapPrimaryDatastoreDriver;
4748
import org.apache.cloudstack.storage.to.VolumeObjectTO;
4849
import org.apache.commons.lang3.StringUtils;
4950
import org.apache.logging.log4j.LogManager;
@@ -176,6 +177,8 @@ public void copyAsync(DataObject srcData, DataObject destData, Host destHost, As
176177
volumeVO.setFormat(ImageFormat.QCOW2);
177178
}
178179
_volumeDao.update(volumeVO.getId(), volumeVO);
180+
// Host-side rewrite clears NFS file QoS; re-attach from the destination volume's IOPS.
181+
reapplyOntapQosAfterContentRewrite(destVolumeInfo);
179182
} catch (Exception ex) {
180183
errMsg = "Primary storage migration failed due to an unexpected error: " + ex.getMessage();
181184
if (ex instanceof CloudRuntimeException) {
@@ -544,6 +547,9 @@ private void handlePostMigration(Map<VolumeInfo, VolumeInfo> srcVolumeInfoToDest
544547
volumeVO.setLastId(srcVolumeInfo.getId());
545548
_volumeDao.update(volumeVO.getId(), volumeVO);
546549

550+
// Live migrate rewrites the NFS qcow2 and clears file QoS; re-attach before source cleanup.
551+
reapplyOntapQosAfterContentRewrite(destVolumeInfo);
552+
547553
if (srcPoolVO != null && srcPoolVO.getPoolType() == StoragePoolType.OntapiSCSI) {
548554
try {
549555
disconnectHostFromVolume(srcHost, srcVolumeInfo.getPoolId(), srcVolumeInfo.get_iScsiName());
@@ -556,6 +562,27 @@ private void handlePostMigration(Map<VolumeInfo, VolumeInfo> srcVolumeInfoToDest
556562
}
557563
}
558564

565+
/**
566+
* NFS createAsync attaches QoS, then qemu-img / libvirt live migrate overwrites the file and
567+
* drops the policy. Re-apply from the destination volume's stored IOPS. iSCSI LUN QoS survives
568+
* block copy; same-SVM offline LUN copy attaches in the driver instead.
569+
*/
570+
private void reapplyOntapQosAfterContentRewrite(VolumeInfo destVolumeInfo) {
571+
if (destVolumeInfo == null || destVolumeInfo.getDataStore() == null) {
572+
return;
573+
}
574+
StoragePoolVO destPool = _storagePoolDao.findById(destVolumeInfo.getPoolId());
575+
if (destPool == null || destPool.getPoolType() != StoragePoolType.NetworkFilesystem
576+
|| !DataStoreProvider.ONTAP_PLUGIN_NAME.equals(destPool.getStorageProviderName())) {
577+
return;
578+
}
579+
if (!(destVolumeInfo.getDataStore().getDriver() instanceof OntapPrimaryDatastoreDriver)) {
580+
return;
581+
}
582+
VolumeInfo refreshed = _volumeDataFactory.getVolume(destVolumeInfo.getId(), destVolumeInfo.getDataStore());
583+
((OntapPrimaryDatastoreDriver)destVolumeInfo.getDataStore().getDriver()).applyQosPolicyToVolume(refreshed);
584+
}
585+
559586
private void handleFailedMigration(Map<VolumeInfo, VolumeInfo> srcVolumeInfoToDestVolumeInfo, VirtualMachineTO vmTO, Host destHost,
560587
boolean preparedForMigration) {
561588
if (preparedForMigration) {

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

Lines changed: 58 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -200,9 +200,47 @@ private void bestEffortDeleteTemplateCacheLun(String svmName, String lunName, St
200200
}
201201
}
202202

203+
/**
204+
* Patches the QoS policy onto an existing LUN. Used after same-SVM LUN copy
205+
* ({@code copy.source} creates the destination without a policy) and after resize
206+
* when only the policy needs updating.
207+
*/
203208
@Override
204209
public CloudStackVolume updateCloudStackVolume(CloudStackVolume cloudstackVolume) {
205-
return null;
210+
if (cloudstackVolume == null || cloudstackVolume.getVolumeInfo() == null) {
211+
throw new CloudRuntimeException("Invalid LUN QoS update request");
212+
}
213+
VolumeQosPolicy qosPolicy = cloudstackVolume.getLun() != null ? cloudstackVolume.getLun().getQosPolicy() : null;
214+
if (qosPolicy == null) {
215+
throw new CloudRuntimeException("QoS policy required for LUN update");
216+
}
217+
if (cloudstackVolume.getLun().getUuid() == null) {
218+
long volumeId = cloudstackVolume.getVolumeInfo().getId();
219+
VolumeDetailVO lunUuidDetail = volumeDetailsDao.findDetail(volumeId, OntapStorageConstants.LUN_DOT_UUID);
220+
if (lunUuidDetail == null || StringUtils.isBlank(lunUuidDetail.getValue())) {
221+
throw new CloudRuntimeException("LUN UUID not found in volume details for volume " + volumeId);
222+
}
223+
cloudstackVolume.getLun().setUuid(lunUuidDetail.getValue());
224+
}
225+
String lunUuid = cloudstackVolume.getLun().getUuid();
226+
try {
227+
String authHeader = OntapStorageUtils.generateAuthHeader(storage.getUsername(), storage.getPassword());
228+
Lun lunPatch = new Lun();
229+
lunPatch.setQosPolicy(qosPolicy);
230+
JobResponse response = sanFeignClient.updateLun(authHeader, lunUuid, lunPatch);
231+
pollJobIfPresent(response, "update Lun QoS [" + lunUuid + "]");
232+
logger.info("Applied QoS policy [{}] to LUN [{}]", qosPolicy.getName(), lunUuid);
233+
return cloudstackVolume;
234+
} catch (FeignException e) {
235+
logger.error("FeignException while applying QoS to LUN [{}], Status: {}, Exception: {}",
236+
lunUuid, e.status(), e.getMessage());
237+
throw new CloudRuntimeException("Failed to apply QoS policy to LUN [" + lunUuid + "]: " + e.getMessage(), e);
238+
} catch (CloudRuntimeException e) {
239+
throw e;
240+
} catch (Exception e) {
241+
logger.error("Exception while applying QoS to LUN [{}]: {}", lunUuid, e.getMessage());
242+
throw new CloudRuntimeException("Failed to apply QoS policy to LUN [" + lunUuid + "]: " + e.getMessage(), e);
243+
}
206244
}
207245

208246
@Override
@@ -345,6 +383,8 @@ private CloudStackVolume buildLunCopyRequest(VolumeInfo srcVolumeInfo, VolumeInf
345383
lun.setSvm(svm);
346384
lun.setName(OntapStorageUtils.getLunName(destStoragePool.getName(), destLunName));
347385
lun.setCopy(copy);
386+
// ONTAP rejects qos_policy.name on a copy.source create. The driver patches the
387+
// policy onto the new LUN after this copy returns.
348388

349389
CloudStackVolume request = new CloudStackVolume();
350390
request.setLun(lun);
@@ -648,7 +688,19 @@ public AccessGroup createAccessGroup(AccessGroup accessGroup) {
648688
} catch (FeignException feignEx) {
649689
if (feignEx.status() == 409) {
650690
logger.warn("createAccessGroup: Igroup with name {} already exists (status 409). Fetching existing Igroup.", igroupName);
651-
// TODO: Currently we aren't doing anything with the returned AccessGroup object, so, haven't added code here to fetch the existing Igroup and set it in AccessGroup.
691+
String lookupAuth = OntapStorageUtils.generateAuthHeader(storage.getUsername(), storage.getPassword());
692+
Map<String, Object> igroupParams = Map.of(
693+
OntapStorageConstants.SVM_DOT_NAME, svmName,
694+
OntapStorageConstants.NAME, igroupName);
695+
OntapResponse<Igroup> existing = sanFeignClient.getIgroupResponse(lookupAuth, igroupParams);
696+
if (existing == null || existing.getRecords() == null || existing.getRecords().isEmpty()) {
697+
String ontapError = feignEx.contentUTF8();
698+
if (StringUtils.isBlank(ontapError)) {
699+
ontapError = feignEx.getMessage();
700+
}
701+
throw new CloudRuntimeException("Igroup [" + igroupName + "] already exists but was not returned: " + ontapError);
702+
}
703+
createdAccessGroup.setIgroup(existing.getRecords().get(0));
652704
return createdAccessGroup;
653705
}
654706
logger.error("createAccessGroup: FeignException during Igroup creation: Status: {}, Exception: {}", feignEx.status(), feignEx.getMessage(), feignEx);
@@ -828,7 +880,8 @@ public String enableLogicalAccess(Map<String, String> values) {
828880
igroup.setName(igroupName);
829881
lunMapRequest.setIgroup(igroup);
830882
try {
831-
sanFeignClient.createLunMap(authHeader, true, lunMapRequest);
883+
// return_records is false. The host path comes from the LUN serial number, not this response.
884+
sanFeignClient.createLunMap(authHeader, false, lunMapRequest);
832885
} catch (Exception feignEx) {
833886
String errMsg = feignEx.getMessage();
834887
if (errMsg != null && errMsg.contains(("LUN already mapped to this group"))) {
@@ -838,29 +891,8 @@ public String enableLogicalAccess(Map<String, String> values) {
838891
throw feignEx;
839892
}
840893
}
841-
// Get the LunMap poolDetails
842-
OntapResponse<LunMap> lunMapResponse = null;
843-
try {
844-
lunMapResponse = sanFeignClient.getLunMapResponse(authHeader,
845-
Map.of(
846-
OntapStorageConstants.SVM_DOT_NAME, svmName,
847-
OntapStorageConstants.LUN_DOT_NAME, lunName,
848-
OntapStorageConstants.IGROUP_DOT_NAME, igroupName,
849-
OntapStorageConstants.FIELDS, OntapStorageConstants.LOGICAL_UNIT_NUMBER
850-
));
851-
if(lunMapResponse != null && lunMapResponse.getRecords() != null && !lunMapResponse.getRecords().isEmpty()
852-
&& lunMapResponse.getRecords().get(0).getLogicalUnitNumber() != null) {
853-
lunNumber = lunMapResponse.getRecords().get(0).getLogicalUnitNumber().toString();
854-
855-
} else {
856-
logger.error("enableLogicalAccess: Failed to fetch LunMap poolDetails for Lun: {} and igroup: {}. LunMap response is null or empty.", lunName, igroupName);
857-
throw new CloudRuntimeException("Failed to fetch LunMap poolDetails for Lun: " + lunName + " and igroup: " + igroupName);
858-
}
859-
} catch (Exception e) {
860-
logger.error("enableLogicalAccess: Failed to fetch LunMap poolDetails for Lun: {} and igroup: {}, Exception: {}", lunName, igroupName, e);
861-
throw new CloudRuntimeException("Failed to fetch LunMap poolDetails for Lun: " + lunName + " and igroup: " + igroupName);
862-
}
863-
logger.trace("enableLogicalAccess: LunMap created successfully, LunMap: {}", lunMapResponse.getRecords().get(0));
894+
logger.trace("enableLogicalAccess: LunMap created for Lun: {} and igroup: {}", lunName, igroupName);
895+
lunNumber = "";
864896
} catch (Exception e) {
865897
logger.error("Exception occurred while creating LunMap", e);
866898
throw new CloudRuntimeException("Failed to create LunMap: " + e.getMessage());

0 commit comments

Comments
 (0)