[Qemu-devel] [PULL v2 for-2.7 12/15] block: Simplify block_set_io_throttle

[Markus Armbruster]

From: Eric Blake <address@hidden>

Now that we can support boxed commands, use it to greatly
reduce the number of parameters (and likelihood of getting
out of sync) when adjusting throttle parameters.

Signed-off-by: Eric Blake <address@hidden>
Reviewed-by: Alberto Garcia <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Markus Armbruster <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>
---
 blockdev.c           | 111 +++++++++++++++++++--------------------------------
 hmp.c                |  45 +++++----------------
 qapi/block-core.json |  20 ++++++++--
 3 files changed, 66 insertions(+), 110 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 384ad3b..c5443c1 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2634,49 +2634,17 @@ fail:
 }
 
 /* throttling disk I/O limits */
-void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
-                               int64_t bps_wr,
-                               int64_t iops,
-                               int64_t iops_rd,
-                               int64_t iops_wr,
-                               bool has_bps_max,
-                               int64_t bps_max,
-                               bool has_bps_rd_max,
-                               int64_t bps_rd_max,
-                               bool has_bps_wr_max,
-                               int64_t bps_wr_max,
-                               bool has_iops_max,
-                               int64_t iops_max,
-                               bool has_iops_rd_max,
-                               int64_t iops_rd_max,
-                               bool has_iops_wr_max,
-                               int64_t iops_wr_max,
-                               bool has_bps_max_length,
-                               int64_t bps_max_length,
-                               bool has_bps_rd_max_length,
-                               int64_t bps_rd_max_length,
-                               bool has_bps_wr_max_length,
-                               int64_t bps_wr_max_length,
-                               bool has_iops_max_length,
-                               int64_t iops_max_length,
-                               bool has_iops_rd_max_length,
-                               int64_t iops_rd_max_length,
-                               bool has_iops_wr_max_length,
-                               int64_t iops_wr_max_length,
-                               bool has_iops_size,
-                               int64_t iops_size,
-                               bool has_group,
-                               const char *group, Error **errp)
+void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp)
 {
     ThrottleConfig cfg;
     BlockDriverState *bs;
     BlockBackend *blk;
     AioContext *aio_context;
 
-    blk = blk_by_name(device);
+    blk = blk_by_name(arg->device);
     if (!blk) {
         error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
-                  "Device '%s' not found", device);
+                  "Device '%s' not found", arg->device);
         return;
     }
 
