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