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

Re: [libvirt] [PATCH 4/4] qemu: support live change of all netdev hostside config



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


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