[libvirt] [PATCH 3/4] qemu: reorganize qemuDomainChangeNet
Laine Stump
laine at laine.org
Fri Oct 12 09:58:24 UTC 2012
* qemuDomainChangeNet is going to need access to the
virDomainDeviceDef that holds the new netdef (so that it can clear out
the virDomainDeviceDef if it ends up using the NetDef to replace the
original), so the virDomainNetDefPtr arg is replaced with a
virDomainDeviceDefPtr.
* qemuDomainChangeNet previously checked for *some* changes to the
interface config, but this check was by no means complete. It was also
a bit disorganized.
This refactoring of the code is (I believe) complete in its check of
all NetDef attributes that might be changed, and either returns a
failure (for changes that are simply impossible), or sets one of three
flags:
needLinkStateChange - if the device link state needs to go up/down
needBridgeChange - if everything else is the same, but it needs
to be connected to a difference linux host
bridge
needReconnect - if the entire host side of the device needs
to be torn down and reconstructed
Note that this function will refuse to make any change that requires
the *guest* size of the device to be detached (e.g. changing the PCI
address or mac address. Those would be disruptive enough to the guest
that it's reasonable to require an explicit detach/attach sequence
from the management application.
This patch *does not* implement the "reconnect" code - there is a
placeholder that turns that into an error as well. Rather, the purpose
of this patch is to replicate existing behavior with code that is
ready to have that functionality plugged in in a later patch.
---
Note that the function qemuDomainChangeNet has essentially been
totally rewritten, so don't waste time trying to correlate between the
"-" and "+" lines in that part of the diff.
src/qemu/qemu_driver.c | 2 +-
src/qemu/qemu_hotplug.c | 360 +++++++++++++++++++++++++++++++++++-------------
src/qemu/qemu_hotplug.h | 4 +-
3 files changed, 271 insertions(+), 95 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ee84b27..323d11e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5984,7 +5984,7 @@ qemuDomainUpdateDeviceLive(virDomainObjPtr vm,
ret = qemuDomainChangeGraphics(driver, vm, dev->data.graphics);
break;
case VIR_DOMAIN_DEVICE_NET:
- ret = qemuDomainChangeNet(driver, vm, dom, dev->data.net);
+ ret = qemuDomainChangeNet(driver, vm, dom, dev);
break;
default:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index a738b19..5284eb7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1236,14 +1236,14 @@ cleanup:
return -1;
}
-static virDomainNetDefPtr qemuDomainFindNet(virDomainObjPtr vm,
- virDomainNetDefPtr dev)
+static virDomainNetDefPtr *qemuDomainFindNet(virDomainObjPtr vm,
+ virDomainNetDefPtr dev)
{
int i;
for (i = 0; i < vm->def->nnets; i++) {
if (virMacAddrCmp(&vm->def->nets[i]->mac, &dev->mac) == 0)
- return vm->def->nets[i];
+ return &vm->def->nets[i];
}
return NULL;
@@ -1327,124 +1327,300 @@ cleanup:
return ret;
}
-int qemuDomainChangeNet(struct qemud_driver *driver,
- virDomainObjPtr vm,
- virDomainPtr dom ATTRIBUTE_UNUSED,
- virDomainNetDefPtr dev)
-
+int
+qemuDomainChangeNet(struct qemud_driver *driver,
+ virDomainObjPtr vm,
+ virDomainPtr dom ATTRIBUTE_UNUSED,
+ virDomainDeviceDefPtr dev)
{
- virDomainNetDefPtr olddev = qemuDomainFindNet(vm, dev);
- int ret = 0;
+ virDomainNetDefPtr newdev = dev->data.net;
+ virDomainNetDefPtr *olddevslot = qemuDomainFindNet(vm, newdev);
+ virDomainNetDefPtr olddev;
+ int oldType, newType;
+ bool needReconnect = false;
+ bool needChangeBridge = false;
+ bool needLinkStateChange = false;
+ int ret = -1;
- if (!olddev) {
+ if (!olddevslot || !(olddev = *olddevslot)) {
virReportError(VIR_ERR_NO_SUPPORT, "%s",
_("cannot find existing network device to modify"));
- return -1;
+ goto cleanup;
}
- if (olddev->type != dev->type) {
+ oldType = virDomainNetGetActualType(olddev);
+ if (oldType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+ /* no changes are possible to a type='hostdev' interface */
+ virReportError(VIR_ERR_NO_SUPPORT,
+ _("cannot change config of '%s' network type"),
+ virDomainNetTypeToString(oldType));
+ goto cleanup;
+ }
+
+ /* Check individual attributes for changes that can't be done to a
+ * live netdev. These checks *mostly* go in order of the
+ * declarations in virDomainNetDef in order to assure nothing is
+ * omitted. (exceptiong where noted in comments - in particular,
+ * some things require that a new "actual device" be allocated
+ * from the network driver first, but we delay doing that until
+ * after we've made as many other checks as possible)
+ */
+
+ /* type: this can change (with some restrictions), but the actual
+ * type of the new device connection isn't known until after we
+ * allocate the "actual" device.
+ */
+
+ if (virMacAddrCmp(&olddev->mac, &newdev->mac)) {
+ char oldmac[VIR_MAC_STRING_BUFLEN], newmac[VIR_MAC_STRING_BUFLEN];
+
+ virReportError(VIR_ERR_NO_SUPPORT,
+ _("cannot change network interface mac address "
+ "from %s to %s"),
+ virMacAddrFormat(&olddev->mac, oldmac),
+ virMacAddrFormat(&newdev->mac, newmac));
+ goto cleanup;
+ }
+
+ if (STRNEQ_NULLABLE(olddev->model, newdev->model)) {
+ virReportError(VIR_ERR_NO_SUPPORT,
+ _("cannot modify network device model from %s to %s"),
+ olddev->model ? olddev->model : "(default)",
+ newdev->model ? newdev->model : "(default)");
+ goto cleanup;
+ }
+
+ if (olddev->model && STREQ(olddev->model, "virtio") &&
+ (olddev->driver.virtio.name != newdev->driver.virtio.name ||
+ olddev->driver.virtio.txmode != newdev->driver.virtio.txmode ||
+ olddev->driver.virtio.ioeventfd != newdev->driver.virtio.ioeventfd ||
+ olddev->driver.virtio.event_idx != newdev->driver.virtio.event_idx)) {
virReportError(VIR_ERR_NO_SUPPORT, "%s",
- _("cannot change network interface type"));
- return -1;
+ _("cannot modify virtio network device driver attributes"));
+ goto cleanup;
}
- if (!virNetDevVPortProfileEqual(olddev->virtPortProfile, dev->virtPortProfile)) {
+ /* data: this union will be examined later, after allocating new actualdev */
+ /* virtPortProfile: will be examined later, after allocating new actualdev */
+
+ if (olddev->tune.sndbuf_specified != newdev->tune.sndbuf_specified ||
+ olddev->tune.sndbuf != newdev->tune.sndbuf) {
+ needReconnect = true;
+ }
+
+ if (STRNEQ_NULLABLE(olddev->script, newdev->script)) {
virReportError(VIR_ERR_NO_SUPPORT, "%s",
- _("cannot change <virtualport> settings"));
+ _("cannot modify network device script attribute"));
+ goto cleanup;
}
- switch (olddev->type) {
- case VIR_DOMAIN_NET_TYPE_USER:
- break;
+ /* ifname: check if it's set in newdev. If not, retain the autogenerated one */
+ if (!(newdev->ifname ||
+ (newdev->ifname = strdup(olddev->ifname)))) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ if (STRNEQ_NULLABLE(olddev->ifname, newdev->ifname)) {
+ virReportError(VIR_ERR_NO_SUPPORT, "%s",
+ _("cannot modify network device tap name"));
+ goto cleanup;
+ }
- case VIR_DOMAIN_NET_TYPE_ETHERNET:
- if (STRNEQ_NULLABLE(olddev->data.ethernet.dev, dev->data.ethernet.dev) ||
- STRNEQ_NULLABLE(olddev->script, dev->script) ||
- STRNEQ_NULLABLE(olddev->data.ethernet.ipaddr, dev->data.ethernet.ipaddr)) {
- virReportError(VIR_ERR_NO_SUPPORT, "%s",
- _("cannot modify ethernet network device configuration"));
- return -1;
- }
- break;
+ /* info: if newdev->info is empty, fill it in from olddev,
+ * otherwise verify that it matches - nothing is allowed to
+ * change. (There is no helper function to do this, so
+ * individually check the few feidls of virDomainDeviceInfo that
+ * are relevant in this case).
+ */
+ if (!virDomainDeviceAddressIsValid(&newdev->info,
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
+ virDomainDeviceInfoCopy(&newdev->info, &olddev->info) < 0) {
+ goto cleanup;
+ }
+ if (!virDevicePCIAddressEqual(&olddev->info.addr.pci,
+ &newdev->info.addr.pci)) {
+ virReportError(VIR_ERR_NO_SUPPORT, "%s",
+ _("cannot modify network device guest PCI address"));
+ goto cleanup;
+ }
+ /* grab alias from olddev if not set in newdev */
+ if (!(newdev->info.alias ||
+ (newdev->info.alias = strdup(olddev->info.alias)))) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ if (STRNEQ_NULLABLE(olddev->info.alias, newdev->info.alias)) {
+ virReportError(VIR_ERR_NO_SUPPORT, "%s",
+ _("cannot modify network device alias"));
+ goto cleanup;
+ }
+ if (olddev->info.rombar != newdev->info.rombar) {
+ virReportError(VIR_ERR_NO_SUPPORT, "%s",
+ _("cannot modify network device rom bar setting"));
+ goto cleanup;
+ }
+ if (STRNEQ_NULLABLE(olddev->info.romfile, newdev->info.romfile)) {
+ virReportError(VIR_ERR_NO_SUPPORT, "%s",
+ _("cannot modify network rom file"));
+ goto cleanup;
+ }
+ if (olddev->info.bootIndex != newdev->info.bootIndex) {
+ virReportError(VIR_ERR_NO_SUPPORT, "%s",
+ _("cannot modify network device boot index setting"));
+ goto cleanup;
+ }
+ /* (end of device info checks) */
- case VIR_DOMAIN_NET_TYPE_SERVER:
- case VIR_DOMAIN_NET_TYPE_CLIENT:
- case VIR_DOMAIN_NET_TYPE_MCAST:
- if (STRNEQ_NULLABLE(olddev->data.socket.address, dev->data.socket.address) ||
- olddev->data.socket.port != dev->data.socket.port) {
- virReportError(VIR_ERR_NO_SUPPORT, "%s",
- _("cannot modify network socket device configuration"));
- return -1;
- }
- break;
+ if (STRNEQ_NULLABLE(olddev->filter, newdev->filter))
+ needReconnect = true;
- case VIR_DOMAIN_NET_TYPE_NETWORK:
- if (STRNEQ_NULLABLE(olddev->data.network.name, dev->data.network.name) ||
- STRNEQ_NULLABLE(olddev->data.network.portgroup, dev->data.network.portgroup)) {
- virReportError(VIR_ERR_NO_SUPPORT, "%s",
- _("cannot modify network device configuration"));
- return -1;
- }
+ /* bandwidth can be modified, and will be checked later */
+ /* vlan can be modified, and will be checked later */
+ /* linkstate can be modified */
- break;
+ /* allocate new actual device to compare to old - we will need to
+ * free it if we fail for any reason
+ */
+ if (newdev->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+ networkAllocateActualDevice(newdev) < 0) {
+ goto cleanup;
+ }
- case VIR_DOMAIN_NET_TYPE_BRIDGE:
- /* allow changing brname */
- break;
+ newType = virDomainNetGetActualType(newdev);
- case VIR_DOMAIN_NET_TYPE_INTERNAL:
- if (STRNEQ_NULLABLE(olddev->data.internal.name, dev->data.internal.name)) {
- virReportError(VIR_ERR_NO_SUPPORT, "%s",
- _("cannot modify internal network device configuration"));
- return -1;
- }
- break;
+ if (newType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+ /* can't turn it into a type='hostdev' interface */
+ virReportError(VIR_ERR_NO_SUPPORT,
+ _("cannot change network interface type to '%s'"),
+ virDomainNetTypeToString(newType));
+ goto cleanup;
+ }
- case VIR_DOMAIN_NET_TYPE_DIRECT:
- if (STRNEQ_NULLABLE(olddev->data.direct.linkdev, dev->data.direct.linkdev) ||
- olddev->data.direct.mode != dev->data.direct.mode) {
- virReportError(VIR_ERR_NO_SUPPORT, "%s",
- _("cannot modify direct network device configuration"));
- return -1;
- }
- break;
+ if (olddev->type != newdev->type || oldType != newType) {
+ /* this will certainly require a total remake of the host
+ * side of the device
+ */
+ needReconnect = true;
+ } else {
- default:
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unable to change config on '%s' network type"),
- virDomainNetTypeToString(dev->type));
+ /* if the type hasn't changed, check the relevant fields for the
+ * type = maybe we don't need to reconnect
+ */
+ switch (olddev->type) {
+ case VIR_DOMAIN_NET_TYPE_USER:
+ break;
+
+ case VIR_DOMAIN_NET_TYPE_ETHERNET:
+ if (STRNEQ_NULLABLE(olddev->data.ethernet.dev,
+ newdev->data.ethernet.dev) ||
+ STRNEQ_NULLABLE(olddev->script, newdev->script) ||
+ STRNEQ_NULLABLE(olddev->data.ethernet.ipaddr,
+ newdev->data.ethernet.ipaddr)) {
+ needReconnect = true;
+ }
break;
- }
+ case VIR_DOMAIN_NET_TYPE_SERVER:
+ case VIR_DOMAIN_NET_TYPE_CLIENT:
+ case VIR_DOMAIN_NET_TYPE_MCAST:
+ if (STRNEQ_NULLABLE(olddev->data.socket.address,
+ newdev->data.socket.address) ||
+ olddev->data.socket.port != newdev->data.socket.port) {
+ needReconnect = true;
+ }
+ break;
- /* all other unmodifiable parameters */
- if (STRNEQ_NULLABLE(olddev->model, dev->model) ||
- STRNEQ_NULLABLE(olddev->filter, dev->filter)) {
- virReportError(VIR_ERR_NO_SUPPORT, "%s",
- _("cannot modify network device configuration"));
- return -1;
- }
+ case VIR_DOMAIN_NET_TYPE_NETWORK:
+ /* other things handled in common code directly below this switch */
+ if (STRNEQ(olddev->data.network.name, newdev->data.network.name))
+ needReconnect = true;
+ break;
- /* check if device name has been set, if no, retain the autogenerated one */
- if (dev->ifname &&
- STRNEQ_NULLABLE(olddev->ifname, dev->ifname)) {
- virReportError(VIR_ERR_NO_SUPPORT, "%s",
- _("cannot modify network device configuration"));
- return -1;
- }
+ case VIR_DOMAIN_NET_TYPE_BRIDGE:
+ /* maintain previous behavior */
+ if (STRNEQ(olddev->data.bridge.brname, olddev->data.bridge.brname))
+ needChangeBridge = true;
+ break;
- if (olddev->type == VIR_DOMAIN_NET_TYPE_BRIDGE
- && STRNEQ_NULLABLE(olddev->data.bridge.brname,
- dev->data.bridge.brname)) {
- if ((ret = qemuDomainChangeNetBridge(vm, olddev, dev)) < 0)
- return ret;
+ case VIR_DOMAIN_NET_TYPE_INTERNAL:
+ if (STRNEQ_NULLABLE(olddev->data.internal.name,
+ newdev->data.internal.name)) {
+ needReconnect = true;
+ }
+ break;
+
+ case VIR_DOMAIN_NET_TYPE_DIRECT:
+ /* all handled in common code directly below this switch */
+ break;
+
+ default:
+ virReportError(VIR_ERR_NO_SUPPORT,
+ _("unable to change config on '%s' network type"),
+ virDomainNetTypeToString(newdev->type));
+ break;
+
+ }
+ }
+ /* now several things that are in multiple (but not all)
+ * different types, and can be safely compared even for those
+ * cases where they don't apply to a particular type.
+ */
+ if (STRNEQ_NULLABLE(virDomainNetGetActualBridgeName(olddev),
+ virDomainNetGetActualBridgeName(newdev)) ||
+ STRNEQ_NULLABLE(virDomainNetGetActualDirectDev(olddev),
+ virDomainNetGetActualDirectDev(newdev)) ||
+ virDomainNetGetActualDirectMode(olddev) != virDomainNetGetActualDirectMode(olddev) ||
+ !virNetDevVPortProfileEqual(virDomainNetGetActualVirtPortProfile(olddev),
+ virDomainNetGetActualVirtPortProfile(newdev)) ||
+ !virNetDevBandwidthEqual(virDomainNetGetActualBandwidth(olddev),
+ virDomainNetGetActualBandwidth(newdev)) ||
+ !virNetDevVlanEqual(virDomainNetGetActualVlan(olddev),
+ virDomainNetGetActualVlan(newdev))) {
+ needReconnect = true;
+ }
+
+ if (olddev->linkstate != newdev->linkstate)
+ needLinkStateChange = true;
+
+ /* 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;
}
- if (olddev->linkstate != dev->linkstate) {
- if ((ret = qemuDomainChangeNetLinkState(driver, vm, olddev, dev->linkstate)) < 0)
- return ret;
+ if (needChangeBridge && qemuDomainChangeNetBridge(vm, olddev, newdev) < 0)
+ goto cleanup;
+
+ if (needLinkStateChange &&
+ qemuDomainChangeNetLinkState(driver, vm, olddev, newdev->linkstate) < 0) {
+ goto cleanup;
}
+ ret = 0;
+cleanup:
+ /* When we get here, we will be in one of these two states:
+ *
+ * 1) newdev has been moved into the domain's list of nets and
+ * newdev set to NULL, and dev->data.net will be NULL (and
+ * dev->type is NONE). olddev will have been completely
+ * released and freed. (aka success) In this case no extra
+ * cleanup is needed.
+ *
+ * 2) newdev has *not* been moved into the domain's list of nets,
+ * and dev->data.net == newdev (and dev->type == NET). In this *
+ * case, we need to at least release the "actual device" from *
+ * newdev (the caller will free dev->data.net a.k.a. newdev, and
+ * the original olddev is still in used)
+ *
+ * Note that case (2) isn't necessarily a failure. It may just be
+ * that the changes were minor enough that we didn't need to
+ * replace the entire device object.
+ */
+ if (newdev)
+ networkReleaseActualDevice(newdev);
+
return ret;
}
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 36c0580..a7864c3 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -1,7 +1,7 @@
/*
* qemu_hotplug.h: QEMU device hotplug management
*
- * Copyright (C) 2006-2007, 2009-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2007, 2009-2012 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -77,7 +77,7 @@ int qemuDomainChangeGraphicsPasswords(struct qemud_driver *driver,
int qemuDomainChangeNet(struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainPtr dom,
- virDomainNetDefPtr dev);
+ virDomainDeviceDefPtr dev);
int qemuDomainChangeNetLinkState(struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainNetDefPtr dev,
--
1.7.11.4
More information about the libvir-list
mailing list