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

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



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
> 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 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(-)
> 
> diff --git a/src/conf/domain_nwfilter.c b/src/conf/domain_nwfilter.c
> index f39c8a1f9b..3e6e462def 100644
> --- a/src/conf/domain_nwfilter.c
> +++ b/src/conf/domain_nwfilter.c
> @@ -85,16 +85,19 @@ int
>  virDomainConfNWFilterInstantiate(const char *vmname,
>                                   const unsigned char *vmuuid,
>                                   virDomainNetDefPtr net,
> -                                 bool ignoreExists)
> +                                 bool ignoreExists,
> +                                 bool ignoreDeleted)
>  {
>      virConnectPtr conn = virGetConnectNWFilter();
>      virNWFilterBindingDefPtr def = NULL;
>      virNWFilterBindingPtr binding = NULL;
> +    virNWFilterPtr nwfilter = NULL;
>      char *xml = NULL;
>      int ret = -1;
>  
> -    VIR_DEBUG("vmname=%s portdev=%s filter=%s ignoreExists=%d",
> -              vmname, NULLSTR(net->ifname), NULLSTR(net->filter), ignoreExists);
> +    VIR_DEBUG("vmname=%s portdev=%s filter=%s ignoreExists=%d ignoreDeleted=%d",
> +              vmname, NULLSTR(net->ifname), NULLSTR(net->filter),
> +              ignoreExists, ignoreDeleted);
>  
>      if (!conn)
>          goto cleanup;
> @@ -113,14 +116,34 @@ virDomainConfNWFilterInstantiate(const char *vmname,
>      if (!(xml = virNWFilterBindingDefFormat(def)))
>          goto cleanup;
>  
> -    if (!(binding = virNWFilterBindingCreateXML(conn, xml, 0)))
> -        goto cleanup;
> +    if (!(binding = virNWFilterBindingCreateXML(conn, xml, 0))) {
> +        virErrorPtr orig_err;
> +
> +        if (!ignoreDeleted)
> +            goto cleanup;
> +
> +        /* Let's determine if the error was because the filter was deleted.
> +         * Save the orig_err just in case it's not a failure to find the
> +         * filter by name. */
> +        orig_err = virSaveLastError();
> +        nwfilter = virNWFilterLookupByName(conn, def->filter);
> +        virSetError(orig_err);
> +        virFreeError(orig_err);
> +        if (nwfilter)
> +            goto cleanup;
> +
> +        VIR_WARN("filter '%s' for binding '%s' has been deleted while the "
> +                 "guest was running, ignoring for restart processing",
> +                 def->filter, def->portdevname);
> +        virResetLastError();
> +    }

I don't get what this code is trying to do. This method is about creating
nwfilter bindings. If virNWFilterBindingCreateXML() fails, that means the
filter binding already exists. But the stated problem was that the admin
had deleted the filter binding. The code is also checking a filter, not
a filter binding.

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


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