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

Thu Mar 25 16:49:49 UTC 2021

Just a friendly ping ;)

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
> 
>