@@ -2685,59 +2653,59 @@ void qmp_block_set_io_throttle(const char *device, 
int64_t bps, int64_t bps_rd,
 
     bs = blk_bs(blk);
     if (!bs) {
-        error_setg(errp, "Device '%s' has no medium", device);
+        error_setg(errp, "Device '%s' has no medium", arg->device);
         goto out;
     }
 
     throttle_config_init(&cfg);
-    cfg.buckets[THROTTLE_BPS_TOTAL].avg = bps;
-    cfg.buckets[THROTTLE_BPS_READ].avg  = bps_rd;
-    cfg.buckets[THROTTLE_BPS_WRITE].avg = bps_wr;
+    cfg.buckets[THROTTLE_BPS_TOTAL].avg = arg->bps;
+    cfg.buckets[THROTTLE_BPS_READ].avg  = arg->bps_rd;
+    cfg.buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr;
 
-    cfg.buckets[THROTTLE_OPS_TOTAL].avg = iops;
-    cfg.buckets[THROTTLE_OPS_READ].avg  = iops_rd;
-    cfg.buckets[THROTTLE_OPS_WRITE].avg = iops_wr;
+    cfg.buckets[THROTTLE_OPS_TOTAL].avg = arg->iops;
+    cfg.buckets[THROTTLE_OPS_READ].avg  = arg->iops_rd;
+    cfg.buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr;
 
-    if (has_bps_max) {
-        cfg.buckets[THROTTLE_BPS_TOTAL].max = bps_max;
+    if (arg->has_bps_max) {
+        cfg.buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max;
     }
-    if (has_bps_rd_max) {
-        cfg.buckets[THROTTLE_BPS_READ].max = bps_rd_max;
+    if (arg->has_bps_rd_max) {
+        cfg.buckets[THROTTLE_BPS_READ].max = arg->bps_rd_max;
     }
-    if (has_bps_wr_max) {
-        cfg.buckets[THROTTLE_BPS_WRITE].max = bps_wr_max;
+    if (arg->has_bps_wr_max) {
+        cfg.buckets[THROTTLE_BPS_WRITE].max = arg->bps_wr_max;
     }
-    if (has_iops_max) {
-        cfg.buckets[THROTTLE_OPS_TOTAL].max = iops_max;
+    if (arg->has_iops_max) {
+        cfg.buckets[THROTTLE_OPS_TOTAL].max = arg->iops_max;
     }
-    if (has_iops_rd_max) {
-        cfg.buckets[THROTTLE_OPS_READ].max = iops_rd_max;
+    if (arg->has_iops_rd_max) {
+        cfg.buckets[THROTTLE_OPS_READ].max = arg->iops_rd_max;
     }
-    if (has_iops_wr_max) {
-        cfg.buckets[THROTTLE_OPS_WRITE].max = iops_wr_max;
+    if (arg->has_iops_wr_max) {
+        cfg.buckets[THROTTLE_OPS_WRITE].max = arg->iops_wr_max;
     }
 
-    if (has_bps_max_length) {
-        cfg.buckets[THROTTLE_BPS_TOTAL].burst_length = bps_max_length;
+    if (arg->has_bps_max_length) {
+        cfg.buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_max_length;
     }
-    if (has_bps_rd_max_length) {
-        cfg.buckets[THROTTLE_BPS_READ].burst_length = bps_rd_max_length;
+    if (arg->has_bps_rd_max_length) {
+        cfg.buckets[THROTTLE_BPS_READ].burst_length = arg->bps_rd_max_length;
     }
-    if (has_bps_wr_max_length) {
-        cfg.buckets[THROTTLE_BPS_WRITE].burst_length = bps_wr_max_length;
+    if (arg->has_bps_wr_max_length) {
+        cfg.buckets[THROTTLE_BPS_WRITE].burst_length = arg->bps_wr_max_length;
     }
-    if (has_iops_max_length) {
-        cfg.buckets[THROTTLE_OPS_TOTAL].burst_length = iops_max_length;
+    if (arg->has_iops_max_length) {
+        cfg.buckets[THROTTLE_OPS_TOTAL].burst_length = arg->iops_max_length;
     }
-    if (has_iops_rd_max_length) {
-        cfg.buckets[THROTTLE_OPS_READ].burst_length = iops_rd_max_length;
+    if (arg->has_iops_rd_max_length) {
+        cfg.buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length;
     }
-    if (has_iops_wr_max_length) {
-        cfg.buckets[THROTTLE_OPS_WRITE].burst_length = iops_wr_max_length;
+    if (arg->has_iops_wr_max_length) {
+        cfg.buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_wr_max_length;
     }
 
-    if (has_iops_size) {
-        cfg.op_size = iops_size;
+    if (arg->has_iops_size) {
+        cfg.op_size = arg->iops_size;
     }
 
     if (!throttle_is_valid(&cfg, errp)) {
@@ -2748,9 +2716,10 @@ void qmp_block_set_io_throttle(const char *device, 
int64_t bps, int64_t bps_rd,
         /* Enable I/O limits if they're not enabled yet, otherwise
          * just update the throttling group. */
         if (!blk_get_public(blk)->throttle_state) {
-            blk_io_limits_enable(blk, has_group ? group : device);
-        } else if (has_group) {
-            blk_io_limits_update_group(blk, group);
+            blk_io_limits_enable(blk,
+                                 arg->has_group ? arg->group : arg->device);
+        } else if (arg->has_group) {
+            blk_io_limits_update_group(blk, arg->group);
         }
         /* Set the new throttling configuration */
         blk_set_io_limits(blk, &cfg);
diff --git a/hmp.c b/hmp.c
index 3ca79c3..ea17d49 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1439,42 +1439,17 @@ void hmp_change(Monitor *mon, const QDict *qdict)
 void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
 {
     Error *err = NULL;
+    BlockIOThrottle throttle = {
+        .device = (char *) qdict_get_str(qdict, "device"),
+        .bps = qdict_get_int(qdict, "bps"),
+        .bps_rd = qdict_get_int(qdict, "bps_rd"),
+        .bps_wr = qdict_get_int(qdict, "bps_wr"),
+        .iops = qdict_get_int(qdict, "iops"),
+        .iops_rd = qdict_get_int(qdict, "iops_rd"),
+        .iops_wr = qdict_get_int(qdict, "iops_wr"),
+    };
 
-    qmp_block_set_io_throttle(qdict_get_str(qdict, "device"),
-                              qdict_get_int(qdict, "bps"),
-                              qdict_get_int(qdict, "bps_rd"),
-                              qdict_get_int(qdict, "bps_wr"),
-                              qdict_get_int(qdict, "iops"),
-                              qdict_get_int(qdict, "iops_rd"),
-                              qdict_get_int(qdict, "iops_wr"),
-                              false, /* no burst max via HMP */
-                              0,
-                              false,
-                              0,
-                              false,
-                              0,
-                              false,
-                              0,
-                              false,
-                              0,
-                              false,
-                              0,
-                              false, /* no burst length via HMP */
-                              0,
-                              false,
-                              0,
-                              false,
-                              0,
-                              false,
-                              0,
-                              false,
-                              0,
-                              false,
-                              0,
-                              false, /* No default I/O size */
-                              0,
-                              false,
-                              NULL, &err);
+    qmp_block_set_io_throttle(&throttle, &err);
     hmp_handle_error(mon, &err);
 }
 
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 3444a9b..3d4b071 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1330,6 +1330,21 @@
 # the device will be removed from its group and the rest of its
 # members will not be affected. The 'group' parameter is ignored.
 #
+# See BlockIOThrottle for parameter descriptions.
+#
+# Returns: Nothing on success
+#          If @device is not a valid block device, DeviceNotFound
+#
+# Since: 1.1
+##
+{ 'command': 'block_set_io_throttle', 'boxed': true,
+  'data': 'BlockIOThrottle' }
+
+##
+# BlockIOThrottle
+#
+# A set of parameters describing block throttling.
+#
 # @device: The name of the device
 #
 # @bps: total throughput limit in bytes per second
@@ -1396,12 +1411,9 @@
 #
 # @group: #optional throttle group name (Since 2.4)
 #
-# Returns: Nothing on success
-#          If @device is not a valid block device, DeviceNotFound
-#
 # Since: 1.1
 ##
-{ 'command': 'block_set_io_throttle',
+{ 'struct': 'BlockIOThrottle',
   'data': { 'device': 'str', 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
             'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
             '*bps_max': 'int', '*bps_rd_max': 'int',
-- 
2.5.5