bhouse-nexthop opened a new pull request, #14623: URL: https://github.com/apache/cloudstack/pull/14623
### Description Resizing a volume on RBD primary storage fails on KVM when the host's libvirt has not cached the image in its view of the pool. A freshly deployed VM's ROOT volume is the usual case: ``` org.libvirt.LibvirtException: Storage volume not found: no storage vol with matching path '<ceph pool>/<uuid>' ``` This is a regression from #14188. Fixes #14622 #### Why it happens `LibvirtResizeVolumeCommandWrapper` gets the disk with `pool.getPhysicalDisk()`, then looks the volume up through libvirt with `conn.storageVolLookupByPath(path)` to call `StorageVol.resize()`. - Before #14188, `getPhysicalDisk()` went through `LibvirtStorageAdaptor.getVolume()`, which refreshes the libvirt pool on a miss. The volume was therefore in libvirt's cache by the time the wrapper looked it up. - #14188 made `getPhysicalDisk()` stat RBD images through librbd, so nothing refreshes the pool any more. - RBD volumes are created through librbd, not libvirt. A new volume is not in libvirt's cache on a host until something refreshes the pool there. #### Fix When `storageVolLookupByPath()` fails with `VIR_ERR_NO_STORAGE_VOL` on an RBD pool, refresh that pool and look the volume up once more. - This is what `getVolume()` did for this path before #14188. The refresh only happens when the volume is not cached. - Any other libvirt error, and every other pool type, fails the resize as before, with no refresh. - Nothing else in the resize changes: the flags, `StorageVol.resize()`, then the resize script, which for RBD is `NOTIFYONLY` and runs `virsh blockresize` on a running VM. I looked at two other fixes: - **Resize RBD images through librbd** and skip libvirt for RBD, as the rest of `LibvirtStorageAdaptor` already does for create, clone and delete. That avoids the refresh entirely, but it replaces the resize itself (shrink handling, the image open, error reporting), so it is a bigger change for a regression fix. It would make a reasonable follow-up. - **Reuse `LibvirtStorageAdaptor.getVolume()`.** It looks the volume up by name on the libvirt pool object and refreshes on any error, so it would change the lookup for volumes that are cached, too. #### Other call sites I checked the KVM plugin for other places that rely on `getPhysicalDisk()` having refreshed the libvirt pool. There are none: - `LibvirtStorageAdaptor.deletePhysicalDisk()` calls `getVolume()` itself, which still refreshes on a miss. - `LibvirtMigrateCommandWrapper.deleteLocalVolume()` uses `storageVolLookupByPath()`, but only for local files. #### Not changed here `execute()` already calls `pool.refresh()` after every resize, before it reads the new size back. For RBD that refresh no longer affects the result, since the size now comes from librbd. On a cache miss, this PR adds a second refresh. Dropping the trailing one for RBD is a separate change. ### 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 #### Bug Severity - [ ] BLOCKER - [ ] Critical - [x] Major - [ ] Minor - [ ] Trivial ### How Has This Been Tested? **Unit tests.** A live lab retest will follow. `LibvirtComputingResourceTest` passes (273 tests, checkstyle on). The new tests are: - `testResizeVolumeCommandRbdVolumeNotInLibvirtPoolCache`: the first lookup throws not found on an RBD pool. The resize refreshes the pool, finds the volume, resizes it, then runs the notify-only script, and answers success with the new size. - `testResizeVolumeCommandRbdVolumeNotFoundAfterRefresh` and `testResizeVolumeCommandRbdVolumeRetriedAfterFailedRefresh`: the volume is still missing after the refresh, whether or not the refresh itself worked. There is exactly one refresh and one retry, then the resize fails. - `testResizeVolumeCommandRbdOtherLookupErrorDoesNotRefresh` and `testResizeVolumeCommandRbdLookupErrorWithoutCodeDoesNotRefresh`: any other lookup error on RBD fails without a refresh. - `testResizeVolumeCommandNonRbdVolumeNotFoundDoesNotRefresh`: a missing volume on NFS fails without a refresh, as before. Without the fix, the first three of these fail. The other three pass with or without it; they check that nothing else changed. -- 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]
