[libvirt] [PATCH v3 17/20] nwfilter: remove virt driver callback layer for rebuilding filters

[Daniel P. Berrangé]

On Mon, Jun 18, 2018 at 04:59:37PM -0400, John Ferlan wrote:
> 
> 
> On 06/14/2018 08:33 AM, Daniel P. Berrangé wrote:
> > Now that the nwfilter driver keeps a list of bindings that it has
> > created, there is no need for the complex virt driver callbacks. It is
> > possible to simply iterate of the list of recorded filter bindings.
> > 
> > This means that rebuilding filters no longer has to acquire any locks on
> > the virDomainObj objects, as they're never touched.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> > ---
> >  src/conf/nwfilter_conf.c               | 134 +++-----------------
> >  src/conf/nwfilter_conf.h               |  51 +-------
> >  src/conf/virnwfilterobj.c              |   4 +-
> >  src/libvirt_private.syms               |   7 +-
> >  src/lxc/lxc_driver.c                   |  28 -----
> >  src/nwfilter/nwfilter_driver.c         |  21 ++--
> >  src/nwfilter/nwfilter_gentech_driver.c | 167 ++++++++++++++++---------
> >  src/nwfilter/nwfilter_gentech_driver.h |   4 +-
> >  src/qemu/qemu_driver.c                 |  25 ----
> >  src/uml/uml_driver.c                   |  29 -----
> >  10 files changed, 141 insertions(+), 329 deletions(-)
> > 
> 
> A diffstat that Jano usually applauds!  Miracles do happen when you
> close your eyes and say 3 times "I wish this code would just go away"
> ;-).  Still this is some of the most "entertaining code" - that now used
> to exist!  It seems I can dig up my nwfilter obj/hash code once this is
> in...
> 
> There's a couple nits below...
> 
> Reviewed-by: John Ferlan <jferlan at redhat.com>
> 
> John
> 
> 
> > diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
> > index de26a6d034..5bb8a0c2e7 100644
> > --- a/src/conf/nwfilter_conf.c
> > +++ b/src/conf/nwfilter_conf.c
> 
> [...]
> 
> > +
> > +
> > +int
> > +virNWFilterTriggerRebuild(void)
> > +{
> > +    return rebuildCallback(rebuildOpaque);
> 
> In the better safe than sorry - should we gate on "if
> (rebuildCallback)"?  I don't think there's a way into here with it being
> NULL, but those are always "famous last words".

Yeah ok.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|