[Qemu-devel] [PULL 19/34] throttle: Merge all functions that check the configuration into one

[Kevin Wolf]

From: Alberto Garcia <address@hidden>

There's no need to keep throttle_conflicting(), throttle_is_valid()
and throttle_max_is_missing_limit() as separate functions, so this
patch merges all three into one.

As a consequence, check_throttle_config() becomes redundant and can be
replaced with throttle_is_valid().

Signed-off-by: Alberto Garcia <address@hidden>
Reviewed-by: Kevin Wolf <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>
---
 blockdev.c              | 21 ++-------------------
 include/qemu/throttle.h |  4 ----
 tests/test-throttle.c   | 18 +++++++++---------
 util/throttle.c         | 40 ++++++++--------------------------------
 4 files changed, 19 insertions(+), 64 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 11a3139..73babeb 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -343,23 +343,6 @@ static bool parse_stats_intervals(BlockAcctStats *stats, 
QList *intervals,
     return true;
 }
 
-static bool check_throttle_config(ThrottleConfig *cfg, Error **errp)
-{
-    if (throttle_conflicting(cfg, errp)) {
-        return false;
-    }
-
-    if (!throttle_is_valid(cfg, errp)) {
-        return false;
-    }
-
-    if (throttle_max_is_missing_limit(cfg, errp)) {
-        return false;
-    }
-
-    return true;
-}
-
 typedef enum { MEDIA_DISK, MEDIA_CDROM } DriveMediaType;
 
 /* All parameters but @opts are optional and may be set to NULL. */
@@ -434,7 +417,7 @@ static void extract_common_blockdev_options(QemuOpts *opts, 
int *bdrv_flags,
         throttle_cfg->op_size =
             qemu_opt_get_number(opts, "throttling.iops-size", 0);
 
-        if (!check_throttle_config(throttle_cfg, errp)) {
+        if (!throttle_is_valid(throttle_cfg, errp)) {
             return;
         }
     }
@@ -2660,7 +2643,7 @@ void qmp_block_set_io_throttle(const char *device, 
int64_t bps, int64_t bps_rd,
         cfg.op_size = iops_size;
     }
 
-    if (!check_throttle_config(&cfg, errp)) {
+    if (!throttle_is_valid(&cfg, errp)) {
         goto out;
     }
 
diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
index ecae621..aec0785 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -106,12 +106,8 @@ bool throttle_timers_are_initialized(ThrottleTimers *tt);
 /* configuration */
 bool throttle_enabled(ThrottleConfig *cfg);
 
-bool throttle_conflicting(ThrottleConfig *cfg, Error **errp);
-
 bool throttle_is_valid(ThrottleConfig *cfg, Error **errp);
 
-bool throttle_max_is_missing_limit(ThrottleConfig *cfg, Error **errp);
-
 void throttle_config(ThrottleState *ts,
                      ThrottleTimers *tt,
                      ThrottleConfig *cfg);
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 3e208a8..a0c17ac 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -257,31 +257,31 @@ static void test_conflicts_for_one_set(bool is_max,
                                        int write)
 {
     memset(&cfg, 0, sizeof(cfg));
-    g_assert(!throttle_conflicting(&cfg, NULL));
+    g_assert(throttle_is_valid(&cfg, NULL));
 
     set_cfg_value(is_max, total, 1);
     set_cfg_value(is_max, read,  1);
-    g_assert(throttle_conflicting(&cfg, NULL));
+    g_assert(!throttle_is_valid(&cfg, NULL));
 
     memset(&cfg, 0, sizeof(cfg));
     set_cfg_value(is_max, total, 1);
     set_cfg_value(is_max, write, 1);
-    g_assert(throttle_conflicting(&cfg, NULL));
+    g_assert(!throttle_is_valid(&cfg, NULL));
 
     memset(&cfg, 0, sizeof(cfg));
     set_cfg_value(is_max, total, 1);
     set_cfg_value(is_max, read,  1);
     set_cfg_value(is_max, write, 1);
-    g_assert(throttle_conflicting(&cfg, NULL));
+    g_assert(!throttle_is_valid(&cfg, NULL));
 
     memset(&cfg, 0, sizeof(cfg));
     set_cfg_value(is_max, total, 1);
-    g_assert(!throttle_conflicting(&cfg, NULL));
+    g_assert(throttle_is_valid(&cfg, NULL));
 
     memset(&cfg, 0, sizeof(cfg));
     set_cfg_value(is_max, read,  1);
     set_cfg_value(is_max, write, 1);
-    g_assert(!throttle_conflicting(&cfg, NULL));
+    g_assert(throttle_is_valid(&cfg, NULL));
 }
 
 static void test_conflicting_config(void)
@@ -340,15 +340,15 @@ static void test_max_is_missing_limit(void)
         memset(&cfg, 0, sizeof(cfg));
         cfg.buckets[i].max = 100;
         cfg.buckets[i].avg = 0;
-        g_assert(throttle_max_is_missing_limit(&cfg, NULL));
+        g_assert(!throttle_is_valid(&cfg, NULL));
 
         cfg.buckets[i].max = 0;
         cfg.buckets[i].avg = 0;
-        g_assert(!throttle_max_is_missing_limit(&cfg, NULL));
+        g_assert(throttle_is_valid(&cfg, NULL));
 
         cfg.buckets[i].max = 0;
         cfg.buckets[i].avg = 100;
-        g_assert(!throttle_max_is_missing_limit(&cfg, NULL));
+        g_assert(throttle_is_valid(&cfg, NULL));
     }
 }
 
