Re: [libvirt PATCH v5 00/30] Add support for persistent mediated devices

Just a friendly ping ;)

On Tue, 2 Mar 2021 16:30:35 -0600 Jonathon Jongsma <jjongsma(a)redhat.com&gt; wrote: > This patch series follows the previously-merged series which added > support for transient mediated devices. This series expands mdev > support to include persistent device definitions. Again, it relies on > mdevctl as the backend. > > It follows the common libvirt pattern of APIs by adding the following > new APIs for node devices: > - virNodeDeviceDefineXML() - defines a persistent device > - virNodeDeviceUndefine() - undefines a persistent device > - virNodeDeviceCreate() - starts a previously-defined device > > It also adds virsh commands mapping to these new APIs: nodedev-define, > nodedev-undefine, and nodedev-start. > > Since we rely on mdevctl for the definition of mediated devices, we > need a way to stay up-to-date with devices that are defined by > mdevctl (outside of libvirt). The method for staying up-to-date is > currently a little bit crude due to the fact that mdevctl does not > emit any events when new devices are added or removed. As a > workaround, we create a file monitor for the mdevctl config directory > and re-query mdevctl when we detect changes within that directory. In > the future, mdevctl may introduce a more elegant solution. > > Changes in v5: > - Rebase to git master > - updated new API version info to 7.2.0 > - capture and relay stderr message from mdevctl > - changed to using GHashTable functions directly instead of > deprecated virHash functions > - protected mdevctlMonitors with a mutex > - added a couple patches to fix the 5s delay when defining a new > mdev. These are currently separate patches added to the end of the > series, but could be squashed into the earlier commits if that's > preferred. > - various other minor review fixes > > Changes in v4: > - rebase to git master > - switch to throwaway thread for querying mdevctl > - fixed a bug when removing devices because I was accidentally using > virHashForEach() instead of virHashForeachSafe() > - use DEFINED/UNDEFINED events instead of STARTED/STOPPED events > - changes related to merging information about mdev devices from > both udev a= nd > mdevctl: > - Re-used the same function to copy extra data from mdevctl > regardless of whether we're processing a udev event or a mdevctl > event (recommended by Erik). This results in slightly more complex > handling of the object lifetimes (see patch 9), but it consolidates > some code. > - nodeDeviceDefCopyFromMdevctl() previously only copied the data > that was unique to mdevctl and didn't exist in udev. It now copies > additional data (possibly overwriting some udev). This solves a > problem where a device = is > defined but not active (i.e. we have not gotten any data from > udev), and then changed (e.g. somebody calls 'mdevctl modify' to > change the mdev type), but libvirt was not updating to the new > definition. > - fix a bug where we were mistakenly emitting 'update' events for > devices th= at > had not changed > - Added the ability to specify a uuid for an mdev via device XML. > - split some commits into multiple patches > - updated new API version info to 7.1.0 > - Fixed a bug reported by Yan Fu which hangs the client when > attempting to destroy a nodedev that is in use by an active vm. See > https://www.redhat.com/archives/libvir-list/2021-February/msg00116.html > for solution suggested by Alex. > - numerous smaller fixes from review findings > > changes in v3: > - streamlined tests -- removed some unnecessary duplication > - split out patch to factor out node device name generation function > - split nodeDeviceParseMdevctlChildDevice() into a separate function > - added follow-up patch to remove space-padded alignment in header > - refactored the mdevctl update handling significantly: > - no longer a separate persistent thread that gets signaled by a > timer > - now piggybacks onto the existing udev thread and signals the > thread in t= he > same way that the udev event does. > - Daniel suggested spawning a throw-away thread to handle mdevctl > updates, but that introduces the complexity of possibly serializing > multiple throw-away threads (e.g. if we get an 'created' event > followed immediate= ly > by a 'deleted' event, two threads may be spawned and we'd need > to ensure they are properly ordered) > - added virNodeDeviceObjListForEach() and > virNodeDeviceObjListRemoveLocked() to simplify removing devices that > are removed from mdevctl. > - coding style fixes > - NOTE: per Erik's request, I experimented with changing the way > that mdevctl commands were generated and tested (e.g. introducing > something like virMdevctlGetCommand(def, > MDEVCTL_COMMAND_<SUBCOMMAND>, ...)), but it was too invasive and > awkward and didn't seem worthwhile > > Changes in v2: > - rebase to latest git master > > Jonathon Jongsma (30): > nodedev: capture and report stderror from mdevctl > tests: remove extra trailing semicolon > nodedev: introduce concept of 'active' node devices > nodedev: Add ability to filter by active state > nodedev: fix docs for virConnectListAllNodeDevices() > nodedev: expose internal helper for naming devices > nodedev: add ability to parse mdevs from mdevctl > nodedev: add ability to list defined mdevs > nodedev: add persistence to virNodeDeviceObj > nodedev: add DEFINED/UNDEFINED lifecycle events > nodedev: add mdevctl devices to node device list > nodedev: add helper functions to remove node devices > nodedev: handle mdevs that disappear from mdevctl > nodedev: Refresh mdev devices when changes are detected > nodedev: add function to generate mdevctl define command > api: add virNodeDeviceDefineXML() > virsh: Add --inactive, --all to nodedev-list > virsh: add nodedev-define command > nodedev: refactor tests to support mdev undefine > api: add virNodeDeviceUndefine() > virsh: Factor out function to find node device > virsh: add nodedev-undefine command > api: add virNodeDeviceCreate() > virsh: add "nodedev-start" command > nodedev: add <uuid> element to mdev caps > nodedev: add ability to specify UUID for new mdevs > nodedev: fix hang when destroying an mdev in use > nodedev: add docs about mdev attribute order > nodedev: factor out function to add mediated devices > nodedev: avoid delay when defining a new mdev > > docs/formatnode.html.in | 5 +- > docs/schemas/nodedev.rng | 43 +- > examples/c/misc/event-test.c | 4 + > include/libvirt/libvirt-nodedev.h | 20 +- > src/access/viraccessperm.c | 2 +- > src/access/viraccessperm.h | 6 + > src/conf/node_device_conf.c | 14 + > src/conf/node_device_conf.h | 8 + > src/conf/virnodedeviceobj.c | 147 +++- > src/conf/virnodedeviceobj.h | 24 + > src/driver-nodedev.h | 14 + > src/libvirt-nodedev.c | 141 +++- > src/libvirt_private.syms | 6 + > src/libvirt_public.syms | 7 + > src/node_device/node_device_driver.c | 743 > +++++++++++++++++- src/node_device/node_device_driver.h | > 50 +- src/node_device/node_device_udev.c | 217 ++++- > src/remote/remote_driver.c | 3 + > src/remote/remote_protocol.x | 40 +- > src/remote_protocol-structs | 16 + > src/rpc/gendispatch.pl | 1 + > ...19_36ea_4111_8f0a_8c9a70e21366-define.argv | 2 + > ...19_36ea_4111_8f0a_8c9a70e21366-define.json | 1 + > ...019_36ea_4111_8f0a_8c9a70e21366-start.argv | 3 +- > ...39_495e_4243_ad9f_beb3f14c23d9-define.argv | 1 + > ...39_495e_4243_ad9f_beb3f14c23d9-define.json | 1 + > ...16_1ca8_49ac_b176_871d16c13076-define.argv | 1 + > ...16_1ca8_49ac_b176_871d16c13076-define.json | 1 + > tests/nodedevmdevctldata/mdevctl-create.argv | 1 + > .../mdevctl-list-defined.argv | 1 + > .../mdevctl-list-multiple.json | 59 ++ > .../mdevctl-list-multiple.out.xml | 43 + > .../nodedevmdevctldata/mdevctl-undefine.argv | 1 + > tests/nodedevmdevctltest.c | 232 +++++- > ...v_d069d019_36ea_4111_8f0a_8c9a70e21366.xml | 1 + > tools/virsh-nodedev.c | 269 ++++++- > 36 files changed, 1935 insertions(+), 193 deletions(-) > create mode 100644 > tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9= > a70e21366-define.argv create mode 100644 > tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9= > a70e21366-define.json create mode 100644 > tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb= > 3f14c23d9-define.argv create mode 100644 > tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb= > 3f14c23d9-define.json create mode 100644 > tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871= > d16c13076-define.argv create mode 100644 > tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871= > d16c13076-define.json create mode 100644 > tests/nodedevmdevctldata/mdevctl-create.argv create mode 100644 > tests/nodedevmdevctldata/mdevctl-list-defined.argv create mode 100644 > tests/nodedevmdevctldata/mdevctl-list-multiple.json create mode > 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml create > mode 100644 tests/nodedevmdevctldata/mdevctl-undefine.argv > > --=20 > 2.26.2 > >