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

[Jonathon Jongsma]

On Wed, 31 Mar 2021 16:00:48 +0200
Erik Skultety <eskultet at redhat.com> wrote:

> On Fri, Mar 26, 2021 at 11:47:56AM -0500, 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 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 v6:
> >  - rebase to git master again
> >  - remove typedefs for various *Ptr types, since they're now
> > discouraged in libvirt.  
> 
> Okay, so I think I ACKed all of the patches now (if not, let me know
> which one I missed). I tested the patches, found 3 leaks, one of them
> I mentioned here, 2 were related to the code but not a direct result
> of this series I believe, so I'll tackle them some other time as a
> follow up. Overall, I think we're good to push this starting with the
> 7.3.0 cycle.

I think everything else has been acked.

> 
> Now, in v4 I promised to share my adjustments I made on top of your
> code. Some of them you already handled yourself in v6, so I rebased
> and performed a bunch of changes, so here they are:
> https://gitlab.com/eskultety/libvirt/-/commits/mdev-adjustments
> 
> Note, that I only split them into multiple patches so that they're
> easier to read, but I'm very well aware that probably none of them
> cannot be compiled on its own (I didn't bother to be that thorough),
> it's just to give you an idea what I've failed to express with words
> until we came to this v6. Please let me know your opinion on the
> changes so that: a) I can send it as a *proper* follow up series
>     b) you can incorporate some of the changes into your series
> 

Thanks for that. I think the changes look reasonable, and I think the
benefits outweigh the drawbacks. I have a few changes I'd like to make
to your commits. See the top 3 commits at [1] for details. Would you
like me to just incorporate your stuff into my original series, or keep
them as separate commits on top of my series?

Jonathon


[1] https://gitlab.com/jjongsma/libvirt/-/commits/mdevctl-adjustments/