Copilot commented on code in PR #13578:
URL: https://github.com/apache/cloudstack/pull/13578#discussion_r3559567618
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -5092,6 +5093,13 @@ private VolumeVO sendAttachVolumeCommand(UserVmVO vm,
VolumeVO volumeToAttach, L
throw new CloudRuntimeException(e.getMessage());
}
+
+ // Reload volume from DB after grantAccess — managed storage
drivers (e.g. ONTAP)
+ // may update the volume's path and iScsiName during
grantAccess, so the local
+ // volumeToAttach object can be stale.
+
if(DataStoreProvider.ONTAP_PLUGIN_NAME.equals(volumeToAttachStoragePool.getStorageProviderName())){
+ volumeToAttach = _volsDao.findById(volumeToAttach.getId());
+ }
Review Comment:
After grantAccess(), volumeToAttach is conditionally reloaded from the DB.
If _volsDao.findById(...) returns null (e.g., volume removed concurrently), the
subsequent dereferences of volumeToAttach (getPath/get_iScsiName/etc.) will
throw a NullPointerException. Add a null-check and fail fast with a clear
CloudRuntimeException.
##########
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java:
##########
@@ -1363,11 +1364,13 @@ private void
createManagedVolumeCopyTemplateAsync(VolumeInfo volumeInfo, Primary
primaryDataStore.setDetails(details);
grantAccess(volumeInfo, destHost, primaryDataStore);
- volumeInfo = volFactory.getVolume(volumeInfo.getId(),
primaryDataStore);
- // For Netapp ONTAP iscsiName or Lun path is available only after
grantAccess
- String managedStoreTarget =
ObjectUtils.defaultIfNull(volumeInfo.get_iScsiName(), volumeInfo.getUuid());
- details.put(PrimaryDataStore.MANAGED_STORE_TARGET,
managedStoreTarget);
- primaryDataStore.setDetails(details);
+ if
(DataStoreProvider.ONTAP_PLUGIN_NAME.equals(primaryDataStore.getStorageProviderName()))
{
+ // For Netapp ONTAP iscsiName or Lun path is available only
after grantAccess
+ volumeInfo = volFactory.getVolume(volumeInfo.getId(),
primaryDataStore);
+ String managedStoreTarget =
ObjectUtils.defaultIfNull(volumeInfo.get_iScsiName(), volumeInfo.getUuid());
+ details.put(PrimaryDataStore.MANAGED_STORE_TARGET,
managedStoreTarget);
Review Comment:
createManagedVolumeCopyTemplateAsync() previously refreshed
MANAGED_STORE_TARGET after grantAccess() (with a UUID fallback). The new
ONTAP-only conditional means non-ONTAP managed storage no longer gets this
post-grantAccess update, so PrimaryDataStore.MANAGED_STORE_TARGET can remain
null/stale if iScsiName is not populated before grantAccess; hypervisor storage
processors (e.g., VMware) use this value directly. Update MANAGED_STORE_TARGET
for all providers, while only reloading volumeInfo from DB for ONTAP if needed.
--
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]
