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

On Wed, 31 Mar 2021 16:00:48 +0200 Erik Skultety <eskultet(a)redhat.com&gt; 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?