Currently, we have one huge function to construct qemu command line. This is very ineffective esp. if there's a fault somewhere. --- src/qemu/qemu_command.c | 323 +++++++++++++++++++++++++----------------------- 1 file changed, 171 insertions(+), 152 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 05c12b2..4af2d04 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5751,6 +5751,172 @@ error: return -1; } +static int +qemuBuildInterfaceCommandLine(virCommandPtr cmd, + virQEMUDriverPtr driver, + virConnectPtr conn, + virDomainDefPtr def, + virDomainNetDefPtr net, + virQEMUCapsPtr qemuCaps, + int vlan, + int bootNet, + enum virNetDevVPortProfileOp vmop) +{ + int ret = -1; + int tapfd = -1; + char *nic = NULL, *host = NULL; + char *tapfdName = NULL; + char *vhostfdName = NULL; + int bootindex = bootNet; + int actualType; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + + if (!bootindex) + bootindex = net->info.bootIndex; + + /* If appropriate, grab a physical device from the configured + * network's pool of devices, or resolve bridge device name + * to the one defined in the network definition. + */ + if (networkAllocateActualDevice(net) < 0) + goto cleanup; + + actualType = virDomainNetGetActualType(net); + if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + virDomainHostdevDefPtr hostdev = virDomainNetGetActualHostdev(net); + virDomainHostdevDefPtr found; + /* For a network with <forward mode='hostdev'>, there is a need to + * add the newly minted hostdev to the hostdevs array. + */ + if (qemuAssignDeviceHostdevAlias(def, hostdev, def->nhostdevs-1) < 0) + goto cleanup; + + if (virDomainHostdevFind(def, hostdev, &found) < 0) { + if (virDomainHostdevInsert(def, hostdev) < 0) { + virReportOOMError(); + goto cleanup; + } + if (qemuPrepareHostdevPCIDevices(driver, def->name, def->uuid, + &hostdev, 1) < 0) { + goto cleanup; + } + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("PCI device %04x:%02x:%02x.%x " + "allocated from network %s is already " + "in use by domain %s"), + hostdev->source.subsys.u.pci.domain, + hostdev->source.subsys.u.pci.bus, + hostdev->source.subsys.u.pci.slot, + hostdev->source.subsys.u.pci.function, + net->data.network.name, + def->name); + goto cleanup; + } + } + /* hostdev interface doesn't require nor -net nor -netdev */ + ret = 0; + goto cleanup; + } + + if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || + actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) { + /* + * 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 || + cfg->privileged || + (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV_BRIDGE))) { + tapfd = qemuNetworkIfaceConnect(def, conn, driver, net, + qemuCaps); + if (tapfd < 0) + goto cleanup; + } + } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { + tapfd = qemuPhysIfaceConnect(def, driver, net, + qemuCaps, vmop); + if (tapfd < 0) + goto cleanup; + } + + if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || + actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || + actualType == VIR_DOMAIN_NET_TYPE_ETHERNET || + actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { + /* Attempt to use vhost-net mode for these types of + network device */ + int vhostfd; + + if (qemuOpenVhostNet(def, net, qemuCaps, &vhostfd) < 0) + goto cleanup; + if (vhostfd >= 0) { + virCommandTransferFD(cmd, vhostfd); + + if (virAsprintf(&vhostfdName, "%d", vhostfd) < 0) { + virReportOOMError(); + goto cleanup; + } + } + } + + if (tapfd >= 0) { + virCommandTransferFD(cmd, tapfd); + + if (virAsprintf(&tapfdName, "%d", tapfd) < 0) { + virReportOOMError(); + goto cleanup; + } + } + + /* Possible combinations: + * + * 1. Old way: -net nic,model=e1000,vlan=1 -net tap,vlan=1 + * 2. Semi-new: -device e1000,vlan=1 -net tap,vlan=1 + * 3. Best way: -netdev type=tap,id=netdev1 -device e1000,id=netdev1 + * + * NB, no support for -netdev without use of -device + */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + if (!(host = qemuBuildHostNetStr(net, driver, qemuCaps, + ',', vlan, tapfdName, + vhostfdName))) + goto cleanup; + virCommandAddArgList(cmd, "-netdev", host, NULL); + } + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + if (!(nic = qemuBuildNicDevStr(net, vlan, bootindex, qemuCaps))) + goto cleanup; + virCommandAddArgList(cmd, "-device", nic, NULL); + } else { + if (!(nic = qemuBuildNicStr(net, "nic,", vlan))) + goto cleanup; + virCommandAddArgList(cmd, "-net", nic, NULL); + } + if (!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE))) { + if (!(host = qemuBuildHostNetStr(net, driver, qemuCaps, + ',', vlan, tapfdName, + vhostfdName))) + goto cleanup; + virCommandAddArgList(cmd, "-net", host, NULL); + } + + ret = 0; +cleanup: + if (ret < 0) + virDomainConfNWFilterTeardown(net); + VIR_FREE(nic); + VIR_FREE(host); + VIR_FREE(tapfdName); + VIR_FREE(vhostfdName); + virObjectUnref(cfg); + return ret; +} + /* * Constructs a argv suitable for launching qemu with config defined * for a given virtual machine. @@ -6713,16 +6879,7 @@ qemuBuildCommandLine(virConnectPtr conn, for (i = 0 ; i < def->nnets ; i++) { virDomainNetDefPtr net = def->nets[i]; - char *nic, *host; - char tapfd_name[50] = ""; - char vhostfd_name[50] = ""; int vlan; - int bootindex = bootNet; - int actualType; - - bootNet = 0; - if (!bootindex) - bootindex = net->info.bootIndex; /* VLANs are not used with -netdev, so don't record them */ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV) && @@ -6731,149 +6888,11 @@ qemuBuildCommandLine(virConnectPtr conn, else vlan = i; - /* If appropriate, grab a physical device from the configured - * network's pool of devices, or resolve bridge device name - * to the one defined in the network definition. - */ - if (networkAllocateActualDevice(net) < 0) - goto error; - - actualType = virDomainNetGetActualType(net); - if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { - if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { - virDomainHostdevDefPtr hostdev = virDomainNetGetActualHostdev(net); - virDomainHostdevDefPtr found; - /* For a network with <forward mode='hostdev'>, there is a need to - * add the newly minted hostdev to the hostdevs array. - */ - if (qemuAssignDeviceHostdevAlias(def, hostdev, - (def->nhostdevs-1)) < 0) { - goto error; - } - - if (virDomainHostdevFind(def, hostdev, &found) < 0) { - if (virDomainHostdevInsert(def, hostdev) < 0) { - virReportOOMError(); - goto error; - } - if (qemuPrepareHostdevPCIDevices(driver, def->name, def->uuid, - &hostdev, 1) < 0) { - goto error; - } - } - else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("PCI device %04x:%02x:%02x.%x " - "allocated from network %s is already " - "in use by domain %s"), - hostdev->source.subsys.u.pci.domain, - hostdev->source.subsys.u.pci.bus, - hostdev->source.subsys.u.pci.slot, - hostdev->source.subsys.u.pci.function, - net->data.network.name, - def->name); - goto error; - } - } - continue; - } - - if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || - actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) { - /* - * 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 || - cfg->privileged || - (!virQEMUCapsGet(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); - 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 (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || - actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || - actualType == VIR_DOMAIN_NET_TYPE_ETHERNET || - actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { - /* Attempt to use vhost-net mode for these types of - network device */ - int vhostfd; - - if (qemuOpenVhostNet(def, net, qemuCaps, &vhostfd) < 0) - goto error; - if (vhostfd >= 0) { - virCommandTransferFD(cmd, vhostfd); - - if (snprintf(vhostfd_name, sizeof(vhostfd_name), "%d", - vhostfd) >= sizeof(vhostfd_name)) - goto no_memory; - } - } - /* Possible combinations: - * - * 1. Old way: -net nic,model=e1000,vlan=1 -net tap,vlan=1 - * 2. Semi-new: -device e1000,vlan=1 -net tap,vlan=1 - * 3. Best way: -netdev type=tap,id=netdev1 -device e1000,id=netdev1 - * - * NB, no support for -netdev without use of -device - */ - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - virCommandAddArg(cmd, "-netdev"); - if (!(host = qemuBuildHostNetStr(net, driver, qemuCaps, - ',', vlan, tapfd_name, - vhostfd_name))) - goto error; - virCommandAddArg(cmd, host); - VIR_FREE(host); - } - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - virCommandAddArg(cmd, "-device"); - nic = qemuBuildNicDevStr(net, vlan, bootindex, qemuCaps); - if (!nic) - goto error; - virCommandAddArg(cmd, nic); - VIR_FREE(nic); - } else { - virCommandAddArg(cmd, "-net"); - if (!(nic = qemuBuildNicStr(net, "nic,", vlan))) - goto error; - virCommandAddArg(cmd, nic); - VIR_FREE(nic); - } - if (!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE))) { - virCommandAddArg(cmd, "-net"); - if (!(host = qemuBuildHostNetStr(net, driver, qemuCaps, - ',', vlan, tapfd_name, - vhostfd_name))) - goto error; - virCommandAddArg(cmd, host); - VIR_FREE(host); - } + if (qemuBuildInterfaceCommandLine(cmd, driver, conn, def, net, + qemuCaps, vlan, bootNet, vmop) < 0) + goto error; + last_good_net = i; + bootNet = 0; } } -- 1.8.1.5

