On Wed, 7 Apr 2021 08:05:17 +0200 Erik Skultety <eskultet@xxxxxxxxxx> 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@xxxxxxxxxx> 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
