Skip to content

Commit c5210eb

Browse files
Addressed the comments received on upstream PR for refactoring the fix for NFS Cancel maintenance (#131)
### Description This PR addresses the comments received on upstream PR apache#13897 to refactor the code written for fixing the Cancel Maintenance where the Pool wasn't getting mounted back in case of NFS. <!--- ******************************************************************************* --> <!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. --> <!--- PLEASE PUT AN 'X' in only **ONE** box --> <!--- ******************************************************************************* --> ### Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [X] Bug fix (non-breaking change which fixes an issue) - [ ] Enhancement (improves an existing feature and functionality) - [ ] Cleanup (Code refactoring and cleanup, that may add test cases) - [ ] Build/CI - [ ] Test (unit or integration test code) ### Feature/Enhancement Scale or Bug Severity #### Feature/Enhancement Scale - [ ] Major - [ ] Minor #### Bug Severity - [ ] BLOCKER - [ ] Critical - [X] Major - [ ] Minor - [ ] Trivial ### Screenshots (if appropriate): <img width="1045" height="339" alt="Screenshot 2026-10-22 at 8 08 49 PM" src="https://github.com/user-attachments/assets/438b7f0e-8b45-4b39-a425-3fb4bd4451ab" /> <img width="1298" height="183" alt="Screenshot 2026-10-22 at 8 10 22 PM" src="https://github.com/user-attachments/assets/58507563-828d-4ff2-b50c-0decd4674578" /> <img width="1049" height="358" alt="Screenshot 2026-10-22 at 8 11 18 PM" src="https://github.com/user-attachments/assets/0ad746fd-137a-4e76-92c8-8bddf3d28692" /> <img width="1302" height="216" alt="Screenshot 2026-10-22 at 8 12 02 PM" src="https://github.com/user-attachments/assets/c0efeadf-a2b6-4328-adb3-928f6bfee618" /> <img width="1048" height="360" alt="Screenshot 2026-10-22 at 8 13 28 PM" src="https://github.com/user-attachments/assets/f6eae7fb-08fc-4d29-94b6-678371f0054c" /> <img width="1301" height="314" alt="Screenshot 2026-10-22 at 8 12 51 PM" src="https://github.com/user-attachments/assets/0eeba7e2-2e98-4e9c-b463-5606f1838776" /> ### How Has This Been Tested? - Created `Primary1` storage pool of `NetworkFileSystem` type using `DefaultPrimary` storage plugin. - Created `testPool` storage pool of `NetworkFileSystem` type using `NetApp ONTAP` storage plugin. - Enter Maintenance on both the pools. - Cancel Maintenance on both the pools. - At each stage, both `mount | grep nfs` and libvirtd's `virst pool-list --all` should reflect the pools appropriately. - Before Enter Maintenance, both the commands should have mount and pool listed respectively. - On Enter Maintenance, neither should be listed. - On Cancel Maintenance, both should be listed again.
1 parent 99acce7 commit c5210eb

4 files changed

Lines changed: 13 additions & 11 deletions

File tree

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ public void addStoragePool(HAStoragePool pool) {
5050
}
5151
}
5252

53+
public void unregisterStoragePool(String uuid) {
54+
synchronized (haStoragePools) {
55+
haStoragePools.remove(uuid);
56+
}
57+
}
58+
5359
public void removeStoragePool(String uuid) {
5460
synchronized (haStoragePools) {
5561
HAStoragePool pool = haStoragePools.get(uuid);

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -457,11 +457,12 @@ public synchronized boolean deleteStoragePool(StoragePoolType type, String uuid)
457457

458458
public boolean deleteStoragePool(StoragePoolType type, String uuid, Map<String, String> details) {
459459
StorageAdaptor adaptor = getStorageAdaptor(type);
460-
// For NetworkFilesystem, libvirt will take care of unmounting the nfs mount. If nfs mount has been removed before libvirt's pool
461-
// delete, libvirt will throw an error.
462-
boolean deleteStatus = adaptor.deleteStoragePool(uuid, details);
460+
boolean deleteStatus;
463461
if (type == StoragePoolType.NetworkFilesystem) {
464-
_haMonitor.removeStoragePool(uuid);
462+
_haMonitor.unregisterStoragePool(uuid);
463+
deleteStatus = adaptor.deleteStoragePool(uuid);
464+
} else {
465+
deleteStatus = adaptor.deleteStoragePool(uuid, details);
465466
}
466467
synchronized (_storagePools) {
467468
_storagePools.remove(uuid);

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -891,11 +891,6 @@ private boolean destroyStoragePoolHandleException(Connect conn, String uuid)
891891
return false;
892892
}
893893

894-
@Override
895-
public boolean deleteStoragePool(String uuid, Map<String, String> details) {
896-
return deleteStoragePool(uuid);
897-
}
898-
899894
@Override
900895
public boolean deleteStoragePool(String uuid) {
901896
logger.info("Attempting to remove storage pool " + uuid + " from libvirt");

‎server/src/main/java/com/cloud/storage/StorageManagerImpl.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1276,7 +1276,7 @@ public PrimaryDataStoreInfo updateStoragePool(UpdateStoragePoolCmd cmd) throws I
12761276
if (capacityBytes != null) {
12771277
if (capacityBytes != pool.getCapacityBytes()) {
12781278
updatedCapacityBytes = capacityBytes;
1279-
details.put(PrimaryDataStoreLifeCycle.CAPACITY_BYTES, String.valueOf(updatedCapacityBytes));
1279+
details.put(PrimaryDataStoreLifeCycle.CAPACITY_BYTES, String.valueOf(updatedCapacityBytes));
12801280
changes = true;
12811281
}
12821282
}
@@ -1286,7 +1286,7 @@ public PrimaryDataStoreInfo updateStoragePool(UpdateStoragePoolCmd cmd) throws I
12861286
if (capacityIops != null) {
12871287
if (!capacityIops.equals(pool.getCapacityIops())) {
12881288
updatedCapacityIops = capacityIops;
1289-
details.put(PrimaryDataStoreLifeCycle.CAPACITY_IOPS, String.valueOf(updatedCapacityIops));
1289+
details.put(PrimaryDataStoreLifeCycle.CAPACITY_IOPS, String.valueOf(updatedCapacityIops));
12901290
changes = true;
12911291
}
12921292
}

0 commit comments

Comments
 (0)