[libvirt PATCH v3 00/21] Add support for persistent mediated devices

[Erik Skultety]

On Thu, Dec 24, 2020 at 08:14:24AM -0600, Jonathon Jongsma 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 ediated 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 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

Thanks for giving ^this a try :)

I think we're converging nicely with the only remaining bit to agree on being
the mdevctl monitoring thread. I think we can target 7.1.0 with this series.

Regards,
Erik