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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jun 18, 2018 at 04:59:12PM -0400, John Ferlan wrote:
> 
> 
> 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@xxxxxxxxxx>
> > ---
> >  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.

The current semantics are that nwfilterInstantiateFilter will not
report an error if the filter already exists, so I think I'll just
not report an error here. This method will go away anyway, so no
great loss.

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

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 :|

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux