Skip to content

Fix for NFS3 primary storage pool is failing to come out of maintenan… #71

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sandeeplocharla wants to merge 3 commits into
base: main
Choose a base branch
from
+9 −2
Open

Fix for NFS3 primary storage pool is failing to come out of maintenan… #71

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c57e358
Fix for NFS3 primary storage pool is failing to come out of maintenan…
sandeeplocharla Jun 30, 2026
5aba1d6
Addressed review comments
sandeeplocharla Jul 8, 2026
705151d
Corrected a comment
sandeeplocharla Jul 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions ...hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ public boolean deleteStoragePool(StoragePoolType type, String uuid) {
if (type == StoragePoolType.NetworkFilesystem) {
_haMonitor.removeStoragePool(uuid);
}
boolean deleteStatus = adaptor.deleteStoragePool(uuid);;
boolean deleteStatus = adaptor.deleteStoragePool(uuid);
synchronized (_storagePools) {
_storagePools.remove(uuid);
}
Expand All @@ -426,10 +426,12 @@ public 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

@rajiv-jain-netapp rajiv-jain-netapp Jul 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be applicable to all vendors?

@rajiv-jain-netapp rajiv-jain-netapp Jul 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

??

@rajiv-jain-netapp rajiv-jain-netapp Jul 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since both changes are in common/shared components, have we assessed the impact on other storage vendors? I just want to ensure that these updates don't introduce any unintended behaviour changes outside the ONTAP implementation.

@sandeeplocharla sandeeplocharla Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this impacts all the vendors using this. Currently, apart from our plugin, I've noticed only DefaultPrimary plugin using it. So, I've tested the same with that plugin and confirmed that the same issue exists even there. And these code changes have fixed it.

// delete, libvirt will throw an error.
boolean deleteStatus = adaptor.deleteStoragePool(uuid, details);
Comment thread
sandeeplocharla marked this conversation as resolved.
Comment thread
sandeeplocharla marked this conversation as resolved.
if (type == StoragePoolType.NetworkFilesystem) {
_haMonitor.removeStoragePool(uuid);
}
Comment thread
sandeeplocharla marked this conversation as resolved.
boolean deleteStatus = adaptor.deleteStoragePool(uuid, details);
synchronized (_storagePools) {
_storagePools.remove(uuid);
}
Expand Down
5 changes: 5 additions & 0 deletions ...hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -905,6 +905,11 @@ private boolean destroyStoragePoolHandleException(Connect conn, String uuid)
return false;
}

@Override
public boolean deleteStoragePool(String uuid, Map<String, String> details) {

@piyush5netapp piyush5netapp Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you are not using details at all, why will a client code will use this method and not use public boolean deleteStoragePool(String uuid) ?

@sandeeplocharla sandeeplocharla Jul 31, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public boolean deleteStoragePool(String uuid, Map<String, String> details) is being called within public boolean deleteStoragePool(StoragePoolType type, String uuid, Map<String, String> details) in KVMStoragePoolManager.java. Without this overload method here, the default implementation is being invoked, which just has status change.

return deleteStoragePool(uuid);
}

@Override
public boolean deleteStoragePool(String uuid) {
logger.info("Attempting to remove storage pool " + uuid + " from libvirt");
Expand Down
Loading