[Qemu-devel] [PATCH v2 02/17] throttle: Make throttle_conflicting() set errp

[Alberto Garcia]

The caller does not need to set it, and this will allow us to refactor
this function later.

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

diff --git a/blockdev.c b/blockdev.c
index 1f73478..95bc2fa 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -345,9 +345,7 @@ static bool parse_stats_intervals(BlockAcctStats *stats, 
QList *intervals,
 
 static bool check_throttle_config(ThrottleConfig *cfg, Error **errp)
 {
-    if (throttle_conflicting(cfg)) {
-        error_setg(errp, "bps/iops/max total values and read/write values"
-                         " cannot be used at the same time");
+    if (throttle_conflicting(cfg, errp)) {
         return false;
     }
 
diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
index 52c98d9..69cf171 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -106,7 +106,7 @@ bool throttle_timers_are_initialized(ThrottleTimers *tt);
 /* configuration */
 bool throttle_enabled(ThrottleConfig *cfg);
 
-bool throttle_conflicting(ThrottleConfig *cfg);
+bool throttle_conflicting(ThrottleConfig *cfg, Error **errp);
 
 bool throttle_is_valid(ThrottleConfig *cfg);
 
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 858f1aa..579b8af 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -255,31 +255,31 @@ static void test_conflicts_for_one_set(bool is_max,
                                        int write)
 {
     memset(&cfg, 0, sizeof(cfg));
-    g_assert(!throttle_conflicting(&cfg));
+    g_assert(!throttle_conflicting(&cfg, NULL));
 
     set_cfg_value(is_max, total, 1);
     set_cfg_value(is_max, read,  1);
-    g_assert(throttle_conflicting(&cfg));
+    g_assert(throttle_conflicting(&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));
+    g_assert(throttle_conflicting(&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));
+    g_assert(throttle_conflicting(&cfg, NULL));
 
     memset(&cfg, 0, sizeof(cfg));
     set_cfg_value(is_max, total, 1);
-    g_assert(!throttle_conflicting(&cfg));
+    g_assert(!throttle_conflicting(&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));
+    g_assert(!throttle_conflicting(&cfg, NULL));
 }
 
 static void test_conflicting_config(void)
diff --git a/util/throttle.c b/util/throttle.c
index c21043a..564e132 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -252,8 +252,9 @@ bool throttle_enabled(ThrottleConfig *cfg)
  *
  * @cfg: the throttling configuration to inspect
  * @ret: true if any conflict detected else false
+ * @errp: error object
  */
-bool throttle_conflicting(ThrottleConfig *cfg)
+bool throttle_conflicting(ThrottleConfig *cfg, Error **errp)
 {
     bool bps_flag, ops_flag;
     bool bps_max_flag, ops_max_flag;
@@ -274,7 +275,13 @@ bool throttle_conflicting(ThrottleConfig *cfg)
                    (cfg->buckets[THROTTLE_OPS_READ].max ||
                    cfg->buckets[THROTTLE_OPS_WRITE].max);
 
-    return bps_flag || ops_flag || bps_max_flag || ops_max_flag;
+    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;
 }
 
 /* check if a throttling configuration is valid
-- 
2.7.0