All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v5 0/6] fsdev-throttle-qmp: qmp interface for fsdev io throttling
@ 2018-11-16  7:58 xiezhide
  2018-11-16  7:59 ` [Qemu-devel] [PATCH v5 1/6] fsdev-throttle-qmp: factor out throttle code to reuse code xiezhide
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: xiezhide @ 2018-11-16  7:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: groug, aneesh.kumar, eblake, armbru, berto, zengcanfu,
	jinxuefeng, chenhui.rtos

This patches provide qmp interface to query/set io throttle parameters of a fsdev.
Some of patches also refactor the code and structure that was present in block and fsdev files. 

xiezhide (6):
  fsdev-throttle-qmp: factor out throttle code to reuse code
  fsdev-throttle-qmp: Rename the ThrottleLimits member names
  fsdev-throttle-qmp: Rewrite BlockIOThrottle with ThrottleLimits as its
    base class
  fsdev-throttle-qmp: Move ThrottleLimits into a new file for future
    reuse
  fsdev-throttle-qmp: qmp interface for fsdev io throttling
  fsdev-throttle-qmp: hmp interface for fsdev io throttling

 Makefile                        |  18 ++++
 Makefile.objs                   |   8 ++
 block/throttle.c                |   6 +-
 blockdev.c                      |  45 +-------
 fsdev/qemu-fsdev-dummy.c        |  11 ++
 fsdev/qemu-fsdev-throttle.c     | 142 +++++++++++++++---------
 fsdev/qemu-fsdev-throttle.h     |   6 +-
 fsdev/qemu-fsdev.c              |  29 +++++
 hmp-commands-info.hx            |  15 +++
 hmp-commands.hx                 |  15 +++
 hmp.c                           |  81 ++++++++++++--
 hmp.h                           |   4 +
 include/qemu/throttle-options.h |   2 +
 include/qemu/throttle.h         |   4 +-
 include/qemu/typedefs.h         |   1 +
 qapi/block-core.json            | 122 +--------------------
 qapi/fsdev.json                 |  96 +++++++++++++++++
 qapi/qapi-schema.json           |   2 +
 qapi/tlimits.json               |  53 +++++++++
 qmp.c                           |  13 +++
 util/throttle.c                 | 231 ++++++++++++++++++++++++++--------------
 21 files changed, 598 insertions(+), 306 deletions(-)
 create mode 100644 qapi/fsdev.json
 create mode 100644 qapi/tlimits.json

 v0 -> v1:
  Addressed comments from Eric Blake and Greg Kurz.
  Fix patch corrupt issue due to email client change the patch format with copy-to-paster
  Break patch to patches

 v1 -> v2:
  Addressed comments from Greg Kurz.
  send patch with outlook,patch corrupted, resend with git send-email

 v2 -> v3:
  Addressed comments from Eric Blake.
  split patch1 into two patches

 v3 -> v4:
  Fix issue cause test failed on centos and fedora

 v4 -> v5:
  Addressed comments from Eric Blake.
  split patch3 into three patches  

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v5 1/6] fsdev-throttle-qmp: factor out throttle code to reuse code
  2018-11-16  7:58 [Qemu-devel] [PATCH v5 0/6] fsdev-throttle-qmp: qmp interface for fsdev io throttling xiezhide
@ 2018-11-16  7:59 ` xiezhide
  2018-11-22 14:46   ` Greg Kurz
  2018-11-16  7:59 ` [Qemu-devel] [PATCH v5 2/6] fsdev-throttle-qmp: Rename the ThrottleLimits member names xiezhide
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: xiezhide @ 2018-11-16  7:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: groug, aneesh.kumar, eblake, armbru, berto, zengcanfu,
	jinxuefeng, chenhui.rtos

Factor out throttle parameter parsing code to a new common
function which will be used by block and fsdev.
Rename function throttle_parse_options to throttle_parse_group
to resolve function name conflict

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: xiezhide <xiezhide@huawei.com>
---
 block/throttle.c                |  6 ++--
 blockdev.c                      | 43 +-------------------------
 fsdev/qemu-fsdev-throttle.c     | 44 ++------------------------
 include/qemu/throttle-options.h |  2 ++
 include/qemu/throttle.h         |  4 +--
 include/qemu/typedefs.h         |  1 +
 util/throttle.c                 | 68 +++++++++++++++++++++++++++++++++++++++++
 7 files changed, 79 insertions(+), 89 deletions(-)

diff --git a/block/throttle.c b/block/throttle.c
index 636c976..bd23c58 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -41,7 +41,7 @@ static QemuOptsList throttle_opts = {
  * @group and must be freed by the caller.
  * If there's an error then @group remains unmodified.
  */
-static int throttle_parse_options(QDict *options, char **group, Error **errp)
+static int throttle_parse_group(QDict *options, char **group, Error **errp)
 {
     int ret;
     const char *group_name;
@@ -90,7 +90,7 @@ static int throttle_open(BlockDriverState *bs, QDict *options,
     bs->supported_zero_flags = bs->file->bs->supported_zero_flags |
                                BDRV_REQ_WRITE_UNCHANGED;
 
-    ret = throttle_parse_options(options, &group, errp);
+    ret = throttle_parse_group(options, &group, errp);
     if (ret == 0) {
         /* Register membership to group with name group_name */
         throttle_group_register_tgm(tgm, group, bdrv_get_aio_context(bs));
@@ -179,7 +179,7 @@ static int throttle_reopen_prepare(BDRVReopenState *reopen_state,
     assert(reopen_state != NULL);
     assert(reopen_state->bs != NULL);
 
-    ret = throttle_parse_options(reopen_state->options, &group, errp);
+    ret = throttle_parse_group(reopen_state->options, &group, errp);
     reopen_state->opaque = group;
     return ret;
 }
diff --git a/blockdev.c b/blockdev.c
index 81f95d9..fce5d8f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -400,48 +400,7 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
     }
 
     if (throttle_cfg) {
-        throttle_config_init(throttle_cfg);
-        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
-            qemu_opt_get_number(opts, "throttling.bps-total", 0);
-        throttle_cfg->buckets[THROTTLE_BPS_READ].avg  =
-            qemu_opt_get_number(opts, "throttling.bps-read", 0);
-        throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
-            qemu_opt_get_number(opts, "throttling.bps-write", 0);
-        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
-            qemu_opt_get_number(opts, "throttling.iops-total", 0);
-        throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
-            qemu_opt_get_number(opts, "throttling.iops-read", 0);
-        throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
-            qemu_opt_get_number(opts, "throttling.iops-write", 0);
-
-        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
-            qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
-        throttle_cfg->buckets[THROTTLE_BPS_READ].max  =
-            qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
-        throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
-            qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
-        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
-            qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
-        throttle_cfg->buckets[THROTTLE_OPS_READ].max =
-            qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
-        throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
-            qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
-
-        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
-            qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
-        throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length  =
-            qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
-        throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
-            qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
-        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
-            qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
-        throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
-            qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
-        throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
-            qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
-
-        throttle_cfg->op_size =
-            qemu_opt_get_number(opts, "throttling.iops-size", 0);
+        throttle_parse_options(throttle_cfg, opts);
 
         if (!throttle_is_valid(throttle_cfg, errp)) {
             return;
diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
index cfd8641..6a4108a 100644
--- a/fsdev/qemu-fsdev-throttle.c
+++ b/fsdev/qemu-fsdev-throttle.c
@@ -17,6 +17,7 @@
 #include "qemu-fsdev-throttle.h"
 #include "qemu/iov.h"
 #include "qemu/option.h"
+#include "qemu/throttle-options.h"
 
 static void fsdev_throttle_read_timer_cb(void *opaque)
 {
@@ -32,48 +33,7 @@ static void fsdev_throttle_write_timer_cb(void *opaque)
 
 void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error **errp)
 {
-    throttle_config_init(&fst->cfg);
-    fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =
-        qemu_opt_get_number(opts, "throttling.bps-total", 0);
-    fst->cfg.buckets[THROTTLE_BPS_READ].avg  =
-        qemu_opt_get_number(opts, "throttling.bps-read", 0);
-    fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =
-        qemu_opt_get_number(opts, "throttling.bps-write", 0);
-    fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =
-        qemu_opt_get_number(opts, "throttling.iops-total", 0);
-    fst->cfg.buckets[THROTTLE_OPS_READ].avg =
-        qemu_opt_get_number(opts, "throttling.iops-read", 0);
-    fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =
-        qemu_opt_get_number(opts, "throttling.iops-write", 0);
-
-    fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =
-        qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
-    fst->cfg.buckets[THROTTLE_BPS_READ].max  =
-        qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
-    fst->cfg.buckets[THROTTLE_BPS_WRITE].max =
-        qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
-    fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =
-        qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
-    fst->cfg.buckets[THROTTLE_OPS_READ].max =
-        qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
-    fst->cfg.buckets[THROTTLE_OPS_WRITE].max =
-        qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
-
-    fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =
-        qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
-    fst->cfg.buckets[THROTTLE_BPS_READ].burst_length  =
-        qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
-    fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =
-        qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
-    fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =
-        qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
-    fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =
-        qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
-    fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =
-        qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
-    fst->cfg.op_size =
-        qemu_opt_get_number(opts, "throttling.iops-size", 0);
-
+    throttle_parse_options(&fst->cfg, opts);
     throttle_is_valid(&fst->cfg, errp);
 }
 
diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
index 3528a8f..944a08c 100644
--- a/include/qemu/throttle-options.h
+++ b/include/qemu/throttle-options.h
@@ -111,4 +111,6 @@
             .help = "when limiting by iops max size of an I/O in bytes",\
         }
 
+void throttle_parse_options(ThrottleConfig *, QemuOpts *);
+
 #endif
diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
index abeb886..f379d91 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -90,10 +90,10 @@ typedef struct LeakyBucket {
  * However it allows to keep the code clean and the bucket field is reset to
  * zero at the right time.
  */
-typedef struct ThrottleConfig {
+struct ThrottleConfig {
     LeakyBucket buckets[BUCKETS_COUNT]; /* leaky buckets */
     uint64_t op_size;         /* size of an operation in bytes */
-} ThrottleConfig;
+};
 
 typedef struct ThrottleState {
     ThrottleConfig cfg;       /* configuration */
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 3ec0e13..0d75edc 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -109,6 +109,7 @@ typedef struct SerialState SerialState;
 typedef struct SHPCDevice SHPCDevice;
 typedef struct SMBusDevice SMBusDevice;
 typedef struct SSIBus SSIBus;
+typedef struct ThrottleConfig ThrottleConfig;
 typedef struct uWireSlave uWireSlave;
 typedef struct VirtIODevice VirtIODevice;
 typedef struct Visitor Visitor;
diff --git a/util/throttle.c b/util/throttle.c
index b38e742..e7db2ad 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -27,6 +27,8 @@
 #include "qemu/throttle.h"
 #include "qemu/timer.h"
 #include "block/aio.h"
+#include "qemu/option.h"
+#include "qemu/throttle-options.h"
 
 /* This function make a bucket leak
  *
@@ -636,3 +638,69 @@ void throttle_config_to_limits(ThrottleConfig *cfg, ThrottleLimits *var)
     var->has_iops_write_max_length = true;
     var->has_iops_size = true;
 }
+
+/* parse the throttle options
+ *
+ * @opts: qemu options
+ * @throttle_cfg: throttle configuration
+ */
+void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts *opts)
+{
+    throttle_config_init(throttle_cfg);
+    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
+        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
+                            QEMU_OPT_BPS_TOTAL, 0);
+    throttle_cfg->buckets[THROTTLE_BPS_READ].avg  =
+        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
+                            QEMU_OPT_BPS_READ, 0);
+    throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
+        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
+                            QEMU_OPT_BPS_WRITE, 0);
+    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
+        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
+                            QEMU_OPT_IOPS_TOTAL, 0);
+    throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
+        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
+                            QEMU_OPT_IOPS_READ, 0);
+    throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
+        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
+                            QEMU_OPT_IOPS_WRITE, 0);
+    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
+        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
+                            QEMU_OPT_BPS_TOTAL_MAX, 0);
+    throttle_cfg->buckets[THROTTLE_BPS_READ].max  =
+        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
+                            QEMU_OPT_BPS_READ_MAX, 0);
+    throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
+        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
+                            QEMU_OPT_BPS_WRITE_MAX, 0);
+    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
+        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
+                            QEMU_OPT_IOPS_TOTAL_MAX, 0);
+    throttle_cfg->buckets[THROTTLE_OPS_READ].max =
+        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
+                            QEMU_OPT_IOPS_READ_MAX, 0);
+    throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
+        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
+                            QEMU_OPT_IOPS_WRITE_MAX, 0);
+    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
+        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
+                            QEMU_OPT_BPS_TOTAL_MAX_LENGTH, 1);
+    throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length  =
+        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
+                            QEMU_OPT_BPS_READ_MAX_LENGTH, 1);
+    throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
+        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
+                            QEMU_OPT_BPS_WRITE_MAX_LENGTH, 1);
+    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
+        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
+                            QEMU_OPT_IOPS_TOTAL_MAX_LENGTH, 1);
+    throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
+        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
+                            QEMU_OPT_IOPS_READ_MAX_LENGTH, 1);
+    throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
+        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
+                            QEMU_OPT_IOPS_WRITE_MAX_LENGTH, 1);
+    throttle_cfg->op_size =
+        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_SIZE, 0);
+}
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v5 2/6] fsdev-throttle-qmp: Rename the ThrottleLimits member names
  2018-11-16  7:58 [Qemu-devel] [PATCH v5 0/6] fsdev-throttle-qmp: qmp interface for fsdev io throttling xiezhide
  2018-11-16  7:59 ` [Qemu-devel] [PATCH v5 1/6] fsdev-throttle-qmp: factor out throttle code to reuse code xiezhide
@ 2018-11-16  7:59 ` xiezhide
  2018-11-28  9:25   ` Markus Armbruster
  2018-11-16  7:59 ` [Qemu-devel] [PATCH v5 3/6] fsdev-throttle-qmp: Rewrite BlockIOThrottle with ThrottleLimits as its base class xiezhide
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: xiezhide @ 2018-11-16  7:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: groug, aneesh.kumar, eblake, armbru, berto, zengcanfu,
	jinxuefeng, chenhui.rtos

Rename the ThrottleLimits member names and modify related code

Signed-off-by: xiezhide <xiezhide@huawei.com>
---
 qapi/block-core.json |  70 +++++++++++-----------
 util/throttle.c      | 163 +++++++++++++++++++++++++--------------------------
 2 files changed, 116 insertions(+), 117 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index d4fe710..4ffaaea 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2240,45 +2240,45 @@
 # transaction. All fields are optional. When setting limits, if a field is
 # missing the current value is not changed.
 #
-# @iops-total:             limit total I/O operations per second
-# @iops-total-max:         I/O operations burst
-# @iops-total-max-length:  length of the iops-total-max burst period, in seconds
-#                          It must only be set if @iops-total-max is set as well.
-# @iops-read:              limit read operations per second
-# @iops-read-max:          I/O operations read burst
-# @iops-read-max-length:   length of the iops-read-max burst period, in seconds
-#                          It must only be set if @iops-read-max is set as well.
-# @iops-write:             limit write operations per second
-# @iops-write-max:         I/O operations write burst
-# @iops-write-max-length:  length of the iops-write-max burst period, in seconds
-#                          It must only be set if @iops-write-max is set as well.
-# @bps-total:              limit total bytes per second
-# @bps-total-max:          total bytes burst
-# @bps-total-max-length:   length of the bps-total-max burst period, in seconds.
-#                          It must only be set if @bps-total-max is set as well.
-# @bps-read:               limit read bytes per second
-# @bps-read-max:           total bytes read burst
-# @bps-read-max-length:    length of the bps-read-max burst period, in seconds
-#                          It must only be set if @bps-read-max is set as well.
-# @bps-write:              limit write bytes per second
-# @bps-write-max:          total bytes write burst
-# @bps-write-max-length:   length of the bps-write-max burst period, in seconds
-#                          It must only be set if @bps-write-max is set as well.
-# @iops-size:              when limiting by iops max size of an I/O in bytes
+# @iops:             limit total I/O operations per second
+# @iops_max:         I/O operations burst
+# @iops_max_length:  length of the iops_total_max burst period, in seconds
+#                          It must only be set if @iops_total_max is set as well.
+# @iops_rd:              limit read operations per second
+# @iops_rd_max:          I/O operations read burst
+# @iops_rd_max_length:   length of the iops_read_max burst period, in seconds
+#                          It must only be set if @iops_read_max is set as well.
+# @iops_wr:             limit write operations per second
+# @iops_wr_max:         I/O operations write burst
+# @iops_wr_max_length:  length of the iops_write_max burst period, in seconds
+#                          It must only be set if @iops_write_max is set as well.
+# @bps:              limit total bytes per second
+# @bps_max:          total bytes burst
+# @bps_max_length:   length of the bps_total_max burst period, in seconds.
+#                          It must only be set if @bps_total_max is set as well.
+# @bps_rd:               limit read bytes per second
+# @bps_rd_max:           total bytes read burst
+# @bps_rd_max_length:    length of the bps_read_max burst period, in seconds
+#                          It must only be set if @bps_read_max is set as well.
+# @bps_wr:              limit write bytes per second
+# @bps_wr_max:          total bytes write burst
+# @bps_wr_max_length:   length of the bps_write_max burst period, in seconds
+#                          It must only be set if @bps_write_max is set as well.
+# @iops_size:              when limiting by iops max size of an I/O in bytes
 #
 # Since: 2.11
 ##
 { 'struct': 'ThrottleLimits',
-  'data': { '*iops-total' : 'int', '*iops-total-max' : 'int',
-            '*iops-total-max-length' : 'int', '*iops-read' : 'int',
-            '*iops-read-max' : 'int', '*iops-read-max-length' : 'int',
-            '*iops-write' : 'int', '*iops-write-max' : 'int',
-            '*iops-write-max-length' : 'int', '*bps-total' : 'int',
-            '*bps-total-max' : 'int', '*bps-total-max-length' : 'int',
-            '*bps-read' : 'int', '*bps-read-max' : 'int',
-            '*bps-read-max-length' : 'int', '*bps-write' : 'int',
-            '*bps-write-max' : 'int', '*bps-write-max-length' : 'int',
-            '*iops-size' : 'int' } }
+  'data': { '*iops' : 'int', '*iops_max' : 'int',
+            '*iops_max_length' : 'int', '*iops_rd' : 'int',
+            '*iops_rd_max' : 'int', '*iops_rd_max_length' : 'int',
+            '*iops_wr' : 'int', '*iops_wr_max' : 'int',
+            '*iops_wr_max_length' : 'int', '*bps' : 'int',
+            '*bps_max' : 'int', '*bps_max_length' : 'int',
+            '*bps_rd' : 'int', '*bps_rd_max' : 'int',
+            '*bps_rd_max_length' : 'int', '*bps_wr' : 'int',
+            '*bps_wr_max' : 'int', '*bps_wr_max_length' : 'int',
+            '*iops_size' : 'int' } }
 
 ##
 # @block-stream:
