* [Qemu-devel] [PATCH v4 1/4] fsdev-throttle-qmp: factor out throttle code to reuse code
2018-11-15 8:54 [Qemu-devel] [PATCH v4 0/4] fsdev-throttle-qmp: qmp interface for fsdev io throttling xiezhide
@ 2018-11-15 8:54 ` xiezhide
2018-11-15 20:55 ` Eric Blake
2018-11-15 8:55 ` [Qemu-devel] [PATCH v4 2/4] fsdev-throttle-qmp: move struct ThrottleLimits to new file xiezhide
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: xiezhide @ 2018-11-15 8:54 UTC (permalink / raw)
To: qemu-devel
Cc: groug, aneesh.kumar, eblake, armbru, berto, zengcanfu, jinxuefeng,
chenhui.rtos
This patch factor out throttle parameter parse code to common function
which will be used by block and fsdev.
rename function throttle_parse_options to throttle_parse_group to resolve
function name conflict
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..1d335aa 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -113,6 +113,7 @@ typedef struct uWireSlave uWireSlave;
typedef struct VirtIODevice VirtIODevice;
typedef struct Visitor Visitor;
typedef struct node_info NodeInfo;
+typedef struct ThrottleConfig ThrottleConfig;
typedef void SaveStateHandler(QEMUFile *f, void *opaque);
typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);
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] 12+ messages in thread* Re: [Qemu-devel] [PATCH v4 1/4] fsdev-throttle-qmp: factor out throttle code to reuse code
2018-11-15 8:54 ` [Qemu-devel] [PATCH v4 1/4] fsdev-throttle-qmp: factor out throttle code to reuse code xiezhide
@ 2018-11-15 20:55 ` Eric Blake
2018-11-16 8:24 ` xiezhide
0 siblings, 1 reply; 12+ messages in thread
From: Eric Blake @ 2018-11-15 20:55 UTC (permalink / raw)
To: xiezhide, qemu-devel
Cc: groug, aneesh.kumar, armbru, berto, zengcanfu, jinxuefeng,
chenhui.rtos
On 11/15/18 2:54 AM, xiezhide wrote:
> This patch factor out throttle parameter parse code to common function
s/factor/factors/
Actually, when I write commit messages, I like to use the imperative
mood, with an implicit "Apply this patch in order to" unspoken prefix.
Starting with an explicit "This patch does" is more of a descriptive
mood. So I might have written:
Factor out the throttle parameter parsing code to a new common function
which will be used by block and fsdev.
> which will be used by block and fsdev.
> rename function throttle_parse_options to throttle_parse_group to resolve
> function name conflict
>
> 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(-)
>
> +++ 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 *);
It's okay to use parameter names in function declarations.
> +++ b/include/qemu/typedefs.h
> @@ -113,6 +113,7 @@ typedef struct uWireSlave uWireSlave;
> typedef struct VirtIODevice VirtIODevice;
> typedef struct Visitor Visitor;
> typedef struct node_info NodeInfo;
> +typedef struct ThrottleConfig ThrottleConfig;
Please keep this in the right sorted location, right after SSIBus.
[Hmm, we already have an inconsistency on whether we are sorting by
en_US rules, which are case-insensitive and therefore has 'struct
node_info' in the wrong location, or by C rules which sort lowercase
later and therefore has 'struct uWireSlave' in the wrong position. For
that matter, why are we renaming 'struct node_info NodeInfo', and why
does uWireSlave violate our naming conventions? But those are
independent cleanups, and not necessarily something for you to worry about]
With the sorting fixed,
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] fsdev-throttle-qmp: factor out throttle code to reuse code
2018-11-15 20:55 ` Eric Blake
@ 2018-11-16 8:24 ` xiezhide
0 siblings, 0 replies; 12+ messages in thread
From: xiezhide @ 2018-11-16 8:24 UTC (permalink / raw)
To: Eric Blake, qemu-devel@nongnu.org
Cc: groug@kaod.org, aneesh.kumar@linux.vnet.ibm.com,
armbru@redhat.com, berto@igalia.com, zengcanfu 00215970,
Jinxuefeng, Chenhui (Felix, Euler)
-----Original Message-----
From: Eric Blake [mailto:eblake@redhat.com]
Sent: 2018年11月16日 4:56
To: xiezhide <xiezhide@huawei.com>; qemu-devel@nongnu.org
Cc: groug@kaod.org; aneesh.kumar@linux.vnet.ibm.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 v4 1/4] fsdev-throttle-qmp: factor out throttle code to reuse code
On 11/15/18 2:54 AM, xiezhide wrote:
> This patch factor out throttle parameter parse code to common function
s/factor/factors/
Actually, when I write commit messages, I like to use the imperative mood, with an implicit "Apply this patch in order to" unspoken prefix.
Starting with an explicit "This patch does" is more of a descriptive mood. So I might have written:
Factor out the throttle parameter parsing code to a new common function which will be used by block and fsdev.
Very helpful, thanks
> which will be used by block and fsdev.
> rename function throttle_parse_options to throttle_parse_group to
> resolve function name conflict
>
> 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(-)
>
> +++ 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 *);
It's okay to use parameter names in function declarations.
> +++ b/include/qemu/typedefs.h
> @@ -113,6 +113,7 @@ typedef struct uWireSlave uWireSlave;
> typedef struct VirtIODevice VirtIODevice;
> typedef struct Visitor Visitor;
> typedef struct node_info NodeInfo;
> +typedef struct ThrottleConfig ThrottleConfig;
Please keep this in the right sorted location, right after SSIBus.
[Hmm, we already have an inconsistency on whether we are sorting by en_US rules, which are case-insensitive and therefore has 'struct node_info' in the wrong location, or by C rules which sort lowercase later and therefore has 'struct uWireSlave' in the wrong position. For that matter, why are we renaming 'struct node_info NodeInfo', and why does uWireSlave violate our naming conventions? But those are independent cleanups, and not necessarily something for you to worry about]
With the sorting fixed,
-----Fix it now
Thanks
Kidd
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v4 2/4] fsdev-throttle-qmp: move struct ThrottleLimits to new file
2018-11-15 8:54 [Qemu-devel] [PATCH v4 0/4] fsdev-throttle-qmp: qmp interface for fsdev io throttling xiezhide
2018-11-15 8:54 ` [Qemu-devel] [PATCH v4 1/4] fsdev-throttle-qmp: factor out throttle code to reuse code xiezhide
@ 2018-11-15 8:55 ` xiezhide
2018-11-15 21:41 ` Eric Blake
2018-11-15 8:55 ` [Qemu-devel] [PATCH v4 3/4] fsdev-throttle-qmp: qmp interface for fsdev io throttling xiezhide
2018-11-15 8:55 ` [Qemu-devel] [PATCH v4 4/4] fsdev-throttle-qmp: hmp " xiezhide
3 siblings, 1 reply; 12+ messages in thread
From: xiezhide @ 2018-11-15 8:55 UTC (permalink / raw)
To: qemu-devel
Cc: groug, aneesh.kumar, eblake, armbru, berto, zengcanfu, jinxuefeng,
chenhui.rtos
this patch move ThrottleLimits to new file and rename struct
field with common format
Signed-off-by: xiezhide <xiezhide@huawei.com>
---
Makefile | 9 +++
Makefile.objs | 4 ++
blockdev.c | 51 +---------------
qapi/block-core.json | 122 +------------------------------------
qapi/qapi-schema.json | 1 +
qapi/tlimits.json | 89 +++++++++++++++++++++++++++
util/throttle.c | 163 +++++++++++++++++++++++++-------------------------
7 files changed, 188 insertions(+), 251 deletions(-)
create mode 100644 qapi/tlimits.json
diff --git a/Makefile b/Makefile
index f294718..8990081 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
@@ -598,6 +602,7 @@ qapi-modules = $(SRC_PATH)/qapi/qapi-schema.json $(SRC_PATH)/qapi/common.json \
$(SRC_PATH)/qapi/tpm.json \
$(SRC_PATH)/qapi/trace.json \
$(SRC_PATH)/qapi/transaction.json \
+ $(SRC_PATH)/qapi/tlimits.json \
$(SRC_PATH)/qapi/ui.json
qapi/qapi-builtin-types.c qapi/qapi-builtin-types.h \
@@ -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/blockdev.c b/blockdev.c
index fce5d8f..0683c07 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2701,56 +2701,7 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp)
goto out;
}
- throttle_config_init(&cfg);
- cfg.buckets[THROTTLE_BPS_TOTAL].avg = arg->bps;
- cfg.buckets[THROTTLE_BPS_READ].avg = arg->bps_rd;
- cfg.buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr;
-
- cfg.buckets[THROTTLE_OPS_TOTAL].avg = arg->iops;
- cfg.buckets[THROTTLE_OPS_READ].avg = arg->iops_rd;
- cfg.buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr;
-
- if (arg->has_bps_max) {
- cfg.buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max;
- }
- if (arg->has_bps_rd_max) {
- cfg.buckets[THROTTLE_BPS_READ].max = arg->bps_rd_max;
- }
- if (arg->has_bps_wr_max) {
- cfg.buckets[THROTTLE_BPS_WRITE].max = arg->bps_wr_max;
- }
- if (arg->has_iops_max) {
- cfg.buckets[THROTTLE_OPS_TOTAL].max = arg->iops_max;
- }
- if (arg->has_iops_rd_max) {
- cfg.buckets[THROTTLE_OPS_READ].max = arg->iops_rd_max;
- }
- if (arg->has_iops_wr_max) {
- cfg.buckets[THROTTLE_OPS_WRITE].max = arg->iops_wr_max;
- }
-
- if (arg->has_bps_max_length) {
- cfg.buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_max_length;
- }
- if (arg->has_bps_rd_max_length) {
- cfg.buckets[THROTTLE_BPS_READ].burst_length = arg->bps_rd_max_length;
- }
- if (arg->has_bps_wr_max_length) {
- cfg.buckets[THROTTLE_BPS_WRITE].burst_length = arg->bps_wr_max_length;
- }
- if (arg->has_iops_max_length) {
- cfg.buckets[THROTTLE_OPS_TOTAL].burst_length = arg->iops_max_length;
- }
- if (arg->has_iops_rd_max_length) {
- cfg.buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length;
- }
- if (arg->has_iops_wr_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_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 d4fe710..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:
@@ -2155,130 +2156,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' } }
-
-##
-# @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-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
-#
-# 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' } }
+ 'base': 'ThrottleLimits',
+ 'data': { '*device': 'str', '*id': 'str', '*group': 'str' } }
##
# @block-stream:
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index 65b6dc2..13d7c0f 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -94,3 +94,4 @@
{ 'include': 'trace.json' }
{ 'include': 'introspect.json' }
{ 'include': 'misc.json' }
+{ 'include': 'tlimits.json' }
diff --git a/qapi/tlimits.json b/qapi/tlimits.json
new file mode 100644
index 0000000..6bcbaf6
--- /dev/null
+++ b/qapi/tlimits.json
@@ -0,0 +1,89 @@
+# -*- 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.
+#
+# @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)
+#
+# Since: 4.0
+#
+##
+{ 'struct': 'ThrottleLimits',
+ 'data': { '*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' } }
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] 12+ messages in thread* Re: [Qemu-devel] [PATCH v4 2/4] fsdev-throttle-qmp: move struct ThrottleLimits to new file
2018-11-15 8:55 ` [Qemu-devel] [PATCH v4 2/4] fsdev-throttle-qmp: move struct ThrottleLimits to new file xiezhide
@ 2018-11-15 21:41 ` Eric Blake
2018-11-15 22:02 ` Eric Blake
2018-11-16 8:21 ` xiezhide
0 siblings, 2 replies; 12+ messages in thread
From: Eric Blake @ 2018-11-15 21:41 UTC (permalink / raw)
To: xiezhide, qemu-devel
Cc: groug, aneesh.kumar, armbru, berto, zengcanfu, jinxuefeng,
chenhui.rtos
On 11/15/18 2:55 AM, xiezhide wrote:
> this patch move ThrottleLimits to new file and rename struct
> field with common format
As written, you need s/move/moves/ and s/rename/renames/ to match the
singular actor 'this patch'. Or, if you stick with my preference for
imperative sense, s/this patch move/Move/
s/to new/to a new/
>
> Signed-off-by: xiezhide <xiezhide@huawei.com>
> ---
> +++ 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
tlimits comes before tpm, not after transaction. (Multiple times in
this file)
> +++ 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
Here too.
> { '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' } }
> -
> -{ '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' } }
> + 'base': 'ThrottleLimits',
> + 'data': { '*device': 'str', '*id': 'str', '*group': 'str' } }
The old code has QMP using 'bps_wr' for BlockIOThrottle, but 'bps-write'
for ThrottleLimits. The new code...
> +++ b/qapi/tlimits.json
> +{ 'struct': 'ThrottleLimits',
> + 'data': { '*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' } }
...is sticking with the BlockIOThrottle naming. I don't see any use of
ThrottleLimits in QAPI code (quick check: grep bps-wr
qapi/qapi-introspect.c comes up empty), so we SHOULD be okay with
regards to back-compat. But I'd still split this patch into multiple
pieces: 1. Rename the ThrottleLimits member names (and give
justification why such rename doesn't break back-compat). 2. Rewrite
BlockIOThrottle with ThrottleLimits as its base class. 3. Move
ThrottleLimits into a new file for future reuse. (Maybe 2 and 3 can be
squashed into a single patch)
> 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;
Otherwise, the churn from renaming members (part 1) makes it hard to see
if the code was properly moved into a new file.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [Qemu-devel] [PATCH v4 2/4] fsdev-throttle-qmp: move struct ThrottleLimits to new file
2018-11-15 21:41 ` Eric Blake
@ 2018-11-15 22:02 ` Eric Blake
2018-11-16 8:19 ` xiezhide
2018-11-16 8:21 ` xiezhide
1 sibling, 1 reply; 12+ messages in thread
From: Eric Blake @ 2018-11-15 22:02 UTC (permalink / raw)
To: xiezhide, qemu-devel
Cc: berto, armbru, zengcanfu, groug, aneesh.kumar, jinxuefeng,
chenhui.rtos
On 11/15/18 3:41 PM, Eric Blake wrote:
> On 11/15/18 2:55 AM, xiezhide wrote:
>> this patch move ThrottleLimits to new file and rename struct
>> field with common format
>
> As written, you need s/move/moves/ and s/rename/renames/ to match the
> singular actor 'this patch'. Or, if you stick with my preference for
> imperative sense, s/this patch move/Move/
>
> s/to new/to a new/
>
>>
>> Signed-off-by: xiezhide <xiezhide@huawei.com>
>> ---
>
>> +++ 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
>
> tlimits comes before tpm, not after transaction. (Multiple times in
> this file)
Or, just apply your patch after mine[1], for a much simpler task of
inserting 'tlimits' in the right place within QAPI_MODULES.
[1] https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg03070.html
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/4] fsdev-throttle-qmp: move struct ThrottleLimits to new file
2018-11-15 22:02 ` Eric Blake
@ 2018-11-16 8:19 ` xiezhide
0 siblings, 0 replies; 12+ messages in thread
From: xiezhide @ 2018-11-16 8:19 UTC (permalink / raw)
To: Eric Blake, qemu-devel@nongnu.org
Cc: berto@igalia.com, armbru@redhat.com, zengcanfu 00215970,
groug@kaod.org, aneesh.kumar@linux.vnet.ibm.com, Jinxuefeng,
Chenhui (Felix, Euler)
-----Original Message-----
From: Eric Blake [mailto:eblake@redhat.com]
Sent: 2018年11月16日 6:03
To: xiezhide <xiezhide@huawei.com>; qemu-devel@nongnu.org
Cc: berto@igalia.com; armbru@redhat.com; zengcanfu 00215970 <zengcanfu@huawei.com>; groug@kaod.org; aneesh.kumar@linux.vnet.ibm.com; Jinxuefeng <jinxuefeng@huawei.com>; Chenhui (Felix, Euler) <chenhui.rtos@huawei.com>
Subject: Re: [Qemu-devel] [PATCH v4 2/4] fsdev-throttle-qmp: move struct ThrottleLimits to new file
On 11/15/18 3:41 PM, Eric Blake wrote:
> On 11/15/18 2:55 AM, xiezhide wrote:
>> this patch move ThrottleLimits to new file and rename struct field
>> with common format
>
> As written, you need s/move/moves/ and s/rename/renames/ to match the
> singular actor 'this patch'. Or, if you stick with my preference for
> imperative sense, s/this patch move/Move/
>
> s/to new/to a new/
>
>>
>> Signed-off-by: xiezhide <xiezhide@huawei.com>
>> ---
>
>> +++ 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
>
> tlimits comes before tpm, not after transaction. (Multiple times in
> this file)
Or, just apply your patch after mine[1], for a much simpler task of inserting 'tlimits' in the right place within QAPI_MODULES.
[1] https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg03070.html
Great, it makes the task very simple. thanks
Thanks
Kidd
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/4] fsdev-throttle-qmp: move struct ThrottleLimits to new file
2018-11-15 21:41 ` Eric Blake
2018-11-15 22:02 ` Eric Blake
@ 2018-11-16 8:21 ` xiezhide
2018-11-16 15:03 ` Eric Blake
1 sibling, 1 reply; 12+ messages in thread
From: xiezhide @ 2018-11-16 8:21 UTC (permalink / raw)
To: Eric Blake, qemu-devel@nongnu.org
Cc: groug@kaod.org, aneesh.kumar@linux.vnet.ibm.com,
armbru@redhat.com, berto@igalia.com, zengcanfu 00215970,
Jinxuefeng, Chenhui (Felix, Euler)
-----Original Message-----
From: Eric Blake [mailto:eblake@redhat.com]
Sent: 2018年11月16日 5:41
To: xiezhide <xiezhide@huawei.com>; qemu-devel@nongnu.org
Cc: groug@kaod.org; aneesh.kumar@linux.vnet.ibm.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 v4 2/4] fsdev-throttle-qmp: move struct ThrottleLimits to new file
On 11/15/18 2:55 AM, xiezhide wrote:
> this patch move ThrottleLimits to new file and rename struct field
> with common format
As written, you need s/move/moves/ and s/rename/renames/ to match the singular actor 'this patch'. Or, if you stick with my preference for imperative sense, s/this patch move/Move/
s/to new/to a new/
>
> Signed-off-by: xiezhide <xiezhide@huawei.com>
> ---
> +++ 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
tlimits comes before tpm, not after transaction. (Multiple times in this file)
> +++ 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
Here too.
> { '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' } }
> -
> -{ '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' } }
> + 'base': 'ThrottleLimits',
> + 'data': { '*device': 'str', '*id': 'str', '*group': 'str' } }
The old code has QMP using 'bps_wr' for BlockIOThrottle, but 'bps-write'
for ThrottleLimits. The new code...
> +++ b/qapi/tlimits.json
> +{ 'struct': 'ThrottleLimits',
> + 'data': { '*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' } }
...is sticking with the BlockIOThrottle naming. I don't see any use of ThrottleLimits in QAPI code (quick check: grep bps-wr qapi/qapi-introspect.c comes up empty), so we SHOULD be okay with regards to back-compat. But I'd still split this patch into multiple
pieces: 1. Rename the ThrottleLimits member names (and give justification why such rename doesn't break back-compat). 2. Rewrite BlockIOThrottle with ThrottleLimits as its base class. 3. Move ThrottleLimits into a new file for future reuse. (Maybe 2 and 3 can be squashed into a single patch)
> 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;
Otherwise, the churn from renaming members (part 1) makes it hard to see if the code was properly moved into a new file.
Split this patch to three patches in v5
Thanks
Kidd
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [Qemu-devel] [PATCH v4 2/4] fsdev-throttle-qmp: move struct ThrottleLimits to new file
2018-11-16 8:21 ` xiezhide
@ 2018-11-16 15:03 ` Eric Blake
0 siblings, 0 replies; 12+ messages in thread
From: Eric Blake @ 2018-11-16 15:03 UTC (permalink / raw)
To: xiezhide, qemu-devel@nongnu.org
Cc: groug@kaod.org, aneesh.kumar@linux.vnet.ibm.com,
armbru@redhat.com, berto@igalia.com, zengcanfu 00215970,
Jinxuefeng, Chenhui (Felix, Euler)
On 11/16/18 2:21 AM, xiezhide wrote:
>
Here's a couple of meta observations:
>
>>
>> Signed-off-by: xiezhide <xiezhide@huawei.com>
Your sign-off line ...[1]
>> - 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;
>
> Otherwise, the churn from renaming members (part 1) makes it hard to see if the code was properly moved into a new file.
>
> Split this patch to three patches in v5
Your mailer's quoting style makes it very hard to read your replies.
You are including both a line that I originally wrote and your reply
with no difference in quoting levels between the two, which makes it
look like you are writing both the question and the answer. It is MUCH
easier to read mails where quoted material is prefixed by some form of
quoting (and '>' is probably the easiest form).
>
> Thanks
> Kidd
[1] ...does not match your informal signature. That's not necessarily a
problem, but the project does prefer that Signed-off-by lines use a
preferred legal name rather than merely a nickname or username. It is
acceptable for a sign off to include multiple variations of your name
(I've seen people use UTF-8 characters to represent their name natively,
followed by a Latinized transcription of the name they use in English),
as well as signatures that use just a UTF-8 form or just a Latinized
form; at the same time, I won't reject your patch as written, because if
you look hard enough in git history, you'll find other commit authors
that got away with a username or nickname. If you prefer to be
addressed by 'Kidd' when speaking in English, then including that as
part of your S-o-b lines will help other reviewers know the best name to
use for mentioning you by name.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v4 3/4] fsdev-throttle-qmp: qmp interface for fsdev io throttling
2018-11-15 8:54 [Qemu-devel] [PATCH v4 0/4] fsdev-throttle-qmp: qmp interface for fsdev io throttling xiezhide
2018-11-15 8:54 ` [Qemu-devel] [PATCH v4 1/4] fsdev-throttle-qmp: factor out throttle code to reuse code xiezhide
2018-11-15 8:55 ` [Qemu-devel] [PATCH v4 2/4] fsdev-throttle-qmp: move struct ThrottleLimits to new file xiezhide
@ 2018-11-15 8:55 ` xiezhide
2018-11-15 8:55 ` [Qemu-devel] [PATCH v4 4/4] fsdev-throttle-qmp: hmp " xiezhide
3 siblings, 0 replies; 12+ messages in thread
From: xiezhide @ 2018-11-15 8:55 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 | 9 +++++
Makefile.objs | 4 ++
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, 259 insertions(+), 8 deletions(-)
create mode 100644 qapi/fsdev.json
diff --git a/Makefile b/Makefile
index 8990081..4b71efd 100644
--- a/Makefile
+++ b/Makefile
@@ -107,6 +107,7 @@ 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-fsdev.h qapi/qapi-types-fsdev.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
@@ -127,6 +128,7 @@ 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-fsdev.h qapi/qapi-visit-fsdev.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
@@ -146,6 +148,7 @@ 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-fsdev.h qapi/qapi-commands-fsdev.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
@@ -165,6 +168,7 @@ 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-fsdev.h qapi/qapi-events-fsdev.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
@@ -603,6 +607,7 @@ qapi-modules = $(SRC_PATH)/qapi/qapi-schema.json $(SRC_PATH)/qapi/common.json \
$(SRC_PATH)/qapi/trace.json \
$(SRC_PATH)/qapi/transaction.json \
$(SRC_PATH)/qapi/tlimits.json \
+ $(SRC_PATH)/qapi/fsdev.json \
$(SRC_PATH)/qapi/ui.json
qapi/qapi-builtin-types.c qapi/qapi-builtin-types.h \
@@ -624,6 +629,7 @@ 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-fsdev.c qapi/qapi-types-fsdev.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 \
@@ -644,6 +650,7 @@ 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-fsdev.c qapi/qapi-visit-fsdev.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 \
@@ -663,6 +670,7 @@ 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-fsdev.c qapi/qapi-commands-fsdev.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 \
@@ -682,6 +690,7 @@ 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-fsdev.c qapi/qapi-events-fsdev.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..72946f2 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -21,6 +21,7 @@ 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-fsdev.o
util-obj-y += qapi/qapi-types-ui.o
util-obj-y += qapi/qapi-builtin-visit.o
util-obj-y += qapi/qapi-visit.o
@@ -41,6 +42,7 @@ 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-fsdev.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
@@ -60,6 +62,7 @@ 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-visit-fsdev.o
util-obj-y += qapi/qapi-events-ui.o
util-obj-y += qapi/qapi-introspect.o
@@ -158,6 +161,7 @@ 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-fsdev.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 13d7c0f..7bfda5e 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -95,3 +95,4 @@
{ 'include': 'introspect.json' }
{ 'include': 'misc.json' }
{ 'include': 'tlimits.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] 12+ messages in thread* [Qemu-devel] [PATCH v4 4/4] fsdev-throttle-qmp: hmp interface for fsdev io throttling
2018-11-15 8:54 [Qemu-devel] [PATCH v4 0/4] fsdev-throttle-qmp: qmp interface for fsdev io throttling xiezhide
` (2 preceding siblings ...)
2018-11-15 8:55 ` [Qemu-devel] [PATCH v4 3/4] fsdev-throttle-qmp: qmp interface for fsdev io throttling xiezhide
@ 2018-11-15 8:55 ` xiezhide
3 siblings, 0 replies; 12+ messages in thread
From: xiezhide @ 2018-11-15 8:55 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 | 83 +++++++++++++++++++++++++++++++++++++++++++++++-----
hmp.h | 4 +++
qapi/tlimits.json | 2 +-
5 files changed, 110 insertions(+), 9 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..4cc4146 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,23 @@ 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;
+ ThrottleLimits *tlimits;
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 +1916,71 @@ void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
throttle.id = device;
}
+ tlimits = qapi_BlockIOThrottle_base(&throttle);
+ hmp_initialize_throttle_limits(tlimits, 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);
diff --git a/qapi/tlimits.json b/qapi/tlimits.json
index 6bcbaf6..76b13c5 100644
--- a/qapi/tlimits.json
+++ b/qapi/tlimits.json
@@ -74,7 +74,7 @@
#
# @iops_size: an I/O size in bytes (Since 1.7)
#
-# Since: 4.0
+# Since: 2.11
#
##
{ 'struct': 'ThrottleLimits',
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread