util: use a single flags arg for virNetDevBandwidthSet(), not multiple bools

Having two bools in the arg list is on the borderline of being
confusing to anyone trying to read the code, but we're about to add a
3rd. This patch replaces the two bools with a single flags argument
which will instead have one or more bits from virNetDevBandwidthFlags
set.

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

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 0e31e5e4b9894d0d92e79a6bb0b30839828832d9..a64cfef1a0be79c5a87137e8057210612fb570d6 100644 (file)
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3570,8 +3570,12 @@ lxcDomainAttachDeviceNetLive(virLXCDriver *driver,
     actualBandwidth = virDomainNetGetActualBandwidth(net);
     if (actualBandwidth) {
         if (virNetDevSupportsBandwidth(actualType)) {
-            if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false,
-                                      !virDomainNetTypeSharesHostView(net)) < 0)
+            unsigned int flags = 0;
+
+            if (!virDomainNetTypeSharesHostView(net))
+                flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
+
+            if (virNetDevBandwidthSet(net->ifname, actualBandwidth, flags) < 0)
                 goto cleanup;
         } else {
             VIR_WARN("setting bandwidth on interfaces of "

diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 083ab83ec6ad8874823478de2c1de753b0043385..7eba7a2c464b568857520ff2c9b51561dce563cd 100644 (file)
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -609,8 +609,12 @@ virLXCProcessSetupInterfaces(virLXCDriver *driver,
         actualBandwidth = virDomainNetGetActualBandwidth(net);
         if (actualBandwidth) {
             if (virNetDevSupportsBandwidth(type)) {
-                if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false,
-                                          !virDomainNetTypeSharesHostView(net)) < 0)
+                unsigned int flags = 0;
+
+                if (!virDomainNetTypeSharesHostView(net))
+                    flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
+
+                if (virNetDevBandwidthSet(net->ifname, actualBandwidth, flags) < 0)
                     goto cleanup;
             } else {
                 VIR_WARN("setting bandwidth on interfaces of "

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index d408f17de73a205df065e5136c26ca33a7d8795f..e700a614a9a369e3afb67b838d2828431bc2861a 100644 (file)
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2109,8 +2109,11 @@ networkStartNetworkVirtual(virNetworkDriverState *driver,
         }
     }
 
-    if (virNetDevBandwidthSet(def->bridge, def->bandwidth, true, true) < 0)
+    if (virNetDevBandwidthSet(def->bridge, def->bandwidth,
+                              VIR_NETDEV_BANDWIDTH_SET_HIERARCHICAL_CLASS
+                              | VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED) < 0) {
         goto error;
+    }
 
     return 0;
 
@@ -2190,8 +2193,11 @@ networkStartNetworkBridge(virNetworkObj *obj)
      * type BRIDGE, is started. On failure, undo anything you've done,
      * and return -1. On success return 0.
      */
-    if (virNetDevBandwidthSet(def->bridge, def->bandwidth, true, true) < 0)
+    if (virNetDevBandwidthSet(def->bridge, def->bandwidth,
+                              VIR_NETDEV_BANDWIDTH_SET_HIERARCHICAL_CLASS
+                              | VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED) < 0) {
         goto error;
+    }
 
     if (networkStartHandleMACTableManagerMode(obj) < 0)
         goto error;

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 98211f4cd665a84da8fef2d4977ea1b126c43a60..07fffaf852c4f3f2e80d850e714d949477b6327d 100644 (file)
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8694,9 +8694,14 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver,
                                                         def->uuid,
                                                         !virDomainNetTypeSharesHostView(net)) < 0)
                     goto cleanup;
-            } else if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false,
-                                             !virDomainNetTypeSharesHostView(net)) < 0) {
-                goto cleanup;
+            } else {
+                unsigned int flags = 0;
+
+                if (!virDomainNetTypeSharesHostView(net))
+                    flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
+
+                if (virNetDevBandwidthSet(net->ifname, actualBandwidth, flags) < 0)
+                    goto cleanup;
             }
         } else {
             VIR_WARN("setting bandwidth on interfaces of "

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5b9c55f704cb57e2f1229e5975d09f4457114b68..425d583e99690200fe07b36358e0282886addf09 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9938,21 +9938,22 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
                 virErrorRestore(&orig_err);
                 goto endjob;
             }
-        } else if (virNetDevBandwidthSet(net->ifname, newBandwidth, false,
-                                         !virDomainNetTypeSharesHostView(net)) < 0) {
-            virErrorPtr orig_err;
-
-            virErrorPreserveLast(&orig_err);
-            ignore_value(virNetDevBandwidthSet(net->ifname,
-                                               net->bandwidth,
-                                               false,
-                                               !virDomainNetTypeSharesHostView(net)));
-            if (net->bandwidth) {
-                ignore_value(virDomainNetBandwidthUpdate(net,
-                                                         net->bandwidth));
+        } else {
+            unsigned int bwflags = 0;
+
+            if (!virDomainNetTypeSharesHostView(net))
+                bwflags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
+
+            if (virNetDevBandwidthSet(net->ifname, newBandwidth, bwflags) < 0) {
+                virErrorPtr orig_err;
+
+                virErrorPreserveLast(&orig_err);
+                ignore_value(virNetDevBandwidthSet(net->ifname, net->bandwidth, bwflags));
+                if (net->bandwidth)
+                    ignore_value(virDomainNetBandwidthUpdate(net, net->bandwidth));
+                virErrorRestore(&orig_err);
+                goto endjob;
             }
-            virErrorRestore(&orig_err);
-            goto endjob;
         }
 
         /* If the old bandwidth was cleared out, restore qdisc. */

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 55512476e4771161819609b25df3c219d00617da..b06ae61a4404fbb2ef1d4cdd9db5adfd0612653b 100644 (file)
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1331,9 +1331,14 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver,
                                                         vm->def->uuid,
                                                         !virDomainNetTypeSharesHostView(net)) < 0)
                     goto cleanup;
