Re: [libvirt] [PATCHv2] qemu: support live change of the bridge used by a guest network device

From: Hendrik Schwartke <hendrik(a)os-t.de&gt; Previously the only attribute of a network device that could be modified by virUpdateDeviceFlags() ("virsh update-device") was the link state; attempts to change any other attribute would log an error and fail. (One notable exception to this was changing the bridge used by an interface of type='bridge' - that wasn't flagged as an error, but still did nothing.) This patch adds recognition of a change in bridge device name, and supports reconnecting the guest's interface to the new device. --- This had too many modifications from the original patch for me to push without sending in for a 3rd party review. Here are the changes from Hendrik's original patch: Things I noted in my original review: 1) remove unnecessary "util/" from #includes 2) change qemuDomainChangeNetBridge from global to static, since it's only used in the one file. 3) Don't free olddev->data.bridge.brname after failure to add the tap to the new bridge, since the caller will free all of olddev anyway. Things I didn't notice until after I reviewed and ACKed (but fortunately before I pushed): 4) When adding the tap to the new bridge, use olddev->ifname instead of newdev->ifname - the one in newdef is evilly cleared out by the parser, and ifname isn't allowed to change anyway, so using olddev->ifname is proper. 5) Add a case for VIR_DOMAIN_NET_TYPE_BRIDGE to the switch that checks for changes to disallowed items for each type of interface. Even before adding the new functionality, absence of this case had meant that attempts to change link state of a type='bridge' interface would have failed (so I was right that this patch fixes an existing bug, but was wrong about exactly what the bug was.) I have run make check and done functional checking on this code - it does properly change the bridge of an existing guest interface without needing to detach it. src/qemu/qemu_hotplug.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 54 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 38163ba..66837c4 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -40,6 +40,8 @@ #include "qemu_cgroup.h" #include "locking/domain_lock.h" #include "network/bridge_driver.h" +#include "virnetdev.h" +#include "virnetdevbridge.h" #include "virnetdevtap.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -1191,6 +1193,40 @@ static virDomainNetDefPtr qemuDomainFindNet(virDomainObjPtr vm, return NULL; } +static +int qemuDomainChangeNetBridge(virDomainNetDefPtr olddev, + virDomainNetDefPtr newdev) +{ + const char *oldbridge = olddev->data.bridge.brname; + const char *newbridge = newdev->data.bridge.brname; + + VIR_DEBUG("Change bridge for interface %s: %s -> %s", + olddev->ifname, oldbridge, newbridge); + + if (virNetDevExists(newbridge) != 1) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("bridge %s doesn't exist"), newbridge); + return -1; + } + + if (oldbridge && + virNetDevBridgeRemovePort(oldbridge, olddev->ifname) < 0) { + return -1; + } + if (virNetDevBridgeAddPort(newbridge, olddev->ifname) < 0) { + if (virNetDevBridgeAddPort(oldbridge, olddev->ifname) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("unable to recover former state by adding port" + "to bridge %s"), oldbridge); + } + return -1; + }