diff --git a/util/throttle.c b/util/throttle.c
index e7db2ad..b421e33 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -496,98 +496,97 @@ void throttle_account(ThrottleState *ts, bool is_write, uint64_t size)
 void throttle_limits_to_config(ThrottleLimits *arg, ThrottleConfig *cfg,
                                Error **errp)
 {
-    if (arg->has_bps_total) {
-        cfg->buckets[THROTTLE_BPS_TOTAL].avg = arg->bps_total;
+    if (arg->has_bps) {
+        cfg->buckets[THROTTLE_BPS_TOTAL].avg = arg->bps;
     }
-    if (arg->has_bps_read) {
-        cfg->buckets[THROTTLE_BPS_READ].avg  = arg->bps_read;
+    if (arg->has_bps_rd) {
+        cfg->buckets[THROTTLE_BPS_READ].avg  = arg->bps_rd;
     }
-    if (arg->has_bps_write) {
-        cfg->buckets[THROTTLE_BPS_WRITE].avg = arg->bps_write;
+    if (arg->has_bps_wr) {
+        cfg->buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr;
     }
 
-    if (arg->has_iops_total) {
-        cfg->buckets[THROTTLE_OPS_TOTAL].avg = arg->iops_total;
+    if (arg->has_iops) {
+        cfg->buckets[THROTTLE_OPS_TOTAL].avg = arg->iops;
     }
-    if (arg->has_iops_read) {
-        cfg->buckets[THROTTLE_OPS_READ].avg  = arg->iops_read;
+    if (arg->has_iops_rd) {
+        cfg->buckets[THROTTLE_OPS_READ].avg  = arg->iops_rd;
     }
-    if (arg->has_iops_write) {
-        cfg->buckets[THROTTLE_OPS_WRITE].avg = arg->iops_write;
+    if (arg->has_iops_wr) {
+        cfg->buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr;
     }
 
-    if (arg->has_bps_total_max) {
-        cfg->buckets[THROTTLE_BPS_TOTAL].max = arg->bps_total_max;
+    if (arg->has_bps_max) {
+        cfg->buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max;
     }
-    if (arg->has_bps_read_max) {
-        cfg->buckets[THROTTLE_BPS_READ].max = arg->bps_read_max;
+    if (arg->has_bps_rd_max) {
+        cfg->buckets[THROTTLE_BPS_READ].max = arg->bps_rd_max;
     }
-    if (arg->has_bps_write_max) {
-        cfg->buckets[THROTTLE_BPS_WRITE].max = arg->bps_write_max;
+    if (arg->has_bps_wr_max) {
+        cfg->buckets[THROTTLE_BPS_WRITE].max = arg->bps_wr_max;
     }
-    if (arg->has_iops_total_max) {
-        cfg->buckets[THROTTLE_OPS_TOTAL].max = arg->iops_total_max;
+    if (arg->has_iops_max) {
+        cfg->buckets[THROTTLE_OPS_TOTAL].max = arg->iops_max;
     }
-    if (arg->has_iops_read_max) {
-        cfg->buckets[THROTTLE_OPS_READ].max = arg->iops_read_max;
+    if (arg->has_iops_rd_max) {
+        cfg->buckets[THROTTLE_OPS_READ].max = arg->iops_rd_max;
     }
-    if (arg->has_iops_write_max) {
-        cfg->buckets[THROTTLE_OPS_WRITE].max = arg->iops_write_max;
+    if (arg->has_iops_wr_max) {
+        cfg->buckets[THROTTLE_OPS_WRITE].max = arg->iops_wr_max;
     }
 
-    if (arg->has_bps_total_max_length) {
-        if (arg->bps_total_max_length > UINT_MAX) {
+    if (arg->has_bps_max_length) {
+        if (arg->bps_max_length > UINT_MAX) {
             error_setg(errp, "bps-total-max-length value must be in"
                              " the range [0, %u]", UINT_MAX);
             return;
         }
-        cfg->buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_total_max_length;
+        cfg->buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_max_length;
     }
-    if (arg->has_bps_read_max_length) {
-        if (arg->bps_read_max_length > UINT_MAX) {
+    if (arg->has_bps_rd_max_length) {
+        if (arg->bps_rd_max_length > UINT_MAX) {
             error_setg(errp, "bps-read-max-length value must be in"
                              " the range [0, %u]", UINT_MAX);
             return;
         }
-        cfg->buckets[THROTTLE_BPS_READ].burst_length = arg->bps_read_max_length;
+        cfg->buckets[THROTTLE_BPS_READ].burst_length = arg->bps_rd_max_length;
     }
-    if (arg->has_bps_write_max_length) {
-        if (arg->bps_write_max_length > UINT_MAX) {
+    if (arg->has_bps_wr_max_length) {
+        if (arg->bps_wr_max_length > UINT_MAX) {
             error_setg(errp, "bps-write-max-length value must be in"
                              " the range [0, %u]", UINT_MAX);
             return;
         }
-        cfg->buckets[THROTTLE_BPS_WRITE].burst_length = arg->bps_write_max_length;
+        cfg->buckets[THROTTLE_BPS_WRITE].burst_length = arg->bps_wr_max_length;
     }
-    if (arg->has_iops_total_max_length) {
-        if (arg->iops_total_max_length > UINT_MAX) {
+    if (arg->has_iops_max_length) {
+        if (arg->iops_max_length > UINT_MAX) {
             error_setg(errp, "iops-total-max-length value must be in"
                              " the range [0, %u]", UINT_MAX);
             return;
         }
-        cfg->buckets[THROTTLE_OPS_TOTAL].burst_length = arg->iops_total_max_length;
+        cfg->buckets[THROTTLE_OPS_TOTAL].burst_length = arg->iops_max_length;
     }
-    if (arg->has_iops_read_max_length) {
-        if (arg->iops_read_max_length > UINT_MAX) {
+    if (arg->has_iops_rd_max_length) {
+        if (arg->iops_rd_max_length > UINT_MAX) {
             error_setg(errp, "iops-read-max-length value must be in"
                              " the range [0, %u]", UINT_MAX);
             return;
         }
-        cfg->buckets[THROTTLE_OPS_READ].burst_length = arg->iops_read_max_length;
+        cfg->buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length;
     }
-    if (arg->has_iops_write_max_length) {
-        if (arg->iops_write_max_length > UINT_MAX) {
+    if (arg->has_iops_wr_max_length) {
+        if (arg->iops_wr_max_length > UINT_MAX) {
             error_setg(errp, "iops-write-max-length value must be in"
                              " the range [0, %u]", UINT_MAX);
             return;
         }
-        cfg->buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_write_max_length;
+        cfg->buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_wr_max_length;
     }
 
     if (arg->has_iops_size) {
         cfg->op_size = arg->iops_size;
     }
-
     throttle_is_valid(cfg, errp);
 }
 
@@ -598,45 +597,45 @@ void throttle_limits_to_config(ThrottleLimits *arg, ThrottleConfig *cfg,
  */
 void throttle_config_to_limits(ThrottleConfig *cfg, ThrottleLimits *var)
 {
-    var->bps_total               = cfg->buckets[THROTTLE_BPS_TOTAL].avg;
-    var->bps_read                = cfg->buckets[THROTTLE_BPS_READ].avg;
-    var->bps_write               = cfg->buckets[THROTTLE_BPS_WRITE].avg;
-    var->iops_total              = cfg->buckets[THROTTLE_OPS_TOTAL].avg;
-    var->iops_read               = cfg->buckets[THROTTLE_OPS_READ].avg;
-    var->iops_write              = cfg->buckets[THROTTLE_OPS_WRITE].avg;
-    var->bps_total_max           = cfg->buckets[THROTTLE_BPS_TOTAL].max;
-    var->bps_read_max            = cfg->buckets[THROTTLE_BPS_READ].max;
-    var->bps_write_max           = cfg->buckets[THROTTLE_BPS_WRITE].max;
-    var->iops_total_max          = cfg->buckets[THROTTLE_OPS_TOTAL].max;
-    var->iops_read_max           = cfg->buckets[THROTTLE_OPS_READ].max;
-    var->iops_write_max          = cfg->buckets[THROTTLE_OPS_WRITE].max;
-    var->bps_total_max_length    = cfg->buckets[THROTTLE_BPS_TOTAL].burst_length;
-    var->bps_read_max_length     = cfg->buckets[THROTTLE_BPS_READ].burst_length;
-    var->bps_write_max_length    = cfg->buckets[THROTTLE_BPS_WRITE].burst_length;
-    var->iops_total_max_length   = cfg->buckets[THROTTLE_OPS_TOTAL].burst_length;
-    var->iops_read_max_length    = cfg->buckets[THROTTLE_OPS_READ].burst_length;
-    var->iops_write_max_length   = cfg->buckets[THROTTLE_OPS_WRITE].burst_length;
-    var->iops_size               = cfg->op_size;
-
-    var->has_bps_total = true;
-    var->has_bps_read = true;
-    var->has_bps_write = true;
-    var->has_iops_total = true;
-    var->has_iops_read = true;
-    var->has_iops_write = true;
-    var->has_bps_total_max = true;
-    var->has_bps_read_max = true;
-    var->has_bps_write_max = true;
-    var->has_iops_total_max = true;
-    var->has_iops_read_max = true;
-    var->has_iops_write_max = true;
-    var->has_bps_read_max_length = true;
-    var->has_bps_total_max_length = true;
-    var->has_bps_write_max_length = true;
-    var->has_iops_total_max_length = true;
-    var->has_iops_read_max_length = true;
-    var->has_iops_write_max_length = true;
-    var->has_iops_size = true;
+    var->bps                    = cfg->buckets[THROTTLE_BPS_TOTAL].avg;
+    var->bps_rd                 = cfg->buckets[THROTTLE_BPS_READ].avg;
+    var->bps_wr                 = cfg->buckets[THROTTLE_BPS_WRITE].avg;
+    var->iops                   = cfg->buckets[THROTTLE_OPS_TOTAL].avg;
+    var->iops_rd                = cfg->buckets[THROTTLE_OPS_READ].avg;
+    var->iops_wr                = cfg->buckets[THROTTLE_OPS_WRITE].avg;
+    var->bps_max                = cfg->buckets[THROTTLE_BPS_TOTAL].max;
+    var->bps_rd_max             = cfg->buckets[THROTTLE_BPS_READ].max;
+    var->bps_wr_max             = cfg->buckets[THROTTLE_BPS_WRITE].max;
+    var->iops_max               = cfg->buckets[THROTTLE_OPS_TOTAL].max;
+    var->iops_rd_max            = cfg->buckets[THROTTLE_OPS_READ].max;
+    var->iops_wr_max            = cfg->buckets[THROTTLE_OPS_WRITE].max;
+    var->bps_max_length         = cfg->buckets[THROTTLE_BPS_TOTAL].burst_length;
+    var->bps_rd_max_length      = cfg->buckets[THROTTLE_BPS_READ].burst_length;
+    var->bps_wr_max_length      = cfg->buckets[THROTTLE_BPS_WRITE].burst_length;
+    var->iops_max_length        = cfg->buckets[THROTTLE_OPS_TOTAL].burst_length;
+    var->iops_rd_max_length     = cfg->buckets[THROTTLE_OPS_READ].burst_length;
+    var->iops_wr_max_length     = cfg->buckets[THROTTLE_OPS_WRITE].burst_length;
+    var->iops_size              = cfg->op_size;
+
+    var->has_bps                = true;
+    var->has_bps_rd             = true;
+    var->has_bps_wr             = true;
+    var->has_iops               = true;
+    var->has_iops_rd            = true;
+    var->has_iops_wr            = true;
+    var->has_bps_max            = true;
+    var->has_bps_rd_max         = true;
+    var->has_bps_wr_max         = true;
+    var->has_iops_max           = true;
+    var->has_iops_rd_max        = true;
+    var->has_iops_wr_max        = true;
+    var->has_bps_rd_max_length  = true;
+    var->has_bps_max_length     = true;
+    var->has_bps_wr_max_length  = true;
+    var->has_iops_max_length    = true;
+    var->has_iops_rd_max_length = true;
+    var->has_iops_wr_max_length = true;
+    var->has_iops_size          = true;
 }
 
 /* parse the throttle options
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v5 3/6] fsdev-throttle-qmp: Rewrite BlockIOThrottle with ThrottleLimits as its base class
  2018-11-16  7:58 [Qemu-devel] [PATCH v5 0/6] fsdev-throttle-qmp: qmp interface for fsdev io throttling xiezhide
  2018-11-16  7:59 ` [Qemu-devel] [PATCH v5 1/6] fsdev-throttle-qmp: factor out throttle code to reuse code xiezhide
  2018-11-16  7:59 ` [Qemu-devel] [PATCH v5 2/6] fsdev-throttle-qmp: Rename the ThrottleLimits member names xiezhide
@ 2018-11-16  7:59 ` xiezhide
  2018-11-16  8:00 ` [Qemu-devel] [PATCH v5 4/6] fsdev-throttle-qmp: Move ThrottleLimits into a new file for future reuse xiezhide
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: xiezhide @ 2018-11-16  7:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: groug, aneesh.kumar, eblake, armbru, berto, zengcanfu,
	jinxuefeng, chenhui.rtos

Rewrite BlockIOThrottle with ThrottleLimits as its base class and modify related code

Signed-off-by: xiezhide <xiezhide@huawei.com>
---
 blockdev.c           |  2 ++
 qapi/block-core.json | 73 ++--------------------------------------------------
 2 files changed, 4 insertions(+), 71 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index fce5d8f..89921ec 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2752,6 +2752,8 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp)
         cfg.op_size = arg->iops_size;
     }
 
+    throttle_limits_to_config(qapi_BlockIOThrottle_base(arg), &cfg, errp);
+
     if (!throttle_is_valid(&cfg, errp)) {
         goto out;
     }
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 4ffaaea..3abd6af 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2155,82 +2155,13 @@
 #
 # @id: The name or QOM path of the guest device (since: 2.8)
 #
-# @bps: total throughput limit in bytes per second
-#
-# @bps_rd: read throughput limit in bytes per second
-#
-# @bps_wr: write throughput limit in bytes per second
-#
-# @iops: total I/O operations per second
-#
-# @iops_rd: read I/O operations per second
-#
-# @iops_wr: write I/O operations per second
-#
-# @bps_max: total throughput limit during bursts,
-#                     in bytes (Since 1.7)
-#
-# @bps_rd_max: read throughput limit during bursts,
-#                        in bytes (Since 1.7)
-#
-# @bps_wr_max: write throughput limit during bursts,
-#                        in bytes (Since 1.7)
-#
-# @iops_max: total I/O operations per second during bursts,
-#                      in bytes (Since 1.7)
-#
-# @iops_rd_max: read I/O operations per second during bursts,
-#                         in bytes (Since 1.7)
-#
-# @iops_wr_max: write I/O operations per second during bursts,
-#                         in bytes (Since 1.7)
-#
-# @bps_max_length: maximum length of the @bps_max burst
-#                            period, in seconds. It must only
-#                            be set if @bps_max is set as well.
-#                            Defaults to 1. (Since 2.6)
-#
-# @bps_rd_max_length: maximum length of the @bps_rd_max
-#                               burst period, in seconds. It must only
-#                               be set if @bps_rd_max is set as well.
-#                               Defaults to 1. (Since 2.6)
-#
-# @bps_wr_max_length: maximum length of the @bps_wr_max
-#                               burst period, in seconds. It must only
-#                               be set if @bps_wr_max is set as well.
-#                               Defaults to 1. (Since 2.6)
-#
-# @iops_max_length: maximum length of the @iops burst
-#                             period, in seconds. It must only
-#                             be set if @iops_max is set as well.
-#                             Defaults to 1. (Since 2.6)
-#
-# @iops_rd_max_length: maximum length of the @iops_rd_max
-#                                burst period, in seconds. It must only
-#                                be set if @iops_rd_max is set as well.
-#                                Defaults to 1. (Since 2.6)
-#
-# @iops_wr_max_length: maximum length of the @iops_wr_max
-#                                burst period, in seconds. It must only
-#                                be set if @iops_wr_max is set as well.
-#                                Defaults to 1. (Since 2.6)
-#
-# @iops_size: an I/O size in bytes (Since 1.7)
-#
 # @group: throttle group name (Since 2.4)
 #
 # Since: 1.1
 ##
 { 'struct': 'BlockIOThrottle',
-  'data': { '*device': 'str', '*id': 'str', 'bps': 'int', 'bps_rd': 'int',
-            'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
-            '*bps_max': 'int', '*bps_rd_max': 'int',
-            '*bps_wr_max': 'int', '*iops_max': 'int',
-            '*iops_rd_max': 'int', '*iops_wr_max': 'int',
-            '*bps_max_length': 'int', '*bps_rd_max_length': 'int',
-            '*bps_wr_max_length': 'int', '*iops_max_length': 'int',
-            '*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int',
-            '*iops_size': 'int', '*group': 'str' } }
+  'base': 'ThrottleLimits',
+  'data': { '*device': 'str', '*id': 'str', '*group': 'str' } }
 
 ##
 # @ThrottleLimits:
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v5 4/6] fsdev-throttle-qmp: Move ThrottleLimits into a new file for future reuse
  2018-11-16  7:58 [Qemu-devel] [PATCH v5 0/6] fsdev-throttle-qmp: qmp interface for fsdev io throttling xiezhide
                   ` (2 preceding siblings ...)
  2018-11-16  7:59 ` [Qemu-devel] [PATCH v5 3/6] fsdev-throttle-qmp: Rewrite BlockIOThrottle with ThrottleLimits as its base class xiezhide
@ 2018-11-16  8:00 ` xiezhide
  2018-11-16  8:00 ` [Qemu-devel] [PATCH v5 5/6] fsdev-throttle-qmp: qmp interface for fsdev io throttling xiezhide
  2018-11-16  8:00 ` [Qemu-devel] [PATCH v5 6/6] fsdev-throttle-qmp: hmp " xiezhide
  5 siblings, 0 replies; 15+ messages in thread
From: xiezhide @ 2018-11-16  8:00 UTC (permalink / raw)
  To: qemu-devel
  Cc: groug, aneesh.kumar, eblake, armbru, berto, zengcanfu,
	jinxuefeng, chenhui.rtos

Move ThrottleLimits into a new file for future reuse.

Signed-off-by: xiezhide <xiezhide@huawei.com>
---
 Makefile              |  9 +++++++++
 Makefile.objs         |  4 ++++
 qapi/block-core.json  | 49 +----------------------------------------------
 qapi/qapi-schema.json |  1 +
 qapi/tlimits.json     | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 68 insertions(+), 48 deletions(-)
 create mode 100644 qapi/tlimits.json

diff --git a/Makefile b/Makefile
index f294718..b546e98 100644
--- a/Makefile
+++ b/Makefile
@@ -106,6 +106,7 @@ GENERATED_FILES += qapi/qapi-types-sockets.h qapi/qapi-types-sockets.c
 GENERATED_FILES += qapi/qapi-types-tpm.h qapi/qapi-types-tpm.c
 GENERATED_FILES += qapi/qapi-types-trace.h qapi/qapi-types-trace.c
 GENERATED_FILES += qapi/qapi-types-transaction.h qapi/qapi-types-transaction.c
+GENERATED_FILES += qapi/qapi-types-tlimits.h qapi/qapi-types-tlimits.c
 GENERATED_FILES += qapi/qapi-types-ui.h qapi/qapi-types-ui.c
 GENERATED_FILES += qapi/qapi-builtin-visit.h qapi/qapi-builtin-visit.c
 GENERATED_FILES += qapi/qapi-visit.h qapi/qapi-visit.c
@@ -125,6 +126,7 @@ GENERATED_FILES += qapi/qapi-visit-sockets.h qapi/qapi-visit-sockets.c
 GENERATED_FILES += qapi/qapi-visit-tpm.h qapi/qapi-visit-tpm.c
 GENERATED_FILES += qapi/qapi-visit-trace.h qapi/qapi-visit-trace.c
 GENERATED_FILES += qapi/qapi-visit-transaction.h qapi/qapi-visit-transaction.c
+GENERATED_FILES += qapi/qapi-visit-tlimits.h qapi/qapi-visit-tlimits.c
 GENERATED_FILES += qapi/qapi-visit-ui.h qapi/qapi-visit-ui.c
 GENERATED_FILES += qapi/qapi-commands.h qapi/qapi-commands.c
 GENERATED_FILES += qapi/qapi-commands-block-core.h qapi/qapi-commands-block-core.c
@@ -143,6 +145,7 @@ GENERATED_FILES += qapi/qapi-commands-sockets.h qapi/qapi-commands-sockets.c
 GENERATED_FILES += qapi/qapi-commands-tpm.h qapi/qapi-commands-tpm.c
 GENERATED_FILES += qapi/qapi-commands-trace.h qapi/qapi-commands-trace.c
 GENERATED_FILES += qapi/qapi-commands-transaction.h qapi/qapi-commands-transaction.c
+GENERATED_FILES += qapi/qapi-commands-tlimits.h qapi/qapi-commands-tlimits.c
 GENERATED_FILES += qapi/qapi-commands-ui.h qapi/qapi-commands-ui.c
 GENERATED_FILES += qapi/qapi-events.h qapi/qapi-events.c
 GENERATED_FILES += qapi/qapi-events-block-core.h qapi/qapi-events-block-core.c
@@ -161,6 +164,7 @@ GENERATED_FILES += qapi/qapi-events-sockets.h qapi/qapi-events-sockets.c
 GENERATED_FILES += qapi/qapi-events-tpm.h qapi/qapi-events-tpm.c
 GENERATED_FILES += qapi/qapi-events-trace.h qapi/qapi-events-trace.c
 GENERATED_FILES += qapi/qapi-events-transaction.h qapi/qapi-events-transaction.c
+GENERATED_FILES += qapi/qapi-events-tlimits.h qapi/qapi-events-tlimits.c
 GENERATED_FILES += qapi/qapi-events-ui.h qapi/qapi-events-ui.c
 GENERATED_FILES += qapi/qapi-introspect.c qapi/qapi-introspect.h
 GENERATED_FILES += qapi/qapi-doc.texi
@@ -596,6 +600,7 @@ qapi-modules = $(SRC_PATH)/qapi/qapi-schema.json $(SRC_PATH)/qapi/common.json \
                $(SRC_PATH)/qapi/run-state.json \
                $(SRC_PATH)/qapi/sockets.json \
                $(SRC_PATH)/qapi/tpm.json \
+               $(SRC_PATH)/qapi/tlimits.json \
                $(SRC_PATH)/qapi/trace.json \
                $(SRC_PATH)/qapi/transaction.json \
                $(SRC_PATH)/qapi/ui.json
@@ -618,6 +623,7 @@ qapi/qapi-types-sockets.c qapi/qapi-types-sockets.h \
 qapi/qapi-types-tpm.c qapi/qapi-types-tpm.h \
 qapi/qapi-types-trace.c qapi/qapi-types-trace.h \
 qapi/qapi-types-transaction.c qapi/qapi-types-transaction.h \
+qapi/qapi-types-tlimits.c qapi/qapi-types-tlimits.h \
 qapi/qapi-types-ui.c qapi/qapi-types-ui.h \
 qapi/qapi-builtin-visit.c qapi/qapi-builtin-visit.h \
 qapi/qapi-visit.c qapi/qapi-visit.h \
@@ -637,6 +643,7 @@ qapi/qapi-visit-sockets.c qapi/qapi-visit-sockets.h \
 qapi/qapi-visit-tpm.c qapi/qapi-visit-tpm.h \
 qapi/qapi-visit-trace.c qapi/qapi-visit-trace.h \
 qapi/qapi-visit-transaction.c qapi/qapi-visit-transaction.h \
+qapi/qapi-visit-tlimits.c qapi/qapi-visit-tlimits.h \
 qapi/qapi-visit-ui.c qapi/qapi-visit-ui.h \
 qapi/qapi-commands.h qapi/qapi-commands.c \
 qapi/qapi-commands-block-core.c qapi/qapi-commands-block-core.h \
@@ -655,6 +662,7 @@ qapi/qapi-commands-sockets.c qapi/qapi-commands-sockets.h \
 qapi/qapi-commands-tpm.c qapi/qapi-commands-tpm.h \
 qapi/qapi-commands-trace.c qapi/qapi-commands-trace.h \
 qapi/qapi-commands-transaction.c qapi/qapi-commands-transaction.h \
+qapi/qapi-commands-tlimits.c qapi/qapi-commands-tlimits.h \
 qapi/qapi-commands-ui.c qapi/qapi-commands-ui.h \
 qapi/qapi-events.c qapi/qapi-events.h \
 qapi/qapi-events-block-core.c qapi/qapi-events-block-core.h \
@@ -673,6 +681,7 @@ qapi/qapi-events-sockets.c qapi/qapi-events-sockets.h \
 qapi/qapi-events-tpm.c qapi/qapi-events-tpm.h \
 qapi/qapi-events-trace.c qapi/qapi-events-trace.h \
 qapi/qapi-events-transaction.c qapi/qapi-events-transaction.h \
+qapi/qapi-events-tlimits.c qapi/qapi-events-tlimits.h \
 qapi/qapi-events-ui.c qapi/qapi-events-ui.h \
 qapi/qapi-introspect.h qapi/qapi-introspect.c \
 qapi/qapi-doc.texi: \
diff --git a/Makefile.objs b/Makefile.objs
index 1e1ff38..682e6ba 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -20,6 +20,7 @@ util-obj-y += qapi/qapi-types-sockets.o
 util-obj-y += qapi/qapi-types-tpm.o
 util-obj-y += qapi/qapi-types-trace.o
 util-obj-y += qapi/qapi-types-transaction.o
+util-obj-y += qapi/qapi-types-tlimits.o
 util-obj-y += qapi/qapi-types-ui.o
 util-obj-y += qapi/qapi-builtin-visit.o
 util-obj-y += qapi/qapi-visit.o
@@ -39,6 +40,7 @@ util-obj-y += qapi/qapi-visit-sockets.o
 util-obj-y += qapi/qapi-visit-tpm.o
 util-obj-y += qapi/qapi-visit-trace.o
 util-obj-y += qapi/qapi-visit-transaction.o
+util-obj-y += qapi/qapi-visit-tlimits.o
 util-obj-y += qapi/qapi-visit-ui.o
 util-obj-y += qapi/qapi-events.o
 util-obj-y += qapi/qapi-events-block-core.o
@@ -57,6 +59,7 @@ util-obj-y += qapi/qapi-events-sockets.o
 util-obj-y += qapi/qapi-events-tpm.o
 util-obj-y += qapi/qapi-events-trace.o
 util-obj-y += qapi/qapi-events-transaction.o
+util-obj-y += qapi/qapi-events-tlimits.o
 util-obj-y += qapi/qapi-events-ui.o
 util-obj-y += qapi/qapi-introspect.o
 
@@ -154,6 +157,7 @@ common-obj-y += qapi/qapi-commands-sockets.o
 common-obj-y += qapi/qapi-commands-tpm.o
 common-obj-y += qapi/qapi-commands-trace.o
 common-obj-y += qapi/qapi-commands-transaction.o
+common-obj-y += qapi/qapi-commands-tlimits.o
 common-obj-y += qapi/qapi-commands-ui.o
 common-obj-y += qapi/qapi-introspect.o
 common-obj-y += qmp.o hmp.o
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 3abd6af..05296b0 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -8,6 +8,7 @@
 { 'include': 'crypto.json' }
 { 'include': 'job.json' }
 { 'include': 'sockets.json' }
+{ 'include': 'tlimits.json' }
 
 ##
 # @SnapshotInfo:
@@ -2164,54 +2165,6 @@
   'data': { '*device': 'str', '*id': 'str', '*group': 'str' } }
 
 ##
-# @ThrottleLimits:
-#
-# Limit parameters for throttling.
-# Since some limit combinations are illegal, limits should always be set in one
-# transaction. All fields are optional. When setting limits, if a field is
-# missing the current value is not changed.
-#
-# @iops:             limit total I/O operations per second
-# @iops_max:         I/O operations burst
-# @iops_max_length:  length of the iops_total_max burst period, in seconds
-#                          It must only be set if @iops_total_max is set as well.
-# @iops_rd:              limit read operations per second
-# @iops_rd_max:          I/O operations read burst
-# @iops_rd_max_length:   length of the iops_read_max burst period, in seconds
-#                          It must only be set if @iops_read_max is set as well.
-# @iops_wr:             limit write operations per second
-# @iops_wr_max:         I/O operations write burst
-# @iops_wr_max_length:  length of the iops_write_max burst period, in seconds
-#                          It must only be set if @iops_write_max is set as well.
-# @bps:              limit total bytes per second
-# @bps_max:          total bytes burst
-# @bps_max_length:   length of the bps_total_max burst period, in seconds.
-#                          It must only be set if @bps_total_max is set as well.
-# @bps_rd:               limit read bytes per second
-# @bps_rd_max:           total bytes read burst
-# @bps_rd_max_length:    length of the bps_read_max burst period, in seconds
-#                          It must only be set if @bps_read_max is set as well.
-# @bps_wr:              limit write bytes per second
-# @bps_wr_max:          total bytes write burst
-# @bps_wr_max_length:   length of the bps_write_max burst period, in seconds
-#                          It must only be set if @bps_write_max is set as well.
-# @iops_size:              when limiting by iops max size of an I/O in bytes
-#
-# Since: 2.11
-##
-{ 'struct': 'ThrottleLimits',
-  'data': { '*iops' : 'int', '*iops_max' : 'int',
-            '*iops_max_length' : 'int', '*iops_rd' : 'int',
-            '*iops_rd_max' : 'int', '*iops_rd_max_length' : 'int',
-            '*iops_wr' : 'int', '*iops_wr_max' : 'int',
-            '*iops_wr_max_length' : 'int', '*bps' : 'int',
-            '*bps_max' : 'int', '*bps_max_length' : 'int',
-            '*bps_rd' : 'int', '*bps_rd_max' : 'int',
-            '*bps_rd_max_length' : 'int', '*bps_wr' : 'int',
-            '*bps_wr_max' : 'int', '*bps_wr_max_length' : 'int',
-            '*iops_size' : 'int' } }
-
-##
 # @block-stream:
 #
 # Copy data from a backing file into a block device.
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index 65b6dc2..e9f594e 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -90,6 +90,7 @@
 { 'include': 'tpm.json' }
 { 'include': 'ui.json' }
 { 'include': 'migration.json' }
+{ 'include': 'tlimits.json' }
 { 'include': 'transaction.json' }
 { 'include': 'trace.json' }
 { 'include': 'introspect.json' }
diff --git a/qapi/tlimits.json b/qapi/tlimits.json
new file mode 100644
index 0000000..ad867de
--- /dev/null
+++ b/qapi/tlimits.json
@@ -0,0 +1,53 @@
+# -*- Mode: Python -*-
+
+##
+# == Throttle limits
+##
+
+##
+# @ThrottleLimits:
+#
+# Limit parameters for throttling.
+# Since some limit combinations are illegal, limits should always be set in one
+# transaction. All fields are optional. When setting limits, if a field is
+# missing the current value is not changed.
+#
+# @iops:             limit total I/O operations per second
+# @iops_max:         I/O operations burst
+# @iops_max_length:  length of the iops_total_max burst period, in seconds
+#                          It must only be set if @iops_total_max is set as well.
+# @iops_rd:              limit read operations per second
+# @iops_rd_max:          I/O operations read burst
+# @iops_rd_max_length:   length of the iops_read_max burst period, in seconds
+#                          It must only be set if @iops_read_max is set as well.
+# @iops_wr:             limit write operations per second
+# @iops_wr_max:         I/O operations write burst
+# @iops_wr_max_length:  length of the iops_write_max burst period, in seconds
+#                          It must only be set if @iops_write_max is set as well.
+# @bps:              limit total bytes per second
+# @bps_max:          total bytes burst
+# @bps_max_length:   length of the bps_total_max burst period, in seconds.
+#                          It must only be set if @bps_total_max is set as well.
+# @bps_rd:               limit read bytes per second
+# @bps_rd_max:           total bytes read burst
+# @bps_rd_max_length:    length of the bps_read_max burst period, in seconds
+#                          It must only be set if @bps_read_max is set as well.
+# @bps_wr:              limit write bytes per second
+# @bps_wr_max:          total bytes write burst
+# @bps_wr_max_length:    length of the bps_read_max burst period, in seconds
+#                          It must only be set if @bps_read_max is set as well.
+# @iops_size:              when limiting by iops max size of an I/O in bytes
+#
+# Since: 2.11
+##
+{ 'struct': 'ThrottleLimits',
+  'data': { '*iops' : 'int', '*iops_max' : 'int',
+            '*iops_max_length' : 'int', '*iops_rd' : 'int',
+            '*iops_rd_max' : 'int', '*iops_rd_max_length' : 'int',
+            '*iops_wr' : 'int', '*iops_wr_max' : 'int',
+            '*iops_wr_max_length' : 'int', '*bps' : 'int',
+            '*bps_max' : 'int', '*bps_max_length' : 'int',
+            '*bps_rd' : 'int', '*bps_rd_max' : 'int',
+            '*bps_rd_max_length' : 'int', '*bps_wr' : 'int',
+            '*bps_wr_max' : 'int', '*bps_wr_max_length' : 'int',
+            '*iops_size' : 'int' } }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v5 5/6] fsdev-throttle-qmp: qmp interface for fsdev io throttling
  2018-11-16  7:58 [Qemu-devel] [PATCH v5 0/6] fsdev-throttle-qmp: qmp interface for fsdev io throttling xiezhide
                   ` (3 preceding siblings ...)
  2018-11-16  8:00 ` [Qemu-devel] [PATCH v5 4/6] fsdev-throttle-qmp: Move ThrottleLimits into a new file for future reuse xiezhide
@ 2018-11-16  8:00 ` xiezhide
  2018-11-16  8:00 ` [Qemu-devel] [PATCH v5 6/6] fsdev-throttle-qmp: hmp " xiezhide
  5 siblings, 0 replies; 15+ messages in thread
From: xiezhide @ 2018-11-16  8:00 UTC (permalink / raw)
  To: qemu-devel
  Cc: groug, aneesh.kumar, eblake, armbru, berto, zengcanfu,
	jinxuefeng, chenhui.rtos

provides two interfaces:
1. set the IO limits for the required fsdev device
2. query info of all the fsdev devices.

Signed-off-by: xiezhide <xiezhide@huawei.com>
---
 Makefile                    | 27 ++++++++-----
 Makefile.objs               | 12 ++++--
 fsdev/qemu-fsdev-dummy.c    | 11 +++++
 fsdev/qemu-fsdev-throttle.c | 98 ++++++++++++++++++++++++++++++++++++++++++---
 fsdev/qemu-fsdev-throttle.h |  6 ++-
 fsdev/qemu-fsdev.c          | 29 ++++++++++++++
 qapi/fsdev.json             | 96 ++++++++++++++++++++++++++++++++++++++++++++
 qapi/qapi-schema.json       |  1 +
 qmp.c                       | 13 ++++++
 9 files changed, 272 insertions(+), 21 deletions(-)
 create mode 100644 qapi/fsdev.json

diff --git a/Makefile b/Makefile
index b546e98..e2c0e92 100644
--- a/Makefile
+++ b/Makefile
@@ -95,6 +95,7 @@ GENERATED_FILES += qapi/qapi-types-block.h qapi/qapi-types-block.c
 GENERATED_FILES += qapi/qapi-types-char.h qapi/qapi-types-char.c
 GENERATED_FILES += qapi/qapi-types-common.h qapi/qapi-types-common.c
 GENERATED_FILES += qapi/qapi-types-crypto.h qapi/qapi-types-crypto.c
+GENERATED_FILES += qapi/qapi-types-fsdev.h qapi/qapi-types-fsdev.c
 GENERATED_FILES += qapi/qapi-types-introspect.h qapi/qapi-types-introspect.c
 GENERATED_FILES += qapi/qapi-types-job.h qapi/qapi-types-job.c
 GENERATED_FILES += qapi/qapi-types-migration.h qapi/qapi-types-migration.c
@@ -103,10 +104,10 @@ GENERATED_FILES += qapi/qapi-types-net.h qapi/qapi-types-net.c
 GENERATED_FILES += qapi/qapi-types-rocker.h qapi/qapi-types-rocker.c
 GENERATED_FILES += qapi/qapi-types-run-state.h qapi/qapi-types-run-state.c
 GENERATED_FILES += qapi/qapi-types-sockets.h qapi/qapi-types-sockets.c
+GENERATED_FILES += qapi/qapi-types-tlimits.h qapi/qapi-types-tlimits.c
 GENERATED_FILES += qapi/qapi-types-tpm.h qapi/qapi-types-tpm.c
 GENERATED_FILES += qapi/qapi-types-trace.h qapi/qapi-types-trace.c
 GENERATED_FILES += qapi/qapi-types-transaction.h qapi/qapi-types-transaction.c
-GENERATED_FILES += qapi/qapi-types-tlimits.h qapi/qapi-types-tlimits.c
 GENERATED_FILES += qapi/qapi-types-ui.h qapi/qapi-types-ui.c
 GENERATED_FILES += qapi/qapi-builtin-visit.h qapi/qapi-builtin-visit.c
 GENERATED_FILES += qapi/qapi-visit.h qapi/qapi-visit.c
@@ -115,6 +116,7 @@ GENERATED_FILES += qapi/qapi-visit-block.h qapi/qapi-visit-block.c
 GENERATED_FILES += qapi/qapi-visit-char.h qapi/qapi-visit-char.c
 GENERATED_FILES += qapi/qapi-visit-common.h qapi/qapi-visit-common.c
 GENERATED_FILES += qapi/qapi-visit-crypto.h qapi/qapi-visit-crypto.c
+GENERATED_FILES += qapi/qapi-visit-fsdev.h qapi/qapi-visit-fsdev.c
 GENERATED_FILES += qapi/qapi-visit-introspect.h qapi/qapi-visit-introspect.c
 GENERATED_FILES += qapi/qapi-visit-job.h qapi/qapi-visit-job.c
 GENERATED_FILES += qapi/qapi-visit-migration.h qapi/qapi-visit-migration.c
@@ -123,10 +125,10 @@ GENERATED_FILES += qapi/qapi-visit-net.h qapi/qapi-visit-net.c
 GENERATED_FILES += qapi/qapi-visit-rocker.h qapi/qapi-visit-rocker.c
 GENERATED_FILES += qapi/qapi-visit-run-state.h qapi/qapi-visit-run-state.c
 GENERATED_FILES += qapi/qapi-visit-sockets.h qapi/qapi-visit-sockets.c
+GENERATED_FILES += qapi/qapi-visit-tlimits.h qapi/qapi-visit-tlimits.c
 GENERATED_FILES += qapi/qapi-visit-tpm.h qapi/qapi-visit-tpm.c
 GENERATED_FILES += qapi/qapi-visit-trace.h qapi/qapi-visit-trace.c
 GENERATED_FILES += qapi/qapi-visit-transaction.h qapi/qapi-visit-transaction.c
-GENERATED_FILES += qapi/qapi-visit-tlimits.h qapi/qapi-visit-tlimits.c
 GENERATED_FILES += qapi/qapi-visit-ui.h qapi/qapi-visit-ui.c
 GENERATED_FILES += qapi/qapi-commands.h qapi/qapi-commands.c
 GENERATED_FILES += qapi/qapi-commands-block-core.h qapi/qapi-commands-block-core.c
@@ -134,6 +136,7 @@ GENERATED_FILES += qapi/qapi-commands-block.h qapi/qapi-commands-block.c
 GENERATED_FILES += qapi/qapi-commands-char.h qapi/qapi-commands-char.c
 GENERATED_FILES += qapi/qapi-commands-common.h qapi/qapi-commands-common.c
 GENERATED_FILES += qapi/qapi-commands-crypto.h qapi/qapi-commands-crypto.c
+GENERATED_FILES += qapi/qapi-commands-fsdev.h qapi/qapi-commands-fsdev.c
 GENERATED_FILES += qapi/qapi-commands-introspect.h qapi/qapi-commands-introspect.c
 GENERATED_FILES += qapi/qapi-commands-job.h qapi/qapi-commands-job.c
 GENERATED_FILES += qapi/qapi-commands-migration.h qapi/qapi-commands-migration.c
@@ -142,10 +145,10 @@ GENERATED_FILES += qapi/qapi-commands-net.h qapi/qapi-commands-net.c
 GENERATED_FILES += qapi/qapi-commands-rocker.h qapi/qapi-commands-rocker.c
 GENERATED_FILES += qapi/qapi-commands-run-state.h qapi/qapi-commands-run-state.c
 GENERATED_FILES += qapi/qapi-commands-sockets.h qapi/qapi-commands-sockets.c
+GENERATED_FILES += qapi/qapi-commands-tlimits.h qapi/qapi-commands-tlimits.c
 GENERATED_FILES += qapi/qapi-commands-tpm.h qapi/qapi-commands-tpm.c
 GENERATED_FILES += qapi/qapi-commands-trace.h qapi/qapi-commands-trace.c
 GENERATED_FILES += qapi/qapi-commands-transaction.h qapi/qapi-commands-transaction.c
-GENERATED_FILES += qapi/qapi-commands-tlimits.h qapi/qapi-commands-tlimits.c
 GENERATED_FILES += qapi/qapi-commands-ui.h qapi/qapi-commands-ui.c
 GENERATED_FILES += qapi/qapi-events.h qapi/qapi-events.c
 GENERATED_FILES += qapi/qapi-events-block-core.h qapi/qapi-events-block-core.c
@@ -153,6 +156,7 @@ GENERATED_FILES += qapi/qapi-events-block.h qapi/qapi-events-block.c
 GENERATED_FILES += qapi/qapi-events-char.h qapi/qapi-events-char.c
 GENERATED_FILES += qapi/qapi-events-common.h qapi/qapi-events-common.c
 GENERATED_FILES += qapi/qapi-events-crypto.h qapi/qapi-events-crypto.c
+GENERATED_FILES += qapi/qapi-events-fsdev.h qapi/qapi-events-fsdev.c
 GENERATED_FILES += qapi/qapi-events-introspect.h qapi/qapi-events-introspect.c
 GENERATED_FILES += qapi/qapi-events-job.h qapi/qapi-events-job.c
 GENERATED_FILES += qapi/qapi-events-migration.h qapi/qapi-events-migration.c
@@ -161,10 +165,10 @@ GENERATED_FILES += qapi/qapi-events-net.h qapi/qapi-events-net.c
 GENERATED_FILES += qapi/qapi-events-rocker.h qapi/qapi-events-rocker.c
 GENERATED_FILES += qapi/qapi-events-run-state.h qapi/qapi-events-run-state.c
 GENERATED_FILES += qapi/qapi-events-sockets.h qapi/qapi-events-sockets.c
+GENERATED_FILES += qapi/qapi-events-tlimits.h qapi/qapi-events-tlimits.c
 GENERATED_FILES += qapi/qapi-events-tpm.h qapi/qapi-events-tpm.c
 GENERATED_FILES += qapi/qapi-events-trace.h qapi/qapi-events-trace.c
 GENERATED_FILES += qapi/qapi-events-transaction.h qapi/qapi-events-transaction.c
-GENERATED_FILES += qapi/qapi-events-tlimits.h qapi/qapi-events-tlimits.c
 GENERATED_FILES += qapi/qapi-events-ui.h qapi/qapi-events-ui.c
 GENERATED_FILES += qapi/qapi-introspect.c qapi/qapi-introspect.h
 GENERATED_FILES += qapi/qapi-doc.texi
@@ -591,6 +595,7 @@ qapi-modules = $(SRC_PATH)/qapi/qapi-schema.json $(SRC_PATH)/qapi/common.json \
                $(SRC_PATH)/qapi/block.json $(SRC_PATH)/qapi/block-core.json \
                $(SRC_PATH)/qapi/char.json \
                $(SRC_PATH)/qapi/crypto.json \
+               $(SRC_PATH)/qapi/fsdev.json \
                $(SRC_PATH)/qapi/introspect.json \
                $(SRC_PATH)/qapi/job.json \
                $(SRC_PATH)/qapi/migration.json \
@@ -599,8 +604,8 @@ qapi-modules = $(SRC_PATH)/qapi/qapi-schema.json $(SRC_PATH)/qapi/common.json \
                $(SRC_PATH)/qapi/rocker.json \
                $(SRC_PATH)/qapi/run-state.json \
                $(SRC_PATH)/qapi/sockets.json \
-               $(SRC_PATH)/qapi/tpm.json \
                $(SRC_PATH)/qapi/tlimits.json \
+               $(SRC_PATH)/qapi/tpm.json \
                $(SRC_PATH)/qapi/trace.json \
                $(SRC_PATH)/qapi/transaction.json \
                $(SRC_PATH)/qapi/ui.json
@@ -612,6 +617,7 @@ qapi/qapi-types-block.c qapi/qapi-types-block.h \
 qapi/qapi-types-char.c qapi/qapi-types-char.h \
 qapi/qapi-types-common.c qapi/qapi-types-common.h \
 qapi/qapi-types-crypto.c qapi/qapi-types-crypto.h \
+qapi/qapi-types-fsdev.c qapi/qapi-types-fsdev.h \
 qapi/qapi-types-introspect.c qapi/qapi-types-introspect.h \
 qapi/qapi-types-job.c qapi/qapi-types-job.h \
 qapi/qapi-types-migration.c qapi/qapi-types-migration.h \
@@ -620,10 +626,10 @@ qapi/qapi-types-net.c qapi/qapi-types-net.h \
 qapi/qapi-types-rocker.c qapi/qapi-types-rocker.h \
 qapi/qapi-types-run-state.c qapi/qapi-types-run-state.h \
 qapi/qapi-types-sockets.c qapi/qapi-types-sockets.h \
+qapi/qapi-types-tlimits.c qapi/qapi-types-tlimits.h \
 qapi/qapi-types-tpm.c qapi/qapi-types-tpm.h \
 qapi/qapi-types-trace.c qapi/qapi-types-trace.h \
 qapi/qapi-types-transaction.c qapi/qapi-types-transaction.h \
-qapi/qapi-types-tlimits.c qapi/qapi-types-tlimits.h \
 qapi/qapi-types-ui.c qapi/qapi-types-ui.h \
 qapi/qapi-builtin-visit.c qapi/qapi-builtin-visit.h \
 qapi/qapi-visit.c qapi/qapi-visit.h \
@@ -632,6 +638,7 @@ qapi/qapi-visit-block.c qapi/qapi-visit-block.h \
 qapi/qapi-visit-char.c qapi/qapi-visit-char.h \
 qapi/qapi-visit-common.c qapi/qapi-visit-common.h \
 qapi/qapi-visit-crypto.c qapi/qapi-visit-crypto.h \
+qapi/qapi-visit-fsdev.c qapi/qapi-visit-fsdev.h \
 qapi/qapi-visit-introspect.c qapi/qapi-visit-introspect.h \
 qapi/qapi-visit-job.c qapi/qapi-visit-job.h \
 qapi/qapi-visit-migration.c qapi/qapi-visit-migration.h \
@@ -640,10 +647,10 @@ qapi/qapi-visit-net.c qapi/qapi-visit-net.h \
 qapi/qapi-visit-rocker.c qapi/qapi-visit-rocker.h \
 qapi/qapi-visit-run-state.c qapi/qapi-visit-run-state.h \
 qapi/qapi-visit-sockets.c qapi/qapi-visit-sockets.h \
+qapi/qapi-visit-tlimits.c qapi/qapi-visit-tlimits.h \
 qapi/qapi-visit-tpm.c qapi/qapi-visit-tpm.h \
 qapi/qapi-visit-trace.c qapi/qapi-visit-trace.h \
 qapi/qapi-visit-transaction.c qapi/qapi-visit-transaction.h \
-qapi/qapi-visit-tlimits.c qapi/qapi-visit-tlimits.h \
 qapi/qapi-visit-ui.c qapi/qapi-visit-ui.h \
 qapi/qapi-commands.h qapi/qapi-commands.c \
 qapi/qapi-commands-block-core.c qapi/qapi-commands-block-core.h \
@@ -651,6 +658,7 @@ qapi/qapi-commands-block.c qapi/qapi-commands-block.h \
 qapi/qapi-commands-char.c qapi/qapi-commands-char.h \
 qapi/qapi-commands-common.c qapi/qapi-commands-common.h \
 qapi/qapi-commands-crypto.c qapi/qapi-commands-crypto.h \
+qapi/qapi-commands-fsdev.c qapi/qapi-commands-fsdev.h \
 qapi/qapi-commands-introspect.c qapi/qapi-commands-introspect.h \
 qapi/qapi-commands-job.c qapi/qapi-commands-job.h \
 qapi/qapi-commands-migration.c qapi/qapi-commands-migration.h \
@@ -659,10 +667,10 @@ qapi/qapi-commands-net.c qapi/qapi-commands-net.h \
 qapi/qapi-commands-rocker.c qapi/qapi-commands-rocker.h \
 qapi/qapi-commands-run-state.c qapi/qapi-commands-run-state.h \
 qapi/qapi-commands-sockets.c qapi/qapi-commands-sockets.h \
+qapi/qapi-commands-tlimits.c qapi/qapi-commands-tlimits.h \
 qapi/qapi-commands-tpm.c qapi/qapi-commands-tpm.h \
 qapi/qapi-commands-trace.c qapi/qapi-commands-trace.h \
 qapi/qapi-commands-transaction.c qapi/qapi-commands-transaction.h \
-qapi/qapi-commands-tlimits.c qapi/qapi-commands-tlimits.h \
 qapi/qapi-commands-ui.c qapi/qapi-commands-ui.h \
 qapi/qapi-events.c qapi/qapi-events.h \
 qapi/qapi-events-block-core.c qapi/qapi-events-block-core.h \
@@ -670,6 +678,7 @@ qapi/qapi-events-block.c qapi/qapi-events-block.h \
 qapi/qapi-events-char.c qapi/qapi-events-char.h \
 qapi/qapi-events-common.c qapi/qapi-events-common.h \
 qapi/qapi-events-crypto.c qapi/qapi-events-crypto.h \
+qapi/qapi-events-fsdev.c qapi/qapi-events-fsdev.h \
 qapi/qapi-events-introspect.c qapi/qapi-events-introspect.h \
 qapi/qapi-events-job.c qapi/qapi-events-job.h \
 qapi/qapi-events-migration.c qapi/qapi-events-migration.h \
@@ -678,10 +687,10 @@ qapi/qapi-events-net.c qapi/qapi-events-net.h \
 qapi/qapi-events-rocker.c qapi/qapi-events-rocker.h \
 qapi/qapi-events-run-state.c qapi/qapi-events-run-state.h \
 qapi/qapi-events-sockets.c qapi/qapi-events-sockets.h \
+qapi/qapi-events-tlimits.c qapi/qapi-events-tlimits.h \
 qapi/qapi-events-tpm.c qapi/qapi-events-tpm.h \
 qapi/qapi-events-trace.c qapi/qapi-events-trace.h \
 qapi/qapi-events-transaction.c qapi/qapi-events-transaction.h \
-qapi/qapi-events-tlimits.c qapi/qapi-events-tlimits.h \
 qapi/qapi-events-ui.c qapi/qapi-events-ui.h \
 qapi/qapi-introspect.h qapi/qapi-introspect.c \
 qapi/qapi-doc.texi: \
diff --git a/Makefile.objs b/Makefile.objs
index 682e6ba..1c4bf88 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -9,6 +9,7 @@ util-obj-y += qapi/qapi-types-block.o
 util-obj-y += qapi/qapi-types-char.o
 util-obj-y += qapi/qapi-types-common.o
 util-obj-y += qapi/qapi-types-crypto.o
+util-obj-y += qapi/qapi-types-fsdev.o
 util-obj-y += qapi/qapi-types-introspect.o
 util-obj-y += qapi/qapi-types-job.o
 util-obj-y += qapi/qapi-types-migration.o
@@ -17,10 +18,10 @@ util-obj-y += qapi/qapi-types-net.o
 util-obj-y += qapi/qapi-types-rocker.o
 util-obj-y += qapi/qapi-types-run-state.o
 util-obj-y += qapi/qapi-types-sockets.o
+util-obj-y += qapi/qapi-types-tlimits.o
 util-obj-y += qapi/qapi-types-tpm.o
 util-obj-y += qapi/qapi-types-trace.o
 util-obj-y += qapi/qapi-types-transaction.o
-util-obj-y += qapi/qapi-types-tlimits.o
 util-obj-y += qapi/qapi-types-ui.o
 util-obj-y += qapi/qapi-builtin-visit.o
 util-obj-y += qapi/qapi-visit.o
@@ -29,6 +30,7 @@ util-obj-y += qapi/qapi-visit-block.o
 util-obj-y += qapi/qapi-visit-char.o
 util-obj-y += qapi/qapi-visit-common.o
 util-obj-y += qapi/qapi-visit-crypto.o
+util-obj-y += qapi/qapi-visit-fsdev.o
 util-obj-y += qapi/qapi-visit-introspect.o
 util-obj-y += qapi/qapi-visit-job.o
 util-obj-y += qapi/qapi-visit-migration.o
@@ -37,10 +39,10 @@ util-obj-y += qapi/qapi-visit-net.o
 util-obj-y += qapi/qapi-visit-rocker.o
 util-obj-y += qapi/qapi-visit-run-state.o
 util-obj-y += qapi/qapi-visit-sockets.o
+util-obj-y += qapi/qapi-visit-tlimits.o
 util-obj-y += qapi/qapi-visit-tpm.o
 util-obj-y += qapi/qapi-visit-trace.o
 util-obj-y += qapi/qapi-visit-transaction.o
-util-obj-y += qapi/qapi-visit-tlimits.o
 util-obj-y += qapi/qapi-visit-ui.o
 util-obj-y += qapi/qapi-events.o
 util-obj-y += qapi/qapi-events-block-core.o
@@ -48,6 +50,7 @@ util-obj-y += qapi/qapi-events-block.o
 util-obj-y += qapi/qapi-events-char.o
 util-obj-y += qapi/qapi-events-common.o
 util-obj-y += qapi/qapi-events-crypto.o
+util-obj-y += qapi/qapi-events-fsdev.o
 util-obj-y += qapi/qapi-events-introspect.o
 util-obj-y += qapi/qapi-events-job.o
 util-obj-y += qapi/qapi-events-migration.o
@@ -56,10 +59,10 @@ util-obj-y += qapi/qapi-events-net.o
 util-obj-y += qapi/qapi-events-rocker.o
 util-obj-y += qapi/qapi-events-run-state.o
 util-obj-y += qapi/qapi-events-sockets.o
+util-obj-y += qapi/qapi-events-tlimits.o
 util-obj-y += qapi/qapi-events-tpm.o
 util-obj-y += qapi/qapi-events-trace.o
 util-obj-y += qapi/qapi-events-transaction.o
-util-obj-y += qapi/qapi-events-tlimits.o
 util-obj-y += qapi/qapi-events-ui.o
 util-obj-y += qapi/qapi-introspect.o
 
@@ -146,6 +149,7 @@ common-obj-y += qapi/qapi-commands-block.o
 common-obj-y += qapi/qapi-commands-char.o
 common-obj-y += qapi/qapi-commands-common.o
 common-obj-y += qapi/qapi-commands-crypto.o
+common-obj-y += qapi/qapi-commands-fsdev.o
 common-obj-y += qapi/qapi-commands-introspect.o
 common-obj-y += qapi/qapi-commands-job.o
 common-obj-y += qapi/qapi-commands-migration.o
@@ -154,10 +158,10 @@ common-obj-y += qapi/qapi-commands-net.o
 common-obj-y += qapi/qapi-commands-rocker.o
 common-obj-y += qapi/qapi-commands-run-state.o
 common-obj-y += qapi/qapi-commands-sockets.o
+common-obj-y += qapi/qapi-commands-tlimits.o
 common-obj-y += qapi/qapi-commands-tpm.o
 common-obj-y += qapi/qapi-commands-trace.o
 common-obj-y += qapi/qapi-commands-transaction.o
-common-obj-y += qapi/qapi-commands-tlimits.o
 common-obj-y += qapi/qapi-commands-ui.o
 common-obj-y += qapi/qapi-introspect.o
 common-obj-y += qmp.o hmp.o
diff --git a/fsdev/qemu-fsdev-dummy.c b/fsdev/qemu-fsdev-dummy.c
index 489cd29..9a90960 100644
--- a/fsdev/qemu-fsdev-dummy.c
+++ b/fsdev/qemu-fsdev-dummy.c
@@ -14,8 +14,19 @@
 #include "qemu-fsdev.h"
 #include "qemu/config-file.h"
 #include "qemu/module.h"
+#include "qapi/qapi-commands-fsdev.h"
 
 int qemu_fsdev_add(QemuOpts *opts, Error **errp)
 {
     return 0;
 }
+
+void qmp_fsdev_set_io_throttle(FsdevIOThrottle *arg, Error **errp)
+{
+    return;
+}
+
+FsdevIOThrottleList *qmp_query_fsdev_io_throttle(Error **errp)
+{
+    return NULL;
+}
diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
index 6a4108a..720fea9 100644
--- a/fsdev/qemu-fsdev-throttle.c
+++ b/fsdev/qemu-fsdev-throttle.c
@@ -17,6 +17,7 @@
 #include "qemu-fsdev-throttle.h"
 #include "qemu/iov.h"
 #include "qemu/option.h"
+#include "qemu/main-loop.h"
 #include "qemu/throttle-options.h"
 
 static void fsdev_throttle_read_timer_cb(void *opaque)
@@ -31,6 +32,94 @@ static void fsdev_throttle_write_timer_cb(void *opaque)
     qemu_co_enter_next(&fst->throttled_reqs[true], NULL);
 }
 
+typedef struct {
+    FsThrottle *fst;
+    bool is_write;
+} RestartData;
+
+static bool coroutine_fn throttle_co_restart_queue(FsThrottle *fst,
+                                                   bool is_write)
+{
+    return qemu_co_queue_next(&fst->throttled_reqs[is_write]);
+}
+
+static void schedule_next_request(FsThrottle *fst, bool is_write)
+{
+    bool must_wait = throttle_schedule_timer(&fst->ts, &fst->tt, is_write);
+    if (!must_wait) {
+        if (qemu_in_coroutine() &&
+            throttle_co_restart_queue(fst, is_write)) {
+            return;
+        } else {
+            int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
+            timer_mod(fst->tt.timers[is_write], now);
+        }
+    }
+}
+
+static void coroutine_fn throttle_restart_queue_entry(void *opaque)
+{
+    RestartData *data = opaque;
+    bool is_write = data->is_write;
+    bool empty_queue = !throttle_co_restart_queue(data->fst, is_write);
+    if (empty_queue) {
+        schedule_next_request(data->fst, is_write);
+    }
+}
+
+static void throttle_restart_queues(FsThrottle *fst)
+{
+    Coroutine *co;
+    RestartData rd = {
+        .fst = fst,
+        .is_write = true
+    };
+     co = qemu_coroutine_create(throttle_restart_queue_entry, &rd);
+    aio_co_enter(fst->ctx, co);
+     rd.is_write = false;
+     co = qemu_coroutine_create(throttle_restart_queue_entry, &rd);
+    aio_co_enter(fst->ctx, co);
+}
+
+static void coroutine_fn fsdev_throttle_config(FsThrottle *fst)
+{
+    if (throttle_enabled(&fst->cfg)) {
+        throttle_config(&fst->ts, QEMU_CLOCK_REALTIME, &fst->cfg);
+    } else {
+        throttle_restart_queues(fst);
+    }
+}
+
+void fsdev_set_io_throttle(FsdevIOThrottle *arg, FsThrottle *fst, Error **errp)
+{
+    ThrottleConfig cfg;
+
+    throttle_get_config(&fst->ts, &cfg);
+    throttle_limits_to_config(qapi_FsdevIOThrottle_base(arg), &cfg, errp);
+
+    if (*errp == NULL) {
+        fst->cfg = cfg;
+        if (!throttle_timers_are_initialized(&fst->tt)) {
+            fsdev_throttle_init(fst);
+        } else {
+            fsdev_throttle_config(fst);
+        }
+    }
+}
+
+void fsdev_get_io_throttle(FsThrottle *fst, FsdevIOThrottle **fs9pcfg,
+                           char *fsdevice)
+{
+    ThrottleConfig cfg = fst->cfg;
+    ThrottleLimits *tlimits;
+    FsdevIOThrottle *fscfg = g_malloc(sizeof(*fscfg));
+    tlimits = qapi_FsdevIOThrottle_base(fscfg);
+    fscfg->has_id = true;
+    fscfg->id = g_strdup(fsdevice);
+    throttle_config_to_limits(&cfg, tlimits);
+    *fs9pcfg = fscfg;
+}
+
 void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error **errp)
 {
     throttle_parse_options(&fst->cfg, opts);
@@ -41,8 +130,9 @@ void fsdev_throttle_init(FsThrottle *fst)
 {
     if (throttle_enabled(&fst->cfg)) {
         throttle_init(&fst->ts);
+        fst->ctx = qemu_get_aio_context();
         throttle_timers_init(&fst->tt,
-                             qemu_get_aio_context(),
+                             fst->ctx,
                              QEMU_CLOCK_REALTIME,
                              fsdev_throttle_read_timer_cb,
                              fsdev_throttle_write_timer_cb,
@@ -63,11 +153,7 @@ void coroutine_fn fsdev_co_throttle_request(FsThrottle *fst, bool is_write,
         }
 
         throttle_account(&fst->ts, is_write, iov_size(iov, iovcnt));
-
-        if (!qemu_co_queue_empty(&fst->throttled_reqs[is_write]) &&
-            !throttle_schedule_timer(&fst->ts, &fst->tt, is_write)) {
-            qemu_co_queue_next(&fst->throttled_reqs[is_write]);
-        }
+        schedule_next_request(fst, is_write);
     }
 }
 
diff --git a/fsdev/qemu-fsdev-throttle.h b/fsdev/qemu-fsdev-throttle.h
index 4e83bda..7107769 100644
--- a/fsdev/qemu-fsdev-throttle.h
+++ b/fsdev/qemu-fsdev-throttle.h
@@ -15,15 +15,15 @@
 #ifndef _FSDEV_THROTTLE_H
 #define _FSDEV_THROTTLE_H
 
-#include "block/aio.h"
-#include "qemu/main-loop.h"
 #include "qemu/coroutine.h"
 #include "qemu/throttle.h"
+#include "qapi/qapi-types-fsdev.h"
 
 typedef struct FsThrottle {
     ThrottleState ts;
     ThrottleTimers tt;
     ThrottleConfig cfg;
+    AioContext *ctx;
     CoQueue      throttled_reqs[2];
 } FsThrottle;
 
@@ -35,4 +35,6 @@ void coroutine_fn fsdev_co_throttle_request(FsThrottle *, bool ,
                                             struct iovec *, int);
 
 void fsdev_throttle_cleanup(FsThrottle *);
+void fsdev_set_io_throttle(FsdevIOThrottle *, FsThrottle *, Error **errp);
+void fsdev_get_io_throttle(FsThrottle *, FsdevIOThrottle **iothp, char *);
 #endif /* _FSDEV_THROTTLE_H */
diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
index 7a3b87c..609d8fc 100644
--- a/fsdev/qemu-fsdev.c
+++ b/fsdev/qemu-fsdev.c
@@ -17,6 +17,7 @@
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
 #include "qemu/option.h"
+#include "qapi/qapi-commands-fsdev.h"
 
 static QTAILQ_HEAD(FsDriverEntry_head, FsDriverListEntry) fsdriver_entries =
     QTAILQ_HEAD_INITIALIZER(fsdriver_entries);
@@ -99,3 +100,31 @@ FsDriverEntry *get_fsdev_fsentry(char *id)
     }
     return NULL;
 }
+
+void qmp_fsdev_set_io_throttle(FsdevIOThrottle *arg, Error **errp)
+{
+    FsDriverEntry *fse;
+
+    fse = get_fsdev_fsentry(arg->has_id ? arg->id : NULL);
+    if (!fse) {
+        error_setg(errp, "Not a valid fsdev device");
+        return;
+    }
+
+    fsdev_set_io_throttle(arg, &fse->fst, errp);
+}
+
+FsdevIOThrottleList *qmp_query_fsdev_io_throttle(Error **errp)
+{
+    FsdevIOThrottleList *head = NULL, *p_next;
+    struct FsDriverListEntry *fsle;
+
+    QTAILQ_FOREACH(fsle, &fsdriver_entries, next) {
+        p_next = g_new0(FsdevIOThrottleList, 1);
+        fsdev_get_io_throttle(&fsle->fse.fst, &p_next->value,
+                              fsle->fse.fsdev_id);
+        p_next->next = head;
+        head = p_next;
+    }
+    return head;
+}
diff --git a/qapi/fsdev.json b/qapi/fsdev.json
new file mode 100644
index 0000000..5c8e7de
--- /dev/null
+++ b/qapi/fsdev.json
@@ -0,0 +1,96 @@
+# -*- Mode: Python -*-
+
+##
+# == QAPI fsdev definitions
+##
+
+{ 'include': 'tlimits.json' }
+
+##
+# @FsdevIOThrottle:
+#
+# A set of parameters describing fsdev throttling.
+#
+# @id: device id
+#
+# Since: 3.2
+##
+{ 'struct': 'FsdevIOThrottle',
+  'base': 'ThrottleLimits',
+  'data': { '*id': 'str' } }
+
+##
+# @fsdev-set-io-throttle:
+#
+# Change I/O limits for a 9p/fsdev device.
+#
+# I/O limits can be enabled by setting throttle value to non-zero number.
+#
+# I/O limits can be disabled by setting all throttle values to 0.
+#
+# Returns: Nothing on success
+#          If @device is not a valid fsdev device, GenericError
+#
+# Since: 3.2
+#
+# Example:
+#
+# -> { "execute": "fsdev-set-io-throttle",
+#      "arguments": { "id": "id0-1-0",
+#                     "bps": 1000000,
+#                     "bps_rd": 0,
+#                     "bps_wr": 0,
+#                     "iops": 0,
+#                     "iops_rd": 0,
+#                     "iops_wr": 0,
+#                     "bps_max": 8000000,
+#                     "bps_rd_max": 0,
+#                     "bps_wr_max": 0,
+#                     "iops_max": 0,
+#                     "iops_rd_max": 0,
+#                     "iops_wr_max": 0,
+#                     "bps_max_length": 60,
+#                     "iops_size": 0 } }
+# <- { "returns": {} }
+##
+{ 'command': 'fsdev-set-io-throttle', 'boxed': true,
+  'data': 'FsdevIOThrottle' }
+##
+# @query-fsdev-io-throttle:
+#
+# Returns: a list of @IOThrottle describing I/O throttle
+#          values of each fsdev device
+#
+# Since: 3.2
+#
+# Example:
+#
+# -> { "Execute": "query-fsdev-io-throttle" }
+# <- { "returns" : [
+#          {
+#              "id": "id0-hd0",
+#              "bps":1000000,
+#              "bps_rd":0,
+#              "bps_wr":0,
+#              "iops":1000000,
+#              "iops_rd":0,
+#              "iops_wr":0,
+#              "bps_max": 8000000,
+#              "bps_rd_max": 0,
+#              "bps_wr_max": 0,
+#              "iops_max": 0,
+#              "iops_rd_max": 0,
+#              "iops_wr_max": 0,
+#              "bps_max_length": 0,
+#              "bps_rd_max_length": 0,
+#              "bps_wr_max_length": 0,
+#              "iops_max_length": 0,
+#              "iops_rd_max_length": 0,
+#              "iops_wr_max_length": 0,
+#              "iops_size": 0
+#            }
+#          ]
+#      }
+#
+##
+{ 'command': 'query-fsdev-io-throttle', 'returns': [ 'FsdevIOThrottle' ] }
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index e9f594e..8fce6d9 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -95,3 +95,4 @@
 { 'include': 'trace.json' }
 { 'include': 'introspect.json' }
 { 'include': 'misc.json' }
+{ 'include': 'fsdev.json' }
diff --git a/qmp.c b/qmp.c
index e7c0a2f..3f3171a 100644
--- a/qmp.c
+++ b/qmp.c
@@ -32,6 +32,7 @@
 #include "qom/qom-qobject.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-block-core.h"
+#include "qapi/qapi-commands-fsdev.h"
 #include "qapi/qapi-commands-misc.h"
 #include "qapi/qapi-commands-ui.h"
 #include "qapi/qmp/qdict.h"
@@ -737,3 +738,15 @@ MemoryInfo *qmp_query_memory_size_summary(Error **errp)
 
     return mem_info;
 }
+
+#if defined(_WIN64) || defined(_WIN32) || defined(__FreeBSD__)
+void qmp_fsdev_set_io_throttle(FsdevIOThrottle *arg, Error **errp)
+{
+    return;
+}
+
+FsdevIOThrottleList *qmp_query_fsdev_io_throttle(Error **errp)
+{
+    return NULL;
+}
+#endif
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v5 6/6] fsdev-throttle-qmp: hmp interface for fsdev io throttling
  2018-11-16  7:58 [Qemu-devel] [PATCH v5 0/6] fsdev-throttle-qmp: qmp interface for fsdev io throttling xiezhide
                   ` (4 preceding siblings ...)
  2018-11-16  8:00 ` [Qemu-devel] [PATCH v5 5/6] fsdev-throttle-qmp: qmp interface for fsdev io throttling xiezhide
@ 2018-11-16  8:00 ` xiezhide
  5 siblings, 0 replies; 15+ messages in thread
From: xiezhide @ 2018-11-16  8:00 UTC (permalink / raw)
  To: qemu-devel
  Cc: groug, aneesh.kumar, eblake, armbru, berto, zengcanfu,
	jinxuefeng, chenhui.rtos

introduces io throttling hmp interfaces for the fsdev devices

Signed-off-by: xiezhide <xiezhide@huawei.com>
---
 hmp-commands-info.hx | 15 ++++++++++
 hmp-commands.hx      | 15 ++++++++++
 hmp.c                | 81 ++++++++++++++++++++++++++++++++++++++++++++++------
 hmp.h                |  4 +++
 4 files changed, 107 insertions(+), 8 deletions(-)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index cbee8b9..eaf0ff5 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -100,6 +100,21 @@ STEXI
 Show progress of ongoing block device operations.
 ETEXI
 
+#if defined(CONFIG_VIRTFS)
+     {
+        .name       = "fsdev_iothrottle",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show fsdev iothrottle information",
+        .cmd        = hmp_info_fsdev_iothrottle,
+    },
+#endif
+STEXI
+@item info fsdev_iothrottle
+@findex fsdev_iothrottle
+Show fsdev device throttle info.
+ETEXI
+
     {
         .name       = "registers",
         .args_type  = "cpustate_all:-a",
diff --git a/hmp-commands.hx b/hmp-commands.hx
index db0c681..40ca7fe 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1716,6 +1716,21 @@ Change I/O throttle limits for a block drive to @var{bps} @var{bps_rd} @var{bps_
 @var{device} can be a block device name, a qdev ID or a QOM path.
 ETEXI
 
+#if defined(CONFIG_VIRTFS)
+     {
+        .name       = "fsdev_set_io_throttle",
+        .args_type  = "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l",
+        .params     = "device bps bps_rd bps_wr iops iops_rd iops_wr",
+        .help       = "change I/O throttle limits for a fs devices",
+        .cmd        = hmp_fsdev_set_io_throttle,
+    },
+#endif
+STEXI
+@item fsdev_set_io_throttle @var{device} @var{bps} @var{bps_rd} @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
+@findex fsdev_set_io_throttle
+Change I/O throttle limits for a fs devices to @var{bps} @var{bps_rd} @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
+ETEXI
+
     {
         .name       = "set_password",
         .args_type  = "protocol:s,password:s,connected:s?",
diff --git a/hmp.c b/hmp.c
index 7828f93..7162d87 100644
--- a/hmp.c
+++ b/hmp.c
@@ -38,6 +38,7 @@
 #include "qapi/qapi-commands-run-state.h"
 #include "qapi/qapi-commands-tpm.h"
 #include "qapi/qapi-commands-ui.h"
+#include "qapi/qapi-commands-fsdev.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/string-input-visitor.h"
@@ -1886,18 +1887,22 @@ void hmp_change(Monitor *mon, const QDict *qdict)
     hmp_handle_error(mon, &err);
 }
 
+static void hmp_initialize_throttle_limits(ThrottleLimits *iot,
+                                           const QDict *qdict)
+{
+    iot->bps = qdict_get_int(qdict, "bps");
+    iot->bps_rd = qdict_get_int(qdict, "bps_rd");
+    iot->bps_wr = qdict_get_int(qdict, "bps_wr");
+    iot->iops = qdict_get_int(qdict, "iops");
+    iot->iops_rd = qdict_get_int(qdict, "iops_rd");
+    iot->iops_wr = qdict_get_int(qdict, "iops_wr");
+}
+
 void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
 {
     Error *err = NULL;
     char *device = (char *) qdict_get_str(qdict, "device");
-    BlockIOThrottle throttle = {
-        .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"),
-    };
+    BlockIOThrottle throttle = {0};
 
     /* qmp_block_set_io_throttle has separate parameters for the
      * (deprecated) block device name and the qdev ID but the HMP
@@ -1910,10 +1915,70 @@ void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
         throttle.id = device;
     }
 
+    hmp_initialize_throttle_limits(qapi_BlockIOThrottle_base(&throttle), qdict);
     qmp_block_set_io_throttle(&throttle, &err);
     hmp_handle_error(mon, &err);
 }
 
+#ifdef CONFIG_VIRTFS
+void hmp_fsdev_set_io_throttle(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+    ThrottleLimits *tlimits;
+    FsdevIOThrottle throttle = {
+        .has_id = true,
+        .id = (char *) qdict_get_str(qdict, "device"),
+    };
+
+    tlimits = qapi_FsdevIOThrottle_base(&throttle);
+    hmp_initialize_throttle_limits(tlimits, qdict);
+    qmp_fsdev_set_io_throttle(&throttle, &err);
+    hmp_handle_error(mon, &err);
+}
+
+static void print_fsdev_throttle_config(Monitor *mon, FsdevIOThrottle *fscfg)
+{
+    monitor_printf(mon, "%s", fscfg->id);
+    monitor_printf(mon, "    I/O throttling:"
+                   " bps=%" PRId64
+                   " bps_rd=%" PRId64  " bps_wr=%" PRId64
+                   " bps_max=%" PRId64
+                   " bps_rd_max=%" PRId64
+                   " bps_wr_max=%" PRId64
+                   " iops=%" PRId64 " iops_rd=%" PRId64
+                   " iops_wr=%" PRId64
+                   " iops_max=%" PRId64
+                   " iops_rd_max=%" PRId64
+                   " iops_wr_max=%" PRId64
+                   " iops_size=%" PRId64
+                   "\n",
+                   fscfg->bps,
+                   fscfg->bps_rd,
+                   fscfg->bps_wr,
+                   fscfg->bps_max,
+                   fscfg->bps_rd_max,
+                   fscfg->bps_wr_max,
+                   fscfg->iops,
+                   fscfg->iops_rd,
+                   fscfg->iops_wr,
+                   fscfg->iops_max,
+                   fscfg->iops_rd_max,
+                   fscfg->iops_wr_max,
+                   fscfg->iops_size);
+}
+
+void hmp_info_fsdev_iothrottle(Monitor *mon, const QDict *qdict)
+{
+    FsdevIOThrottleList *fsdev_list, *info;
+    fsdev_list = qmp_query_fsdev_io_throttle(NULL);
+
+    for (info = fsdev_list; info; info = info->next) {
+        print_fsdev_throttle_config(mon, info->value);
+    }
+    qapi_free_FsdevIOThrottleList(fsdev_list);
+}
+#endif
+
 void hmp_block_stream(Monitor *mon, const QDict *qdict)
 {
     Error *error = NULL;
diff --git a/hmp.h b/hmp.h
index 5f1addc..9330e11 100644
--- a/hmp.h
+++ b/hmp.h
@@ -91,6 +91,10 @@ void hmp_block_job_cancel(Monitor *mon, const QDict *qdict);
 void hmp_block_job_pause(Monitor *mon, const QDict *qdict);
 void hmp_block_job_resume(Monitor *mon, const QDict *qdict);
 void hmp_block_job_complete(Monitor *mon, const QDict *qdict);
+#ifdef CONFIG_VIRTFS
+void hmp_fsdev_set_io_throttle(Monitor *mon, const QDict *qdict);
+void hmp_info_fsdev_iothrottle(Monitor *mon, const QDict *qdict);
+#endif
 void hmp_migrate(Monitor *mon, const QDict *qdict);
 void hmp_device_add(Monitor *mon, const QDict *qdict);
 void hmp_device_del(Monitor *mon, const QDict *qdict);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v5 1/6] fsdev-throttle-qmp: factor out throttle code to reuse code
  2018-11-16  7:59 ` [Qemu-devel] [PATCH v5 1/6] fsdev-throttle-qmp: factor out throttle code to reuse code xiezhide
@ 2018-11-22 14:46   ` Greg Kurz
  2018-11-23  6:42     ` xiezhide
  0 siblings, 1 reply; 15+ messages in thread
From: Greg Kurz @ 2018-11-22 14:46 UTC (permalink / raw)
  To: xiezhide
  Cc: qemu-devel, aneesh.kumar, eblake, armbru, berto, zengcanfu,
	jinxuefeng, chenhui.rtos

On Fri, 16 Nov 2018 15:59:16 +0800
xiezhide <xiezhide@huawei.com> wrote:

> Factor out throttle parameter parsing code to a new common
> function which will be used by block and fsdev.
> Rename function throttle_parse_options to throttle_parse_group
> to resolve function name conflict
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: xiezhide <xiezhide@huawei.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

And, since I guess this will likely go through someone else's tree,
for the fsdev changes:

Acked-by: Greg Kurz <groug@kaod.org>

>  block/throttle.c                |  6 ++--
>  blockdev.c                      | 43 +-------------------------
>  fsdev/qemu-fsdev-throttle.c     | 44 ++------------------------
>  include/qemu/throttle-options.h |  2 ++
>  include/qemu/throttle.h         |  4 +--
>  include/qemu/typedefs.h         |  1 +
>  util/throttle.c                 | 68 +++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 79 insertions(+), 89 deletions(-)
> 
> diff --git a/block/throttle.c b/block/throttle.c
> index 636c976..bd23c58 100644
> --- a/block/throttle.c
> +++ b/block/throttle.c
> @@ -41,7 +41,7 @@ static QemuOptsList throttle_opts = {
>   * @group and must be freed by the caller.
>   * If there's an error then @group remains unmodified.
>   */
> -static int throttle_parse_options(QDict *options, char **group, Error **errp)
> +static int throttle_parse_group(QDict *options, char **group, Error **errp)
>  {
>      int ret;
>      const char *group_name;
> @@ -90,7 +90,7 @@ static int throttle_open(BlockDriverState *bs, QDict *options,
>      bs->supported_zero_flags = bs->file->bs->supported_zero_flags |
>                                 BDRV_REQ_WRITE_UNCHANGED;
>  
> -    ret = throttle_parse_options(options, &group, errp);
> +    ret = throttle_parse_group(options, &group, errp);
>      if (ret == 0) {
>          /* Register membership to group with name group_name */
>          throttle_group_register_tgm(tgm, group, bdrv_get_aio_context(bs));
> @@ -179,7 +179,7 @@ static int throttle_reopen_prepare(BDRVReopenState *reopen_state,
>      assert(reopen_state != NULL);
>      assert(reopen_state->bs != NULL);
>  
> -    ret = throttle_parse_options(reopen_state->options, &group, errp);
> +    ret = throttle_parse_group(reopen_state->options, &group, errp);
>      reopen_state->opaque = group;
>      return ret;
>  }
> diff --git a/blockdev.c b/blockdev.c
> index 81f95d9..fce5d8f 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -400,48 +400,7 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
>      }
>  
>      if (throttle_cfg) {
> -        throttle_config_init(throttle_cfg);
> -        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
> -            qemu_opt_get_number(opts, "throttling.bps-total", 0);
> -        throttle_cfg->buckets[THROTTLE_BPS_READ].avg  =
> -            qemu_opt_get_number(opts, "throttling.bps-read", 0);
> -        throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
> -            qemu_opt_get_number(opts, "throttling.bps-write", 0);
> -        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
> -            qemu_opt_get_number(opts, "throttling.iops-total", 0);
> -        throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
> -            qemu_opt_get_number(opts, "throttling.iops-read", 0);
> -        throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
> -            qemu_opt_get_number(opts, "throttling.iops-write", 0);
> -
> -        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
> -            qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
> -        throttle_cfg->buckets[THROTTLE_BPS_READ].max  =
> -            qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
> -        throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
> -            qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
> -        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
> -            qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
> -        throttle_cfg->buckets[THROTTLE_OPS_READ].max =
> -            qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
> -        throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
> -            qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
> -
> -        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
> -            qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
> -        throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length  =
> -            qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
> -        throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
> -            qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
> -        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
> -            qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
> -        throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
> -            qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
> -        throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
> -            qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
> -
> -        throttle_cfg->op_size =
> -            qemu_opt_get_number(opts, "throttling.iops-size", 0);
> +        throttle_parse_options(throttle_cfg, opts);
>  
>          if (!throttle_is_valid(throttle_cfg, errp)) {
>              return;
> diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
> index cfd8641..6a4108a 100644
> --- a/fsdev/qemu-fsdev-throttle.c
> +++ b/fsdev/qemu-fsdev-throttle.c
> @@ -17,6 +17,7 @@
>  #include "qemu-fsdev-throttle.h"
>  #include "qemu/iov.h"
>  #include "qemu/option.h"
> +#include "qemu/throttle-options.h"
>  
>  static void fsdev_throttle_read_timer_cb(void *opaque)
>  {
> @@ -32,48 +33,7 @@ static void fsdev_throttle_write_timer_cb(void *opaque)
>  
>  void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error **errp)
>  {
> -    throttle_config_init(&fst->cfg);
> -    fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =
> -        qemu_opt_get_number(opts, "throttling.bps-total", 0);
> -    fst->cfg.buckets[THROTTLE_BPS_READ].avg  =
> -        qemu_opt_get_number(opts, "throttling.bps-read", 0);
> -    fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =
> -        qemu_opt_get_number(opts, "throttling.bps-write", 0);
> -    fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =
> -        qemu_opt_get_number(opts, "throttling.iops-total", 0);
> -    fst->cfg.buckets[THROTTLE_OPS_READ].avg =
> -        qemu_opt_get_number(opts, "throttling.iops-read", 0);
> -    fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =
> -        qemu_opt_get_number(opts, "throttling.iops-write", 0);
> -
> -    fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =
> -        qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
> -    fst->cfg.buckets[THROTTLE_BPS_READ].max  =
> -        qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
> -    fst->cfg.buckets[THROTTLE_BPS_WRITE].max =
> -        qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
> -    fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =
> -        qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
> -    fst->cfg.buckets[THROTTLE_OPS_READ].max =
> -        qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
> -    fst->cfg.buckets[THROTTLE_OPS_WRITE].max =
> -        qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
> -
> -    fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =
> -        qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
> -    fst->cfg.buckets[THROTTLE_BPS_READ].burst_length  =
> -        qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
> -    fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =
> -        qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
> -    fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =
> -        qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
> -    fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =
> -        qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
> -    fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =
> -        qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
> -    fst->cfg.op_size =
> -        qemu_opt_get_number(opts, "throttling.iops-size", 0);
> -
> +    throttle_parse_options(&fst->cfg, opts);
>      throttle_is_valid(&fst->cfg, errp);
>  }
>  
> diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
> index 3528a8f..944a08c 100644
> --- a/include/qemu/throttle-options.h
> +++ b/include/qemu/throttle-options.h
> @@ -111,4 +111,6 @@
>              .help = "when limiting by iops max size of an I/O in bytes",\
>          }
>  
> +void throttle_parse_options(ThrottleConfig *, QemuOpts *);
> +
>  #endif
> diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
> index abeb886..f379d91 100644
> --- a/include/qemu/throttle.h
> +++ b/include/qemu/throttle.h
> @@ -90,10 +90,10 @@ typedef struct LeakyBucket {
>   * However it allows to keep the code clean and the bucket field is reset to
>   * zero at the right time.
>   */
> -typedef struct ThrottleConfig {
> +struct ThrottleConfig {
>      LeakyBucket buckets[BUCKETS_COUNT]; /* leaky buckets */
>      uint64_t op_size;         /* size of an operation in bytes */
> -} ThrottleConfig;
> +};
>  
>  typedef struct ThrottleState {
>      ThrottleConfig cfg;       /* configuration */
> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> index 3ec0e13..0d75edc 100644
> --- a/include/qemu/typedefs.h
> +++ b/include/qemu/typedefs.h
> @@ -109,6 +109,7 @@ typedef struct SerialState SerialState;
>  typedef struct SHPCDevice SHPCDevice;
>  typedef struct SMBusDevice SMBusDevice;
>  typedef struct SSIBus SSIBus;
> +typedef struct ThrottleConfig ThrottleConfig;
>  typedef struct uWireSlave uWireSlave;
>  typedef struct VirtIODevice VirtIODevice;
>  typedef struct Visitor Visitor;
> diff --git a/util/throttle.c b/util/throttle.c
> index b38e742..e7db2ad 100644
> --- a/util/throttle.c
> +++ b/util/throttle.c
> @@ -27,6 +27,8 @@
>  #include "qemu/throttle.h"
>  #include "qemu/timer.h"
>  #include "block/aio.h"
> +#include "qemu/option.h"
> +#include "qemu/throttle-options.h"
>  
>  /* This function make a bucket leak
>   *
> @@ -636,3 +638,69 @@ void throttle_config_to_limits(ThrottleConfig *cfg, ThrottleLimits *var)
>      var->has_iops_write_max_length = true;
>      var->has_iops_size = true;
>  }
> +
> +/* parse the throttle options
> + *
> + * @opts: qemu options
> + * @throttle_cfg: throttle configuration
> + */
> +void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts *opts)
> +{
> +    throttle_config_init(throttle_cfg);
> +    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
> +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> +                            QEMU_OPT_BPS_TOTAL, 0);
> +    throttle_cfg->buckets[THROTTLE_BPS_READ].avg  =
> +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> +                            QEMU_OPT_BPS_READ, 0);
> +    throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
> +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> +                            QEMU_OPT_BPS_WRITE, 0);
> +    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
> +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> +                            QEMU_OPT_IOPS_TOTAL, 0);
> +    throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
> +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> +                            QEMU_OPT_IOPS_READ, 0);
> +    throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
> +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> +                            QEMU_OPT_IOPS_WRITE, 0);
> +    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
> +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> +                            QEMU_OPT_BPS_TOTAL_MAX, 0);
> +    throttle_cfg->buckets[THROTTLE_BPS_READ].max  =
> +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> +                            QEMU_OPT_BPS_READ_MAX, 0);
> +    throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
> +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> +                            QEMU_OPT_BPS_WRITE_MAX, 0);
> +    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
> +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> +                            QEMU_OPT_IOPS_TOTAL_MAX, 0);
> +    throttle_cfg->buckets[THROTTLE_OPS_READ].max =
> +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> +                            QEMU_OPT_IOPS_READ_MAX, 0);
> +    throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
> +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> +                            QEMU_OPT_IOPS_WRITE_MAX, 0);
> +    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
> +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> +                            QEMU_OPT_BPS_TOTAL_MAX_LENGTH, 1);
> +    throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length  =
> +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> +                            QEMU_OPT_BPS_READ_MAX_LENGTH, 1);
> +    throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
> +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> +                            QEMU_OPT_BPS_WRITE_MAX_LENGTH, 1);
> +    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
> +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> +                            QEMU_OPT_IOPS_TOTAL_MAX_LENGTH, 1);
> +    throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
> +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> +                            QEMU_OPT_IOPS_READ_MAX_LENGTH, 1);
> +    throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
> +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> +                            QEMU_OPT_IOPS_WRITE_MAX_LENGTH, 1);
> +    throttle_cfg->op_size =
> +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_SIZE, 0);
> +}

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v5 1/6] fsdev-throttle-qmp: factor out throttle code to reuse code
  2018-11-22 14:46   ` Greg Kurz
@ 2018-11-23  6:42     ` xiezhide
  0 siblings, 0 replies; 15+ messages in thread
From: xiezhide @ 2018-11-23  6:42 UTC (permalink / raw)
  To: Greg Kurz
  Cc: qemu-devel, aneesh.kumar, eblake, armbru, berto,
	zengcanfu 00215970, Jinxuefeng, Chenhui (Felix, Euler),
	Pradeep Jagadeesh



> -----Original Message-----
> From: Greg Kurz [mailto:groug@kaod.org]
> Sent: Thursday, November 22, 2018 10:46 PM
> To: xiezhide <xiezhide@huawei.com>
> Cc: qemu-devel@nongnu.org; aneesh.kumar@linux.vnet.ibm.com;
> eblake@redhat.com; armbru@redhat.com; berto@igalia.com; zengcanfu
> 00215970 <zengcanfu@huawei.com>; Jinxuefeng <jinxuefeng@huawei.com>;
> Chenhui (Felix, Euler) <chenhui.rtos@huawei.com>
> Subject: Re: [PATCH v5 1/6] fsdev-throttle-qmp: factor out throttle code to
> reuse code
> 
> On Fri, 16 Nov 2018 15:59:16 +0800
> xiezhide <xiezhide@huawei.com> wrote:
> 
> > Factor out throttle parameter parsing code to a new common function
> > which will be used by block and fsdev.
> > Rename function throttle_parse_options to throttle_parse_group to
> > resolve function name conflict
> >
> > Reviewed-by: Eric Blake <eblake@redhat.com>
> > Signed-off-by: xiezhide <xiezhide@huawei.com>
> > ---
> 
> Reviewed-by: Greg Kurz <groug@kaod.org>
> 
> And, since I guess this will likely go through someone else's tree, for the fsdev
> changes:

Yes,Pradeep Jagadeesh<pradeep.jagadeesh@huawei.com> had done some work for this, and I take it over now

> 
> Acked-by: Greg Kurz <groug@kaod.org>
> 
> >  block/throttle.c                |  6 ++--
> >  blockdev.c                      | 43 +-------------------------
> >  fsdev/qemu-fsdev-throttle.c     | 44 ++------------------------
> >  include/qemu/throttle-options.h |  2 ++
> >  include/qemu/throttle.h         |  4 +--
> >  include/qemu/typedefs.h         |  1 +
> >  util/throttle.c                 | 68
> +++++++++++++++++++++++++++++++++++++++++
> >  7 files changed, 79 insertions(+), 89 deletions(-)
> >
> > diff --git a/block/throttle.c b/block/throttle.c index
> > 636c976..bd23c58 100644
> > --- a/block/throttle.c
> > +++ b/block/throttle.c
> > @@ -41,7 +41,7 @@ static QemuOptsList throttle_opts = {
> >   * @group and must be freed by the caller.
> >   * If there's an error then @group remains unmodified.
> >   */
> > -static int throttle_parse_options(QDict *options, char **group, Error
> > **errp)
> > +static int throttle_parse_group(QDict *options, char **group, Error
> > +**errp)
> >  {
> >      int ret;
> >      const char *group_name;
> > @@ -90,7 +90,7 @@ static int throttle_open(BlockDriverState *bs, QDict
> *options,
> >      bs->supported_zero_flags = bs->file->bs->supported_zero_flags |
> >                                 BDRV_REQ_WRITE_UNCHANGED;
> >
> > -    ret = throttle_parse_options(options, &group, errp);
> > +    ret = throttle_parse_group(options, &group, errp);
> >      if (ret == 0) {
> >          /* Register membership to group with name group_name */
> >          throttle_group_register_tgm(tgm, group,
> > bdrv_get_aio_context(bs)); @@ -179,7 +179,7 @@ static int
> throttle_reopen_prepare(BDRVReopenState *reopen_state,
> >      assert(reopen_state != NULL);
> >      assert(reopen_state->bs != NULL);
> >
> > -    ret = throttle_parse_options(reopen_state->options, &group, errp);
> > +    ret = throttle_parse_group(reopen_state->options, &group, errp);
> >      reopen_state->opaque = group;
> >      return ret;
> >  }
> > diff --git a/blockdev.c b/blockdev.c
> > index 81f95d9..fce5d8f 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -400,48 +400,7 @@ static void
> extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
> >      }
> >
> >      if (throttle_cfg) {
> > -        throttle_config_init(throttle_cfg);
> > -        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
> > -            qemu_opt_get_number(opts, "throttling.bps-total", 0);
> > -        throttle_cfg->buckets[THROTTLE_BPS_READ].avg  =
> > -            qemu_opt_get_number(opts, "throttling.bps-read", 0);
> > -        throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
> > -            qemu_opt_get_number(opts, "throttling.bps-write", 0);
> > -        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
> > -            qemu_opt_get_number(opts, "throttling.iops-total", 0);
> > -        throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
> > -            qemu_opt_get_number(opts, "throttling.iops-read", 0);
> > -        throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
> > -            qemu_opt_get_number(opts, "throttling.iops-write", 0);
> > -
> > -        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
> > -            qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
> > -        throttle_cfg->buckets[THROTTLE_BPS_READ].max  =
> > -            qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
> > -        throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
> > -            qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
> > -        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
> > -            qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
> > -        throttle_cfg->buckets[THROTTLE_OPS_READ].max =
> > -            qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
> > -        throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
> > -            qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
> > -
> > -        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
> > -            qemu_opt_get_number(opts,
> "throttling.bps-total-max-length", 1);
> > -        throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length  =
> > -            qemu_opt_get_number(opts,
> "throttling.bps-read-max-length", 1);
> > -        throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
> > -            qemu_opt_get_number(opts,
> "throttling.bps-write-max-length", 1);
> > -        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
> > -            qemu_opt_get_number(opts,
> "throttling.iops-total-max-length", 1);
> > -        throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
> > -            qemu_opt_get_number(opts,
> "throttling.iops-read-max-length", 1);
> > -        throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
> > -            qemu_opt_get_number(opts,
> "throttling.iops-write-max-length", 1);
> > -
> > -        throttle_cfg->op_size =
> > -            qemu_opt_get_number(opts, "throttling.iops-size", 0);
> > +        throttle_parse_options(throttle_cfg, opts);
> >
> >          if (!throttle_is_valid(throttle_cfg, errp)) {
> >              return;
> > diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
> > index cfd8641..6a4108a 100644
> > --- a/fsdev/qemu-fsdev-throttle.c
> > +++ b/fsdev/qemu-fsdev-throttle.c
> > @@ -17,6 +17,7 @@
> >  #include "qemu-fsdev-throttle.h"
> >  #include "qemu/iov.h"
> >  #include "qemu/option.h"
> > +#include "qemu/throttle-options.h"
> >
> >  static void fsdev_throttle_read_timer_cb(void *opaque)  { @@ -32,48
> > +33,7 @@ static void fsdev_throttle_write_timer_cb(void *opaque)
> >
> >  void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error
> > **errp)  {
> > -    throttle_config_init(&fst->cfg);
> > -    fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =
> > -        qemu_opt_get_number(opts, "throttling.bps-total", 0);
> > -    fst->cfg.buckets[THROTTLE_BPS_READ].avg  =
> > -        qemu_opt_get_number(opts, "throttling.bps-read", 0);
> > -    fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =
> > -        qemu_opt_get_number(opts, "throttling.bps-write", 0);
> > -    fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =
> > -        qemu_opt_get_number(opts, "throttling.iops-total", 0);
> > -    fst->cfg.buckets[THROTTLE_OPS_READ].avg =
> > -        qemu_opt_get_number(opts, "throttling.iops-read", 0);
> > -    fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =
> > -        qemu_opt_get_number(opts, "throttling.iops-write", 0);
> > -
> > -    fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =
> > -        qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
> > -    fst->cfg.buckets[THROTTLE_BPS_READ].max  =
> > -        qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
> > -    fst->cfg.buckets[THROTTLE_BPS_WRITE].max =
> > -        qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
> > -    fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =
> > -        qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
> > -    fst->cfg.buckets[THROTTLE_OPS_READ].max =
> > -        qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
> > -    fst->cfg.buckets[THROTTLE_OPS_WRITE].max =
> > -        qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
> > -
> > -    fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =
> > -        qemu_opt_get_number(opts, "throttling.bps-total-max-length",
> 1);
> > -    fst->cfg.buckets[THROTTLE_BPS_READ].burst_length  =
> > -        qemu_opt_get_number(opts, "throttling.bps-read-max-length",
> 1);
> > -    fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =
> > -        qemu_opt_get_number(opts, "throttling.bps-write-max-length",
> 1);
> > -    fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =
> > -        qemu_opt_get_number(opts, "throttling.iops-total-max-length",
> 1);
> > -    fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =
> > -        qemu_opt_get_number(opts, "throttling.iops-read-max-length",
> 1);
> > -    fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =
> > -        qemu_opt_get_number(opts, "throttling.iops-write-max-length",
> 1);
> > -    fst->cfg.op_size =
> > -        qemu_opt_get_number(opts, "throttling.iops-size", 0);
> > -
> > +    throttle_parse_options(&fst->cfg, opts);
> >      throttle_is_valid(&fst->cfg, errp);  }
> >
> > diff --git a/include/qemu/throttle-options.h
> > b/include/qemu/throttle-options.h index 3528a8f..944a08c 100644
> > --- a/include/qemu/throttle-options.h
> > +++ b/include/qemu/throttle-options.h
> > @@ -111,4 +111,6 @@
> >              .help = "when limiting by iops max size of an I/O in bytes",\
> >          }
> >
> > +void throttle_parse_options(ThrottleConfig *, QemuOpts *);
> > +
> >  #endif
> > diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h index
> > abeb886..f379d91 100644
> > --- a/include/qemu/throttle.h
> > +++ b/include/qemu/throttle.h
> > @@ -90,10 +90,10 @@ typedef struct LeakyBucket {
> >   * However it allows to keep the code clean and the bucket field is reset to
> >   * zero at the right time.
> >   */
> > -typedef struct ThrottleConfig {
> > +struct ThrottleConfig {
> >      LeakyBucket buckets[BUCKETS_COUNT]; /* leaky buckets */
> >      uint64_t op_size;         /* size of an operation in bytes */
> > -} ThrottleConfig;
> > +};
> >
> >  typedef struct ThrottleState {
> >      ThrottleConfig cfg;       /* configuration */
> > diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h index
> > 3ec0e13..0d75edc 100644
> > --- a/include/qemu/typedefs.h
> > +++ b/include/qemu/typedefs.h
> > @@ -109,6 +109,7 @@ typedef struct SerialState SerialState;  typedef
> > struct SHPCDevice SHPCDevice;  typedef struct SMBusDevice SMBusDevice;
> > typedef struct SSIBus SSIBus;
> > +typedef struct ThrottleConfig ThrottleConfig;
> >  typedef struct uWireSlave uWireSlave;  typedef struct VirtIODevice
> > VirtIODevice;  typedef struct Visitor Visitor; diff --git
> > a/util/throttle.c b/util/throttle.c index b38e742..e7db2ad 100644
> > --- a/util/throttle.c
> > +++ b/util/throttle.c
> > @@ -27,6 +27,8 @@
> >  #include "qemu/throttle.h"
> >  #include "qemu/timer.h"
> >  #include "block/aio.h"
> > +#include "qemu/option.h"
> > +#include "qemu/throttle-options.h"
> >
> >  /* This function make a bucket leak
> >   *
> > @@ -636,3 +638,69 @@ void throttle_config_to_limits(ThrottleConfig *cfg,
> ThrottleLimits *var)
> >      var->has_iops_write_max_length = true;
> >      var->has_iops_size = true;
> >  }
> > +
> > +/* parse the throttle options
> > + *
> > + * @opts: qemu options
> > + * @throttle_cfg: throttle configuration  */ void
> > +throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts *opts)
> > +{
> > +    throttle_config_init(throttle_cfg);
> > +    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
> > +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> > +                            QEMU_OPT_BPS_TOTAL, 0);
> > +    throttle_cfg->buckets[THROTTLE_BPS_READ].avg  =
> > +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> > +                            QEMU_OPT_BPS_READ, 0);
> > +    throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
> > +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> > +                            QEMU_OPT_BPS_WRITE, 0);
> > +    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
> > +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> > +                            QEMU_OPT_IOPS_TOTAL, 0);
> > +    throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
> > +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> > +                            QEMU_OPT_IOPS_READ, 0);
> > +    throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
> > +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> > +                            QEMU_OPT_IOPS_WRITE, 0);
> > +    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
> > +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> > +                            QEMU_OPT_BPS_TOTAL_MAX, 0);
> > +    throttle_cfg->buckets[THROTTLE_BPS_READ].max  =
> > +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> > +                            QEMU_OPT_BPS_READ_MAX, 0);
> > +    throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
> > +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> > +                            QEMU_OPT_BPS_WRITE_MAX, 0);
> > +    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
> > +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> > +                            QEMU_OPT_IOPS_TOTAL_MAX, 0);
> > +    throttle_cfg->buckets[THROTTLE_OPS_READ].max =
> > +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> > +                            QEMU_OPT_IOPS_READ_MAX, 0);
> > +    throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
> > +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> > +                            QEMU_OPT_IOPS_WRITE_MAX, 0);
> > +    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
> > +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> > +                            QEMU_OPT_BPS_TOTAL_MAX_LENGTH,
> 1);
> > +    throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length  =
> > +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> > +                            QEMU_OPT_BPS_READ_MAX_LENGTH,
> 1);
> > +    throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
> > +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> > +                            QEMU_OPT_BPS_WRITE_MAX_LENGTH,
> 1);
> > +    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
> > +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> > +                            QEMU_OPT_IOPS_TOTAL_MAX_LENGTH,
> 1);
> > +    throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
> > +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> > +                            QEMU_OPT_IOPS_READ_MAX_LENGTH,
> 1);
> > +    throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
> > +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> > +                            QEMU_OPT_IOPS_WRITE_MAX_LENGTH,
> 1);
> > +    throttle_cfg->op_size =
> > +        qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX
> > +QEMU_OPT_IOPS_SIZE, 0); }


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v5 2/6] fsdev-throttle-qmp: Rename the ThrottleLimits member names
  2018-11-16  7:59 ` [Qemu-devel] [PATCH v5 2/6] fsdev-throttle-qmp: Rename the ThrottleLimits member names xiezhide
@ 2018-11-28  9:25   ` Markus Armbruster
  2018-11-28 13:09     ` Eric Blake
  2018-11-29  7:10     ` xiezhide
  0 siblings, 2 replies; 15+ messages in thread