Currently, only one tapfd and one vhostfd could be passed. However, multiqueue network requires several FDs to be passed to qemu so we must adapt out monitor handling functions to cope with that. --- src/qemu/qemu_hotplug.c | 7 +++++-- src/qemu/qemu_monitor.c | 39 +++++++++++++++++++++++---------------- src/qemu/qemu_monitor.h | 4 ++-- 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 254ab4a..d1347c6 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -830,8 +830,11 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, goto cleanup; } } else { - if (qemuMonitorAddHostNetwork(priv->mon, netstr, tapfd, tapfd_name, - vhostfd, vhostfd_name) < 0) { + if (qemuMonitorAddHostNetwork(priv->mon, netstr, + &tapfd, &tapfd_name, + tapfd_name ? 1 : 0, + &vhostfd, &vhostfd_name, + vhostfd_name ? 1 : 0) < 0) { qemuDomainObjExitMonitor(driver, vm); virDomainAuditNet(vm, NULL, net, "attach", false); goto cleanup; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 4ca80f4..763dcb2 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2460,14 +2460,16 @@ cleanup: int qemuMonitorAddHostNetwork(qemuMonitorPtr mon, const char *netstr, - int tapfd, const char *tapfd_name, - int vhostfd, const char *vhostfd_name) + int *tapfd, char **tapfdName, int tapfdSize, + int *vhostfd, char **vhostfdName, int vhostfdSize) { int ret = -1; - VIR_DEBUG("mon=%p netstr=%s tapfd=%d tapfd_name=%s " - "vhostfd=%d vhostfd_name=%s", - mon, netstr, tapfd, NULLSTR(tapfd_name), - vhostfd, NULLSTR(vhostfd_name)); + int i = 0, j = 0; + + VIR_DEBUG("mon=%p netstr=%s tapfd=%p tapfdName=%p tapfdSize=%d " + "vhostfd=%p vhostfdName=%p vhostfdSize=%d", + mon, netstr, tapfd, tapfdName, tapfdSize, + vhostfd, vhostfdName, vhostfdSize); if (!mon) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -2475,12 +2477,13 @@ int qemuMonitorAddHostNetwork(qemuMonitorPtr mon, return -1; } - if (tapfd >= 0 && qemuMonitorSendFileHandle(mon, tapfd_name, tapfd) < 0) - return -1; - if (vhostfd >= 0 && - qemuMonitorSendFileHandle(mon, vhostfd_name, vhostfd) < 0) { - vhostfd = -1; - goto cleanup; + for (i = 0; i < tapfdSize; i++) { + if (qemuMonitorSendFileHandle(mon, tapfdName[i], tapfd[i]) < 0) + goto cleanup; + } + for (j = 0; j < vhostfdSize; j++) { + if (qemuMonitorSendFileHandle(mon, vhostfdName[j], vhostfd[j]) < 0) + goto cleanup; } if (mon->json) @@ -2491,10 +2494,14 @@ int qemuMonitorAddHostNetwork(qemuMonitorPtr mon, cleanup: if (ret < 0) { - if (tapfd >= 0 && qemuMonitorCloseFileHandle(mon, tapfd_name) < 0) - VIR_WARN("failed to close device handle '%s'", tapfd_name); - if (vhostfd >= 0 && qemuMonitorCloseFileHandle(mon, vhostfd_name) < 0) - VIR_WARN("failed to close device handle '%s'", vhostfd_name); + while (i--) { + if (qemuMonitorCloseFileHandle(mon, tapfdName[i]) < 0) + VIR_WARN("failed to close device handle '%s'", tapfdName[i]); + } + while (j--) { + if (qemuMonitorCloseFileHandle(mon, vhostfdName[j]) < 0) + VIR_WARN("failed to close device handle '%s'", vhostfdName[j]); + } } return ret; diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 580e42b..45257a4 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -497,8 +497,8 @@ int qemuMonitorRemoveFd(qemuMonitorPtr mon, int fdset, int fd); */ int qemuMonitorAddHostNetwork(qemuMonitorPtr mon, const char *netstr, - int tapfd, const char *tapfd_name, - int vhostfd, const char *vhostfd_name); + int *tapfd, char **tapfdName, int tapfdSize, + int *vhostfd, char **vhostfdName, int vhostfdSize); int qemuMonitorRemoveHostNetwork(qemuMonitorPtr mon, int vlan, -- 1.8.1.5

Currently, the function knows how to open a TAP device for a single time. However, in case of multiqueue network we need to open it multiple times. Moreover, when doing TUNSETIFF ioctl, the IFF_MULTI_QUEUE flag shall be requested. This commit changes a behaviour slightly as well. Till now it was possible to pass NULL as an @fd address by which caller didn't care about returned FD. While this was used only in UML driver, the appropriate UML code snippets has been changed as well. --- src/network/bridge_driver.c | 2 +- src/qemu/qemu_command.c | 2 +- src/uml/uml_conf.c | 5 ++- src/util/virnetdevtap.c | 105 ++++++++++++++++++++++++-------------------- src/util/virnetdevtap.h | 2 + 5 files changed, 66 insertions(+), 50 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 27dd230..7118702 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2424,7 +2424,7 @@ networkStartNetworkVirtual(struct network_driver *driver, /* Keep tun fd open and interface up to allow for IPv6 DAD to happen */ if (virNetDevTapCreateInBridgePort(network->def->bridge, &macTapIfName, &network->def->mac, - NULL, &tapfd, NULL, NULL, + NULL, &tapfd, 1, NULL, NULL, VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE | VIR_NETDEV_TAP_CREATE_IFUP | VIR_NETDEV_TAP_CREATE_PERSIST) < 0) { diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b5374e0..f5d11ad 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -272,7 +272,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, } err = virNetDevTapCreateInBridgePort(brname, &net->ifname, &net->mac, - def->uuid, &tapfd, + def->uuid, &tapfd, 1, virDomainNetGetActualVirtPortProfile(net), virDomainNetGetActualVlan(net), tap_create_flags); diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 4fa7927..4816f06 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -110,6 +110,7 @@ umlConnectTapDevice(virConnectPtr conn, const char *bridge) { bool template_ifname = false; + int tapfd; if (!net->ifname || STRPREFIX(net->ifname, VIR_NET_GENERATED_PREFIX) || @@ -122,7 +123,7 @@ umlConnectTapDevice(virConnectPtr conn, } if (virNetDevTapCreateInBridgePort(bridge, &net->ifname, &net->mac, - vm->uuid, NULL, + vm->uuid, &tapfd, 1, virDomainNetGetActualVirtPortProfile(net), virDomainNetGetActualVlan(net), VIR_NETDEV_TAP_CREATE_IFUP | @@ -140,11 +141,13 @@ umlConnectTapDevice(virConnectPtr conn, } } + VIR_FORCE_CLOSE(tapfd); return 0; no_memory: virReportOOMError(); error: + VIR_FORCE_CLOSE(tapfd); return -1; } diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index e4ce223..cdf6bd3 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -106,7 +106,8 @@ virNetDevProbeVnetHdr(int tapfd) /** * virNetDevTapCreate: * @ifname: the interface name - * @tapfd: file descriptor return value for the new tap device + * @tapfds: file descriptor return value for the new tap device + * @tapfdSize: size of @tapfd * @flags: OR of virNetDevTapCreateFlags. Only one flag is recognized: * * VIR_NETDEV_TAP_CREATE_VNET_HDR @@ -114,76 +115,83 @@ virNetDevProbeVnetHdr(int tapfd) * VIR_NETDEV_TAP_CREATE_PERSIST * - The device will persist after the file descriptor is closed * - * Creates a tap interface. - * If the @tapfd parameter is supplied, the open tap device file descriptor - * will be returned, otherwise the TAP device will be closed. The caller must - * use virNetDevTapDelete to remove a persistent TAP device when it is no - * longer needed. + * Creates a tap interface. The caller must use virNetDevTapDelete to + * remove a persistent TAP device when it is no longer needed. In case + * @tapfdSize is greater than one, multiqueue extension is requested + * from kernel. * * Returns 0 in case of success or -1 on failure. */ int virNetDevTapCreate(char **ifname, int *tapfd, + int tapfdSize, unsigned int flags) { - int fd; + int i; struct ifreq ifr; int ret = -1; - if ((fd = open("/dev/net/tun", O_RDWR)) < 0) { - virReportSystemError(errno, "%s", - _("Unable to open /dev/net/tun, is tun module loaded?")); - return -1; - } - memset(&ifr, 0, sizeof(ifr)); + for (i = 0; i < tapfdSize; i++) { + int fd; + + if ((fd = open("/dev/net/tun", O_RDWR)) < 0) { + virReportSystemError(errno, "%s", + _("Unable to open /dev/net/tun, is tun module loaded?")); + goto cleanup; + } - ifr.ifr_flags = IFF_TAP|IFF_NO_PI; + memset(&ifr, 0, sizeof(ifr)); + + ifr.ifr_flags = IFF_TAP | IFF_NO_PI; + /* If tapfdSize is greater than one, request multiqueue */ + if (tapfdSize > 1) + ifr.ifr_flags |= IFF_MULTI_QUEUE; # ifdef IFF_VNET_HDR - if ((flags & VIR_NETDEV_TAP_CREATE_VNET_HDR) && - virNetDevProbeVnetHdr(fd)) - ifr.ifr_flags |= IFF_VNET_HDR; + if ((flags & VIR_NETDEV_TAP_CREATE_VNET_HDR) && + virNetDevProbeVnetHdr(fd)) + ifr.ifr_flags |= IFF_VNET_HDR; # endif - if (virStrcpyStatic(ifr.ifr_name, *ifname) == NULL) { - virReportSystemError(ERANGE, - _("Network interface name '%s' is too long"), - *ifname); - goto cleanup; + if (virStrcpyStatic(ifr.ifr_name, *ifname) == NULL) { + virReportSystemError(ERANGE, + _("Network interface name '%s' is too long"), + *ifname); + goto cleanup; - } + } - if (ioctl(fd, TUNSETIFF, &ifr) < 0) { - virReportSystemError(errno, - _("Unable to create tap device %s"), - NULLSTR(*ifname)); - goto cleanup; - } + if (ioctl(fd, TUNSETIFF, &ifr) < 0) { + virReportSystemError(errno, + _("Unable to create tap device %s"), + NULLSTR(*ifname)); + goto cleanup; + } - if ((flags & VIR_NETDEV_TAP_CREATE_PERSIST) && - (errno = ioctl(fd, TUNSETPERSIST, 1))) { - virReportSystemError(errno, - _("Unable to set tap device %s to persistent"), - NULLSTR(*ifname)); - goto cleanup; + if ((flags & VIR_NETDEV_TAP_CREATE_PERSIST) && + (errno = ioctl(fd, TUNSETPERSIST, 1))) { + virReportSystemError(errno, + _("Unable to set tap device %s to persistent"), + NULLSTR(*ifname)); + goto cleanup; + } + tapfd[i] = fd; } VIR_FREE(*ifname); - if (!(*ifname = strdup(ifr.ifr_name))) { + if (ifr.ifr_name && !(*ifname = strdup(ifr.ifr_name))) { virReportOOMError(); goto cleanup; } - if (tapfd) - *tapfd = fd; - else - VIR_FORCE_CLOSE(fd); ret = 0; cleanup: - if (ret < 0) - VIR_FORCE_CLOSE(fd); + if (ret < 0) { + for (i = 0; i < tapfdSize; i++) + VIR_FORCE_CLOSE(tapfd[i]); + } return ret; } @@ -232,6 +240,7 @@ cleanup: #else /* ! TUNSETIFF */ int virNetDevTapCreate(char **ifname ATTRIBUTE_UNUSED, int *tapfd ATTRIBUTE_UNUSED, + int tapfdSize ATTRIBUTE_UNUSED, unsigned int flags ATTRIBUTE_UNUSED) { virReportSystemError(ENOSYS, "%s", @@ -252,7 +261,8 @@ int virNetDevTapDelete(const char *ifname ATTRIBUTE_UNUSED) * @brname: the bridge name * @ifname: the interface name (or name template) * @macaddr: desired MAC address - * @tapfd: file descriptor return value for the new tap device + * @tapfd: array of file descriptor return value for the new tap device + * @tapfdSize: size of @tapfd * @virtPortProfile: bridge/port specific configuration * @flags: OR of virNetDevTapCreateFlags: @@ -280,6 +290,7 @@ int virNetDevTapCreateInBridgePort(const char *brname, const virMacAddrPtr macaddr, const unsigned char *vmuuid, int *tapfd, + int tapfdSize, virNetDevVPortProfilePtr virtPortProfile, virNetDevVlanPtr virtVlan, unsigned int flags) @@ -287,7 +298,7 @@ int virNetDevTapCreateInBridgePort(const char *brname, virMacAddr tapmac; char macaddrstr[VIR_MAC_STRING_BUFLEN]; - if (virNetDevTapCreate(ifname, tapfd, flags) < 0) + if (virNetDevTapCreate(ifname, tapfd, tapfdSize, flags) < 0) return -1; /* We need to set the interface MAC before adding it @@ -338,9 +349,9 @@ int virNetDevTapCreateInBridgePort(const char *brname, return 0; - error: - if (tapfd) - VIR_FORCE_CLOSE(*tapfd); +error: + while (tapfdSize) + VIR_FORCE_CLOSE(tapfd[--tapfdSize]); return -1; } diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h index 980db61..6e46ac8 100644 --- a/src/util/virnetdevtap.h +++ b/src/util/virnetdevtap.h @@ -29,6 +29,7 @@ int virNetDevTapCreate(char **ifname, int *tapfd, + int tapfdSize, unsigned int flags) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; @@ -52,6 +53,7 @@ int virNetDevTapCreateInBridgePort(const char *brname, const virMacAddrPtr macaddr, const unsigned char *vmuuid, int *tapfd, + int tapfdSize, virNetDevVPortProfilePtr virtPortProfile, virNetDevVlanPtr virtVlan, unsigned int flags) -- 1.8.1.5

For future work it's better, if tapfd is passed as pointer. Moreover, we need to be able to return multiple values now. --- src/qemu/qemu_command.c | 70 +++++++++++++++++++++++++------------------------ src/qemu/qemu_command.h | 4 ++- src/qemu/qemu_hotplug.c | 4 +-- 3 files changed, 41 insertions(+), 37 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7337069..58c81d7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -199,11 +199,12 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, virConnectPtr conn, virQEMUDriverPtr driver, virDomainNetDefPtr net, - virQEMUCapsPtr qemuCaps) + virQEMUCapsPtr qemuCaps, + int *tapfd, + int tapfdSize) { char *brname = NULL; - int err; - int tapfd = -1; + int ret = -1; unsigned int tap_create_flags = VIR_NETDEV_TAP_CREATE_IFUP; bool template_ifname = false; int actualType = virDomainNetGetActualType(net); @@ -215,7 +216,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, virNetworkPtr network = virNetworkLookupByName(conn, net->data.network.name); if (!network) - return -1; + return ret; active = virNetworkIsActive(network); if (active != 1) { @@ -240,18 +241,18 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, virFreeError(errobj); if (fail) - return -1; + return ret; } else if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) { if (!(brname = strdup(virDomainNetGetActualBridgeName(net)))) { virReportOOMError(); - return -1; + return ret; } } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("Network type %d is not supported"), virDomainNetGetActualType(net)); - return -1; + return ret; } if (!net->ifname || @@ -271,49 +272,51 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR; } - err = virNetDevTapCreateInBridgePort(brname, &net->ifname, &net->mac, - def->uuid, &tapfd, 1, - virDomainNetGetActualVirtPortProfile(net), - virDomainNetGetActualVlan(net), - tap_create_flags); - virDomainAuditNetDevice(def, net, "/dev/net/tun", tapfd >= 0); - if (err < 0) { + if (virNetDevTapCreateInBridgePort(brname, &net->ifname, &net->mac, + def->uuid, tapfd, tapfdSize, + virDomainNetGetActualVirtPortProfile(net), + virDomainNetGetActualVlan(net), + tap_create_flags) < 0) { + virDomainAuditNetDevice(def, net, "/dev/net/tun", false); if (template_ifname) VIR_FREE(net->ifname); - tapfd = -1; + goto cleanup; } + virDomainAuditNetDevice(def, net, "/dev/net/tun", true); - if (cfg->macFilter) { - if ((err = networkAllowMacOnPort(driver, net->ifname, &net->mac))) { - virReportSystemError(err, - _("failed to add ebtables rule to allow MAC address on '%s'"), - net->ifname); - } + if (cfg->macFilter && + (ret = networkAllowMacOnPort(driver, net->ifname, &net->mac)) < 0) { + virReportSystemError(ret, + _("failed to add ebtables rule " + "to allow MAC address on '%s'"), + net->ifname); } - if (tapfd >= 0 && - virNetDevBandwidthSet(net->ifname, + if (virNetDevBandwidthSet(net->ifname, virDomainNetGetActualBandwidth(net), false) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot set bandwidth limits on %s"), net->ifname); - VIR_FORCE_CLOSE(tapfd); goto cleanup; } - if (tapfd >= 0) { - if ((net->filter) && (net->ifname)) { - if (virDomainConfNWFilterInstantiate(conn, def->uuid, net) < 0) - VIR_FORCE_CLOSE(tapfd); - } - } + if (net->filter && net->ifname && + virDomainConfNWFilterInstantiate(conn, def->uuid, net) < 0) + goto cleanup; + + + ret = 0; cleanup: + if (ret < 0) { + while (tapfdSize--) + VIR_FORCE_CLOSE(tapfd[tapfdSize]); + } VIR_FREE(brname); virObjectUnref(cfg); - return tapfd; + return ret; } @@ -5868,9 +5871,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || cfg->privileged || (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV_BRIDGE))) { - tapfd = qemuNetworkIfaceConnect(def, conn, driver, net, - qemuCaps); - if (tapfd < 0) + if (qemuNetworkIfaceConnect(def, conn, driver, net, + qemuCaps, &tapfd, 1) < 0) goto cleanup; } } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index db14d8f..4a225b3 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -150,7 +150,9 @@ int qemuNetworkIfaceConnect(virDomainDefPtr def, virConnectPtr conn, virQEMUDriverPtr driver, virDomainNetDefPtr net, - virQEMUCapsPtr qemuCaps) + virQEMUCapsPtr qemuCaps, + int *tapfd, + int tapfdSize) ATTRIBUTE_NONNULL(2); int qemuPhysIfaceConnect(virDomainDefPtr def, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index b34160a..3505c52 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -740,8 +740,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || cfg->privileged || (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV_BRIDGE))) { - if ((tapfd = qemuNetworkIfaceConnect(vm->def, conn, driver, net, - priv->qemuCaps)) < 0) + if (qemuNetworkIfaceConnect(vm->def, conn, driver, net, + priv->qemuCaps, &tapfd, 1) < 0) goto cleanup; iface_connected = true; if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, &vhostfd, 1) < 0) -- 1.8.1.5

Currently, need for use of vhost-net is signalized by returning zero, and setting passed FD to a value different to negative one. However, when using multiple vhost-net devices, it is not so easy so we should use return value for that. --- src/qemu/qemu_command.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 58c81d7..dfcfedc 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -320,6 +320,19 @@ cleanup: } +/** + * qemuOpenVhostNet: + * @def: domain definition + * @net: network definition + * @qemuCaps: qemu binary capabilities + * @vhostfd: array of opened vhost-net device + * @vhostfdSize: size of @vhostfd array + * + * Open vhost-net, multiple times - if request. + * Returns: 1 if no vhost-net is required for @net type + * 0 on success + * -1 on failure + */ int qemuOpenVhostNet(virDomainDefPtr def, virDomainNetDefPtr net, @@ -333,9 +346,8 @@ qemuOpenVhostNet(virDomainDefPtr def, vhostfd[i] = -1; /* assume we won't use vhost */ /* If the config says explicitly to not use vhost, return now */ - if (net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_QEMU) { - return 0; - } + if (net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_QEMU) + return 1; /* If qemu doesn't support vhost-net mode (including the -netdev command * option), don't try to open the device. @@ -349,7 +361,7 @@ qemuOpenVhostNet(virDomainDefPtr def, "this QEMU binary")); return -1; } - return 0; + return 1; } /* If the nic model isn't virtio, don't try to open. */ @@ -360,7 +372,7 @@ qemuOpenVhostNet(virDomainDefPtr def, "virtio network interfaces")); return -1; } - return 0; + return 1; } for (i = 0; i < vhostfdSize; i++) { -- 1.8.1.5

--- src/qemu/qemu_command.c | 10 ++++++---- src/qemu/qemu_hotplug.c | 5 +++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 60873c7..a7d422a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5887,12 +5887,13 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || cfg->privileged || (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV_BRIDGE))) { - if (VIR_ALLOC(tapfd) < 0 || VIR_ALLOC(tapfdName) < 0) { + if (VIR_ALLOC_N(tapfd, net->driver.virtio.queues) < 0 || + VIR_ALLOC_N(tapfdName, net->driver.virtio.queues) < 0) { virReportOOMError(); goto cleanup; } - tapfdSize = 1; + tapfdSize = net->driver.virtio.queues; if (qemuNetworkIfaceConnect(def, conn, driver, net, qemuCaps, tapfd, tapfdSize) < 0) goto cleanup; @@ -5916,11 +5917,12 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, int useVhost; /* Attempt to use vhost-net mode for these types of network device */ - if (VIR_ALLOC(vhostfd) < 0 || VIR_ALLOC(vhostfdName)) { + if (VIR_ALLOC_N(vhostfd, net->driver.virtio.queues) < 0 || + VIR_ALLOC_N(vhostfdName, net->driver.virtio.queues)) { virReportOOMError(); goto cleanup; } - vhostfdSize = 1; + vhostfdSize = 1 + net->driver.virtio.queues; useVhost = qemuOpenVhostNet(def, net, qemuCaps, vhostfd, vhostfdSize); if (useVhost < 0) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 3a00811..d35801d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -743,11 +743,12 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || cfg->privileged || (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV_BRIDGE))) { - if (VIR_ALLOC(tapfd) < 0 || VIR_ALLOC(vhostfd) < 0) { + if (VIR_ALLOC_N(tapfd, net->driver.virtio.queues) < 0 || + VIR_ALLOC_N(vhostfd, net->driver.virtio.queues) < 0) { virReportOOMError(); goto cleanup; } - tapfdSize = vhostfdSize = 1; + tapfdSize = vhostfdSize = net->driver.virtio.queues; if (qemuNetworkIfaceConnect(vm->def, conn, driver, net, priv->qemuCaps, tapfd, tapfdSize) < 0) goto cleanup; -- 1.8.1.5