[libvirt] [PATCH] nwfilter: Handle libvirtd restart if nwfilter binding deleted

[Daniel P. Berrangé]

On Wed, Aug 22, 2018 at 05:43:21PM -0400, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1607202
> 
> It's stated that if the admin wants to shoot themselves in
> the foot by removing the nwfilter binding while the domain

So based on your explanation in the other reply, this message
is what was misleading me. s/nwfilter binding/nwfilter/

> is running we will certainly allow that.  However, in doing
> so we also run the risk that a libvirtd restart will cause
> the domain to be shutdown, which isn't a good thing.
> 
> So add another boolean to virDomainConfNWFilterInstantiate
> which allows us to recover somewhat gracefully in the event
> the virNWFilterBindingCreateXML fails when we come from
> qemuProcessReconnect and we determine that the filter has
> been deleted. It was there at some point (it had to be), but
> if it's missing, then we don't want to cause the guest to
> stop running, so issue a warning and continue on.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/conf/domain_nwfilter.c | 33 ++++++++++++++++++++++++++++-----
>  src/conf/domain_nwfilter.h |  3 ++-
>  src/lxc/lxc_process.c      |  3 ++-
>  src/qemu/qemu_hotplug.c    |  7 ++++---
>  src/qemu/qemu_interface.c  |  6 ++++--
>  src/qemu/qemu_process.c    | 10 +++++++---
>  src/uml/uml_conf.c         |  3 ++-
>  7 files changed, 49 insertions(+), 16 deletions(-)

[snip]

>  static int
> -qemuProcessFiltersInstantiate(virDomainDefPtr def, bool ignoreExists)
> +qemuProcessFiltersInstantiate(virDomainDefPtr def,
> +                              bool ignoreExists,
> +                              bool ignoreDeleted)
>  {
>      size_t i;
>  
>      for (i = 0; i < def->nnets; i++) {
>          virDomainNetDefPtr net = def->nets[i];
>          if ((net->filter) && (net->ifname)) {
> -            if (virDomainConfNWFilterInstantiate(def->name, def->uuid, net, ignoreExists) < 0)
> +            if (virDomainConfNWFilterInstantiate(def->name, def->uuid, net,
> +                                                 ignoreExists,
> +                                                 ignoreDeleted) < 0)
>                  return 1;
>          }

Rather than this extra "ignoreDeleted" arg, why can't we just do

           if (virDomainConfNWFilterInstantiate(def->name, def->uuid, net,
                                                 ignoreExists) < 0 &&
						 ignoreDeleted)
                return 1;           

This ensures that all things which can cause a nwfilter binding failure
on startup will be handled by avoiding tearing down the running guest.


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