diff --git a/util/throttle.c b/util/throttle.c
index 9046dd8..f8bf03c 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -248,14 +248,14 @@ bool throttle_enabled(ThrottleConfig *cfg)
     return false;
 }
 
-/* return true if any two throttling parameters conflicts
- *
+/* check if a throttling configuration is valid
  * @cfg: the throttling configuration to inspect
- * @ret: true if any conflict detected else false
+ * @ret: true if valid else false
  * @errp: error object
  */
-bool throttle_conflicting(ThrottleConfig *cfg, Error **errp)
+bool throttle_is_valid(ThrottleConfig *cfg, Error **errp)
 {
+    int i;
     bool bps_flag, ops_flag;
     bool bps_max_flag, ops_max_flag;
 
@@ -278,21 +278,9 @@ bool throttle_conflicting(ThrottleConfig *cfg, Error 
**errp)
     if (bps_flag || ops_flag || bps_max_flag || ops_max_flag) {
         error_setg(errp, "bps/iops/max total values and read/write values"
                    " cannot be used at the same time");
-        return true;
+        return false;
     }
 
-    return false;
-}
-
-/* check if a throttling configuration is valid
- * @cfg: the throttling configuration to inspect
- * @ret: true if valid else false
- * @errp: error object
- */
-bool throttle_is_valid(ThrottleConfig *cfg, Error **errp)
-{
-    int i;
-
     for (i = 0; i < BUCKETS_COUNT; i++) {
         if (cfg->buckets[i].avg < 0 ||
             cfg->buckets[i].max < 0 ||
@@ -302,27 +290,15 @@ bool throttle_is_valid(ThrottleConfig *cfg, Error **errp)
                        THROTTLE_VALUE_MAX);
             return false;
         }
-    }
-
-    return true;
-}
 
-/* check if bps_max/iops_max is used without bps/iops
- * @cfg: the throttling configuration to inspect
- * @errp: error object
- */
-bool throttle_max_is_missing_limit(ThrottleConfig *cfg, Error **errp)
-{
-    int i;
-
-    for (i = 0; i < BUCKETS_COUNT; i++) {
         if (cfg->buckets[i].max && !cfg->buckets[i].avg) {
             error_setg(errp, "bps_max/iops_max require corresponding"
                        " bps/iops values");
-            return true;
+            return false;
         }
     }
-    return false;
+
+    return true;
 }
 
 /* fix bucket parameters */
-- 
1.8.3.1