Since we previously only supported vlan tagging for interfaces
connected to an OVS bridge [*], the code in qemuChangeNet() (used by
the update-device API) assumed an interface with modified vlan config
was on an OVS bridge, and would call the OVS-specific
virNetDevOpenvswitchUpdateVlan().

Now that we support vlan tagging for interfaces connected to a
standard Linux host bridge, we must check the type of connection and
only call the OVS function when connected to an OVS bridge *both
before and after the update*. Otherwise we just set the flag to
re-connect to the bridge, which has the side effect of redoing the
vlan setup.

([*] or an SRIOV VF assigned using VFIO, but we don't support *any
runtime changes to that type of netdev so it's irrelevant here.)

Signed-off-by: Laine Stump <laine@redhat.com>
---

I'm fine with this going in either before or after the 11.0.0 release
(would prefer before since it is a bugfix), but if it's going into the
release I'd rather it be included with RC2 rather than only in the
final release. For that reason, if someone ACKs the patch and agrees
that it's safe to go in, please push it for me (since Jiri will almost
certainly tag RC2 before I'm conscious on Monday morning)

 src/qemu/qemu_hotplug.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index de0777d330..5a7e6c3b12 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -4144,8 +4144,13 @@ qemuDomainChangeNet(virQEMUDriver *driver,
      * they don't apply to a particular type.
      */
 
-    if (!virNetDevVlanEqual(virDomainNetGetActualVlan(olddev),
-                             virDomainNetGetActualVlan(newdev))) {
+    /* since attaching to a new bridge will re-do the vlan setup,
+     * we don't need to separately do that in the case that we're
+     * already switching to a different bridge
+     */
+    if (!(needBridgeChange ||
+          virNetDevVlanEqual(virDomainNetGetActualVlan(olddev),
+                             virDomainNetGetActualVlan(newdev)))) {
         needVlanUpdate = true;
     }
 
@@ -4215,6 +4220,23 @@ qemuDomainChangeNet(virQEMUDriver *driver,
         needReplaceDevDef = true;
     }
 
+    if (needVlanUpdate) {
+        if (virDomainNetDefIsOvsport(olddev) && virDomainNetDefIsOvsport(newdev)) {
+            /* optimization if we're attached to an OVS bridge. This
+             * will redo vlan setup without needing to re-attach the
+             * tap device to the bridge
+             */
+            if (virNetDevOpenvswitchUpdateVlan(newdev->ifname, &newdev->vlan) < 0)
+                goto cleanup;
+        } else {
+             /* vlan setup is done as a part of reconnecting the tap
+              * device to a new bridge (either OVS or Linux host bridge).
+              */
+            needBridgeChange = true;
+        }
+        needReplaceDevDef = true;
+    }
+
     if (needBridgeChange) {
         if (qemuDomainChangeNetBridge(vm, olddev, newdev) < 0)
             goto cleanup;
@@ -4266,12 +4288,6 @@ qemuDomainChangeNet(virQEMUDriver *driver,
         goto cleanup;
     }
 
-    if (needVlanUpdate) {
-        if (virNetDevOpenvswitchUpdateVlan(newdev->ifname, &newdev->vlan) < 0)
-            goto cleanup;
-        needReplaceDevDef = true;
-    }
-
     if (needReplaceDevDef) {
         /* the changes above warrant replacing olddev with newdev in
          * the domain's nets list.
-- 
2.47.1

On 1/12/25 03:10, Laine Stump wrote:
> Since we previously only supported vlan tagging for interfaces
> connected to an OVS bridge [*], the code in qemuChangeNet() (used by
> the update-device API) assumed an interface with modified vlan config
> was on an OVS bridge, and would call the OVS-specific
> virNetDevOpenvswitchUpdateVlan().
> 
> Now that we support vlan tagging for interfaces connected to a
> standard Linux host bridge, we must check the type of connection and
> only call the OVS function when connected to an OVS bridge *both
> before and after the update*. Otherwise we just set the flag to
> re-connect to the bridge, which has the side effect of redoing the
> vlan setup.
> 
> ([*] or an SRIOV VF assigned using VFIO, but we don't support *any
> runtime changes to that type of netdev so it's irrelevant here.)
> 
> Signed-off-by: Laine Stump <laine@redhat.com>
> ---
> 
> I'm fine with this going in either before or after the 11.0.0 release
> (would prefer before since it is a bugfix), but if it's going into the
> release I'd rather it be included with RC2 rather than only in the
> final release. For that reason, if someone ACKs the patch and agrees
> that it's safe to go in, please push it for me (since Jiri will almost
> certainly tag RC2 before I'm conscious on Monday morning)
> 
>  src/qemu/qemu_hotplug.c | 32 ++++++++++++++++++++++++--------
>  1 file changed, 24 insertions(+), 8 deletions(-)

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

and I agree this is safe for freeze. So I've merged it.

Michal