Re: [PATCH 2/3] qemu: move runtime netdev validation into a separate function

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

 



On 9/13/19 4:52 PM, Laine Stump wrote:
> The same validation should be done for both static network devices and
> hotplugged devices, but they are currently inconsistent. Move all the
> relevant validation from qemuBuildInterfaceCommandLine() into the new
> function qemuDomainValidateActualNetDef() and call the latter from
> the former.
> 
> Signed-off-by: Laine Stump <laine@xxxxxxxxxx>
> ---
>  src/qemu/qemu_command.c | 52 +--------------------------
>  src/qemu/qemu_domain.c  | 80 +++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_domain.h  |  4 +++
>  3 files changed, 85 insertions(+), 51 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index f795f2e987..2acae3bf33 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -8352,50 +8352,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
>      if (!bootindex)
>          bootindex = net->info.bootIndex;
>  
> -    /* Currently nothing besides TAP devices supports multiqueue. */
> -    if (net->driver.virtio.queues > 0 &&
> -        !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
> -          actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
> -          actualType == VIR_DOMAIN_NET_TYPE_DIRECT ||
> -          actualType == VIR_DOMAIN_NET_TYPE_ETHERNET ||
> -          actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER)) {
> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                       _("Multiqueue network is not supported for: %s"),
> -                       virDomainNetTypeToString(actualType));
> +    if (qemuDomainValidateActualNetDef(net, qemuCaps) < 0)
>          return -1;
> -    }
> -
> -    /* and only TAP devices support nwfilter rules */
> -    if (net->filter) {
> -        virNetDevVPortProfilePtr vport = virDomainNetGetActualVirtPortProfile(net);
> -        if (!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
> -              actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
> -              actualType == VIR_DOMAIN_NET_TYPE_ETHERNET)) {
> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                           _("filterref is not supported for "
> -                             "network interfaces of type %s"),
> -                           virDomainNetTypeToString(actualType));
> -            return -1;
> -        }
> -        if (vport && vport->virtPortType != VIR_NETDEV_VPORT_PROFILE_NONE) {
> -            /* currently none of the defined virtualport types support iptables */
> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                           _("filterref is not supported for "
> -                             "network interfaces with virtualport type %s"),
> -                           virNetDevVPortTypeToString(vport->virtPortType));
> -            return -1;
> -        }
> -    }
> -
> -    if (net->backend.tap &&
> -        !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
> -          actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
> -          actualType == VIR_DOMAIN_NET_TYPE_ETHERNET)) {
> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                       _("Custom tap device path is not supported for: %s"),
> -                       virDomainNetTypeToString(actualType));
> -        return -1;
> -    }
>  
>      switch (actualType) {
>      case VIR_DOMAIN_NET_TYPE_NETWORK:
> @@ -8458,14 +8416,6 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
>      case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
>          requireNicdev = true;
>  
> -        if (net->driver.virtio.queues > 1 &&
> -            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQUEUE)) {
> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                           _("multi-queue is not supported for vhost-user "
> -                             "with this QEMU binary"));
> -            goto cleanup;
> -        }
> -
>          if (qemuInterfaceVhostuserConnect(driver, logManager, secManager,
>                                            cmd, def, net, qemuCaps, &chardev) < 0)
>              goto cleanup;
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index bd247628cb..ebbe1a85db 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -5300,6 +5300,86 @@ qemuDomainWatchdogDefValidate(const virDomainWatchdogDef *dev,
>  }
>  
>  
> +

3 empty lines instead of 2.

