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

Re: [libvirt] [PATCHv4 4/4] qemu: Enable multiqueue network



On 05/21/2013 10:18 AM, Michal Privoznik wrote:
> ---
>  src/qemu/qemu_command.c | 36 ++++++++++++++++++++++++++++--------
>  src/qemu/qemu_hotplug.c | 37 ++++++++++++++++++++++++++-----------
>  2 files changed, 54 insertions(+), 19 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 7059b08..0474670 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6503,14 +6503,28 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
>      if (!bootindex)
>          bootindex = net->info.bootIndex;
>  
> +    /* Currently nothing else then TAP devices supports multiqueue. */


s/nothing else/nothing besides/   (or "nothing other than")


> +    if (net->driver.virtio.queues &&
> +        !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
> +          actualType == VIR_DOMAIN_NET_TYPE_BRIDGE)) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Multiqueue network is not supported for: %s"),
> +                       virDomainNetTypeToString(actualType));
> +        return -1;

You probably want to check for queues > 1 rather than != 0.

(I would *almost* suggest logging this error anytime tapfdSize came back
from the subordinate function(s) smaller than net->driver.virtio.queues;
that way would wouldn't have to have top change stuff about non-support
in multiple places when macvtap gets the support later. However, if you
did the error reporting that way, you wouldn't be able to give as many
specifics about the reasoning.)

> +    }
> +
>      if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
>          actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
> -        if (VIR_ALLOC(tapfd) < 0 || VIR_ALLOC(tapfdName) < 0) {
> +        tapfdSize = net->driver.virtio.queues;
> +        if (!tapfdSize)
> +            tapfdSize = 1;
> +
> +        if (VIR_ALLOC_N(tapfd, tapfdSize) < 0 ||
> +            VIR_ALLOC_N(tapfdName, tapfdSize) < 0) {
>              virReportOOMError();
>              goto cleanup;
>          }
>  
> -        tapfdSize = 1;
>          if (qemuNetworkIfaceConnect(def, conn, driver, net,
>                                      qemuCaps, tapfd, &tapfdSize) < 0)
>              goto cleanup;
> @@ -6532,11 +6546,15 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
>          actualType == VIR_DOMAIN_NET_TYPE_DIRECT) {
>          /* Attempt to use vhost-net mode for these types of
>             network device */
> -        if (VIR_ALLOC(vhostfd) < 0 || VIR_ALLOC(vhostfdName)) {
> +        vhostfdSize = net->driver.virtio.queues;
> +        if (!vhostfdSize)
> +            vhostfdSize = 1;
> +
> +        if (VIR_ALLOC_N(vhostfd, vhostfdSize) < 0 ||
> +            VIR_ALLOC_N(vhostfdName, vhostfdSize)) {
>              virReportOOMError();
>              goto cleanup;
>          }
> -        vhostfdSize = 1;
>  
>          if (qemuOpenVhostNet(def, net, qemuCaps, vhostfd, &vhostfdSize) < 0)
>              goto cleanup;
> @@ -6600,15 +6618,17 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
>  cleanup:
>      if (ret < 0)
>          virDomainConfNWFilterTeardown(net);
> -    for (i = 0; i < tapfdSize; i++) {
> +    for (i = 0; tapfd && i < tapfdSize; i++) {
>          if (ret < 0)
>              VIR_FORCE_CLOSE(tapfd[i]);
> -        VIR_FREE(tapfdName[i]);
> +        if (tapfdName)
> +            VIR_FREE(tapfdName[i]);
>      }
> -    for (i = 0; i < vhostfdSize; i++) {
> +    for (i = 0; vhostfd && i < vhostfdSize; i++) {
>          if (ret < 0)
>              VIR_FORCE_CLOSE(vhostfd[i]);
> -        VIR_FREE(vhostfdName[i]);
> +        if (vhostfdName)
> +            VIR_FREE(vhostfdName[i]);
>      }
>      VIR_FREE(tapfd);
>      VIR_FREE(vhostfd);
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 7e50592..e7d200f 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -740,13 +740,26 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
>          goto cleanup;
>      }
>  
> +    /* Currently nothing else then TAP devices supports multiqueue. */
> +    if (net->driver.virtio.queues &&
> +        !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
> +          actualType == VIR_DOMAIN_NET_TYPE_BRIDGE)) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Multiqueue network is not supported for: %s"),
> +                       virDomainNetTypeToString(actualType));
> +        return -1;
> +    }
> +
>      if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
>          actualType == VIR_DOMAIN_NET_TYPE_NETWORK) {
> -        if (VIR_ALLOC(tapfd) < 0 || VIR_ALLOC(vhostfd) < 0) {
> +        tapfdSize = vhostfdSize = net->driver.virtio.queues;
> +        if (!tapfdSize)
> +            tapfdSize = vhostfdSize = 1;
> +        if (VIR_ALLOC_N(tapfd, tapfdSize) < 0 ||
> +            VIR_ALLOC_N(vhostfd, vhostfdSize) < 0) {
>              virReportOOMError();
>              goto cleanup;
>          }
> -        tapfdSize = vhostfdSize = 1;
>          if (qemuNetworkIfaceConnect(vm->def, conn, driver, net,
>                                      priv->qemuCaps, tapfd, &tapfdSize) < 0)
>              goto cleanup;
> @@ -754,11 +767,11 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
>          if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, vhostfd, &vhostfdSize) < 0)
>              goto cleanup;
>      } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) {
> +        tapfdSize = vhostfdSize = 1;
>          if (VIR_ALLOC(tapfd) < 0 || VIR_ALLOC(vhostfd) < 0) {
>              virReportOOMError();
>              goto cleanup;
>          }
> -        tapfdSize = vhostfdSize = 1;
>          if ((tapfd[0] = qemuPhysIfaceConnect(vm->def, driver, net,
>                                               priv->qemuCaps,
>                                               VIR_NETDEV_VPORT_PROFILE_OP_CREATE)) < 0)
> @@ -767,11 +780,11 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
>          if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, vhostfd, &vhostfdSize) < 0)
>              goto cleanup;
>      } else if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) {
> -        if (VIR_ALLOC(vhostfd) < 0) {
> -            virReportOOMError();
> -            goto cleanup;
> -        }
>          vhostfdSize = 1;
> +        if (VIR_ALLOC(vhostfd) < 0) {
> +            virReportOOMError();
> +            goto cleanup;
> +        }
>          if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, vhostfd, &vhostfdSize) < 0)
>              goto cleanup;
>      }
> @@ -961,15 +974,17 @@ cleanup:
>  
>      VIR_FREE(nicstr);
>      VIR_FREE(netstr);
> -    for (i = 0; i < tapfdSize; i++) {
> +    for (i = 0; tapfd && i < tapfdSize; i++) {
>          VIR_FORCE_CLOSE(tapfd[i]);
> -        VIR_FREE(tapfdName[i]);
> +        if (tapfdName)
> +            VIR_FREE(tapfdName[i]);
>      }
>      VIR_FREE(tapfd);
>      VIR_FREE(tapfdName);
> -    for (i = 0; i < vhostfdSize; i++) {
> +    for (i = 0; vhostfd && i < vhostfdSize; i++) {
>          VIR_FORCE_CLOSE(vhostfd[i]);
> -        VIR_FREE(vhostfdName[i]);
> +        if (vhostfdName)
> +            VIR_FREE(vhostfdName[i]);
>      }
>      VIR_FREE(vhostfd);
>      VIR_FREE(vhostfdName);

ACK with those two small fixes.


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