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>

diff --git a/src/hypervisor/domain_interface.c b/src/hypervisor/domain_interface.c
index 0a9cad80111722df2a9877a5661689751a6dfedc..cc6aa8551a54831f4d9df10e79f1b467f4aad6ed 100644 (file)
--- 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 bb212cf3b827dad950ab22d8f82a081d62302940..572b4dd8c558240ccd4f29a4d68aa2ac0f6378f2 100644 (file)
--- 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 328f5b347be49de2629dd5416b73051e2439e608..839fe4f545f1e21486c68b63ed0135fbe9332f6c 100644 (file)
--- 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 89ef9416aaf07f33895159e93b9bc32a2f6a5ee7..1842ae8f0e99cc7cc4ed71e02587f6c21a29db97 100644 (file)
--- 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 054053729cfa1e5d81b6b0b6d5ed20fb01d4c86c..774962b0df2c67a3faf04253116346b243b2850b 100644 (file)
--- 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