winterhazel commented on code in PR #13897:
URL: https://github.com/apache/cloudstack/pull/13897#discussion_r4319963488
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java:
##########
@@ -457,10 +457,12 @@ public synchronized boolean
deleteStoragePool(StoragePoolType type, String uuid)
public boolean deleteStoragePool(StoragePoolType type, String uuid,
Map<String, String> details) {
StorageAdaptor adaptor = getStorageAdaptor(type);
+ // For NetworkFilesystem, libvirt will take care of unmounting the nfs
mount. If nfs mount has been removed before libvirt's pool
+ // delete, libvirt will throw an error.
+ boolean deleteStatus = adaptor.deleteStoragePool(uuid, details);
Review Comment:
@sandeeplocharla I think this specific change (the unmount cancel fix) needs
more discussion and testing, as it impacts other storage pool types as well. It
fixes the NFS case, but seems to break SharedMountPoint storages. With this
change, SMPs will be unmounted by the HA monitor on the next heartbeat after
being put into maintenance, resulting in a "Up but unmounted" storage after the
maintenance is canceled, since CloudStack doesn't remount them automatically.
Could you remove the changes of #14187 from this PR? Then it's good to merge
from my side, and we can discuss this fix in #14187.
--
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]