From: Markus Armbruster @ 2018-11-28  9:25 UTC (permalink / raw)
  To: xiezhide
  Cc: qemu-devel, berto, armbru, zengcanfu, groug, aneesh.kumar,
	jinxuefeng, chenhui.rtos

xiezhide <xiezhide@huawei.com> writes:

> Rename the ThrottleLimits member names and modify related code
>
> Signed-off-by: xiezhide <xiezhide@huawei.com>
> ---
>  qapi/block-core.json |  70 +++++++++++-----------
>  util/throttle.c      | 163 +++++++++++++++++++++++++--------------------------
>  2 files changed, 116 insertions(+), 117 deletions(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index d4fe710..4ffaaea 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2240,45 +2240,45 @@
>  # transaction. All fields are optional. When setting limits, if a field is
>  # missing the current value is not changed.
>  #
> -# @iops-total:             limit total I/O operations per second
> -# @iops-total-max:         I/O operations burst
> -# @iops-total-max-length:  length of the iops-total-max burst period, in seconds
> -#                          It must only be set if @iops-total-max is set as well.
> -# @iops-read:              limit read operations per second
> -# @iops-read-max:          I/O operations read burst
> -# @iops-read-max-length:   length of the iops-read-max burst period, in seconds
> -#                          It must only be set if @iops-read-max is set as well.
> -# @iops-write:             limit write operations per second
> -# @iops-write-max:         I/O operations write burst
> -# @iops-write-max-length:  length of the iops-write-max burst period, in seconds
> -#                          It must only be set if @iops-write-max is set as well.
> -# @bps-total:              limit total bytes per second
> -# @bps-total-max:          total bytes burst
> -# @bps-total-max-length:   length of the bps-total-max burst period, in seconds.
> -#                          It must only be set if @bps-total-max is set as well.
> -# @bps-read:               limit read bytes per second
> -# @bps-read-max:           total bytes read burst
> -# @bps-read-max-length:    length of the bps-read-max burst period, in seconds
> -#                          It must only be set if @bps-read-max is set as well.
> -# @bps-write:              limit write bytes per second
> -# @bps-write-max:          total bytes write burst
> -# @bps-write-max-length:   length of the bps-write-max burst period, in seconds
> -#                          It must only be set if @bps-write-max is set as well.
> -# @iops-size:              when limiting by iops max size of an I/O in bytes
> +# @iops:             limit total I/O operations per second
> +# @iops_max:         I/O operations burst
> +# @iops_max_length:  length of the iops_total_max burst period, in seconds
> +#                          It must only be set if @iops_total_max is set as well.
> +# @iops_rd:              limit read operations per second
> +# @iops_rd_max:          I/O operations read burst
> +# @iops_rd_max_length:   length of the iops_read_max burst period, in seconds
> +#                          It must only be set if @iops_read_max is set as well.
> +# @iops_wr:             limit write operations per second
> +# @iops_wr_max:         I/O operations write burst
> +# @iops_wr_max_length:  length of the iops_write_max burst period, in seconds
> +#                          It must only be set if @iops_write_max is set as well.
> +# @bps:              limit total bytes per second
> +# @bps_max:          total bytes burst
> +# @bps_max_length:   length of the bps_total_max burst period, in seconds.
> +#                          It must only be set if @bps_total_max is set as well.
> +# @bps_rd:               limit read bytes per second
> +# @bps_rd_max:           total bytes read burst
> +# @bps_rd_max_length:    length of the bps_read_max burst period, in seconds
> +#                          It must only be set if @bps_read_max is set as well.
> +# @bps_wr:              limit write bytes per second
> +# @bps_wr_max:          total bytes write burst
> +# @bps_wr_max_length:   length of the bps_write_max burst period, in seconds
> +#                          It must only be set if @bps_write_max is set as well.
> +# @iops_size:              when limiting by iops max size of an I/O in bytes
>  #
>  # Since: 2.11
>  ##
>  { 'struct': 'ThrottleLimits',
> -  'data': { '*iops-total' : 'int', '*iops-total-max' : 'int',
> -            '*iops-total-max-length' : 'int', '*iops-read' : 'int',
> -            '*iops-read-max' : 'int', '*iops-read-max-length' : 'int',
> -            '*iops-write' : 'int', '*iops-write-max' : 'int',
> -            '*iops-write-max-length' : 'int', '*bps-total' : 'int',
> -            '*bps-total-max' : 'int', '*bps-total-max-length' : 'int',
> -            '*bps-read' : 'int', '*bps-read-max' : 'int',
> -            '*bps-read-max-length' : 'int', '*bps-write' : 'int',
> -            '*bps-write-max' : 'int', '*bps-write-max-length' : 'int',
> -            '*iops-size' : 'int' } }
> +  'data': { '*iops' : 'int', '*iops_max' : 'int',
> +            '*iops_max_length' : 'int', '*iops_rd' : 'int',
> +            '*iops_rd_max' : 'int', '*iops_rd_max_length' : 'int',
> +            '*iops_wr' : 'int', '*iops_wr_max' : 'int',
> +            '*iops_wr_max_length' : 'int', '*bps' : 'int',
> +            '*bps_max' : 'int', '*bps_max_length' : 'int',
> +            '*bps_rd' : 'int', '*bps_rd_max' : 'int',
> +            '*bps_rd_max_length' : 'int', '*bps_wr' : 'int',
> +            '*bps_wr_max' : 'int', '*bps_wr_max_length' : 'int',
> +            '*iops_size' : 'int' } }

