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