-            } else if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false,
-                                             !virDomainNetTypeSharesHostView(net)) < 0) {
-                goto cleanup;
+            } else {
+                int flags = 0;
+
+                if (!virDomainNetTypeSharesHostView(net))
+                    flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
+
+                if (virNetDevBandwidthSet(net->ifname, actualBandwidth, flags) < 0)
+                    goto cleanup;
             }
         } else {
             VIR_WARN("setting bandwidth on interfaces of "
@@ -4181,9 +4186,14 @@ qemuDomainChangeNet(virQEMUDriver *driver,
                                                         vm->def->uuid,
                                                         !virDomainNetTypeSharesHostView(newdev)) < 0)
                     goto cleanup;
-            } else if (virNetDevBandwidthSet(newdev->ifname, newb, false,
-                                             !virDomainNetTypeSharesHostView(newdev)) < 0) {
-                goto cleanup;
+            } else {
+                int flags = 0;
+
+                if (!virDomainNetTypeSharesHostView(newdev))
+                    flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
+
+                if (virNetDevBandwidthSet(newdev->ifname, newb, flags) < 0)
+                    goto cleanup;
             }
         } else {
             if (virDomainInterfaceClearQoS(vm->def, olddev) < 0)

diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
index 2b58c58d3e940ca1181bac230a92efa2e5d5765b..1baad849c65d780c328d573f4ed5991b51f15184 100644 (file)
--- a/src/util/virnetdevbandwidth.c
+++ b/src/util/virnetdevbandwidth.c
@@ -173,30 +173,35 @@ virNetDevBandwidthManipulateFilter(const char *ifname,
  * virNetDevBandwidthSet:
  * @ifname: on which interface
  * @bandwidth: rates to set (may be NULL)
- * @hierarchical_class: whether to create hierarchical class
- * @swapped: true if IN/OUT should be set contrariwise
+ * @flags: bits indicating certain optional actions
  *
+
  * This function enables QoS on specified interface
  * and set given traffic limits for both, incoming
- * and outgoing traffic. Any previous setting get
- * overwritten. If @hierarchical_class is TRUE, create
- * hierarchical class. It is used to guarantee minimal
- * throughput ('floor' attribute in NIC).
+ * and outgoing traffic.
+ *
+ * @flags bits and their meanings:
+ *
+ *   VIR_NETDEV_BANDWIDTH_SET_HIERARCHICAL_CLASS
+ *     whether to create a hierarchical class
+ *     A hiearchical class structure is used to implement a minimal
+ *     throughput guarantee ('floor' attribute in NIC).
  *
- * If @swapped is set, the IN part of @bandwidth is set on
- * @ifname's TX, and vice versa. If it is not set, IN is set on
- * RX and OUT on TX. This is because for some types of interfaces
- * domain and the host live on the same side of the 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_DIR_SWAPPED
+ *     set if IN/OUT should be set backwards from what's indicated in
+ *     the bandwidth, i.e. the IN part of @bandwidth is set on
+ *     @ifname's TX, and the OUT part of @bandwidth is set on
+ *     @ifname's RX.  This is needed because for some types of
+ *     interfaces the domain and the host live on the same side of the
+ *     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).
  *
  * Return 0 on success, -1 otherwise.
  */
 int
 virNetDevBandwidthSet(const char *ifname,
                       const virNetDevBandwidth *bandwidth,
-                      bool hierarchical_class,
-                      bool swapped)
+                      unsigned int flags)
 {
     int ret = -1;
     virNetDevBandwidthRate *rx = NULL; /* From domain POV */
@@ -205,6 +210,7 @@ virNetDevBandwidthSet(const char *ifname,
     char *average = NULL;
     char *peak = NULL;
     char *burst = NULL;
+    bool hierarchical_class = flags & VIR_NETDEV_BANDWIDTH_SET_HIERARCHICAL_CLASS;
 
     if (!bandwidth) {
         /* nothing to be enabled */
@@ -224,7 +230,7 @@ virNetDevBandwidthSet(const char *ifname,
         return -1;
     }
 
-    if (swapped) {
+    if (flags & VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED) {
         rx = bandwidth->out;
         tx = bandwidth->in;
     } else {

diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h
index 6d268fb1190de07e386ab85cfd7af6c386aaae1b..80dc654486872288459bf2a32da6c21fb6024d72 100644 (file)
--- a/src/util/virnetdevbandwidth.h
+++ b/src/util/virnetdevbandwidth.h
@@ -39,11 +39,16 @@ void virNetDevBandwidthFree(virNetDevBandwidth *def);
 
 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),
+} virNetDevBandwidthSetFlags;
+
 int virNetDevBandwidthSet(const char *ifname,
                           const virNetDevBandwidth *bandwidth,
-                          bool hierarchical_class,
-                          bool swapped)
+                          unsigned int flags)
     G_GNUC_WARN_UNUSED_RESULT;
+
 int virNetDevBandwidthClear(const char *ifname);
 int virNetDevBandwidthCopy(virNetDevBandwidth **dest,
                            const virNetDevBandwidth *src)

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