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

From: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com&gt; 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(a)linux.vnet.ibm.com&gt; Signed-off-by: Corey Bryant&lt;coreyb(a)linux.vnet.ibm.com&gt; --- 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; }