[libvirt] [PATCH 2/2] Error out when custom tap device path makes no sense

[John Ferlan]

On 02/05/2015 07:52 AM, Ján Tomko wrote:
> It is only usable for NETWORK and BRIDGE type interfaces.
> Error out when trying to start a domain where the custom
> tap device path is specified for interfaces of other types,
> or when the daemon is not privileged.
> 
> Note that this cannot be checked at definition time, because
> the comparison is against actual type.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1147195
> ---
>  src/qemu/qemu_command.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 

Note: Something I forgot for [1/2]

The <backend> is not well described in formatdomain.html.in - perhaps
take the opportunity to indicate that <backend> attribute "vhost" is
only valid for virtio network interfaces. Do you think it's reasonable
to list the default of value "/dev/vhost-net"

Then of course for this one - formatdomain.html.in - the <backend>
attribute "tap" is only usable for NETWORK and BRIDGE type interfaces.
Furthermore, custom "tap" devices are not supported in session mode. Do
you think it's reasonable to list the default value "/dev/net/tun"?


> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 3b6eddc..06a59d0 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -299,8 +299,14 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
>      virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>      const char *tunpath = "/dev/net/tun";
>  
> -    if (net->backend.tap)
> +    if (net->backend.tap) {
>          tunpath = net->backend.tap;
> +        if (!cfg->privileged) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("cannot use custom tap device in session mode"));
> +            goto cleanup;
> +        }
> +    }

This seems reasonable, wasn't part of the bz, but still should be
documented...

ACK - with the doc changes...

John
>  
>      if (!(brname = virDomainNetGetActualBridgeName(net))) {
>          virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing bridge name"));
> @@ -7721,6 +7727,15 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
>          return -1;
>      }
>  
> +    if (net->backend.tap &&
> +        !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
> +          actualType == VIR_DOMAIN_NET_TYPE_BRIDGE)) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Custom tap device path is not supported for: %s"),
> +                       virDomainNetTypeToString(actualType));
> +        return -1;
> +    }
> +
>      if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
>          actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
>          tapfdSize = net->driver.virtio.queues;
>