Compatibility break.  Why is that okay?

Even if it is, you still run afoul of docs/devel/qapi-code-gen.txt:

    Command names, and member names within a type, should be all lower
    case with words separated by a hyphen.  However, some existing older
    commands and complex types use underscore; when extending such
    expressions, consistency is preferred over blindly avoiding
    underscore.

The exception doesn't apply here.

>  
>  ##
>  # @block-stream:
[...]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v5 2/6] fsdev-throttle-qmp: Rename the ThrottleLimits member names
  2018-11-28  9:25   ` Markus Armbruster
@ 2018-11-28 13:09     ` Eric Blake
  2018-11-29  7:23       ` xiezhide
  2018-11-29  8:59       ` Markus Armbruster
  2018-11-29  7:10     ` xiezhide
  1 sibling, 2 replies; 15+ messages in thread
From: Eric Blake @ 2018-11-28 13:09 UTC (permalink / raw)
  To: Markus Armbruster, xiezhide
  Cc: berto, qemu-devel, groug, zengcanfu, aneesh.kumar, jinxuefeng,
	chenhui.rtos

On 11/28/18 3:25 AM, Markus Armbruster wrote:
> xiezhide <xiezhide@huawei.com> writes:
> 
>> Rename the ThrottleLimits member names and modify related code
>>
>> Signed-off-by: xiezhide <xiezhide@huawei.com>
>> ---
>>   qapi/block-core.json |  70 +++++++++++-----------
>>   util/throttle.c      | 163 +++++++++++++++++++++++++--------------------------
>>   2 files changed, 116 insertions(+), 117 deletions(-)
>>
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index d4fe710..4ffaaea 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json

>>   ##
>>   { 'struct': 'ThrottleLimits',
>> -  'data': { '*iops-total' : 'int', '*iops-total-max' : 'int',

>> +  'data': { '*iops' : 'int', '*iops_max' : 'int',
>> +            '*iops_max_length' : 'int', '*iops_rd' : 'int',
>> +            '*iops_rd_max' : 'int', '*iops_rd_max_length' : 'int',
>> +            '*iops_wr' : 'int', '*iops_wr_max' : 'int',
>> +            '*iops_wr_max_length' : 'int', '*bps' : 'int',
>> +            '*bps_max' : 'int', '*bps_max_length' : 'int',
>> +            '*bps_rd' : 'int', '*bps_rd_max' : 'int',
>> +            '*bps_rd_max_length' : 'int', '*bps_wr' : 'int',
>> +            '*bps_wr_max' : 'int', '*bps_wr_max_length' : 'int',
>> +            '*iops_size' : 'int' } }
> 
> Compatibility break.  Why is that okay?

Grepping qapi/qapi-introspection.c shows 0 hits for either 
ThrottleLimits or for iops-total, so there are no QMP commands affected. 
There might, however, be command line and/or QOM paths affected, which 
is harder to audit since those don't affect instrospection.

> 
> Even if it is, you still run afoul of docs/devel/qapi-code-gen.txt:
> 
>      Command names, and member names within a type, should be all lower
>      case with words separated by a hyphen.  However, some existing older
>      commands and complex types use underscore; when extending such
>      expressions, consistency is preferred over blindly avoiding
>      underscore.
> 
> The exception doesn't apply here.

Ah, but it does, because we are refactoring code to share a common QAPI 
struct in a later patch, where we need this exact naming to avoid 
breaking that command.

So the REAL problem with this commit is that the commit message does not 
give enough details, either why this is safe (because it does not impact 
existing QMP commands) or needed (because we will be using it to rewrite 
an existing QMP command that needs this spelling).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v5 2/6] fsdev-throttle-qmp: Rename the ThrottleLimits member names
  2018-11-28  9:25   ` Markus Armbruster
  2018-11-28 13:09     ` Eric Blake
@ 2018-11-29  7:10     ` xiezhide
  1 sibling, 0 replies; 15+ messages in thread
