[libvirt PATCH v5 00/30] Add support for persistent mediated devices
Erik Skultety
eskultet at redhat.com
Fri Mar 26 06:27:46 UTC 2021
On Thu, Mar 25, 2021 at 11:49:49AM -0500, Jonathon Jongsma wrote:
> Just a friendly ping ;)
I'm sorry I've been neglecting this for the past 3 weeks or so, I'll dive right
into it starting Monday next week, we're entering freeze today anyway.
Would you mind resending a rebased version? There were a couple of conflicts
wrt to RPC, I fixed them all locally, but it's always better to start the
review with fresh content.
Thanks,
Erik
>
> On Tue, 2 Mar 2021 16:30:35 -0600
> Jonathon Jongsma <jjongsma at redhat.com> 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
> >
> >
>
More information about the libvir-list
mailing list