[libvirt] [PATCH v3 16/20] nwfilter: keep track of active filter bindings

[John Ferlan]

On 06/14/2018 08:33 AM, Daniel P. Berrangé wrote:
> Currently the nwfilter driver does not keep any record of what filter
> bindings it has active. This means that when it needs to recreate
> filters, it has to rely on triggering callbacks provided by the virt
> drivers. This introduces a hash table recording the virNWFilterBinding
> objects so the driver has a record of all active filters.
> 
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
>  src/conf/virnwfilterobj.h      |  4 ++
>  src/nwfilter/nwfilter_driver.c | 86 ++++++++++++++++++++++++----------
>  2 files changed, 65 insertions(+), 25 deletions(-)
> 

[...]

> diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
> index 7202691646..2388e925fc 100644
> --- a/src/nwfilter/nwfilter_driver.c
> +++ b/src/nwfilter/nwfilter_driver.c
> @@ -38,7 +38,6 @@
>  #include "domain_conf.h"
>  #include "domain_nwfilter.h"
>  #include "nwfilter_driver.h"
> -#include "virnwfilterbindingdef.h"
>  #include "nwfilter_gentech_driver.h"
>  #include "configmake.h"
>  #include "virfile.h"
> @@ -174,7 +173,6 @@ nwfilterStateInitialize(bool privileged,
>                          virStateInhibitCallback callback ATTRIBUTE_UNUSED,
>                          void *opaque ATTRIBUTE_UNUSED)
>  {

[...]

>  
>      if (virNWFilterObjListLoadAllConfigs(driver->nwfilters, driver->configDir) < 0)
>          goto error;
>  
> +    if (virNWFilterBindingObjListLoadAllConfigs(driver->bindings, driver->bindingDir) < 0)
> +        goto error;
> +

Because of this... [1]

>      nwfilterDriverUnlock();
>  
>      return 0;
>  

[...]

> @@ -647,13 +656,37 @@ nwfilterInstantiateFilter(const char *vmname,
>                            const unsigned char *vmuuid,
>                            virDomainNetDefPtr net)
>  {
> -    virNWFilterBindingDefPtr binding;
> +    virNWFilterBindingObjPtr obj;
> +    virNWFilterBindingDefPtr def;
>      int ret;
>  
> -    if (!(binding = virNWFilterBindingDefForNet(vmname, vmuuid, net)))
> +    obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, net->ifname);
> +    if (obj) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Filter already present for NIC %s"), net->ifname);

[1]
If I stop at this patch, start a domain with a filter, then restart
libvirtd, then that will cause a guest running with a filter to exit and
not just disappear - as it can be restarted. The error is:

2018-06-18 16:49:07.405+0000: 3978: error :
nwfilterInstantiateFilter:666 : internal error: Filter already present
for NIC vnet1

Because once I have this patch built/running the
/var/run/libvirt/nwfilter-binding/vnet1.xml exists and get's loaded when
virNWFilterBindingObjListLoadAllConfigs is called at StateInitialize.

I only saw this because I found later in patch 20 the failure comes from
nwfilterBindingCreateXML instead when virDomainConfNWFilterInstantiate
is called. I worked my way back to this point.

Not sure which would be the "best" solution at this point. Should we
wait to do [1] until patch 20 or perhaps just not have an error here.

NB: If the guest was running at a point up through patch 15 then it
won't exit on the first libvirtd restart (since the obj dir doesn't
exist), but the issue shows up on the 2nd restart.

In general the code is fine to me, but just need to handle this one
issue in some way.

John

> +        virNWFilterBindingObjEndAPI(&obj);
> +        return -1;
> +    }
> +
> +    if (!(def = virNWFilterBindingDefForNet(vmname, vmuuid, net)))
>          return -1;
> -    ret = virNWFilterInstantiateFilter(driver, binding);
> -    virNWFilterBindingDefFree(binding);

[...]