From: xiezhide @ 2018-11-29  7:10 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: qemu-devel, berto, zengcanfu 00215970, groug, aneesh.kumar,
	Jinxuefeng, Chenhui (Felix, Euler)


> Subject: Re: [Qemu-devel] [PATCH v5 2/6] fsdev-throttle-qmp: Rename the
> ThrottleLimits member names
> 
> xiezhide <xiezhide@huawei.com> writes:
> 
> > Rename the ThrottleLimits member names and modify related code
> >
> > Signed-off-by: xiezhide <xiezhide@huawei.com>
> > ---
> >  qapi/block-core.json |  70 +++++++++++-----------
> >  util/throttle.c      | 163
> +++++++++++++++++++++++++--------------------------
> >  2 files changed, 116 insertions(+), 117 deletions(-)
> >
> > diff --git a/qapi/block-core.json b/qapi/block-core.json index
> > d4fe710..4ffaaea 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -2240,45 +2240,45 @@
> >  # transaction. All fields are optional. When setting limits, if a
> > field is  # missing the current value is not changed.
> >  #
> > -# @iops-total:             limit total I/O operations per second
> > -# @iops-total-max:         I/O operations burst
> > -# @iops-total-max-length:  length of the iops-total-max burst period, in
> seconds
> > -#                          It must only be set if @iops-total-max is set
> as well.
> > -# @iops-read:              limit read operations per second
> > -# @iops-read-max:          I/O operations read burst
> > -# @iops-read-max-length:   length of the iops-read-max burst period, in
> seconds
> > -#                          It must only be set if @iops-read-max is set
> as well.
> > -# @iops-write:             limit write operations per second
> > -# @iops-write-max:         I/O operations write burst
> > -# @iops-write-max-length:  length of the iops-write-max burst period, in
> seconds
> > -#                          It must only be set if @iops-write-max is
> set as well.
> > -# @bps-total:              limit total bytes per second
> > -# @bps-total-max:          total bytes burst
> > -# @bps-total-max-length:   length of the bps-total-max burst period, in
> seconds.
> > -#                          It must only be set if @bps-total-max is set
> as well.
> > -# @bps-read:               limit read bytes per second
> > -# @bps-read-max:           total bytes read burst
> > -# @bps-read-max-length:    length of the bps-read-max burst period, in
> seconds
> > -#                          It must only be set if @bps-read-max is set
> as well.
> > -# @bps-write:              limit write bytes per second
> > -# @bps-write-max:          total bytes write burst
> > -# @bps-write-max-length:   length of the bps-write-max burst period, in
> seconds
> > -#                          It must only be set if @bps-write-max is set
> as well.
> > -# @iops-size:              when limiting by iops max size of an I/O in
> bytes
> > +# @iops:             limit total I/O operations per second
> > +# @iops_max:         I/O operations burst
> > +# @iops_max_length:  length of the iops_total_max burst period, in
> seconds
> > +#                          It must only be set if @iops_total_max is
> set as well.
> > +# @iops_rd:              limit read operations per second
> > +# @iops_rd_max:          I/O operations read burst
> > +# @iops_rd_max_length:   length of the iops_read_max burst period, in
> seconds
> > +#                          It must only be set if @iops_read_max is
> set as well.
> > +# @iops_wr:             limit write operations per second
> > +# @iops_wr_max:         I/O operations write burst
> > +# @iops_wr_max_length:  length of the iops_write_max burst period, in
> seconds
> > +#                          It must only be set if @iops_write_max is
> set as well.
> > +# @bps:              limit total bytes per second
> > +# @bps_max:          total bytes burst
> > +# @bps_max_length:   length of the bps_total_max burst period, in
> seconds.
> > +#                          It must only be set if @bps_total_max is
> set as well.
> > +# @bps_rd:               limit read bytes per second
> > +# @bps_rd_max:           total bytes read burst
> > +# @bps_rd_max_length:    length of the bps_read_max burst period, in
> seconds
> > +#                          It must only be set if @bps_read_max is
> set as well.
> > +# @bps_wr:              limit write bytes per second
> > +# @bps_wr_max:          total bytes write burst
> > +# @bps_wr_max_length:   length of the bps_write_max burst period, in
> seconds
> > +#                          It must only be set if @bps_write_max is
> set as well.
> > +# @iops_size:              when limiting by iops max size of an I/O in
> bytes
> >  #
> >  # Since: 2.11
> >  ##
> >  { 'struct': 'ThrottleLimits',
> > -  'data': { '*iops-total' : 'int', '*iops-total-max' : 'int',
> > -            '*iops-total-max-length' : 'int', '*iops-read' : 'int',
> > -            '*iops-read-max' : 'int', '*iops-read-max-length' : 'int',
> > -            '*iops-write' : 'int', '*iops-write-max' : 'int',
> > -            '*iops-write-max-length' : 'int', '*bps-total' : 'int',
> > -            '*bps-total-max' : 'int', '*bps-total-max-length' : 'int',
> > -            '*bps-read' : 'int', '*bps-read-max' : 'int',
> > -            '*bps-read-max-length' : 'int', '*bps-write' : 'int',
> > -            '*bps-write-max' : 'int', '*bps-write-max-length' : 'int',
> > -            '*iops-size' : 'int' } }
> > +  'data': { '*iops' : 'int', '*iops_max' : 'int',
> > +            '*iops_max_length' : 'int', '*iops_rd' : 'int',
> > +            '*iops_rd_max' : 'int', '*iops_rd_max_length' : 'int',
> > +            '*iops_wr' : 'int', '*iops_wr_max' : 'int',
> > +            '*iops_wr_max_length' : 'int', '*bps' : 'int',
> > +            '*bps_max' : 'int', '*bps_max_length' : 'int',
> > +            '*bps_rd' : 'int', '*bps_rd_max' : 'int',
> > +            '*bps_rd_max_length' : 'int', '*bps_wr' : 'int',
> > +            '*bps_wr_max' : 'int', '*bps_wr_max_length' : 'int',
> > +            '*iops_size' : 'int' } }
> 
> Compatibility break.  Why is that okay?
> 
> Even if it is, you still run afoul of docs/devel/qapi-code-gen.txt:
> 
>     Command names, and member names within a type, should be all lower
>     case with words separated by a hyphen.  However, some existing older
>     commands and complex types use underscore; when extending such
>     expressions, consistency is preferred over blindly avoiding
>     underscore.
> 
> The exception doesn't apply here.

