[libvirt] [PATCH 8/9] qemu: explicitly delete standard tap devices only on platforms that require it

[Daniel P. Berrangé]

On Tue, Aug 27, 2019 at 09:46:38PM -0400, Laine Stump wrote:
> libvirt creates its tap devices without the IFF_PERSIST flag, so they
> will be automatically deleted when qemu is finished with them. In the
> case of tap devices created outside of libvirt, if the creating entity
> wants the devices to be deleted, it will also omit IFF_PERSIST, but if
> it wants them to remain (e.g. for re-use), then it will use
> IFF_PERSIST when creating the device.
> 
> Back when support was added for autocreation by libvirt of tap devices
> for <interface type='ethernet'> (commit 9c17d665), code was mistakenly
> put in qemuProcessStop to always delete tap devices for
> type='ethernet'. This should only be done on platforms that have
> VIR_NETDEV_TAP_REQUIRE_MANUAL_CLEANUP #defined (which is only
> FreeBSD).

This isn't right. The tap devices should *always* be deleted as we
don't trust that QEMU hasn't (possibly maliciously) set IFF_PERSIST
itself.

> 
> This mistake has been corrected, along with the unnecessary check for
> non-null net->ifname (it must always be non-null), and erroneous
> VIR_FREE of net->ifname.

There could be a risk of net->ifname being NULL if qemuProcessStart
fails early in startup before all tap devices have finished being
created IIUC.

> 
> Signed-off-by: Laine Stump <laine at redhat.com>
> ---
>  src/qemu/qemu_process.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 11c1ba8fb9..3449abf2ec 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -7548,10 +7548,10 @@ void qemuProcessStop(virQEMUDriverPtr driver,
>                               cfg->stateDir));
>              break;
>          case VIR_DOMAIN_NET_TYPE_ETHERNET:
> -            if (net->managed_tap != VIR_TRISTATE_BOOL_NO && net->ifname) {
> +#ifdef VIR_NETDEV_TAP_REQUIRE_MANUAL_CLEANUP
> +            if (net->managed_tap != VIR_TRISTATE_BOOL_NO)
>                  ignore_value(virNetDevTapDelete(net->ifname, net->backend.tap));
> -                VIR_FREE(net->ifname);
> -            }
> +#endif
>              break;
>          case VIR_DOMAIN_NET_TYPE_BRIDGE:
>          case VIR_DOMAIN_NET_TYPE_NETWORK:
> -- 
> 2.21.0
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

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