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

Re: [libvirt] [Patch v2 2/3] Add -netdev bridge support



On 29.06.2012 20:08, rmarwah linux vnet ibm com wrote:
> From: Richa Marwaha <rmarwah linux vnet ibm com>
> 
> This patch adds the support to run the QEMU network helper
> under unprivileged user. It also adds the support for
> attach-interface option in virsh to run under unprivileged
> user.
> 
> Signed-off-by: Richa Marwaha <rmarwah linux vnet ibm com>
> Signed-off-by: Corey Bryant<coreyb linux vnet ibm com>
> ---
> v2
> - This patch attach-interface option is tested on
> commit cd15303fd123146b0ba53e387d08ef22b707223
> 
>  src/qemu/qemu_command.c |   61 +++++++++++++++++++++++++++++++++-------------
>  src/qemu/qemu_command.h |    2 +
>  src/qemu/qemu_hotplug.c |   31 ++++++++++++++++-------
>  3 files changed, 67 insertions(+), 27 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 6549f57..4eb8cd5 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2851,6 +2851,8 @@ error:
>  
>  char *
>  qemuBuildHostNetStr(virDomainNetDefPtr net,
> +                    struct qemud_driver *driver,
> +                    virBitmapPtr qemuCaps,
>                      char type_sep,
>                      int vlan,
>                      const char *tapfd,
> @@ -2859,6 +2861,7 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
>      bool is_tap = false;
>      virBuffer buf = VIR_BUFFER_INITIALIZER;
>      enum virDomainNetType netType = virDomainNetGetActualType(net);
> +    const char *brname = NULL;
>  
>      if (net->script && netType != VIR_DOMAIN_NET_TYPE_ETHERNET) {
>          qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> @@ -2868,8 +2871,21 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
>      }
>  
>      switch (netType) {
> -    case VIR_DOMAIN_NET_TYPE_NETWORK:
> +    /*
> +     * If type='bridge', and we're running as privileged user
> +     * or -netdev bridge is not supported then it will fall
> +     * through, -net tap,fd
> +     */
>      case VIR_DOMAIN_NET_TYPE_BRIDGE:
> +        if (!driver->privileged &&
> +            qemuCapsGet(qemuCaps, QEMU_CAPS_NETDEV_BRIDGE)) {
> +            brname = virDomainNetGetActualBridgeName(net);
> +            virBufferAsprintf(&buf, "bridge%cbr=%s", type_sep, brname);
> +            type_sep = ',';
> +            is_tap = true;
> +            break;
> +        }
> +    case VIR_DOMAIN_NET_TYPE_NETWORK:
>      case VIR_DOMAIN_NET_TYPE_DIRECT:
>          virBufferAsprintf(&buf, "tap%cfd=%s", type_sep, tapfd);
>          type_sep = ',';
> @@ -4997,7 +5013,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>          for (i = 0 ; i < def->nnets ; i++) {
>              virDomainNetDefPtr net = def->nets[i];
>              char *nic, *host;
> -            char tapfd_name[50];
> +            char tapfd_name[50] = "";
>              char vhostfd_name[50] = "";
>              int vlan;
>              int bootindex = bootNet;
> @@ -5034,17 +5050,26 @@ qemuBuildCommandLine(virConnectPtr conn,
>  
>              if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
>                  actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
> -                int tapfd = qemuNetworkIfaceConnect(def, conn, driver, net,
> -                                                    qemuCaps);
> -                if (tapfd < 0)
> -                    goto error;
> -
> -                last_good_net = i;
> -                virCommandTransferFD(cmd, tapfd);
> -
> -                if (snprintf(tapfd_name, sizeof(tapfd_name), "%d",
> -                             tapfd) >= sizeof(tapfd_name))
> -                    goto no_memory;
> +                /*
> +                 * If type='bridge' then we attempt to allocate the tap fd here only if
> +                 * running under a privilged user or -netdev bridge option is not
> +                 * supported.
> +                 */
> +                 if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
> +                     driver->privileged ||
> +                     (!qemuCapsGet(qemuCaps, QEMU_CAPS_NETDEV_BRIDGE))) {
> +                     int tapfd = qemuNetworkIfaceConnect(def, conn, driver, net,
> +                                                         qemuCaps);
> +                     if (tapfd < 0)
> +                         goto error;
> +
> +                     last_good_net = i;
> +                     virCommandTransferFD(cmd, tapfd);
> +
> +                      if (snprintf(tapfd_name, sizeof(tapfd_name), "%d",
> +                                   tapfd) >= sizeof(tapfd_name))
> +                          goto no_memory;
> +                 }
>              } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) {
>                  int tapfd = qemuPhysIfaceConnect(def, driver, net,
>                                                   qemuCaps, vmop);
> @@ -5087,8 +5112,9 @@ qemuBuildCommandLine(virConnectPtr conn,
>              if (qemuCapsGet(qemuCaps, QEMU_CAPS_NETDEV) &&
>                  qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
>                  virCommandAddArg(cmd, "-netdev");
> -                if (!(host = qemuBuildHostNetStr(net, ',', vlan,
> -                                                 tapfd_name, vhostfd_name)))
> +                if (!(host = qemuBuildHostNetStr(net, driver, qemuCaps,
> +                                                 ',', vlan, tapfd_name,
> +                                                 vhostfd_name)))
>                      goto error;
>                  virCommandAddArg(cmd, host);
>                  VIR_FREE(host);
> @@ -5110,8 +5136,9 @@ qemuBuildCommandLine(virConnectPtr conn,
>              if (!(qemuCapsGet(qemuCaps, QEMU_CAPS_NETDEV) &&
>                    qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE))) {
>                  virCommandAddArg(cmd, "-net");
> -                if (!(host = qemuBuildHostNetStr(net, ',', vlan,
> -                                                 tapfd_name, vhostfd_name)))
> +                if (!(host = qemuBuildHostNetStr(net, driver, qemuCaps,
> +                                                 ',', vlan, tapfd_name,
> +                                                 vhostfd_name)))
>                      goto error;
>                  virCommandAddArg(cmd, host);
>                  VIR_FREE(host);
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index 1eafeb3..ebf7ad0 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -62,6 +62,8 @@ qemuBuildChrDeviceStr (virDomainChrDefPtr serial,
>  
>  /* With vlan == -1, use netdev syntax, else old hostnet */
>  char * qemuBuildHostNetStr(virDomainNetDefPtr net,
> +                           struct qemud_driver *driver,
> +                           virBitmapPtr qemuCaps,
>                             char type_sep,
>                             int vlan,
>                             const char *tapfd,
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index c2fa75b..deca4cb 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -699,12 +699,21 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
>  
>      if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
>          actualType == VIR_DOMAIN_NET_TYPE_NETWORK) {
> -        if ((tapfd = qemuNetworkIfaceConnect(vm->def, conn, driver, net,
> -                                             priv->qemuCaps)) < 0)
> -            goto cleanup;
> -        iface_connected = true;
> -        if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, &vhostfd) < 0)
> -            goto cleanup;
> +        /*
> +         * If type=bridge then we attempt to allocate the tap fd here only if
> +         * running under a privilged user or -netdev bridge option is not
> +         * supported.
> +         */
> +        if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
> +            driver->privileged ||
> +            (!qemuCapsGet (priv->qemuCaps, QEMU_CAPS_NETDEV_BRIDGE))) {
> +            if ((tapfd = qemuNetworkIfaceConnect(vm->def, conn, driver, net,
> +                                                 priv->qemuCaps)) < 0)
> +                goto cleanup;
> +            iface_connected = true;
> +            if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, &vhostfd) < 0)
> +                goto cleanup;
> +        }
>      } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) {
>          if ((tapfd = qemuPhysIfaceConnect(vm->def, driver, net,
>                                            priv->qemuCaps,
> @@ -752,12 +761,14 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
>  
>      if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV) &&
>          qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
> -        if (!(netstr = qemuBuildHostNetStr(net, ',',
> -                                           -1, tapfd_name, vhostfd_name)))
> +        if (!(netstr = qemuBuildHostNetStr(net, driver, priv->qemuCaps,
> +                                           ',', -1, tapfd_name,
> +                                           vhostfd_name)))
>              goto cleanup;
>      } else {
> -        if (!(netstr = qemuBuildHostNetStr(net, ' ',
> -                                           vlan, tapfd_name, vhostfd_name)))
> +        if (!(netstr = qemuBuildHostNetStr(net, driver, priv->qemuCaps,
> +                                           ' ', vlan, tapfd_name,
> +                                           vhostfd_name)))
>              goto cleanup;
>      }
>  
> 

ACK

I am not going to review next apparmor patch which has already been
reviewed.

Michal


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