Why did this changing:
1. ThrottleLimits is just use for block device, this change not break compatibility
2. can share same base struct in json file
3. can reuse same code between block and fsdev device for io throttling
4. can keep the same qmp command style for block and fsdev with less codes

Block:
{ "execute": "block_set_io_throttle",
     "arguments": {
        "device": "virtio0",
        "iops": 100,
        "iops_rd": 0,
        "iops_wr": 0,
        "bps": 0,
        "bps_rd": 0,
        "bps_wr": 0
     }
   }

Fsdev:
{ "execute": "fsdev-set-io-throttle",
    "arguments": {
        "id": "extra-9p-kataTest"
        "iops": 100,
        "iops_rd": 0,
        "iops_wr": 0,
        "bps": 0,
        "bps_rd": 0,
        "bps_wr": 0
     }
   }

Thanks
xiezhide

> 
> >
> >  ##
> >  # @block-stream:
> [...]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v5 2/6] fsdev-throttle-qmp: Rename the ThrottleLimits member names
  2018-11-28 13:09     ` Eric Blake
@ 2018-11-29  7:23       ` xiezhide
  2018-11-29  8:59       ` Markus Armbruster
  1 sibling, 0 replies; 15+ messages in thread
From: xiezhide @ 2018-11-29  7:23 UTC (permalink / raw)
  To: Eric Blake, Markus Armbruster
  Cc: berto, qemu-devel, groug, zengcanfu 00215970, aneesh.kumar,
	Jinxuefeng, Chenhui (Felix, Euler)


> Subject: Re: [Qemu-devel] [PATCH v5 2/6] fsdev-throttle-qmp: Rename the
> ThrottleLimits member names
> 
> On 11/28/18 3:25 AM, Markus Armbruster wrote:
> > xiezhide <xiezhide@huawei.com> writes:
> >
> >> Rename the ThrottleLimits member names and modify related code
> >>
> >> Signed-off-by: xiezhide <xiezhide@huawei.com>
> >> ---
> >>   qapi/block-core.json |  70 +++++++++++-----------
> >>   util/throttle.c      | 163
> +++++++++++++++++++++++++--------------------------
> >>   2 files changed, 116 insertions(+), 117 deletions(-)
> >>
> >> diff --git a/qapi/block-core.json b/qapi/block-core.json index
> >> d4fe710..4ffaaea 100644
> >> --- a/qapi/block-core.json
> >> +++ b/qapi/block-core.json
> 
> >>   ##
> >>   { 'struct': 'ThrottleLimits',
> >> -  'data': { '*iops-total' : 'int', '*iops-total-max' : 'int',
> 
> >> +  'data': { '*iops' : 'int', '*iops_max' : 'int',
> >> +            '*iops_max_length' : 'int', '*iops_rd' : 'int',
> >> +            '*iops_rd_max' : 'int', '*iops_rd_max_length' : 'int',
> >> +            '*iops_wr' : 'int', '*iops_wr_max' : 'int',
> >> +            '*iops_wr_max_length' : 'int', '*bps' : 'int',
> >> +            '*bps_max' : 'int', '*bps_max_length' : 'int',
> >> +            '*bps_rd' : 'int', '*bps_rd_max' : 'int',
> >> +            '*bps_rd_max_length' : 'int', '*bps_wr' : 'int',
> >> +            '*bps_wr_max' : 'int', '*bps_wr_max_length' : 'int',
> >> +            '*iops_size' : 'int' } }
> >
> > Compatibility break.  Why is that okay?
> 
> Grepping qapi/qapi-introspection.c shows 0 hits for either ThrottleLimits or for
> iops-total, so there are no QMP commands affected.
> There might, however, be command line and/or QOM paths affected, which is
> harder to audit since those don't affect instrospection.
> 
> >
> > Even if it is, you still run afoul of docs/devel/qapi-code-gen.txt:
> >
> >      Command names, and member names within a type, should be all
> lower
> >      case with words separated by a hyphen.  However, some existing
> older
> >      commands and complex types use underscore; when extending such
> >      expressions, consistency is preferred over blindly avoiding
> >      underscore.
> >
> > The exception doesn't apply here.
> 
> Ah, but it does, because we are refactoring code to share a common QAPI
> struct in a later patch, where we need this exact naming to avoid breaking that
> command.
> 
> So the REAL problem with this commit is that the commit message does not
> give enough details, either why this is safe (because it does not impact existing
> QMP commands) or needed (because we will be using it to rewrite an existing
> QMP command that needs this spelling).
> 

@Erick, thanks for your simple but exact explaining for purpose of this patch

Thanks
xiezhide


> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v5 2/6] fsdev-throttle-qmp: Rename the ThrottleLimits member names
  2018-11-28 13:09     ` Eric Blake
  2018-11-29  7:23       ` xiezhide
@ 2018-11-29  8:59       ` Markus Armbruster
  2018-11-30  1:39         ` xiezhide
  1 sibling, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2018-11-29  8:59 UTC (permalink / raw)
  To: Eric Blake
  Cc: xiezhide, berto, qemu-devel, zengcanfu, groug, aneesh.kumar,
	jinxuefeng, chenhui.rtos

Eric Blake <eblake@redhat.com> writes:

> On 11/28/18 3:25 AM, Markus Armbruster wrote:
>> xiezhide <xiezhide@huawei.com> writes:
>>
>>> Rename the ThrottleLimits member names and modify related code
>>>
>>> Signed-off-by: xiezhide <xiezhide@huawei.com>
>>> ---
>>>   qapi/block-core.json |  70 +++++++++++-----------
>>>   util/throttle.c      | 163 +++++++++++++++++++++++++--------------------------
>>>   2 files changed, 116 insertions(+), 117 deletions(-)
>>>
>>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>>> index d4fe710..4ffaaea 100644
>>> --- a/qapi/block-core.json
>>> +++ b/qapi/block-core.json
>
>>>   ##
>>>   { 'struct': 'ThrottleLimits',
>>> -  'data': { '*iops-total' : 'int', '*iops-total-max' : 'int',
>
>>> +  'data': { '*iops' : 'int', '*iops_max' : 'int',
>>> +            '*iops_max_length' : 'int', '*iops_rd' : 'int',
>>> +            '*iops_rd_max' : 'int', '*iops_rd_max_length' : 'int',
>>> +            '*iops_wr' : 'int', '*iops_wr_max' : 'int',
>>> +            '*iops_wr_max_length' : 'int', '*bps' : 'int',
>>> +            '*bps_max' : 'int', '*bps_max_length' : 'int',
>>> +            '*bps_rd' : 'int', '*bps_rd_max' : 'int',
>>> +            '*bps_rd_max_length' : 'int', '*bps_wr' : 'int',
>>> +            '*bps_wr_max' : 'int', '*bps_wr_max_length' : 'int',
>>> +            '*iops_size' : 'int' } }
>>
>> Compatibility break.  Why is that okay?
>
> Grepping qapi/qapi-introspection.c shows 0 hits for either
> ThrottleLimits or for iops-total, so there are no QMP commands
> affected.

I see.

>           There might, however, be command line and/or QOM paths
> affected, which is harder to audit since those don't affect
> instrospection.

Yet another argument for QAPIfication...

Let's check.  git-grep ThrottleLimits finds:

* block/throttle-groups.c: getter and setter for QOM class
  "throttle-group" property "limits".  This class is user-creatable.
  Therefore, this is an externally visible interface.

  The question is whether it's a *stable* interface.  The status of QOM
  properties in that regard is not clear to me.  If I remember
  correctly, we treated it as "just for debugging and such" at least
  initially.  But is that still a defensible position?  It ceases to be
  one as soon as you need QOM properties to do something users ought to
  be able to do.  Like configuring stuff that is meant to be configured.
  If that's the case, we need to examine the situation, and clarify our
  ABI promises.  Recommend a separate thread.

* util/throttle.c: Functions to convert to and from ThrottleConfig.  The
  conversion to ThrottleConfig uses hardcoded member names in error
  messages.  Relevant only insofar we need to remember to change them
  when we rename members.

>> Even if it is, you still run afoul of docs/devel/qapi-code-gen.txt:
>>
>>      Command names, and member names within a type, should be all lower
>>      case with words separated by a hyphen.  However, some existing older
>>      commands and complex types use underscore; when extending such
>>      expressions, consistency is preferred over blindly avoiding
>>      underscore.
>>
>> The exception doesn't apply here.
>
> Ah, but it does, because we are refactoring code to share a common
> QAPI struct in a later patch, where we need this exact naming to avoid
> breaking that command.
>
> So the REAL problem with this commit is that the commit message does
> not give enough details, either why this is safe (because it does not
> impact existing QMP commands) or needed (because we will be using it
> to rewrite an existing QMP command that needs this spelling).

Yes, the commit message needs to make explain why the change is useful,
and why it is safe.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v5 2/6] fsdev-throttle-qmp: Rename the ThrottleLimits member names
  2018-11-29  8:59       ` Markus Armbruster
