From 812a146dfe784315edece43d09f8d9e432f8230e Mon Sep 17 00:00:00 2001
From: Michal Privoznik <mprivozn@redhat.com>
Date: Fri, 12 Apr 2024 09:26:20 +0200
Subject: [PATCH] domain_interface: Introduce and use
 virDomainInterfaceClearQoS()

In QEMU and LXC drivers in a few places only
virNetDevBandwidthClear() is called. This means that if an
interface is of openvswitch vport profile, its QoS is not
removed. And to make matters worse - OVS is designed to remember
state even when corresponding interface is gone. This leads to
stale QoS settings piling up in OVS database.

To resolve this, introduce virDomainInterfaceClearQoS() which
looks at given interface and calls corresponding QoS clear
function. Then, basically replace virNetDevBandwidthClear() calls
in those hypervisor drivers with this new function.

Resolves: https://issues.redhat.com/browse/RHEL-30373
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
---
 src/hypervisor/domain_interface.c | 36 ++++++++++++++++++++++++++-----
 src/hypervisor/domain_interface.h |  2 ++
 src/libvirt_private.syms          |  1 +
 src/lxc/lxc_driver.c              |  5 +----
 src/qemu/qemu_hotplug.c           | 18 +++-------------
 5 files changed, 38 insertions(+), 24 deletions(-)

diff --git a/src/hypervisor/domain_interface.c b/src/hypervisor/domain_interface.c
index 0a9cad8011..cc6aa8551a 100644
--- a/src/hypervisor/domain_interface.c
+++ b/src/hypervisor/domain_interface.c
@@ -472,16 +472,42 @@ virDomainInterfaceDeleteDevice(virDomainDef *def,
 }
 
 
+/**
+ * virDomainInterfaceClearQoS
+ * @def: domain definition
+ * @net: a net definition in the VM
+ *
+ * For given interface @net clear its QoS settings in the
+ * host. NOP if @net has no QoS or is of a type that doesn't
+ * support QoS.
+ *
+ * Returns: 0 on success,
+ *         -1 otherwise (with appropriate error reported)
+ */
+int
+virDomainInterfaceClearQoS(virDomainDef *def,
+                           virDomainNetDef *net)
+{
+    if (!virDomainNetGetActualBandwidth(net))
+        return 0;
+
+    if (!virNetDevSupportsBandwidth(virDomainNetGetActualType(net)))
+        return 0;
+
+    if (virDomainNetDefIsOvsport(net)) {
+        return virNetDevOpenvswitchInterfaceClearQos(net->ifname, def->uuid);
+    }
+
+    return virNetDevBandwidthClear(net->ifname);
+}
+
+
 void
 virDomainClearNetBandwidth(virDomainDef *def)
 {
     size_t i;
-    virDomainNetType type;
 
     for (i = 0; i < def->nnets; i++) {
-        type = virDomainNetGetActualType(def->nets[i]);
-        if (virDomainNetGetActualBandwidth(def->nets[i]) &&
-            virNetDevSupportsBandwidth(type))
-            virNetDevBandwidthClear(def->nets[i]->ifname);
+        virDomainInterfaceClearQoS(def, def->nets[i]);
     }
 }
diff --git a/src/hypervisor/domain_interface.h b/src/hypervisor/domain_interface.h
index bb212cf3b8..572b4dd8c5 100644
--- a/src/hypervisor/domain_interface.h
+++ b/src/hypervisor/domain_interface.h
@@ -44,5 +44,7 @@ void virDomainInterfaceDeleteDevice(virDomainDef *def,
                                 virDomainNetDef *net,
                                 bool priv_net_created,
                                 char *stateDir);
+int virDomainInterfaceClearQoS(virDomainDef *def,
+                               virDomainNetDef *net);
 void virDomainClearNetBandwidth(virDomainDef *def)
     ATTRIBUTE_NONNULL(1);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 328f5b347b..839fe4f545 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1635,6 +1635,7 @@ virDomainDriverSetupPersistentDefBlkioParams;
 
 # hypervisor/domain_interface.h
 virDomainClearNetBandwidth;
+virDomainInterfaceClearQoS;
 virDomainInterfaceDeleteDevice;
 virDomainInterfaceEthernetConnect;
 virDomainInterfaceIsVnetCompatModel;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 89ef9416aa..1842ae8f0e 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -4052,10 +4052,7 @@ lxcDomainDetachDeviceNetLive(virDomainObj *vm,
     actualType = virDomainNetGetActualType(detach);
 
     /* clear network bandwidth */
-    if (virDomainNetGetActualBandwidth(detach) &&
-        virNetDevSupportsBandwidth(actualType) &&
-        virNetDevBandwidthClear(detach->ifname))
-        goto cleanup;
+    virDomainInterfaceClearQoS(vm->def, detach);
 
     switch (actualType) {
     case VIR_DOMAIN_NET_TYPE_BRIDGE:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 054053729c..774962b0df 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -4059,11 +4059,8 @@ qemuDomainChangeNet(virQEMUDriver *driver,
                 goto cleanup;
             }
         } else {
-            /*
-             * virNetDevBandwidthSet() doesn't clear any existing
-             * setting unless something new is being set.
-             */
-            virNetDevBandwidthClear(newdev->ifname);
+            if (virDomainInterfaceClearQoS(vm->def, newdev) < 0)
+                goto cleanup;
         }
 
         /* If the old bandwidth was cleared out, restore qdisc. */
@@ -4800,16 +4797,7 @@ qemuDomainRemoveNetDevice(virQEMUDriver *driver,
     if (!(charDevAlias = qemuAliasChardevFromDevAlias(net->info.alias)))
         return -1;
 
-    if (virNetDevSupportsBandwidth(virDomainNetGetActualType(net))) {
-        if (virDomainNetDefIsOvsport(net)) {
-            if (virNetDevOpenvswitchInterfaceClearQos(net->ifname, vm->def->uuid) < 0)
-                VIR_WARN("cannot clear bandwidth setting for ovs device : %s",
-                         net->ifname);
-        } else if (virNetDevBandwidthClear(net->ifname) < 0) {
-            VIR_WARN("cannot clear bandwidth setting for device : %s",
-                     net->ifname);
-        }
-    }
+    virDomainInterfaceClearQoS(vm->def, net);
 
     /* deactivate the tap/macvtap device on the host, which could also
      * affect the parent device (e.g. macvtap passthrough mode sets