Repository navigation
kvm: refresh the RBD pool when resizing a volume libvirt has not cached - #14623
bhouse-nexthop wants to merge 2 commits into
Conversation
LibvirtResizeVolumeCommandWrapper gets the disk with pool.getPhysicalDisk() and then looks the volume up through libvirt with storageVolLookupByPath() to resize it. Before apache#14188, getPhysicalDisk() went through LibvirtStorageAdaptor.getVolume(), which refreshes the libvirt pool when the volume is not found, so the volume was in libvirt's cache by the time the wrapper looked it up. apache#14188 made getPhysicalDisk() stat RBD images through librbd, so nothing refreshes the pool any more. RBD volumes are created through librbd, not libvirt, so a new volume is not in libvirt's cache on a host until something refreshes the pool there, and resizing it fails with: Storage volume not found: no storage vol with matching path '<pool>/<uuid>' When the lookup by path fails with VIR_ERR_NO_STORAGE_VOL on an RBD pool, refresh the pool and look the volume up once more. That is what getVolume() did for this path before apache#14188, and the refresh only happens when the volume is not cached. Other errors and other pool types are handled as before. Signed-off-by: Brad House <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## 4.22 #14623 +/- ##
=============================================
- Coverage 18.32% 4.08% -14.25%
=============================================
Files 5939 449 -5490
Lines 537768 38282 -499486
Branches 65929 7083 -58846
=============================================
- Hits 98551 1564 -96987
+ Misses 428008 36503 -391505
+ Partials 11209 215 -10994
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| final LibvirtUtilitiesHelper libvirtUtilitiesHelper = libvirtComputingResource.getLibvirtUtilitiesHelper(); | ||
|
|
||
| final Connect conn = libvirtUtilitiesHelper.getConnection(); | ||
| final StorageVol v = conn.storageVolLookupByPath(path); |
There was a problem hiding this comment.
can you create a method in libvirtUtilitiesHelper ?
There was a problem hiding this comment.
Done in aa9683a: the lookup with the RBD pool refresh is now LibvirtUtilitiesHelper.getStorageVolumeByPath(Connect, KVMStoragePool, String), and the resize wrapper calls it. Its tests moved to LibvirtUtilitiesHelperTest; LibvirtComputingResourceTest keeps the end-to-end resize of an RBD volume libvirt has not cached, plus a resize that fails when the lookup fails.
Test report: resizing RBD volumes that libvirt has not cached, with no manual pool refreshEnvironment
1. Size-only resize of a fresh VM's ROOT, VM running
On the build without this PR, the same first resize failed with 2. Data volume, created, attached and resized while attached to the running VM
3.
Result: pass.
Not covered
|
Move the lookup by path that refreshes an RBD pool when the volume is not in libvirt's cache out of LibvirtResizeVolumeCommandWrapper and into LibvirtUtilitiesHelper.getStorageVolumeByPath(), and call it from the wrapper. The behaviour and the log messages are unchanged. The tests of the lookup itself move to LibvirtUtilitiesHelperTest. LibvirtComputingResourceTest keeps a resize of an RBD volume that is not in libvirt's cache, through the real helper, and adds a resize that fails when the lookup fails. Signed-off-by: Brad House <[email protected]>
weizhouapache
left a comment
There was a problem hiding this comment.
code lgtm
not tested yet
|
@blueorangutan package |
|
@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
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:
This is a regression from #14188.
Fixes #14622
Why it happens
LibvirtResizeVolumeCommandWrappergets the disk withpool.getPhysicalDisk(), then looks the volume up through libvirt withconn.storageVolLookupByPath(path)to callStorageVol.resize().getPhysicalDisk()went throughLibvirtStorageAdaptor.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.getPhysicalDisk()stat RBD images through librbd, so nothing refreshes the pool any more.Fix
When
storageVolLookupByPath()fails withVIR_ERR_NO_STORAGE_VOLon an RBD pool, refresh that pool and look the volume up once more.getVolume()did for this path before kvm: look RBD volumes up through librbd instead of refreshing the whole pool #14188. The refresh only happens when the volume is not cached.StorageVol.resize(), then the resize script, which for RBD isNOTIFYONLYand runsvirsh blockresizeon a running VM.I looked at two other fixes:
LibvirtStorageAdaptoralready 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.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()callsgetVolume()itself, which still refreshes on a miss.LibvirtMigrateCommandWrapper.deleteLocalVolume()usesstorageVolLookupByPath(), but only for local files.Not changed here
execute()already callspool.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
Feature/Enhancement Scale or Bug Severity
Bug Severity
How Has This Been Tested?
Unit tests. A live lab retest will follow.
LibvirtComputingResourceTestpasses (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.testResizeVolumeCommandRbdVolumeNotFoundAfterRefreshandtestResizeVolumeCommandRbdVolumeRetriedAfterFailedRefresh: 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.testResizeVolumeCommandRbdOtherLookupErrorDoesNotRefreshandtestResizeVolumeCommandRbdLookupErrorWithoutCodeDoesNotRefresh: 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.