This patch moves backend setup code from qemudDomainAttachNetDevice and qemudDomainDetachNetDevice into separate functions. While the intent is to allow the new functions to be called separately without affecting VM NIC devices, this is arguably a worthwhile refactoring on its own. Currently it's based on 0.7.6 with some macvtap patches. I can rebase it against git if there's interest. Signed-off-by: Ed Swierk <eswierk(a)aristanetworks.com&gt; --- Move the code that creates and destroys the network backend and connects it to qemu from the code that creates the virtual NIC. Index: libvirt-0.7.6/src/qemu/qemu_driver.c =================================================================== --- libvirt-0.7.6.orig/src/qemu/qemu_driver.c +++ libvirt-0.7.6/src/qemu/qemu_driver.c @@ -132,6 +132,17 @@ static int qemuDetectVcpuPIDs(virConnect static int qemuUpdateActivePciHostdevs(struct qemud_driver *driver, virDomainDefPtr def); +static int qemudDomainConnectNetBackend(virConnectPtr conn, + struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainNetDefPtr net, + unsigned int qemuCmdFlags); + +static int qemudDomainDisconnectNetBackend(virConnectPtr conn, + struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainNetDefPtr net); + static struct qemud_driver *qemu_driver = NULL; @@ -5609,19 +5620,17 @@ error: } -static int qemudDomainAttachNetDevice(virConnectPtr conn, - struct qemud_driver *driver, - virDomainObjPtr vm, - virDomainNetDefPtr net, - unsigned int qemuCmdFlags) +static int qemudDomainConnectNetBackend(virConnectPtr conn, + struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainNetDefPtr net, + unsigned int qemuCmdFlags) { qemuDomainObjPrivatePtr priv = vm->privateData; char *tapfd_name = NULL; int tapfd = -1; - char *nicstr = NULL; char *netstr = NULL; int ret = -1; - virDomainDevicePCIAddress guestAddr; int vlan; if (!(qemuCmdFlags & QEMUD_CMD_FLAG_HOST_NET_ADD)) { @@ -5630,142 +5639,132 @@ static int qemudDomainAttachNetDevice(vi return -1; } + if (priv->monConfig->type != VIR_DOMAIN_CHR_TYPE_UNIX) { + qemudReportError(conn, dom, NULL, VIR_ERR_NO_SUPPORT, + _("network device type '%s' cannot be attached: " + "qemu is not using a unix socket monitor"), + virDomainNetTypeToString(net->type)); + return -1; + } + + if ((vlan = qemuDomainNetVLAN(net)) < 0) { + qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, "%s", + _("Unable to connect network backend without vlan")); + goto out; + } + if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE || net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { - if (priv->monConfig->type != VIR_DOMAIN_CHR_TYPE_UNIX) { - qemudReportError(conn, dom, NULL, VIR_ERR_NO_SUPPORT, - _("network device type '%s' cannot be attached: " - "qemu is not using a unix socket monitor"), - virDomainNetTypeToString(net->type)); - return -1; - } - - if ((tapfd = qemudNetworkIfaceConnect(conn, driver, net, qemuCmdFlags)) < 0) - return -1; + tapfd = qemudNetworkIfaceConnect(conn, driver, net, qemuCmdFlags); } else if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) { - if (priv->monConfig->type != VIR_DOMAIN_CHR_TYPE_UNIX) { - qemudReportError(conn, dom, NULL, VIR_ERR_NO_SUPPORT, - _("direct device type '%s' cannot be attached: " - "qemu is not using a unix socket monitor"), - virDomainNetTypeToString(net->type)); - return -1; - } + tapfd = qemudPhysIfaceConnect(conn, net, + net->data.direct.linkdev, + net->data.direct.mode); + } + if (tapfd < 0) + return -1; - if ((tapfd = qemudPhysIfaceConnect(conn, net, - net->data.direct.linkdev, - net->data.direct.mode)) < 0) - return -1; + if (virAsprintf(&tapfd_name, "fd-%s", net->info.alias) < 0) { + virReportOOMError(conn); + close(tapfd); + return -1; } - if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets+1) < 0) - goto no_memory; + if (!(netstr = qemuBuildHostNetStr(conn, net, ' ', + vlan, tapfd_name))) { + VIR_FREE(tapfd_name); + close(tapfd); + return -1; + } + + qemuDomainObjEnterMonitorWithDriver(driver, vm); + + if (qemuMonitorSendFileHandle(priv->mon, tapfd_name, tapfd) < 0) + goto out; + + if (qemuMonitorAddHostNetwork(priv->mon, netstr) < 0) { + if (qemuMonitorCloseFileHandle(priv->mon, tapfd_name) < 0) + VIR_WARN(_("Failed to close tapfd with '%s'"), tapfd_name); + goto out; + } + + ret = 0; + +out: + qemuDomainObjExitMonitorWithDriver(driver, vm); + VIR_FREE(netstr); + VIR_FREE(tapfd_name); + close(tapfd); + return ret; +} + +static int qemudDomainAttachNetDevice(virConnectPtr conn, + struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainNetDefPtr net, + unsigned int qemuCmdFlags) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + char *nicstr = NULL; + int ret = -1; + virDomainDevicePCIAddress guestAddr; + int vlan; + + if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets+1) < 0) { + virReportOOMError(conn); + return -1; + } if ((qemuCmdFlags & QEMUD_CMD_FLAG_NET_NAME) || (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { if (qemuAssignDeviceNetAlias(vm->def, net, -1) < 0) - goto cleanup; + goto out; } if ((qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) && qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &net->info) < 0) - goto cleanup; - - vlan = qemuDomainNetVLAN(net); + goto out; - if (vlan < 0) { + if ((vlan = qemuDomainNetVLAN(net)) < 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, "%s", _("Unable to attach network devices without vlan")); - goto cleanup; - } - - if (tapfd != -1) { - if (virAsprintf(&tapfd_name, "fd-%s", net->info.alias) < 0) - goto no_memory; - - qemuDomainObjEnterMonitorWithDriver(driver, vm); - if (qemuMonitorSendFileHandle(priv->mon, tapfd_name, tapfd) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); - goto cleanup; - } - qemuDomainObjExitMonitorWithDriver(driver, vm); + goto out; } - if (!(netstr = qemuBuildHostNetStr(conn, net, ' ', - vlan, tapfd_name))) - goto try_tapfd_close; - - qemuDomainObjEnterMonitorWithDriver(driver, vm); - if (qemuMonitorAddHostNetwork(priv->mon, netstr) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); - goto try_tapfd_close; + if (qemudDomainConnectNetBackend(conn, driver, vm, net, qemuCmdFlags) < 0) { + VIR_WARN0(_("Failed to connect network backend")); + goto out; } - qemuDomainObjExitMonitorWithDriver(driver, vm); - - if (tapfd != -1) - close(tapfd); - tapfd = -1; if (!(nicstr = qemuBuildNicStr(conn, net, NULL, vlan))) - goto try_remove; + goto out_remove; qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuMonitorAddPCINetwork(priv->mon, nicstr, &guestAddr) < 0) { qemuDomainObjExitMonitorWithDriver(driver, vm); - goto try_remove; + goto out_remove; } net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; memcpy(&net->info.addr.pci, &guestAddr, sizeof(guestAddr)); qemuDomainObjExitMonitorWithDriver(driver, vm); + vm->def->nets[vm->def->nnets++] = net; ret = 0; + goto out; - vm->def->nets[vm->def->nnets++] = net; +out_remove: + if (qemudDomainDisconnectNetBackend(conn, driver, vm, net) < 0) + VIR_WARN0("Unable to disconnect network backend"); -cleanup: +out: if ((ret != 0) && (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) && (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &net->info) < 0) VIR_WARN0("Unable to release PCI address on NIC"); - VIR_FREE(nicstr); - VIR_FREE(netstr); - VIR_FREE(tapfd_name); - if (tapfd != -1) - close(tapfd); - return ret; - -try_remove: - if (vlan < 0) { - VIR_WARN0(_("Unable to remove network backend")); - } else { - char *hostnet_name; - if (virAsprintf(&hostnet_name, "host%s", net->info.alias) < 0) - goto no_memory; - qemuDomainObjEnterMonitorWithDriver(driver, vm); - if (qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0) - VIR_WARN(_("Failed to remove network backend for vlan %d, net %s"), - vlan, hostnet_name); - qemuDomainObjExitMonitorWithDriver(driver, vm); - VIR_FREE(hostnet_name); - } - goto cleanup; - -try_tapfd_close: - if (tapfd_name) { - qemuDomainObjEnterMonitorWithDriver(driver, vm); - if (qemuMonitorCloseFileHandle(priv->mon, tapfd_name) < 0) - VIR_WARN(_("Failed to close tapfd with '%s'"), tapfd_name); - qemuDomainObjExitMonitorWithDriver(driver, vm); - } - - goto cleanup; - -no_memory: - virReportOOMError(conn); - goto cleanup; } @@ -6207,82 +6206,95 @@ cleanup: return ret; } -static int -qemudDomainDetachNetDevice(virConnectPtr conn, - struct qemud_driver *driver, - virDomainObjPtr vm, - virDomainDeviceDefPtr dev) +static int qemudDomainDisconnectNetBackend(virConnectPtr conn, + struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainNetDefPtr net) { - int i, ret = -1; - virDomainNetDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; int vlan; char *hostnet_name = NULL; + if ((vlan = qemuDomainNetVLAN(net)) < 0) { + qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("unable to determine original VLAN")); + return -1; + } + + if (virAsprintf(&hostnet_name, "host%s", net->info.alias) < 0) { + virReportOOMError(NULL); + return -1; + } + + qemuDomainObjEnterMonitorWithDriver(driver, vm); + if (qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0) + VIR_WARN0("Failed to remove host network"); + qemuDomainObjExitMonitorWithDriver(driver, vm); + +#if WITH_MACVTAP + if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) { + if (net->ifname) + delMacvtap(conn, net->ifname); + } +#endif + + VIR_FREE(hostnet_name); + return 0; +} + +static int qemudDomainDetachNetDevice(virConnectPtr conn, + struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev) +{ + int i = -1; + virDomainNetDefPtr net = NULL; + qemuDomainObjPrivatePtr priv = vm->privateData; + for (i = 0 ; i < vm->def->nnets ; i++) { - virDomainNetDefPtr net = vm->def->nets[i]; + virDomainNetDefPtr n = vm->def->nets[i]; - if (!memcmp(net->mac, dev->data.net->mac, sizeof(net->mac))) { - detach = net; + if (!memcmp(n->mac, dev->data.net->mac, sizeof(n->mac))) { + net = n; break; } } - if (!detach) { + if (!net) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, _("network device %02x:%02x:%02x:%02x:%02x:%02x not found"), dev->data.net->mac[0], dev->data.net->mac[1], dev->data.net->mac[2], dev->data.net->mac[3], dev->data.net->mac[4], dev->data.net->mac[5]); - goto cleanup; + return -1; } - if (!virDomainDeviceAddressIsValid(&detach->info, + if (!virDomainDeviceAddressIsValid(&net->info, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("device cannot be detached without a PCI address")); - goto cleanup; - } - - if ((vlan = qemuDomainNetVLAN(detach)) < 0) { - qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - "%s", _("unable to determine original VLAN")); - goto cleanup; - } - - if (virAsprintf(&hostnet_name, "host%s", detach->info.alias) < 0) { - virReportOOMError(NULL); - goto cleanup; + return -1; } qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuMonitorRemovePCIDevice(priv->mon, - &detach->info.addr.pci) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); - goto cleanup; - } - - if (qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0) { + &net->info.addr.pci) < 0) { qemuDomainObjExitMonitorWithDriver(driver, vm); - goto cleanup; + return -1; } qemuDomainObjExitMonitorWithDriver(driver, vm); -#if WITH_MACVTAP - if (detach->type == VIR_DOMAIN_NET_TYPE_DIRECT) { - if (detach->ifname) - delMacvtap(conn, detach->ifname); - } -#endif + if (qemudDomainDisconnectNetBackend(conn, driver, vm, net) < 0) + VIR_WARN0("Failed to disconnect network backend"); - if ((driver->macFilter) && (detach->ifname != NULL)) { + if ((driver->macFilter) && (net->ifname != NULL)) { if ((errno = networkDisallowMacOnPort(conn, driver, - detach->ifname, - detach->mac))) { + net->ifname, + net->mac))) { virReportSystemError(conn, errno, _("failed to remove ebtables rule on '%s'"), - detach->ifname); + net->ifname); } } @@ -6299,13 +6311,9 @@ qemudDomainDetachNetDevice(virConnectPtr VIR_FREE(vm->def->nets); vm->def->nnets = 0; } - virDomainNetDefFree(detach); + virDomainNetDefFree(net); - ret = 0; - -cleanup: - VIR_FREE(hostnet_name); - return ret; + return 0; } static int qemudDomainDetachHostPciDevice(virConnectPtr conn,