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

[John Ferlan]

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".

>  }
>  
>  

[...]

> diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
> index 2388e925fc..3b111e3dc7 100644
> --- a/src/nwfilter/nwfilter_driver.c
> +++ b/src/nwfilter/nwfilter_driver.c
> @@ -163,6 +163,14 @@ nwfilterDriverInstallDBusMatches(DBusConnection *sysbus ATTRIBUTE_UNUSED)
>  
>  #endif /* HAVE_FIREWALLD */
>  
> +static int virNWFilterTriggerRebuildImpl(void *opaque)

NIT:

static int
virNWFilterTriggerRebuildImpl(void *opaque)

> +{
> +    virNWFilterDriverStatePtr nwdriver = opaque;
> +
> +    return virNWFilterBuildAll(nwdriver, true);
> +}
> +
> +

[...]