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>

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,
+            unsigned int flags = 0;
-                                      !virDomainNetTypeSharesHostView(net)) < 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 "

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,
+                unsigned int flags = 0;
-                                          !virDomainNetTypeSharesHostView(net)) < 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 "

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;

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,
+            } else {
-                                             !virDomainNetTypeSharesHostView(net)) < 0) {
+                unsigned int flags = 0;
-                goto cleanup;
+
+                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 "

src/qemu/qemu_driver.c

@@ -9938,21 +9938,22 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
                 virErrorRestore(&orig_err);
                 goto endjob;
             }
-        } else if (virNetDevBandwidthSet(net->ifname, newBandwidth, false,
+        } else {
-                                         !virDomainNetTypeSharesHostView(net)) < 0) {
+            unsigned int bwflags = 0;
-            virErrorPtr orig_err;
 
-            virErrorPreserveLast(&orig_err);
+            if (!virDomainNetTypeSharesHostView(net))
-            ignore_value(virNetDevBandwidthSet(net->ifname,
+                bwflags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
-                                               net->bandwidth,
+
-                                               false,
+            if (virNetDevBandwidthSet(net->ifname, newBandwidth, bwflags) < 0) {
-                                               !virDomainNetTypeSharesHostView(net)));
+                virErrorPtr orig_err;
-            if (net->bandwidth) {
+
-                ignore_value(virDomainNetBandwidthUpdate(net,
+                virErrorPreserveLast(&orig_err);
-                                                         net->bandwidth));
+                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. */

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,
+            } else {
-                                             !virDomainNetTypeSharesHostView(net)) < 0) {
+                int flags = 0;
-                goto cleanup;
+
+                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,
+            } else {
-                                             !virDomainNetTypeSharesHostView(newdev)) < 0) {
+                int flags = 0;
-                goto cleanup;
+
+                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)

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
+ * @flags: bits indicating certain optional actions
- * @swapped: true if IN/OUT should be set contrariwise
  *
+
  * This function enables QoS on specified interface
  * and set given traffic limits for both, incoming
- * and outgoing traffic. Any previous setting get
+ * and outgoing traffic.
- * overwritten. If @hierarchical_class is TRUE, create
- * hierarchical class. It is used to guarantee minimal
- * throughput ('floor' attribute in NIC).
  *
- * If @swapped is set, the IN part of @bandwidth is set on
+ * @flags bits and their meanings:
- * @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
+ *   VIR_NETDEV_BANDWIDTH_SET_HIERARCHICAL_CLASS
- * domain and the host live on the same side of the interface (so
+ *     whether to create a hierarchical class
- * domain's RX/TX is host's RX/TX), and for some it's swapped
+ *     A hiearchical class structure is used to implement a minimal
- * (domain's RX/TX is hosts's TX/RX).
+ *     throughput guarantee ('floor' attribute in NIC).
+ *
+ *   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,
+                      unsigned int flags)
-                      bool swapped)
 {
     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 {

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,
+                          unsigned int flags)
-                          bool swapped)
     G_GNUC_WARN_UNUSED_RESULT;
+
 int virNetDevBandwidthClear(const char *ifname);
 int virNetDevBandwidthCopy(virNetDevBandwidth **dest,
                            const virNetDevBandwidth *src)

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;
     }