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

[Laine Stump]

From: Hendrik Schwartke <hendrik at os-t.de>

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;
+    }
+    VIR_FREE(olddev->data.bridge.brname);
+    olddev->data.bridge.brname = newdev->data.bridge.brname;
+    newdev->data.bridge.brname = NULL;
+    return 0;
+}
+
 int qemuDomainChangeNetLinkState(struct qemud_driver *driver,
                                  virDomainObjPtr vm,
                                  virDomainNetDefPtr dev,
@@ -1279,6 +1315,16 @@ int qemuDomainChangeNet(struct qemud_driver *driver,
 
         break;
 
+    case VIR_DOMAIN_NET_TYPE_BRIDGE:
+       /* allow changing brname, but not portprofile */
+       if (!virNetDevVPortProfileEqual(olddev->data.bridge.virtPortProfile,
+                                       dev->data.bridge.virtPortProfile)) {
+           qemuReportError(VIR_ERR_NO_SUPPORT,
+                           _("cannot modify bridge network device configuration"));
+           return -1;
+       }
+       break;
+
     case VIR_DOMAIN_NET_TYPE_INTERNAL:
         if (STRNEQ_NULLABLE(olddev->data.internal.name, dev->data.internal.name)) {
             qemuReportError(VIR_ERR_NO_SUPPORT,
@@ -1321,6 +1367,14 @@ int qemuDomainChangeNet(struct qemud_driver *driver,
         return -1;
     }
 
+    if (olddev->type == VIR_DOMAIN_NET_TYPE_BRIDGE
+        && dev->type == VIR_DOMAIN_NET_TYPE_BRIDGE
+        && STRNEQ_NULLABLE(olddev->data.bridge.brname,
+                           dev->data.bridge.brname)) {
+        if ((ret = qemuDomainChangeNetBridge(olddev, dev)) < 0)
+            return ret;
+    }
+
     if (olddev->linkstate != dev->linkstate) {
         if ((ret = qemuDomainChangeNetLinkState(driver, vm, olddev, dev->linkstate)) < 0)
             return ret;
-- 
1.7.7.6