> +int
> +qemuDomainValidateActualNetDef(const virDomainNetDef *net,
> +                               virQEMUCapsPtr qemuCaps)
> +{
> +    /*
> +     * Validations that can only be properly checked at runtime (after
> +     * an <interface type='network'> has been resolved to its actual
> +     * type.
> +     *
> +     * (In its current form this function can still be called before
> +     * the actual type has been resolved (e.g. at domain definition
> +     * time), but only if the validations would SUCCEED for
> +     * type='network'.)
> +     */
> +    virDomainNetType actualType = virDomainNetGetActualType(net);
> +
> +    /* Only tap/macvtap devices support multiqueue. */
> +    if (net->driver.virtio.queues > 1) {

I don't think that this is right. Take VIR_DOMAIN_NET_TYPE_USER for instance. It doesn't allow anything but queues == 0.

> +
> +        if (!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
> +              actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
> +              actualType == VIR_DOMAIN_NET_TYPE_DIRECT ||
> +              actualType == VIR_DOMAIN_NET_TYPE_ETHERNET ||
> +              actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER)) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("multiqueue network is not supported for: %s"),
> +                           virDomainNetTypeToString(actualType));
> +            return -1;
> +        }
> +> +        if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER &&

This is actually where a single queue can be permitred. At least according to the original code.

> +            qemuCaps && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQUEUE)) {

NULL is never passed to qemuCaps, so no need to check it.

> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("multiqueue network is not supported for vhost-user "
> +                             "with this QEMU binary"));
> +            return -1;
> +        }
> +    }
> +
> +    /*
> +     * Only standard tap devices support nwfilter rules, and even then only
> +     * when *not* connected to an OVS bridge or midonet (indicated by having
> +     * a <virtualport> element in the config)
> +     */
> +    if (net->filter) {
> +        virNetDevVPortProfilePtr vport = virDomainNetGetActualVirtPortProfile(net);
> +        if (!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
> +              actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
> +              actualType == VIR_DOMAIN_NET_TYPE_ETHERNET)) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("filterref is not supported for "
> +                             "network interfaces of type %s"),
> +                           virDomainNetTypeToString(actualType));
> +            return -1;
> +        }
> +        if (vport && vport->virtPortType != VIR_NETDEV_VPORT_PROFILE_NONE) {
> +            /* currently none of the defined virtualport types support iptables */
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("filterref is not supported for "
> +                             "network interfaces with virtualport type %s"),
> +                           virNetDevVPortTypeToString(vport->virtPortType));
> +            return -1;
> +        }
> +    }
> +
> +    if (net->backend.tap &&
> +        !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
> +          actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
> +          actualType == VIR_DOMAIN_NET_TYPE_ETHERNET)) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Custom tap device path is not supported for: %s"),
> +                       virDomainNetTypeToString(actualType));
> +        return -1;
> +    }
> +
> +    return 0;
> + }

s/^ //

ACK with this squashed in:

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ebbe1a85db..fa0dd888c8 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5300,7 +5300,6 @@ qemuDomainWatchdogDefValidate(const virDomainWatchdogDef *dev,
 }
 
 
-
 int
 qemuDomainValidateActualNetDef(const virDomainNetDef *net,
                                virQEMUCapsPtr qemuCaps)
@@ -5318,8 +5317,7 @@ qemuDomainValidateActualNetDef(const virDomainNetDef *net,
     virDomainNetType actualType = virDomainNetGetActualType(net);
 
     /* Only tap/macvtap devices support multiqueue. */
-    if (net->driver.virtio.queues > 1) {
-
+    if (net->driver.virtio.queues > 0) {
         if (!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
               actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
               actualType == VIR_DOMAIN_NET_TYPE_DIRECT ||
@@ -5331,8 +5329,9 @@ qemuDomainValidateActualNetDef(const virDomainNetDef *net,
             return -1;
         }
 
-        if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER &&
-            qemuCaps && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQUEUE)) {
+        if (net->driver.virtio.queues > 1 &&
+            actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER &&
+            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQUEUE)) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                            _("multiqueue network is not supported for vhost-user "
                              "with this QEMU binary"));
@@ -5377,7 +5376,7 @@ qemuDomainValidateActualNetDef(const virDomainNetDef *net,
     }
 
     return 0;
- }
+}


Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux