Copilot commented on code in PR #14192:
URL: https://github.com/apache/cloudstack/pull/14192#discussion_r4059703623
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java:
##########
@@ -156,6 +159,8 @@ public void createAsync(DataStore dataStore, DataObject
dataObject, AsyncComplet
volumeVO.setPoolType(storagePool.getPoolType());
volumeVO.setPoolId(storagePool.getId());
+ volumeVO.setFormat(getImageFormat(storagePool));
Review Comment:
`createCloudStackVolume` has already created the ONTAP LUN by the time this
format check runs. If the hypervisor or pool type is unsupported (including an
upgraded legacy iSCSI pool), `getImageFormat` throws and the catch path does
not delete the backend object, leaving an orphaned LUN. Resolve/validate the
format before the backend creation.
##########
engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java:
##########
@@ -616,8 +616,8 @@ private void
handleVolumeMigrationFromManagedStorageToNonManagedStorage(VolumeIn
private void verifyFormatWithPoolType(ImageFormat imageFormat,
StoragePoolType poolType) {
if (imageFormat != ImageFormat.VHD && imageFormat != ImageFormat.OVA
&& imageFormat != ImageFormat.QCOW2 &&
!(imageFormat == ImageFormat.RAW && (StoragePoolType.PowerFlex
== poolType ||
- StoragePoolType.FiberChannel == poolType))) {
- throw new CloudRuntimeException(String.format("Only the following
image types are currently supported: %s, %s, %s, %s (for PowerFlex and
FiberChannel)",
+ StoragePoolType.FiberChannel == poolType ||
StoragePoolType.OntapiSCSI == poolType))) {
Review Comment:
This only updates `verifyFormatWithPoolType`;
`handleCreateTemplateFromManagedVolume` has a separate guard at lines 2796-2800
that allows RAW only for PowerFlex and FiberChannel. Because ONTAP iSCSI
volumes are now marked RAW, creating a template from one is still rejected
before the copy. Add `OntapiSCSI` to that guard as well.
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java:
##########
@@ -174,7 +174,7 @@ public DataStore initialize(Map<String, Object> dsInfos) {
logger.info("Setting NFS path for storage pool: " + path + ",
port: " + port + " with mount option: vers=3");
break;
case ISCSI:
- parameters.setType(Storage.StoragePoolType.Iscsi);
+ parameters.setType(Storage.StoragePoolType.OntapiSCSI);
Review Comment:
Existing ONTAP iSCSI pools were persisted as `StoragePoolType.Iscsi` before
this branch changed initialization to `OntapiSCSI`, and no upgrade migration
updates those rows. Such pools now reach `getImageFormat`, which rejects
`Iscsi`, while the RAW/KVM detection lists also only recognize the new enum, so
upgrades can no longer create or correctly classify volumes on existing ONTAP
iSCSI storage. Add a data migration or preserve the legacy type in every
affected path.
--
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]