@ 2018-11-30  1:39         ` xiezhide
  0 siblings, 0 replies; 15+ messages in thread
From: xiezhide @ 2018-11-30  1:39 UTC (permalink / raw)
  To: Markus Armbruster, Eric Blake
  Cc: berto, qemu-devel, zengcanfu 00215970, groug, aneesh.kumar,
	Jinxuefeng, Chenhui (Felix, Euler)

> 
> Eric Blake <eblake@redhat.com> writes:
> 
> > On 11/28/18 3:25 AM, Markus Armbruster wrote:
> >> xiezhide <xiezhide@huawei.com> writes:
> >>
> >>> Rename the ThrottleLimits member names and modify related code
> >>>
> >>> Signed-off-by: xiezhide <xiezhide@huawei.com>
> >>> ---
> >>>   qapi/block-core.json |  70 +++++++++++-----------
> >>>   util/throttle.c      | 163
> +++++++++++++++++++++++++--------------------------
> >>>   2 files changed, 116 insertions(+), 117 deletions(-)
> >>>
> >>> diff --git a/qapi/block-core.json b/qapi/block-core.json index
> >>> d4fe710..4ffaaea 100644
> >>> --- a/qapi/block-core.json
> >>> +++ b/qapi/block-core.json
> >
> >>>   ##
> >>>   { 'struct': 'ThrottleLimits',
> >>> -  'data': { '*iops-total' : 'int', '*iops-total-max' : 'int',
> >
> >>> +  'data': { '*iops' : 'int', '*iops_max' : 'int',
> >>> +            '*iops_max_length' : 'int', '*iops_rd' : 'int',
> >>> +            '*iops_rd_max' : 'int', '*iops_rd_max_length' : 'int',
> >>> +            '*iops_wr' : 'int', '*iops_wr_max' : 'int',
> >>> +            '*iops_wr_max_length' : 'int', '*bps' : 'int',
> >>> +            '*bps_max' : 'int', '*bps_max_length' : 'int',
> >>> +            '*bps_rd' : 'int', '*bps_rd_max' : 'int',
> >>> +            '*bps_rd_max_length' : 'int', '*bps_wr' : 'int',
> >>> +            '*bps_wr_max' : 'int', '*bps_wr_max_length' : 'int',
> >>> +            '*iops_size' : 'int' } }
> >>
> >> Compatibility break.  Why is that okay?
> >
> > Grepping qapi/qapi-introspection.c shows 0 hits for either
> > ThrottleLimits or for iops-total, so there are no QMP commands
> > affected.
> 
> I see.
> 
> >           There might, however, be command line and/or QOM paths
> > affected, which is harder to audit since those don't affect
> > instrospection.
> 
> Yet another argument for QAPIfication...
> 
> Let's check.  git-grep ThrottleLimits finds:
> 
> * block/throttle-groups.c: getter and setter for QOM class
>   "throttle-group" property "limits".  This class is user-creatable.
>   Therefore, this is an externally visible interface.
> 
>   The question is whether it's a *stable* interface.  The status of QOM
>   properties in that regard is not clear to me.  If I remember
>   correctly, we treated it as "just for debugging and such" at least
>   initially.  But is that still a defensible position?  It ceases to be
>   one as soon as you need QOM properties to do something users ought to
>   be able to do.  Like configuring stuff that is meant to be configured.
>   If that's the case, we need to examine the situation, and clarify our
>   ABI promises.  Recommend a separate thread.
> 
> * util/throttle.c: Functions to convert to and from ThrottleConfig.  The
>   conversion to ThrottleConfig uses hardcoded member names in error
>   messages.  Relevant only insofar we need to remember to change them
>   when we rename members.
Yes, find few hard coded member names in error message need to change.
Will fix it in new version

 
> >> Even if it is, you still run afoul of docs/devel/qapi-code-gen.txt:
> >>
> >>      Command names, and member names within a type, should be all
> lower
> >>      case with words separated by a hyphen.  However, some existing
> older
> >>      commands and complex types use underscore; when extending such
> >>      expressions, consistency is preferred over blindly avoiding
> >>      underscore.
> >>
> >> The exception doesn't apply here.
> >
> > Ah, but it does, because we are refactoring code to share a common
> > QAPI struct in a later patch, where we need this exact naming to avoid
> > breaking that command.
> >
> > So the REAL problem with this commit is that the commit message does
> > not give enough details, either why this is safe (because it does not
> > impact existing QMP commands) or needed (because we will be using it
> > to rewrite an existing QMP command that needs this spelling).
> 
> Yes, the commit message needs to make explain why the change is useful, and
> why it is safe.


Strongly endorse , will with clear and detail commit message next


Thanks
xiezhide

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2018-11-30  1:39 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-16  7:58 [Qemu-devel] [PATCH v5 0/6] fsdev-throttle-qmp: qmp interface for fsdev io throttling xiezhide
2018-11-16  7:59 ` [Qemu-devel] [PATCH v5 1/6] fsdev-throttle-qmp: factor out throttle code to reuse code xiezhide
2018-11-22 14:46   ` Greg Kurz
2018-11-23  6:42     ` xiezhide
2018-11-16  7:59 ` [Qemu-devel] [PATCH v5 2/6] fsdev-throttle-qmp: Rename the ThrottleLimits member names xiezhide
2018-11-28  9:25   ` Markus Armbruster
2018-11-28 13:09     ` Eric Blake
2018-11-29  7:23       ` xiezhide
2018-11-29  8:59       ` Markus Armbruster
2018-11-30  1:39         ` xiezhide
2018-11-29  7:10     ` xiezhide
2018-11-16  7:59 ` [Qemu-devel] [PATCH v5 3/6] fsdev-throttle-qmp: Rewrite BlockIOThrottle with ThrottleLimits as its base class xiezhide
2018-11-16  8:00 ` [Qemu-devel] [PATCH v5 4/6] fsdev-throttle-qmp: Move ThrottleLimits into a new file for future reuse xiezhide
2018-11-16  8:00 ` [Qemu-devel] [PATCH v5 5/6] fsdev-throttle-qmp: qmp interface for fsdev io throttling xiezhide
2018-11-16  8:00 ` [Qemu-devel] [PATCH v5 6/6] fsdev-throttle-qmp: hmp " xiezhide

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.