* [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* 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
* [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* 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 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
* 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
* [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