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

Re: [libvirt] [PATCH v2 07/12] qemu: Allow multiple vhost-net openings



On 05/13/2013 01:23 PM, Michal Privoznik wrote:
> With multiqueue network feature, we are advised to pass multiple
> vhost-net FDs as well. The ratio should be 1:1. Therefore we must
> alter the qemuOpenVhostNet function to allow that.
> ---
>  src/qemu/qemu_command.c | 40 ++++++++++++++++++++++++++--------------
>  src/qemu/qemu_command.h |  3 ++-
>  src/qemu/qemu_hotplug.c |  6 +++---
>  3 files changed, 31 insertions(+), 18 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index e4bb2b7..13e3488 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -409,9 +409,13 @@ int
>  qemuOpenVhostNet(virDomainDefPtr def,
>                   virDomainNetDefPtr net,
>                   virQEMUCapsPtr qemuCaps,
> -                 int *vhostfd)
> +                 int *vhostfd,
> +                 int vhostfdSize)
>  {
> -    *vhostfd = -1;   /* assume we won't use vhost */
> +    int i;
> +
> +    for (i = 0; i < vhostfdSize; i++)
> +        vhostfd[i] = -1;   /* assume we won't use vhost */


Would it maybe be cleaner for the caller if the args had int
*vhostfdSize, and this function set it to 0 if it determined we didn't
want to use vhostnet?


>  
>      /* If the config says explicitly to not use vhost, return now */
>      if (net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_QEMU) {
> @@ -444,20 +448,28 @@ qemuOpenVhostNet(virDomainDefPtr def,
>          return 0;
>      }
>  
> -    *vhostfd = open("/dev/vhost-net", O_RDWR);
> -    virDomainAuditNetDevice(def, net, "/dev/vhost-net", *vhostfd >= 0);
> +    for (i = 0; i < vhostfdSize; i++) {
> +        vhostfd[i] = open("/dev/vhost-net", O_RDWR);
> +        virDomainAuditNetDevice(def, net, "/dev/vhost-net", vhostfd[i] >= 0);
>  
> -    /* If the config says explicitly to use vhost and we couldn't open it,
> -     * report an error.
> -     */
> -    if ((*vhostfd < 0) &&
> -        (net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_VHOST)) {
> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                       "%s", _("vhost-net was requested for an interface, "
> -                               "but is unavailable"));
> -        return -1;
> +        /* If the config says explicitly to use vhost and we couldn't open it,
> +         * report an error.
> +         */
> +        if (vhostfd[i] < 0 &&
> +            net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_VHOST) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           "%s", _("vhost-net was requested for an interface, "
> +                                   "but is unavailable"));
> +            goto error;
> +        }
>      }
>      return 0;
> +
> +error:
> +    for (i = 0; i < vhostfdSize; i++)
> +        VIR_FORCE_CLOSE(vhostfd[i]);


This time you're saved by the fact that you've initialized vhostfd to
all -1, but I still would prefer if you instead made this a while (i--)
{ } loop so that you never even look at items in the array that aren't
valid fds.

(and if you change it so that vhostfdSize can be changed to 0, you can
then avoid initializing vhostfd[].)

> +
> +    return -1;
>  }
>  
>  int
> @@ -6431,7 +6443,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
>             network device */
>          int vhostfd;
>  
> -        if (qemuOpenVhostNet(def, net, qemuCaps, &vhostfd) < 0)
> +        if (qemuOpenVhostNet(def, net, qemuCaps, &vhostfd, 1) < 0)
>              goto cleanup;
>          if (vhostfd >= 0) {
>              virCommandTransferFD(cmd, vhostfd);
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index 1068b4d..d9e620a 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -167,7 +167,8 @@ int qemuPhysIfaceConnect(virDomainDefPtr def,
>  int qemuOpenVhostNet(virDomainDefPtr def,
>                       virDomainNetDefPtr net,
>                       virQEMUCapsPtr qemuCaps,
> -                     int *vhostfd);
> +                     int *vhostfd,
> +                     int vhostfdSize);
>  
>  int qemuNetworkPrepareDevices(virDomainDefPtr def);
>  
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 0a1845a..9ca1e43 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -738,7 +738,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
>                                               priv->qemuCaps)) < 0)
>              goto cleanup;
>          iface_connected = true;
> -        if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, &vhostfd) < 0)
> +        if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, &vhostfd, 1) < 0)
>              goto cleanup;
>      } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) {
>          if ((tapfd = qemuPhysIfaceConnect(vm->def, driver, net,
> @@ -746,10 +746,10 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
>                                            VIR_NETDEV_VPORT_PROFILE_OP_CREATE)) < 0)
>              goto cleanup;
>          iface_connected = true;
> -        if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, &vhostfd) < 0)
> +        if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, &vhostfd, 1) < 0)
>              goto cleanup;
>      } else if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) {
> -        if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, &vhostfd) < 0)
> +        if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, &vhostfd, 1) < 0)
>              goto cleanup;
>      }
>  


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