From 601f4160b942e4e6ead7767a864220a7ef8d3d36 Mon Sep 17 00:00:00 2001
From: Laine Stump <laine@redhat.com>
Date: Sun, 15 Sep 2024 17:56:02 -0400
Subject: [PATCH] qemu: replace open-coded remove/attach bridge with
 virNetDevTapReattachBridge()

The new function does what the old qemuDomainChangeNetbridge() did
manually, except that:

1) the new function supports changing from a bridge of one type to
   another, e.g. from a Linux host bridge to an OVS
   bridge. (previously that wasn't handled)

2) the new function doesn't emit audit log messages. This is actually
   a good thing, because the old code would just log a "detach"
   followed immediately by "attach" for the same MAC address, so it's
   essentially a NOP. (the audit logs don't have any more detailed
   info about the connection - just the VM name and MAC address, so it
   makes no sense to log the detach/attach pair as it's not providing
   any information).

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_hotplug.c | 55 ++++++++++-------------------------------
 1 file changed, 13 insertions(+), 42 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 001e051485..4739b10e98 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -56,6 +56,7 @@
 #include "virnetdevbridge.h"
 #include "virnetdevopenvswitch.h"
 #include "virnetdevmidonet.h"
+#include "virnetdevtap.h"
 #include "device_conf.h"
 #include "storage_source.h"
 #include "storage_source_conf.h"
@@ -3533,7 +3534,6 @@ qemuDomainChangeNetBridge(virDomainObj *vm,
                           virDomainNetDef *olddev,
                           virDomainNetDef *newdev)
 {
-    int ret = -1;
     const char *oldbridge = virDomainNetGetActualBridgeName(olddev);
     const char *newbridge = virDomainNetGetActualBridgeName(newdev);
 
@@ -3547,50 +3547,21 @@ qemuDomainChangeNetBridge(virDomainObj *vm,
 
     if (virNetDevExists(newbridge) != 1) {
         virReportError(VIR_ERR_OPERATION_FAILED,
-                       _("bridge %1$s doesn't exist"), newbridge);
+                       _("cannot add domain %1$s device %2$s to nonexistent bridge %3$s"),
+                       vm->def->name, newdev->ifname, newbridge);
         return -1;
     }
 
-    ret = virNetDevBridgeRemovePort(oldbridge, olddev->ifname);
-    virDomainAuditNet(vm, olddev, NULL, "detach", ret == 0);
-    if (ret < 0) {
-        /* warn but continue - possibly the old network
-         * had been destroyed and reconstructed, leaving the
-         * tap device orphaned.
-         */
-        VIR_WARN("Unable to detach device %s from bridge %s",
-                 olddev->ifname, oldbridge);
-    }
-
-    ret = virNetDevBridgeAddPort(newbridge, olddev->ifname);
-    if (ret == 0 &&
-        virDomainNetGetActualPortOptionsIsolated(newdev) == VIR_TRISTATE_BOOL_YES) {
-
-        ret = virNetDevBridgePortSetIsolated(newbridge, olddev->ifname, true);
-        if (ret < 0) {
-            virErrorPtr err;
-
-            virErrorPreserveLast(&err);
-            ignore_value(virNetDevBridgeRemovePort(newbridge, olddev->ifname));
-            virErrorRestore(&err);
-        }
-    }
-    virDomainAuditNet(vm, NULL, newdev, "attach", ret == 0);
-    if (ret < 0) {
-        virErrorPtr err;
-
-        virErrorPreserveLast(&err);
-        ret = virNetDevBridgeAddPort(oldbridge, olddev->ifname);
-        if (ret == 0 &&
-            virDomainNetGetActualPortOptionsIsolated(olddev) == VIR_TRISTATE_BOOL_YES) {
-            ignore_value(virNetDevBridgePortSetIsolated(newbridge, olddev->ifname, true));
-        }
-        virDomainAuditNet(vm, NULL, olddev, "attach", ret == 0);
-        virErrorRestore(&err);
-        return -1;
-    }
-    /* caller will replace entire olddev with newdev in domain nets list */
-    return 0;
+    /* force the detach/reattach (final arg) to make sure we pick up virtualport changes
+     * even if the bridge name hasn't changed
+     */
+    return virNetDevTapReattachBridge(newdev->ifname,
+                                      virDomainNetGetActualBridgeName(newdev),
+                                      &newdev->mac, vm->def->uuid,
+                                      virDomainNetGetActualVirtPortProfile(newdev),
+                                      virDomainNetGetActualVlan(newdev),
+                                      virDomainNetGetActualPortOptionsIsolated(newdev),
+                                      newdev->mtu, NULL, true);
 }
 
 static int