On 12.10.2012 11:58, Laine Stump wrote: > This is the final patch to resolve: > > https://bugzilla.redhat.com/show_bug.cgi?id=805071 > > This patch finalizes support live change of any/all hostside config of > a network device as part of the public virDomainUpdateDeviceFlags > API. The changes are done without detaching the device from the guest, > so hopefully disruptions are kept to a minimum. > > Two new functions are created for the task - > qemuDomainDetachNetHostSide() and qemuDomainAttachHostSide() - which > detach and reattach just the host side of a network device, while > leaving the PCI device on the guest side still attached. Calling these > two functions in sequence allows us to completely change the host side > of the device, including type of tap device, where the tap is > connected, bandwidth/vlan/virtualport settings, etc. > > These functions had their starts as cut-pastes of > qemuDomainDetachNetDevice and qemuDomainAttachNetDevice. To avoid the > headaches of duplicated code we may later want those older functions > to call the new functions, but that should wait until the new code > has had some real world testing - better to keep it in a lesser role > for now, with the existing code path unchanged. > > For now, the two new functions are only used by qemuDomainChangeNet() > in circumstances that previously would have resulted in an UNSUPPORTED > error, but now will instead perform a device reconnect using the > modified netdev object. > > One potentially ugly bit is that if the Attach of the new config > fails, we will have already detached the old config; in order to > recover as well as possible under the circumstances, we try to > re-attach the old config, which could fail for the same (or similar) > reason, leaving a discrepancy between the domain object and the actual > state of the domain. There really isn't any good way out of this > unfortunately. > --- > src/qemu/qemu_hotplug.c | 338 +++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 332 insertions(+), 6 deletions(-) > > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c > index 5284eb7..57a2392 100644 > --- a/src/qemu/qemu_hotplug.c > +++ b/src/qemu/qemu_hotplug.c > @@ -33,6 +33,7 @@ > #include "domain_audit.h" > #include "domain_nwfilter.h" > #include "logging.h" > +#include "datatypes.h" > #include "virterror_internal.h" > #include "memory.h" > #include "pci.h" > @@ -646,7 +647,6 @@ error: > return -1; > } > > - > /* XXX conn required for network -> bridge resolution */ > int qemuDomainAttachNetDevice(virConnectPtr conn, > struct qemud_driver *driver, > @@ -1249,6 +1249,304 @@ static virDomainNetDefPtr *qemuDomainFindNet(virDomainObjPtr vm, > return NULL; > } > > +/* qemuDomainDetachNetHostSide: > + * > + * detach only the Host side of the netdev. Leave the Guest side > + * attached. The intent is for this to *only* be called just prior to > + * calling qemuDomainAttachNetHostSide() to reconnect to a different > + * host side source (or with different settings). > + * > + * The device is *not* removed from the domain's list of nets, and > + * the "actual device" is not released. > + * > + * Returns 0 on success, -1 on failure. > + */ > +static int > +qemuDomainDetachNetHostSide(struct qemud_driver *driver, > + virDomainObjPtr vm, > + virDomainNetDefPtr detach) > +{ > + int ret = -1; > + qemuDomainObjPrivatePtr priv = vm->privateData; > + int vlan; > + char *hostnet_name = NULL; > + virNetDevVPortProfilePtr vport = NULL; > + > + if (virDomainNetGetActualType(detach) == VIR_DOMAIN_NET_TYPE_HOSTDEV) { > + /* it's not possible to detach just the host side of a hostdev */ > + virReportError(VIR_ERR_NO_SUPPORT, "%s", > + _("cannot detach only host side of a hostdev network device")); > + goto cleanup; > + } > + > + if ((vlan = qemuDomainNetVLAN(detach)) < 0) { > + virReportError(VIR_ERR_OPERATION_FAILED, > + "%s", _("unable to determine original VLAN")); > + goto cleanup; > + } > + > + if (virAsprintf(&hostnet_name, "host%s", detach->info.alias) < 0) { > + virReportOOMError(); > + goto cleanup; > + } > + > + qemuDomainObjEnterMonitorWithDriver(driver, vm); > + if (qemuCapsGet(priv->caps, QEMU_CAPS_NETDEV) && > + qemuCapsGet(priv->caps, QEMU_CAPS_DEVICE)) { > + ret = qemuMonitorRemoveNetdev(priv->mon, hostnet_name); > + } else { > + ret = qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name); > + } > + qemuDomainObjExitMonitorWithDriver(driver, vm); > + virDomainAuditNet(vm, detach, NULL, "detach", ret == 0); > + if (ret < 0) > + goto cleanup; > + > + virDomainConfNWFilterTeardown(detach); > + > + if (virDomainNetGetActualType(detach) == VIR_DOMAIN_NET_TYPE_DIRECT) { > + ignore_value(virNetDevMacVLanDeleteWithVPortProfile( > + detach->ifname, &detach->mac, > + virDomainNetGetActualDirectDev(detach), > + virDomainNetGetActualDirectMode(detach), > + virDomainNetGetActualVirtPortProfile(detach), > + driver->stateDir)); > + } > + > + if ((driver->macFilter) && (detach->ifname != NULL)) { > + if ((errno = networkDisallowMacOnPort(driver, > + detach->ifname, > + &detach->mac))) { > + virReportSystemError(errno, > + _("failed to remove ebtables rule on '%s'"), > + detach->ifname); > + } > + } > + > + vport = virDomainNetGetActualVirtPortProfile(detach); > + if (vport && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) > + ignore_value(virNetDevOpenvswitchRemovePort( > + virDomainNetGetActualBridgeName(detach), > + detach->ifname)); > + ret = 0; > +cleanup: > + if (!ret) > + networkReleaseActualDevice(detach); > + VIR_FREE(hostnet_name); > + return ret; > +} > + > +/* qemuDomainAttachNetHostSide: > + * > + * attach only the Host side of the netdev. Assume that the Guest side > + * is already attached. The intent is for this to *only* be called > + * just after calling qemuDomainDetachNetHostSide() to reconnect to a > + * different host side source (or with different settings). > + * > + * The device is assumed to be already in the domain's list of nets, > + * and the "actualDevice" is already allocated. Also, it isn't removed > + * from the domain nets list in case of failure. > + * > + * Returns 0 on success, -1 on failure. > + * > + * XXX: conn required for network -> bridge resolution > +*/ > +static int > +qemuDomainAttachNetHostSide(virConnectPtr conn, > + struct qemud_driver *driver, > + virDomainObjPtr vm, > + virDomainNetDefPtr net) > +{ > + qemuDomainObjPrivatePtr priv = vm->privateData; > + char *tapfd_name = NULL; > + int tapfd = -1; > + char *vhostfd_name = NULL; > + int vhostfd = -1; > + char *netstr = NULL; > + virNetDevVPortProfilePtr vport = NULL; > + int ret = -1; > + int vlan; > + bool iface_connected = false; > + int actualType; > + char *netdev_name = NULL; > + > + actualType = virDomainNetGetActualType(net); > + > + if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { > + /* We can't attach just one side of one of these devices */ > + virReportError(VIR_ERR_NO_SUPPORT, "%s", > + _("cannot attach only host side of a hostdev network device")); > + goto cleanup; > + } > + > + if (!qemuCapsGet(priv->caps, QEMU_CAPS_HOST_NET_ADD)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("installed qemu version does not support host_net_add")); > + goto cleanup; > + } > + > + if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || > + actualType == VIR_DOMAIN_NET_TYPE_NETWORK) { > + /* > + * If type=bridge then we attempt to allocate the tap fd here > + * only if running as root or if "-netdev bridge" (using the > + * qemu setuid network helper) is not supported. > + */ > + if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || > + driver->privileged || > + (!qemuCapsGet (priv->caps, QEMU_CAPS_NETDEV_BRIDGE))) { > + if ((tapfd = qemuNetworkIfaceConnect(vm->def, conn, driver, > + net, priv->caps)) < 0) { > + goto cleanup; > + } > + iface_connected = true; > + if (qemuOpenVhostNet(vm->def, net, priv->caps, &vhostfd) < 0) > + goto cleanup; > + } > + } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { > + if ((tapfd = qemuPhysIfaceConnect(vm->def, driver, net, priv->caps, > + VIR_NETDEV_VPORT_PROFILE_OP_CREATE)) < 0) > + goto cleanup; > + iface_connected = true; > + if (qemuOpenVhostNet(vm->def, net, priv->caps, &vhostfd) < 0) > + goto cleanup; > + } > + > + if (qemuCapsGet(priv->caps, QEMU_CAPS_NETDEV) && > + qemuCapsGet(priv->caps, QEMU_CAPS_DEVICE)) { > + vlan = -1; > + } else { > + vlan = qemuDomainNetVLAN(net); > + if (vlan < 0) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%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; > + } > + > + if (vhostfd != -1) { > + if (virAsprintf(&vhostfd_name, "vhostfd-%s", net->info.alias) < 0) > + goto no_memory; > + } > + > + if (qemuCapsGet(priv->caps, QEMU_CAPS_NETDEV) && > + qemuCapsGet(priv->caps, QEMU_CAPS_DEVICE)) { > + if (!(netstr = qemuBuildHostNetStr(net, driver, priv->caps, > + ',', -1, tapfd_name, > + vhostfd_name))) > + goto cleanup; > + } else { > + if (!(netstr = qemuBuildHostNetStr(net, driver, priv->caps, > + ' ', vlan, tapfd_name, > + vhostfd_name))) > + goto cleanup; > + } > + > + qemuDomainObjEnterMonitorWithDriver(driver, vm); > + if (qemuCapsGet(priv->caps, QEMU_CAPS_NETDEV) && > + qemuCapsGet(priv->caps, QEMU_CAPS_DEVICE)) { > + if (qemuMonitorAddNetdev(priv->mon, netstr, tapfd, tapfd_name, > + vhostfd, vhostfd_name) < 0) { > + qemuDomainObjExitMonitorWithDriver(driver, vm); > + virDomainAuditNet(vm, NULL, net, "attach", false); > + goto cleanup; > + } > + } else { > + if (qemuMonitorAddHostNetwork(priv->mon, netstr, tapfd, tapfd_name, > + vhostfd, vhostfd_name) < 0) { > + qemuDomainObjExitMonitorWithDriver(driver, vm); > + virDomainAuditNet(vm, NULL, net, "attach", false); > + goto cleanup; > + } > + } > + qemuDomainObjExitMonitorWithDriver(driver, vm); > + > + VIR_FORCE_CLOSE(tapfd); > + VIR_FORCE_CLOSE(vhostfd); > + > + if (!virDomainObjIsActive(vm)) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("guest unexpectedly quit")); > + goto cleanup; > + } > + > + /* set link state */ > + if (net->linkstate == VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN) { > + if (!net->info.alias) { > + virReportError(VIR_ERR_OPERATION_FAILED, "%s", > + _("device alias not found: cannot set link state to down")); > + } else { > + qemuDomainObjEnterMonitorWithDriver(driver, vm); > + > + if (qemuCapsGet(priv->caps, QEMU_CAPS_NETDEV)) { > + if (qemuMonitorSetLink(priv->mon, net->info.alias, > + VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN) < 0) { > + qemuDomainObjExitMonitorWithDriver(driver, vm); > + virDomainAuditNet(vm, NULL, net, "attach", false); > + goto try_remove; > + } > + } else { > + virReportError(VIR_ERR_OPERATION_FAILED, "%s", > + _("setting of link state not supported: Link is up")); > + } > + > + qemuDomainObjExitMonitorWithDriver(driver, vm); > + } > + /* link set to down */ > + } > + > + virDomainAuditNet(vm, NULL, net, "attach", true); > + > + ret = 0; > +cleanup: > + if (ret < 0 && iface_connected) { > + virDomainConfNWFilterTeardown(net); > + > + vport = virDomainNetGetActualVirtPortProfile(net); > + if (vport && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) > + ignore_value(virNetDevOpenvswitchRemovePort( > + virDomainNetGetActualBridgeName(net), net->ifname)); > + } > + > + VIR_FREE(netstr); > + VIR_FREE(tapfd_name); > + VIR_FORCE_CLOSE(tapfd); > + VIR_FREE(vhostfd_name); > + VIR_FORCE_CLOSE(vhostfd); > + VIR_FREE(netdev_name); > + > + return ret; > + > +try_remove: > + if (!virDomainObjIsActive(vm)) > + goto cleanup; > + > + if (virAsprintf(&netdev_name, "host%s", net->info.alias) < 0) > + goto no_memory; > + > + qemuDomainObjEnterMonitorWithDriver(driver, vm); > + if (vlan < 0) { > + if (qemuMonitorRemoveNetdev(priv->mon, netdev_name) < 0) > + VIR_WARN("Failed to remove network backend for netdev %s", > + netdev_name); > + } else { > + if (qemuMonitorRemoveHostNetwork(priv->mon, vlan, netdev_name) < 0) > + VIR_WARN("Failed to remove network backend for vlan %d, net %s", > + vlan, netdev_name); > + } > + qemuDomainObjExitMonitorWithDriver(driver, vm); > + goto cleanup; > + > +no_memory: > + virReportOOMError(); > + goto cleanup; > +} > + > static > int qemuDomainChangeNetBridge(virDomainObjPtr vm, > virDomainNetDefPtr olddev, > @@ -1330,7 +1628,7 @@ cleanup: > int > qemuDomainChangeNet(struct qemud_driver *driver, > virDomainObjPtr vm, > - virDomainPtr dom ATTRIBUTE_UNUSED, > + virDomainPtr dom, > virDomainDeviceDefPtr dev) > { > virDomainNetDefPtr newdev = dev->data.net; > @@ -1584,10 +1882,38 @@ qemuDomainChangeNet(struct qemud_driver *driver, > > /* FINALLY - actual perform the required actions */ > if (needReconnect) { > - virReportError(VIR_ERR_NO_SUPPORT, > - _("unable to change config on '%s' network type"), > - virDomainNetTypeToString(newdev->type)); > - goto cleanup; > + /* disconnect using the old config */ > + if (qemuDomainDetachNetHostSide(driver, vm, olddev) < 0) > + goto cleanup; > + > + /* reconnect using the new config (note that the new > + * actualDevice was already allocated above) > + */ > + if (qemuDomainAttachNetHostSide(dom->conn, driver, vm, newdev) < 0) { > + virErrorPtr err; > + > + /* try to reconnect olddev */ > + err = virSaveLastError(); > + ignore_value(qemuDomainAttachNetHostSide(dom->conn, driver, vm, newdev)); > + virSetError(err); > + virFreeError(err); > + goto cleanup; > + } > + > + /* we're successfully attached using the new config, so put it > + * in place in the domain nets list, and discard the old config > + */ > + networkReleaseActualDevice(olddev); > + virDomainNetDefFree(olddev); > + /* move newdev into the nets list, and NULL it out from the > + * virDomainDeviceDef that we were given so that the caller > + * won't delete it on return. */ > + *olddevslot = newdev; > + newdev = dev->data.net = NULL; > + dev->type = VIR_DOMAIN_DEVICE_NONE; > + > + needChangeBridge = false; > + needLinkStateChange = false; > } > > if (needChangeBridge && qemuDomainChangeNetBridge(vm, olddev, newdev) < 0) > ACK Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list