Backport libvirt listDevices() fixes to yoga - #4
Merged
Conversation
The call to _get_pci_passthrough_devices could fail because a network device could have disappeared which would cause a traceback in the logs. This wraps the function in a safe way to return an empty array if it fails, which will clean-up the logs if the device disappears Closes-Bug: #1972028 Change-Id: I46d3bbe122d9f8452f168286391bab67ecea3128
This is similar to change I668643c836d46a25df46d4c99a973af5e50a39db where the objects returned in a list from a libvirt call were not tpool.Proxy wrapped. Because the objects are not wrapped, calling methods on them such as listCaps() can block all other greenthreads and can cause nova-compute to freeze for hours in certain scenarios. This adds the same wrapping to libvirt calls which return lists of virNodeDevice. Closes-Bug: #2091033 Change-Id: I60d6f04d374e9ede5895a43b7a75e955b0fea3c5
This a partial revert of change
I60d6f04d374e9ede5895a43b7a75e955b0fea3c5 which added tpool.Proxy
wrapping to the listDevices() and listAllDevices() methods.
The regression was caught during downstream testing with vGPUs and the
update_available_resource() periodic task was failing with:
TypeError: virNodeDeviceLookupByName() argument 2 must be str or
None, not Proxy
It turns out that while the listAllDevices() method returns a list of
virNodeDevice objects [1], the listDevices() method returns a list of
string names [2] and is generated from the corresponding function in C
[3].
The error was not caught by unit or functional testing because those
test environments intentionally do not import the libvirt Python
module -- so mocked code in the LibvirtFixture runs instead. Also, the
update_available_resource() method has a 'except Exception:' at the end
which logs an error but does not re-raise. So it would not cause a
functional test to fail.
This reverts the change that caused the regression, updates potentially
confusing docstrings, adds type annotations to the methods that use
listDevices(), and moves the nodeDeviceLookupByName type checking into
the LibvirtFixture.
Closes-Bug: #2098892
[1] https://github.com/libvirt/libvirt-python/blob/408815a/libvirt-override-virConnect.py#L520-L524
[2] https://github.com/libvirt/libvirt-python/blob/408815a/libvirt-override-api.xml#L448-L453
[3] https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeListDevices
Change-Id: Ib5befdd3c13367daa208ff969f66cba693ae2c76
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backports three upstream OpenStack Nova libvirt fixes related to device listing to the
elastx/yogabranch.Commits
_get_pci_passthrough_devices(8534499) — cherry-picked cleanly.listDevices()andlistAllDevices()(f304b9e) — cherry-picked cleanly.listDevices()return type (2c07aa0) — partial revert of Add code for autostart tagged instances after hypervisor reboot #2; ensureslistDevices()returns a list of name strings (nottpool.Proxy-wrapped objects) whilelistAllDevices()remains wrapped. FixesTypeError: virNodeDeviceLookupByName() argument 2 must be str or None, not Proxy(Closes-Bug: #2098892).Deviation from a straight cherry-pick
Commit #3 also modified
nova/tests/functional/regressions/test_bug_2098892.py, a functional regression test that does not exist onelastx/yoga(it was introduced by a separate upstream commit not being backported). The test could not be carried over as-is because it depends on newer test infrastructure than this branch has (it callsself._run_periodics(raise_on_error=True), but onelastx/yoga_run_periodicstakes no arguments). The test file was therefore dropped; the production fix innova/virt/libvirt/host.pyandnova/tests/fixtures/libvirt.pyis fully applied.