util: make it optional to clear existing tc qdiscs/filters in virNetDevBandwidthSet()

virNetDevBandwidthSet() always clears all existing qdiscs and their
subordinate filters before adding all the new qdiscs/filters. This is
normally exactly what we want, but there is one case (the network
driver) where the Qdisc added by virNetDevBandwidthSet() may already
be in use by the nftables backend (which will add a rule to fix the
checksum of dhcp packets); in that case, we *don't* want
virNetDevBandwidthSet() to clear out the qdisc that was already added
for nftables, and none of the bandwidth filters have been added yet,
so there already aren't any "old" filters that need to be removed
either - it is safe to just skip virNetDevBandwidthClear() in this
case.

To allow the network driver to set bandwidth without first clearing
it, this patch adds the flag VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL to the
virNetDevBandwidthSetFlags enum, and recognizes it in
virNetDevBandwidthSet() - if the flag is set, then
virNetDevBandwidth() will call virNetDevBandwidthClear() just as it
always has. But if the flag isn't set it *won't* call
virNetDevBandwidthClear().

As suggested above, VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL is set for all
calls to virNetdevBandwidthSet() except for two places in the network
driver.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index a64cfef1a0be79c5a87137e8057210612fb570d6..d682e7168a59d2a487f29982a8f24f31c5fa426f 100644 (file)
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3570,7 +3570,7 @@ lxcDomainAttachDeviceNetLive(virLXCDriver *driver,
     actualBandwidth = virDomainNetGetActualBandwidth(net);
     if (actualBandwidth) {
         if (virNetDevSupportsBandwidth(actualType)) {
-            unsigned int flags = 0;
+            unsigned int flags = VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL;
 
             if (!virDomainNetTypeSharesHostView(net))
                 flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;

diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 7eba7a2c464b568857520ff2c9b51561dce563cd..cd8bcfc282f424c7a9e6caceaab10d71b3e0dd74 100644 (file)
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -609,7 +609,7 @@ virLXCProcessSetupInterfaces(virLXCDriver *driver,
         actualBandwidth = virDomainNetGetActualBandwidth(net);
         if (actualBandwidth) {
             if (virNetDevSupportsBandwidth(type)) {
-                unsigned int flags = 0;
+                unsigned int flags = VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL;
 
                 if (!virDomainNetTypeSharesHostView(net))
                     flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 07fffaf852c4f3f2e80d850e714d949477b6327d..dcb9c4934ec68450bc1541349c3bff6112af3038 100644 (file)
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8695,7 +8695,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver,
                                                         !virDomainNetTypeSharesHostView(net)) < 0)
                     goto cleanup;
             } else {
-                unsigned int flags = 0;
+                unsigned int flags = VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL;
 
                 if (!virDomainNetTypeSharesHostView(net))
                     flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 425d583e99690200fe07b36358e0282886addf09..d1b32de56ad7aecd4dbc48eabfa029bf2201db43 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9939,7 +9939,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
                 goto endjob;
             }
         } else {
-            unsigned int bwflags = 0;
+            unsigned int bwflags = VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL;
 
             if (!virDomainNetTypeSharesHostView(net))
                 bwflags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index b06ae61a4404fbb2ef1d4cdd9db5adfd0612653b..3c18af6b0c5f4099a8f85c542ed90533464040a3 100644 (file)
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1332,7 +1332,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver,
                                                         !virDomainNetTypeSharesHostView(net)) < 0)
                     goto cleanup;
             } else {
-                int flags = 0;
+                int flags = VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL;
 
                 if (!virDomainNetTypeSharesHostView(net))
                     flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
@@ -4187,7 +4187,7 @@ qemuDomainChangeNet(virQEMUDriver *driver,
                                                         !virDomainNetTypeSharesHostView(newdev)) < 0)
                     goto cleanup;
             } else {
-                int flags = 0;
+                int flags = VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL;
 
                 if (!virDomainNetTypeSharesHostView(newdev))
                     flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;

diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
index 1baad849c65d780c328d573f4ed5991b51f15184..9c48844c5dd1d297bd9ec99d603ca3b8ce9307ba 100644 (file)
--- a/src/util/virnetdevbandwidth.c
+++ b/src/util/virnetdevbandwidth.c
@@ -196,6 +196,21 @@ virNetDevBandwidthManipulateFilter(const char *ifname,
  *     interface (so domain's RX/TX is host's RX/TX), and for some
  *     it's swapped (domain's RX/TX is hosts's TX/RX).
  *
+ *   VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL
+ *     If VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL is set, then the root
+ *     qdisc is deleted before adding any new qdisc/class/filter,
+ *     which causes any pre-existing filters to also be deleted. If
+ *     not set, then it's assumed that there are no existing rules (or
+ *     that those already there need to be kept). The caller should
+ *     set this flag for an existing interface that is having its
+ *     bandwidth settings modified, but can leave it unset if the
+ *     interface was newly created and this is the first time
+ *     bandwidth has been set, but someone else might have already
+ *     added the qdisc (e.g. this is the case when the network driver
+ *     is setting bandwidth for a virtual network bridge device - the
+ *     nftables backend may have already added qdisc handle 1:0 and a
+ *     filter, and we don't want to delete them)
+ *
  * Return 0 on success, -1 otherwise.
  */
 int
@@ -238,7 +253,11 @@ virNetDevBandwidthSet(const char *ifname,
         tx = bandwidth->out;
     }
 
-    virNetDevBandwidthClear(ifname);
+    /* Only if the caller requests, clear everything including root
+     * qdisc and all filters before adding everything.
+     */
+    if (flags & VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL)
+        virNetDevBandwidthClear(ifname);
 
     if (tx && tx->average) {
         average = g_strdup_printf("%llukbps", tx->average);

diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h
index 80dc654486872288459bf2a32da6c21fb6024d72..744aa4c826593fc796f4c540fbf3fadb47827a8f 100644 (file)
--- a/src/util/virnetdevbandwidth.h
+++ b/src/util/virnetdevbandwidth.h
@@ -42,6 +42,7 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNetDevBandwidth, virNetDevBandwidthFree);
 typedef enum {
     VIR_NETDEV_BANDWIDTH_SET_HIERARCHICAL_CLASS = (1 << 0),
     VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED = (1 << 1),
+    VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL = (1 << 2),
 } virNetDevBandwidthSetFlags;
 
 int virNetDevBandwidthSet(const char *ifname,

diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c
index 6529ff4026c5ed1fc95d8879ab104ea69f290aa6..6d5c847ad7a772749f8fd3052a97794b6601df7b 100644 (file)
--- a/tests/virnetdevbandwidthtest.c
+++ b/tests/virnetdevbandwidthtest.c
@@ -82,7 +82,8 @@ testVirNetDevBandwidthSet(const void *data)
         if (virNetDevOpenvswitchInterfaceSetQos(iface, band, info->uuid, true) < 0)
             return -1;
     } else {
-        unsigned int flags = VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
+        unsigned int flags = VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED |
+                             VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL;
 
         if (info->hierarchical_class)
             flags |= VIR_NETDEV_BANDWIDTH_SET_HIERARCHICAL_CLASS;