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

[Jonathon Jongsma]

On Wed, 7 Apr 2021 08:05:17 +0200
Erik Skultety <eskultet at redhat.com> wrote:

> On Thu, Apr 01, 2021 at 10:18:45AM -0500, Jonathon Jongsma wrote:
> > 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?  
> 
> I like your ^additional cleanup.
> 
> As for the 30 patches - I'd like to avoid having to go through most
> of them again, so no, please go ahead an merge this series as is. The
> follow up changes that started to discuss are not functional anyway,
> so posting them separately is actually preferable.
> 
> As for the follow up series - now that you see what my intentions wrt
> abstracting the mdevctl cmdline building code as much as possible
> were, feel free to take my commits, apply your 3 changes, shuffle
> them around as you need, and post it as a proper follow up series
> that can compile after every single patch :) and I'll review it.

Sounds good. Will send a followup series shortly. Thanks!

Jonathon