[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V11 4/5] block: Add support for throttling burst
|
From: |
Fam Zheng |
|
Subject: |
Re: [Qemu-devel] [PATCH V11 4/5] block: Add support for throttling burst max in QMP and the command line. |
|
Date: |
Mon, 2 Sep 2013 10:50:32 +0800 |
|
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Sun, 09/01 18:39, Benoît Canet wrote:
> The max parameter of the leaky bucket throttling algorithm can be used to
> allow the guest to do bursts.
> The max value is a pool of I/O that the guest can use without being throttled
> at all. Throttling is triggered once this pool is empty.
It would be good to putting this description in comment and/or document strings
(e.g. help text) to make it easier for both users and developers to understand
how is this "max" used, because it's hard to fully understand from "total max
in bytes" or "total max in bytes".
>
> Signed-off-by: Benoit Canet <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>
> ---
> block/qapi.c | 26 ++++++++++++
> blockdev.c | 118
> ++++++++++++++++++++++++++++++++++++++++++++++--------
> hmp.c | 32 +++++++++++++--
> qapi-schema.json | 34 +++++++++++++++-
> qemu-options.hx | 4 +-
> qmp-commands.hx | 28 ++++++++++++-
> 6 files changed, 218 insertions(+), 24 deletions(-)
>
> diff --git a/block/qapi.c b/block/qapi.c
> index cac3919..b1edc66 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -232,6 +232,32 @@ void bdrv_query_info(BlockDriverState *bs,
> info->inserted->iops = cfg.buckets[THROTTLE_OPS_TOTAL].avg;
> info->inserted->iops_rd = cfg.buckets[THROTTLE_OPS_READ].avg;
> info->inserted->iops_wr = cfg.buckets[THROTTLE_OPS_WRITE].avg;
> +
> + info->inserted->has_bps_max =
> + cfg.buckets[THROTTLE_BPS_TOTAL].max;
> + info->inserted->bps_max =
> + cfg.buckets[THROTTLE_BPS_TOTAL].max;
> + info->inserted->has_bps_rd_max =
> + cfg.buckets[THROTTLE_BPS_READ].max;
> + info->inserted->bps_rd_max =
> + cfg.buckets[THROTTLE_BPS_READ].max;
> + info->inserted->has_bps_wr_max =
> + cfg.buckets[THROTTLE_BPS_WRITE].max;
> + info->inserted->bps_wr_max =
> + cfg.buckets[THROTTLE_BPS_WRITE].max;
> +
> + info->inserted->has_iops_max =
> + cfg.buckets[THROTTLE_OPS_TOTAL].max;
> + info->inserted->iops_max =
> + cfg.buckets[THROTTLE_OPS_TOTAL].max;
> + info->inserted->has_iops_rd_max =
> + cfg.buckets[THROTTLE_OPS_READ].max;
> + info->inserted->iops_rd_max =
> + cfg.buckets[THROTTLE_OPS_READ].max;
> + info->inserted->has_iops_wr_max =
> + cfg.buckets[THROTTLE_OPS_WRITE].max;
> + info->inserted->iops_wr_max =
> + cfg.buckets[THROTTLE_OPS_WRITE].max;
> }
>
> bs0 = bs;
> diff --git a/blockdev.c b/blockdev.c
> index 66fce9f..dc0637d 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -495,13 +495,18 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
> cfg.buckets[THROTTLE_OPS_WRITE].avg =
> qemu_opt_get_number(opts, "throttling.iops-write", 0);
>
> - cfg.buckets[THROTTLE_BPS_TOTAL].max = 0;
> - cfg.buckets[THROTTLE_BPS_READ].max = 0;
> - cfg.buckets[THROTTLE_BPS_WRITE].max = 0;
> -
> - cfg.buckets[THROTTLE_OPS_TOTAL].max = 0;
> - cfg.buckets[THROTTLE_OPS_READ].max = 0;
> - cfg.buckets[THROTTLE_OPS_WRITE].max = 0;
> + cfg.buckets[THROTTLE_BPS_TOTAL].max =
> + qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
> + cfg.buckets[THROTTLE_BPS_READ].max =
> + qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
> + cfg.buckets[THROTTLE_BPS_WRITE].max =
> + qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
> + cfg.buckets[THROTTLE_OPS_TOTAL].max =
> + qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
> + cfg.buckets[THROTTLE_OPS_READ].max =
> + qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
> + cfg.buckets[THROTTLE_OPS_WRITE].max =
> + qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
>
> cfg.op_size = 0;
>
> @@ -782,6 +787,14 @@ DriveInfo *drive_init(QemuOpts *all_opts,
> BlockInterfaceType block_default_type)
> qemu_opt_rename(all_opts, "bps_rd", "throttling.bps-read");
> qemu_opt_rename(all_opts, "bps_wr", "throttling.bps-write");
>
> + qemu_opt_rename(all_opts, "iops_max", "throttling.iops-total-max");
> + qemu_opt_rename(all_opts, "iops_rd_max", "throttling.iops-read-max");
> + qemu_opt_rename(all_opts, "iops_wr_max", "throttling.iops-write-max");
> +
> + qemu_opt_rename(all_opts, "bps_max", "throttling.bps-total-max");
> + qemu_opt_rename(all_opts, "bps_rd_max", "throttling.bps-read-max");
> + qemu_opt_rename(all_opts, "bps_wr_max", "throttling.bps-write-max");
> +
> qemu_opt_rename(all_opts, "readonly", "read-only");
>
> value = qemu_opt_get(all_opts, "cache");
> @@ -1266,8 +1279,22 @@ void qmp_change_blockdev(const char *device, const
> char *filename,
>
> /* throttling disk I/O limits */
> void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t
> bps_rd,
> - int64_t bps_wr, int64_t iops, int64_t iops_rd,
> - int64_t iops_wr, Error **errp)
> + int64_t bps_wr,
> + int64_t iops,
> + int64_t iops_rd,
> + int64_t iops_wr,
> + bool has_bps_max,
> + int64_t bps_max,
> + bool has_bps_rd_max,
> + int64_t bps_rd_max,
> + bool has_bps_wr_max,
> + int64_t bps_wr_max,
> + bool has_iops_max,
> + int64_t iops_max,
> + bool has_iops_rd_max,
> + int64_t iops_rd_max,
> + bool has_iops_wr_max,
> + int64_t iops_wr_max, Error **errp)
> {
> ThrottleConfig cfg;
> BlockDriverState *bs;
> @@ -1287,13 +1314,24 @@ void qmp_block_set_io_throttle(const char *device,
> int64_t bps, int64_t bps_rd,
> cfg.buckets[THROTTLE_OPS_READ].avg = iops_rd;
> cfg.buckets[THROTTLE_OPS_WRITE].avg = iops_wr;
>
> - cfg.buckets[THROTTLE_BPS_TOTAL].max = 0;
> - cfg.buckets[THROTTLE_BPS_READ].max = 0;
> - cfg.buckets[THROTTLE_BPS_WRITE].max = 0;
> -
> - cfg.buckets[THROTTLE_OPS_TOTAL].max = 0;
> - cfg.buckets[THROTTLE_OPS_READ].max = 0;
> - cfg.buckets[THROTTLE_OPS_WRITE].max = 0;
> + if (has_bps_max) {
> + cfg.buckets[THROTTLE_BPS_TOTAL].max = bps_max;
> + }
> + if (has_bps_rd_max) {
> + cfg.buckets[THROTTLE_BPS_READ].max = bps_rd_max;
> + }
> + if (has_bps_wr_max) {
> + cfg.buckets[THROTTLE_BPS_WRITE].max = bps_wr_max;
> + }
> + if (has_iops_max) {
> + cfg.buckets[THROTTLE_OPS_TOTAL].max = iops_max;
> + }
> + if (has_iops_rd_max) {
> + cfg.buckets[THROTTLE_OPS_READ].max = iops_rd_max;
> + }
> + if (has_iops_wr_max) {
> + cfg.buckets[THROTTLE_OPS_WRITE].max = iops_wr_max;
> + }
>
> cfg.op_size = 0;
>
> @@ -1997,6 +2035,30 @@ QemuOptsList qemu_common_drive_opts = {
> .type = QEMU_OPT_NUMBER,
> .help = "limit write bytes per second",
> },{
> + .name = "throttling.iops-total-max",
> + .type = QEMU_OPT_NUMBER,
> + .help = "I/O operations burst",
> + },{
> + .name = "throttling.iops-read-max",
> + .type = QEMU_OPT_NUMBER,
> + .help = "I/O operations read burst",
> + },{
> + .name = "throttling.iops-write-max",
> + .type = QEMU_OPT_NUMBER,
> + .help = "I/O operations write burst",
> + },{
> + .name = "throttling.bps-total-max",
> + .type = QEMU_OPT_NUMBER,
> + .help = "total bytes burst",
> + },{
> + .name = "throttling.bps-read-max",
> + .type = QEMU_OPT_NUMBER,
> + .help = "total bytes read burst",
> + },{
> + .name = "throttling.bps-write-max",
> + .type = QEMU_OPT_NUMBER,
> + .help = "total bytes write burst",
> + },{
> .name = "copy-on-read",
> .type = QEMU_OPT_BOOL,
> .help = "copy read data from backing file into image file",
> @@ -2119,6 +2181,30 @@ QemuOptsList qemu_old_drive_opts = {
> .type = QEMU_OPT_NUMBER,
> .help = "limit write bytes per second",
> },{
> + .name = "iops_max",
> + .type = QEMU_OPT_NUMBER,
> + .help = "I/O operations burst",
> + },{
> + .name = "iops_rd_max",
> + .type = QEMU_OPT_NUMBER,
> + .help = "I/O operations read burst",
> + },{
> + .name = "iops_wr_max",
> + .type = QEMU_OPT_NUMBER,
> + .help = "I/O operations write burst",
> + },{
> + .name = "bps_max",
> + .type = QEMU_OPT_NUMBER,
> + .help = "total bytes burst",
> + },{
> + .name = "bps_rd_max",
> + .type = QEMU_OPT_NUMBER,
> + .help = "total bytes read burst",
> + },{
> + .name = "bps_wr_max",
> + .type = QEMU_OPT_NUMBER,
> + .help = "total bytes write burst",
> + },{
> .name = "copy-on-read",
> .type = QEMU_OPT_BOOL,
> .help = "copy read data from backing file into image file",
> diff --git a/hmp.c b/hmp.c
> index fcca6ae..85a6c16 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -344,14 +344,28 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
> {
> 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 "\n",
> + " iops_wr=%" PRId64
> + " iops_max=%" PRId64
> + " iops_rd_max=%" PRId64
> + " iops_wr_max=%" PRId64 "\n",
> info->value->inserted->bps,
> info->value->inserted->bps_rd,
> info->value->inserted->bps_wr,
> + info->value->inserted->bps_max,
> + info->value->inserted->bps_rd_max,
> + info->value->inserted->bps_wr_max,
> info->value->inserted->iops,
> info->value->inserted->iops_rd,
> - info->value->inserted->iops_wr);
> + info->value->inserted->iops_wr,
> + info->value->inserted->iops_max,
> + info->value->inserted->iops_rd_max,
> + info->value->inserted->iops_wr_max);
> + } else {
> + monitor_printf(mon, " [not inserted]");
> }
>
> if (verbose) {
> @@ -1098,7 +1112,19 @@ void hmp_block_set_io_throttle(Monitor *mon, const
> QDict *qdict)
> qdict_get_int(qdict, "bps_wr"),
> qdict_get_int(qdict, "iops"),
> qdict_get_int(qdict, "iops_rd"),
> - qdict_get_int(qdict, "iops_wr"), &err);
> + qdict_get_int(qdict, "iops_wr"),
> + false, /* no burst max via HMP */
> + 0,
> + false,
> + 0,
> + false,
> + 0,
> + false,
> + 0,
> + false,
> + 0,
> + false,
> + 0, &err);
> hmp_handle_error(mon, &err);
> }
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index a51f7d2..5e5461e 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -785,6 +785,18 @@
> #
> # @image: the info of image used (since: 1.6)
> #
> +# @bps_max: #optional total max in bytes (Since 1.7)
> +#
> +# @bps_rd_max: #optional read max in bytes (Since 1.7)
> +#
> +# @bps_wr_max: #optional write max in bytes (Since 1.7)
> +#
> +# @iops_max: #optional total I/O operations max (Since 1.7)
> +#
> +# @iops_rd_max: #optional read I/O operations max (Since 1.7)
> +#
> +# @iops_wr_max: #optional write I/O operations max (Since 1.7)
> +#
> # Since: 0.14.0
> #
> # Notes: This interface is only found in @BlockInfo.
> @@ -795,7 +807,10 @@
> 'encrypted': 'bool', 'encryption_key_missing': 'bool',
> 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
> 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
> - 'image': 'ImageInfo' } }
> + 'image': 'ImageInfo',
> + '*bps_max': 'int', '*bps_rd_max': 'int',
> + '*bps_wr_max': 'int', '*iops_max': 'int',
> + '*iops_rd_max': 'int', '*iops_wr_max': 'int' }}
Please be consistent with brace spaceing: s/}}/} }/
>
> ##
> # @BlockDeviceIoStatus:
> @@ -2174,6 +2189,18 @@
> #
> # @iops_wr: write I/O operations per second
> #
> +# @bps_max: #optional total max in bytes (Since 1.7)
> +#
> +# @bps_rd_max: #optional read max in bytes (Since 1.7)
> +#
> +# @bps_wr_max: #optional write max in bytes (Since 1.7)
> +#
> +# @iops_max: #optional total I/O operations max (Since 1.7)
> +#
> +# @iops_rd_max: #optional read I/O operations max (Since 1.7)
> +#
> +# @iops_wr_max: #optional write I/O operations max (Since 1.7)
> +#
> # Returns: Nothing on success
> # If @device is not a valid block device, DeviceNotFound
> #
> @@ -2181,7 +2208,10 @@
> ##
> { 'command': 'block_set_io_throttle',
> 'data': { 'device': 'str', 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
> - 'iops': 'int', 'iops_rd': 'int', 'iops_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' }}
Please be consistent with brace spaceing: s/}}/} }/
>
> ##
> # @block-stream:
> diff --git a/qemu-options.hx b/qemu-options.hx
> index d15338e..8df7f1f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -409,7 +409,9 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
> "
> [,cache=writethrough|writeback|none|directsync|unsafe][,format=f]\n"
> " [,serial=s][,addr=A][,id=name][,aio=threads|native]\n"
> " [,readonly=on|off][,copy-on-read=on|off]\n"
> - "
> [[,bps=b]|[[,bps_rd=r][,bps_wr=w]]][[,iops=i]|[[,iops_rd=r][,iops_wr=w]]\n"
> + " [[,bps=b]|[[,bps_rd=r][,bps_wr=w]]][[,iops=i]|[[,iops_rd=r]\n"
> + " [,iops_wr=w][,bps_max=bm]|[[,bps_rd_max=rm][,bps_wr_max=wm]]]\n"
> + " [[,iops_max=im]|[[,iops_rd_max=irm][,iops_wr_max=iwm]]\n"
The brackets don't match here, let's expand and indent it to see
what's here:
[
[,bps=b]
|
[
[,bps_rd=r]
[,bps_wr=w]
]
]
[
[,iops=i]
|
[
[,iops_rd=r]
[,iops_wr=w]
[,bps_max=bm]
|
[
[,bps_rd_max=rm]
[,bps_wr_max=wm]
]
]
[
[,iops_max=im]
|
[
[,iops_rd_max=irm]
[,iops_wr_max=iwm]
]\n"
Mismatch here, right? And I think you wanted to move bps_max* group to outmost.
> " use 'file' as a drive image\n", QEMU_ARCH_ALL)
> STEXI
> @item -drive @var{option}[,@var{option}[,@var{option}[,...]]]
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index cf93d9b..a10d82b 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -1389,7 +1389,7 @@ EQMP
>
> {
> .name = "block_set_io_throttle",
> - .args_type =
> "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l",
> + .args_type =
> "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l,bps_max:l?,bps_rd_max:l?,bps_wr_max:l?,iops_max:l?,iops_rd_max:l?,iops_wr_max:l?",
Too long, wrap this line?
> .mhandler.cmd_new = qmp_marshal_input_block_set_io_throttle,
> },
>
> @@ -1408,6 +1408,12 @@ Arguments:
> - "iops": total I/O operations per second (json-int)
> - "iops_rd": read I/O operations per second (json-int)
> - "iops_wr": write I/O operations per second (json-int)
> +- "bps_max": total max in bytes (json-int)
> +- "bps_rd_max": read max in bytes (json-int)
> +- "bps_wr_max": write max in bytes (json-int)
> +- "iops_max": total I/O operations max (json-int)
> +- "iops_rd_max": read I/O operations max (json-int)
> +- "iops_wr_max": write I/O operations max (json-int)
>
> Example:
>
> @@ -1417,7 +1423,13 @@ Example:
> "bps_wr": 0,
> "iops": 0,
> "iops_rd": 0,
> - "iops_wr": 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 } }
OK, good brace spacing.
> <- { "return": {} }
>
> EQMP
> @@ -1758,6 +1770,12 @@ Each json-object contain the following:
> - "iops": limit total I/O operations per second (json-int)
> - "iops_rd": limit read operations per second (json-int)
> - "iops_wr": limit write operations per second (json-int)
> + - "bps_max": total max in bytes (json-int)
> + - "bps_rd_max": read max in bytes (json-int)
> + - "bps_wr_max": write max in bytes (json-int)
> + - "iops_max": total I/O operations max (json-int)
> + - "iops_rd_max": read I/O operations max (json-int)
> + - "iops_wr_max": write I/O operations max (json-int)
> - "image": the detail of the image, it is a json-object containing
> the following:
> - "filename": image file name (json-string)
> @@ -1827,6 +1845,12 @@ Example:
> "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,
> "image":{
> "filename":"disks/test.qcow2",
> "format":"qcow2",
> --
> 1.7.10.4
>
>
- [Qemu-devel] [PATCH V11 1/5] throttle: Add a new throttling API implementing continuous leaky bucket., (continued)
[Qemu-devel] [PATCH V11 2/5] throttle: Add units tests, Benoît Canet, 2013/09/01
[Qemu-devel] [PATCH V11 3/5] block: Enable the new throttling code in the block layer., Benoît Canet, 2013/09/01
[Qemu-devel] [PATCH V11 4/5] block: Add support f or throttling burst max in QMP and the command line., Benoît Canet, 2013/09/01
- Re: [Qemu-devel] [PATCH V11 4/5] block: Add support for throttling burst max in QMP and the command line.,
Fam Zheng <=
[Qemu-devel] [PATCH V11 5/5] block: Add iops_si ze to do the iops accounting for a given io size., Benoît Canet, 2013/09/01