| Message ID | 20170731095443.28211-6-el13635@mail.ntua.gr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jul 31, 2017 at 12:54:41PM +0300, Manos Pitsidianakis wrote: > +static int throttle_configure_tgm(BlockDriverState *bs, > + ThrottleGroupMember *tgm, > + QDict *options, Error **errp) > +{ > + int ret; > + ThrottleConfig cfg; > + const char *group_name = NULL; > + Error *local_err = NULL; > + QemuOpts *opts = qemu_opts_create(&throttle_opts, NULL, 0, &local_err); If there is no scenario where this can fail please use error_abort.
Am 31.07.2017 um 11:54 hat Manos Pitsidianakis geschrieben: > block/throttle.c uses existing I/O throttle infrastructure inside a > block filter driver. I/O operations are intercepted in the filter's > read/write coroutines, and referred to block/throttle-groups.c > > The driver can be used with the syntax > -drive driver=throttle,file.filename=foo.qcow2, \ > limits.iops-total=...,throttle-group=bar > > The configuration flags and their semantics are identical to the > hardcoded throttling ones. > > A node can be created referring to an existing group, and will overwrite > its limits if any are specified, otherwise they are retained. > > Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr> > --- > block/Makefile.objs | 1 + > block/throttle.c | 395 ++++++++++++++++++++++++++++++++++++++++ > include/qemu/throttle-options.h | 1 + > 3 files changed, 397 insertions(+) > create mode 100644 block/throttle.c > > diff --git a/block/Makefile.objs b/block/Makefile.objs > index 2aaede4ae1..6eaf78a046 100644 > --- a/block/Makefile.objs > +++ b/block/Makefile.objs > @@ -25,6 +25,7 @@ block-obj-y += accounting.o dirty-bitmap.o > block-obj-y += write-threshold.o > block-obj-y += backup.o > block-obj-$(CONFIG_REPLICATION) += replication.o > +block-obj-y += throttle.o > > block-obj-y += crypto.o > > diff --git a/block/throttle.c b/block/throttle.c > new file mode 100644 > index 0000000000..f3395462fb > --- /dev/null > +++ b/block/throttle.c > @@ -0,0 +1,395 @@ > +/* > + * QEMU block throttling filter driver infrastructure > + * > + * Copyright (c) 2017 Manos Pitsidianakis > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation; either version 2 or > + * (at your option) version 3 of the License. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, see <http://www.gnu.org/licenses/>. > + */ > + > +#include "qemu/osdep.h" > +#include "block/throttle-groups.h" > +#include "qemu/throttle-options.h" > +#include "qapi/error.h" > + > +#undef THROTTLE_OPT_PREFIX > +#define THROTTLE_OPT_PREFIX "limits." > +static QemuOptsList throttle_opts = { > + .name = "throttle", > + .head = QTAILQ_HEAD_INITIALIZER(throttle_opts.head), > + .desc = { > + THROTTLE_OPTS, > + { > + .name = QEMU_OPT_THROTTLE_GROUP_NAME, > + .type = QEMU_OPT_STRING, > + .help = "throttle group name", > + }, > + { /* end of list */ } > + }, > +}; > + > +/* Extract ThrottleConfig options. Assumes cfg is initialized and will be > + * checked for validity. > + */ > +static int throttle_extract_options(QemuOpts *opts, ThrottleConfig *cfg, > + Error **errp) > +{ > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL)) { > + cfg->buckets[THROTTLE_BPS_TOTAL].avg = > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL, > + 0); > + } > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ)) { > + cfg->buckets[THROTTLE_BPS_READ].avg = > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ, > + 0); > + } > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE)) { > + cfg->buckets[THROTTLE_BPS_WRITE].avg = > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE, > + 0); > + } > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL)) { > + cfg->buckets[THROTTLE_OPS_TOTAL].avg = > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL, > + 0); > + } > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ)) { > + cfg->buckets[THROTTLE_OPS_READ].avg = > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ, > + 0); > + } > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE)) { > + cfg->buckets[THROTTLE_OPS_WRITE].avg = > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE, > + 0); > + } > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL_MAX)) { > + cfg->buckets[THROTTLE_BPS_TOTAL].max = > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > + QEMU_OPT_BPS_TOTAL_MAX, 0); > + } > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ_MAX)) { > + cfg->buckets[THROTTLE_BPS_READ].max = > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > + QEMU_OPT_BPS_READ_MAX, 0); > + } > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE_MAX)) { > + cfg->buckets[THROTTLE_BPS_WRITE].max = > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > + QEMU_OPT_BPS_WRITE_MAX, 0); > + } > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL_MAX)) { > + cfg->buckets[THROTTLE_OPS_TOTAL].max = > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > + QEMU_OPT_IOPS_TOTAL_MAX, 0); > + } > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ_MAX)) { > + cfg->buckets[THROTTLE_OPS_READ].max = > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > + QEMU_OPT_IOPS_READ_MAX, 0); > + } > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE_MAX)) { > + cfg->buckets[THROTTLE_OPS_WRITE].max = > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > + QEMU_OPT_IOPS_WRITE_MAX, 0); > + } > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL_MAX_LENGTH)) { > + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > + QEMU_OPT_BPS_TOTAL_MAX_LENGTH, 1) > UINT_MAX) { > + error_setg(errp, "%s value must be in the range [0, %u]", > + THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL_MAX_LENGTH, > + UINT_MAX); > + return -1; > + } > + cfg->buckets[THROTTLE_BPS_TOTAL].burst_length = > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > + QEMU_OPT_BPS_TOTAL_MAX_LENGTH, 1); > + } > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ_MAX_LENGTH)) { > + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > + QEMU_OPT_BPS_READ_MAX_LENGTH, 1) > UINT_MAX) { > + error_setg(errp, "%s must be in the range [0, %u]", > + THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ_MAX_LENGTH, > + UINT_MAX); > + return -1; > + } > + cfg->buckets[THROTTLE_BPS_READ].burst_length = > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > + QEMU_OPT_BPS_READ_MAX_LENGTH, 1); > + } > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE_MAX_LENGTH)) { > + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > + QEMU_OPT_BPS_WRITE_MAX_LENGTH, 1) > UINT_MAX) { > + error_setg(errp, "%s must be in the range [0, %u]", > + THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE_MAX_LENGTH, > + UINT_MAX); > + return -1; > + } > + cfg->buckets[THROTTLE_BPS_WRITE].burst_length = > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > + QEMU_OPT_BPS_WRITE_MAX_LENGTH, 1); > + } > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL_MAX_LENGTH)) { > + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > + QEMU_OPT_IOPS_TOTAL_MAX_LENGTH, 1) > UINT_MAX) { > + error_setg(errp, "%s must be in the range [0, %u]", > + THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL_MAX_LENGTH, > + UINT_MAX); > + return -1; > + } > + cfg->buckets[THROTTLE_OPS_TOTAL].burst_length = > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > + QEMU_OPT_IOPS_TOTAL_MAX_LENGTH, 1); > + } > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ_MAX_LENGTH)) { > + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > + QEMU_OPT_IOPS_READ_MAX_LENGTH, 1) > UINT_MAX) { > + error_setg(errp, "%s must be in the range [0, %u]", > + THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ_MAX_LENGTH, > + UINT_MAX); > + return -1; > + } > + cfg->buckets[THROTTLE_OPS_READ].burst_length = > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > + QEMU_OPT_IOPS_READ_MAX_LENGTH, 1); > + } > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE_MAX_LENGTH)) { > + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > + QEMU_OPT_IOPS_WRITE_MAX_LENGTH, 1) > UINT_MAX) { > + error_setg(errp, "%s must be in the range [0, %u]", > + THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE_MAX_LENGTH, > + UINT_MAX); > + return -1; > + } > + cfg->buckets[THROTTLE_OPS_WRITE].burst_length = > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > + QEMU_OPT_IOPS_WRITE_MAX_LENGTH, 1); > + } > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_SIZE)) { > + cfg->op_size = > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_SIZE, > + 0); > + } > + return 0; > +} This function is very repetitive, but each block is long enough that you have to look closely to review whether the right constants are used everywhere. Maybe this could become a bit more readable with a macro or two? > +static int throttle_configure_tgm(BlockDriverState *bs, > + ThrottleGroupMember *tgm, > + QDict *options, Error **errp) > +{ > + int ret; > + ThrottleConfig cfg; > + const char *group_name = NULL; > + Error *local_err = NULL; > + QemuOpts *opts = qemu_opts_create(&throttle_opts, NULL, 0, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + return -EINVAL; > + } As Stefan said, qemu_opts_create() can't fail if you pass NULL for id, so you can just use &error_abort. > + > + qemu_opts_absorb_qdict(opts, options, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + goto err; > + } > + > + /* If no name is specified, an anonymous group will be created */ > + group_name = qemu_opt_get(opts, QEMU_OPT_THROTTLE_GROUP_NAME); > + > + /* Register membership to group with name group_name */ > + throttle_group_register_tgm(tgm, group_name, bdrv_get_aio_context(bs)); The documentation of throttle_group_register_tgm() suggests that you have to pass a string here, but group_name can be NULL if the option wasn't given. Probably just means that the comment for that function needs to be updated. > + /* Copy previous configuration */ > + throttle_group_get_config(tgm, &cfg); > + > + /* Change limits if user has specified them */ > + if (throttle_extract_options(opts, &cfg, errp) || > + !throttle_is_valid(&cfg, errp)) { > + throttle_group_unregister_tgm(tgm); > + goto err; > + } > + /* Update group configuration */ > + throttle_group_config(tgm, &cfg); > + > + ret = 0; > + goto fin; > + > +err: > + ret = -EINVAL; > +fin: > + qemu_opts_del(opts); > + return ret; > +} > + > +static int throttle_open(BlockDriverState *bs, QDict *options, > + int flags, Error **errp) > +{ > + ThrottleGroupMember *tgm = bs->opaque; > + > + bs->file = bdrv_open_child(NULL, options, "file", > + bs, &child_file, false, errp); Indentation is off. > + if (!bs->file) { > + return -EINVAL; > + } > + > + return throttle_configure_tgm(bs, tgm, options, errp); > +} > + > +static void throttle_close(BlockDriverState *bs) > +{ > + ThrottleGroupMember *tgm = bs->opaque; > + throttle_group_unregister_tgm(tgm); > +} > + > + > +static int64_t throttle_getlength(BlockDriverState *bs) > +{ > + return bdrv_getlength(bs->file->bs); > +} > + > + > +static int coroutine_fn throttle_co_preadv(BlockDriverState *bs, > + uint64_t offset, uint64_t bytes, > + QEMUIOVector *qiov, int flags) > +{ > + > + ThrottleGroupMember *tgm = bs->opaque; > + throttle_group_co_io_limits_intercept(tgm, bytes, false); > + > + return bdrv_co_preadv(bs->file, offset, bytes, qiov, flags); > +} > + > +static int coroutine_fn throttle_co_pwritev(BlockDriverState *bs, > + uint64_t offset, uint64_t bytes, > + QEMUIOVector *qiov, int flags) > +{ > + ThrottleGroupMember *tgm = bs->opaque; > + throttle_group_co_io_limits_intercept(tgm, bytes, true); > + > + return bdrv_co_preadv(bs->file, offset, bytes, qiov, flags); > +} I think we want to set BlockDriver.supported_write_flags so that passing down flags is actually of any use. > + > +static int coroutine_fn throttle_co_pwrite_zeroes(BlockDriverState *bs, > + int64_t offset, int bytes, BdrvRequestFlags flags) > +{ > + ThrottleGroupMember *tgm = bs->opaque; > + throttle_group_co_io_limits_intercept(tgm, bytes, true); > + > + return bdrv_co_pwrite_zeroes(bs->file, offset, bytes, flags); > +} The same is true for BlockDriver.supported_zero_flags. > +static int coroutine_fn throttle_co_pdiscard(BlockDriverState *bs, > + int64_t offset, int bytes) > +{ > + ThrottleGroupMember *tgm = bs->opaque; > + throttle_group_co_io_limits_intercept(tgm, bytes, true); > + > + return bdrv_co_pdiscard(bs->file->bs, offset, bytes); > +} > + > +static int throttle_co_flush(BlockDriverState *bs) > +{ > + return bdrv_co_flush(bs->file->bs); > +} > + > +static void throttle_detach_aio_context(BlockDriverState *bs) > +{ > + ThrottleGroupMember *tgm = bs->opaque; > + throttle_group_detach_aio_context(tgm); > +} > + > +static void throttle_attach_aio_context(BlockDriverState *bs, > + AioContext *new_context) Indentation is off here... > +{ > + ThrottleGroupMember *tgm = bs->opaque; > + throttle_group_attach_aio_context(tgm, new_context); > +} > + > +static int throttle_reopen_prepare(BDRVReopenState *reopen_state, > + BlockReopenQueue *queue, Error **errp) ...and here. > +{ > + ThrottleGroupMember *tgm = NULL; > + > + assert(reopen_state != NULL); > + assert(reopen_state->bs != NULL); > + > + reopen_state->opaque = g_new0(ThrottleGroupMember, 1); > + tgm = reopen_state->opaque; > + > + return throttle_configure_tgm(reopen_state->bs, tgm, reopen_state->options, > + errp); > +} > + > +static void throttle_reopen_commit(BDRVReopenState *state) > +{ > + ThrottleGroupMember *tgm = state->bs->opaque; > + > + throttle_group_unregister_tgm(tgm); > + g_free(state->bs->opaque); > + state->bs->opaque = state->opaque; > + state->opaque = NULL; > +} > + > +static void throttle_reopen_abort(BDRVReopenState *state) > +{ > + ThrottleGroupMember *tgm = state->opaque; > + > + throttle_group_unregister_tgm(tgm); > + g_free(state->opaque); > + state->opaque = NULL; > +} > + > +static bool throttle_recurse_is_first_non_filter(BlockDriverState *bs, > + BlockDriverState *candidate) > +{ > + return bdrv_recurse_is_first_non_filter(bs->file->bs, candidate); > +} > + > +static BlockDriver bdrv_throttle = { > + .format_name = "throttle", > + .protocol_name = "throttle", > + .instance_size = sizeof(ThrottleGroupMember), > + > + .bdrv_file_open = throttle_open, > + .bdrv_close = throttle_close, > + .bdrv_co_flush = throttle_co_flush, > + > + .bdrv_child_perm = bdrv_filter_default_perms, > + > + .bdrv_getlength = throttle_getlength, > + > + .bdrv_co_preadv = throttle_co_preadv, > + .bdrv_co_pwritev = throttle_co_pwritev, > + > + .bdrv_co_pwrite_zeroes = throttle_co_pwrite_zeroes, > + .bdrv_co_pdiscard = throttle_co_pdiscard, > + > + .bdrv_recurse_is_first_non_filter = throttle_recurse_is_first_non_filter, > + > + .bdrv_attach_aio_context = throttle_attach_aio_context, > + .bdrv_detach_aio_context = throttle_detach_aio_context, > + > + .bdrv_reopen_prepare = throttle_reopen_prepare, > + .bdrv_reopen_commit = throttle_reopen_commit, > + .bdrv_reopen_abort = throttle_reopen_abort, > + > + .is_filter = true, > +}; What about .bdrv_co_get_block_status? > +static void bdrv_throttle_init(void) > +{ > + bdrv_register(&bdrv_throttle); > +} > + > +block_init(bdrv_throttle_init); > diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h > index 182b7896e1..3528a8f4a2 100644 > --- a/include/qemu/throttle-options.h > +++ b/include/qemu/throttle-options.h > @@ -29,6 +29,7 @@ > #define QEMU_OPT_BPS_WRITE_MAX "bps-write-max" > #define QEMU_OPT_BPS_WRITE_MAX_LENGTH "bps-write-max-length" > #define QEMU_OPT_IOPS_SIZE "iops-size" > +#define QEMU_OPT_THROTTLE_GROUP_NAME "throttle-group" > > #define THROTTLE_OPT_PREFIX "throttling." > #define THROTTLE_OPTS \ Kevin
On Thu, Aug 03, 2017 at 10:07:41AM +0200, Kevin Wolf wrote: >Am 31.07.2017 um 11:54 hat Manos Pitsidianakis geschrieben: >> +/* Extract ThrottleConfig options. Assumes cfg is initialized and will be >> + * checked for validity. >> + */ >> +static int throttle_extract_options(QemuOpts *opts, ThrottleConfig *cfg, >> + Error **errp) >> +{ >> + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL)) { >> + cfg->buckets[THROTTLE_BPS_TOTAL].avg = >> + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL, >> + 0); >> + } >> + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ)) { >> + cfg->buckets[THROTTLE_BPS_READ].avg = >> + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ, >> + 0); >> + } >> + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE)) { >> + cfg->buckets[THROTTLE_BPS_WRITE].avg = >> + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE, >> + 0); >> + } >> + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL)) { >> + cfg->buckets[THROTTLE_OPS_TOTAL].avg = >> + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL, >> + 0); >> + } >> + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ)) { >> + cfg->buckets[THROTTLE_OPS_READ].avg = >> + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ, >> + 0); >> + } >> + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE)) { >> + cfg->buckets[THROTTLE_OPS_WRITE].avg = >> + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE, >> + 0); >> + } >> + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL_MAX)) { >> + cfg->buckets[THROTTLE_BPS_TOTAL].max = >> + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX >> + QEMU_OPT_BPS_TOTAL_MAX, 0); >> + } >> + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ_MAX)) { >> + cfg->buckets[THROTTLE_BPS_READ].max = >> + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX >> + QEMU_OPT_BPS_READ_MAX, 0); >> + } >> + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE_MAX)) { >> + cfg->buckets[THROTTLE_BPS_WRITE].max = >> + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX >> + QEMU_OPT_BPS_WRITE_MAX, 0); >> + } >> + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL_MAX)) { >> + cfg->buckets[THROTTLE_OPS_TOTAL].max = >> + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX >> + QEMU_OPT_IOPS_TOTAL_MAX, 0); >> + } >> + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ_MAX)) { >> + cfg->buckets[THROTTLE_OPS_READ].max = >> + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX >> + QEMU_OPT_IOPS_READ_MAX, 0); >> + } >> + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE_MAX)) { >> + cfg->buckets[THROTTLE_OPS_WRITE].max = >> + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX >> + QEMU_OPT_IOPS_WRITE_MAX, 0); >> + } >> + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL_MAX_LENGTH)) { >> + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX >> + QEMU_OPT_BPS_TOTAL_MAX_LENGTH, 1) > UINT_MAX) { >> + error_setg(errp, "%s value must be in the range [0, %u]", >> + THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL_MAX_LENGTH, >> + UINT_MAX); >> + return -1; >> + } >> + cfg->buckets[THROTTLE_BPS_TOTAL].burst_length = >> + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX >> + QEMU_OPT_BPS_TOTAL_MAX_LENGTH, 1); >> + } >> + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ_MAX_LENGTH)) { >> + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX >> + QEMU_OPT_BPS_READ_MAX_LENGTH, 1) > UINT_MAX) { >> + error_setg(errp, "%s must be in the range [0, %u]", >> + THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ_MAX_LENGTH, >> + UINT_MAX); >> + return -1; >> + } >> + cfg->buckets[THROTTLE_BPS_READ].burst_length = >> + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX >> + QEMU_OPT_BPS_READ_MAX_LENGTH, 1); >> + } >> + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE_MAX_LENGTH)) { >> + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX >> + QEMU_OPT_BPS_WRITE_MAX_LENGTH, 1) > UINT_MAX) { >> + error_setg(errp, "%s must be in the range [0, %u]", >> + THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE_MAX_LENGTH, >> + UINT_MAX); >> + return -1; >> + } >> + cfg->buckets[THROTTLE_BPS_WRITE].burst_length = >> + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX >> + QEMU_OPT_BPS_WRITE_MAX_LENGTH, 1); >> + } >> + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL_MAX_LENGTH)) { >> + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX >> + QEMU_OPT_IOPS_TOTAL_MAX_LENGTH, 1) > UINT_MAX) { >> + error_setg(errp, "%s must be in the range [0, %u]", >> + THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL_MAX_LENGTH, >> + UINT_MAX); >> + return -1; >> + } >> + cfg->buckets[THROTTLE_OPS_TOTAL].burst_length = >> + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX >> + QEMU_OPT_IOPS_TOTAL_MAX_LENGTH, 1); >> + } >> + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ_MAX_LENGTH)) { >> + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX >> + QEMU_OPT_IOPS_READ_MAX_LENGTH, 1) > UINT_MAX) { >> + error_setg(errp, "%s must be in the range [0, %u]", >> + THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ_MAX_LENGTH, >> + UINT_MAX); >> + return -1; >> + } >> + cfg->buckets[THROTTLE_OPS_READ].burst_length = >> + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX >> + QEMU_OPT_IOPS_READ_MAX_LENGTH, 1); >> + } >> + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE_MAX_LENGTH)) { >> + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX >> + QEMU_OPT_IOPS_WRITE_MAX_LENGTH, 1) > UINT_MAX) { >> + error_setg(errp, "%s must be in the range [0, %u]", >> + THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE_MAX_LENGTH, >> + UINT_MAX); >> + return -1; >> + } >> + cfg->buckets[THROTTLE_OPS_WRITE].burst_length = >> + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX >> + QEMU_OPT_IOPS_WRITE_MAX_LENGTH, 1); >> + } >> + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_SIZE)) { >> + cfg->op_size = >> + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_SIZE, >> + 0); >> + } >> + return 0; >> +} > >This function is very repetitive, but each block is long enough that >you have to look closely to review whether the right constants are used >everywhere. > >Maybe this could become a bit more readable with a macro or two? How about this? # #define IF_OPT_SET(rvalue,opt_name) \ # if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX opt_name)) { \ # rvalue = qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX opt_name, 0); } # IF_OPT_SET(cfg->buckets[THROTTLE_BPS_TOTAL].avg, QEMU_OPT_BPS_TOTAL); # IF_OPT_SET(cfg->buckets[THROTTLE_BPS_READ].avg, QEMU_OPT_BPS_READ); # IF_OPT_SET(cfg->buckets[THROTTLE_BPS_WRITE].avg, QEMU_OPT_BPS_WRITE); # IF_OPT_SET(cfg->buckets[THROTTLE_OPS_TOTAL].avg, QEMU_OPT_IOPS_TOTAL); # IF_OPT_SET(cfg->buckets[THROTTLE_OPS_READ].avg, QEMU_OPT_IOPS_READ); # IF_OPT_SET(cfg->buckets[THROTTLE_OPS_WRITE].avg, QEMU_OPT_IOPS_WRITE); # IF_OPT_SET(cfg->buckets[THROTTLE_BPS_TOTAL].max, QEMU_OPT_BPS_TOTAL_MAX); # IF_OPT_SET(cfg->buckets[THROTTLE_BPS_READ].max , QEMU_OPT_BPS_READ_MAX); # IF_OPT_SET(cfg->buckets[THROTTLE_BPS_WRITE].max, QEMU_OPT_BPS_WRITE_MAX); # IF_OPT_SET(cfg->buckets[THROTTLE_OPS_TOTAL].max, QEMU_OPT_IOPS_TOTAL_MAX); # IF_OPT_SET(cfg->buckets[THROTTLE_OPS_READ].max, QEMU_OPT_IOPS_READ_MAX); # IF_OPT_SET(cfg->buckets[THROTTLE_OPS_WRITE].max, QEMU_OPT_IOPS_WRITE_MAX); # IF_OPT_SET(cfg->op_size, QEMU_OPT_IOPS_SIZE); # #define IF_OPT_UINT_SET(rvalue,opt_name) \ # if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX opt_name)) { \ # if (qemu_opt_get_number(opts, \ # THROTTLE_OPT_PREFIX opt_name, 1) > UINT_MAX) { \ # error_setg(errp, "%s value must be in the range [0, %u]", \ # THROTTLE_OPT_PREFIX opt_name, UINT_MAX); \ # return -1; \ # } \ # rvalue = qemu_opt_get_number(opts, opt_name, 1); \ # } # IF_OPT_UINT_SET(cfg->buckets[THROTTLE_BPS_TOTAL].burst_length, # QEMU_OPT_BPS_TOTAL_MAX_LENGTH); # IF_OPT_UINT_SET(cfg->buckets[THROTTLE_BPS_READ].burst_length, # QEMU_OPT_BPS_READ_MAX_LENGTH); # IF_OPT_UINT_SET(cfg->buckets[THROTTLE_BPS_WRITE].burst_length, # QEMU_OPT_BPS_WRITE_MAX_LENGTH); # IF_OPT_UINT_SET(cfg->buckets[THROTTLE_OPS_TOTAL].burst_length, # QEMU_OPT_IOPS_TOTAL_MAX_LENGTH); # IF_OPT_UINT_SET(cfg->buckets[THROTTLE_OPS_READ].burst_length, # QEMU_OPT_IOPS_READ_MAX_LENGTH); # IF_OPT_UINT_SET(cfg->buckets[THROTTLE_OPS_WRITE].burst_length, # QEMU_OPT_IOPS_WRITE_MAX_LENGTH); >> +static int throttle_configure_tgm(BlockDriverState *bs, >> + ThrottleGroupMember *tgm, >> + QDict *options, Error **errp) >> +{ >> + int ret; >> + ThrottleConfig cfg; >> + const char *group_name = NULL; >> + Error *local_err = NULL; >> + QemuOpts *opts = qemu_opts_create(&throttle_opts, NULL, 0, &local_err); >> + if (local_err) { >> + error_propagate(errp, local_err); >> + return -EINVAL; >> + } > >As Stefan said, qemu_opts_create() can't fail if you pass NULL for id, >so you can just use &error_abort. > >> + >> + qemu_opts_absorb_qdict(opts, options, &local_err); >> + if (local_err) { >> + error_propagate(errp, local_err); >> + goto err; >> + } >> + >> + /* If no name is specified, an anonymous group will be created */ >> + group_name = qemu_opt_get(opts, QEMU_OPT_THROTTLE_GROUP_NAME); >> + >> + /* Register membership to group with name group_name */ >> + throttle_group_register_tgm(tgm, group_name, bdrv_get_aio_context(bs)); > >The documentation of throttle_group_register_tgm() suggests that you >have to pass a string here, but group_name can be NULL if the option >wasn't given. Probably just means that the comment for that function >needs to be updated. > >> + /* Copy previous configuration */ >> + throttle_group_get_config(tgm, &cfg); >> + >> + /* Change limits if user has specified them */ >> + if (throttle_extract_options(opts, &cfg, errp) || >> + !throttle_is_valid(&cfg, errp)) { >> + throttle_group_unregister_tgm(tgm); >> + goto err; >> + } >> + /* Update group configuration */ >> + throttle_group_config(tgm, &cfg); >> + >> + ret = 0; >> + goto fin; >> + >> +err: >> + ret = -EINVAL; >> +fin: >> + qemu_opts_del(opts); >> + return ret; >> +} >> + >> +static int throttle_open(BlockDriverState *bs, QDict *options, >> + int flags, Error **errp) >> +{ >> + ThrottleGroupMember *tgm = bs->opaque; >> + >> + bs->file = bdrv_open_child(NULL, options, "file", >> + bs, &child_file, false, errp); > >Indentation is off. > >> + if (!bs->file) { >> + return -EINVAL; >> + } >> + >> + return throttle_configure_tgm(bs, tgm, options, errp); >> +} >> + >> +static void throttle_close(BlockDriverState *bs) >> +{ >> + ThrottleGroupMember *tgm = bs->opaque; >> + throttle_group_unregister_tgm(tgm); >> +} >> + >> + >> +static int64_t throttle_getlength(BlockDriverState *bs) >> +{ >> + return bdrv_getlength(bs->file->bs); >> +} >> + >> + >> +static int coroutine_fn throttle_co_preadv(BlockDriverState *bs, >> + uint64_t offset, uint64_t bytes, >> + QEMUIOVector *qiov, int flags) >> +{ >> + >> + ThrottleGroupMember *tgm = bs->opaque; >> + throttle_group_co_io_limits_intercept(tgm, bytes, false); >> + >> + return bdrv_co_preadv(bs->file, offset, bytes, qiov, flags); >> +} >> + >> +static int coroutine_fn throttle_co_pwritev(BlockDriverState *bs, >> + uint64_t offset, uint64_t bytes, >> + QEMUIOVector *qiov, int flags) >> +{ >> + ThrottleGroupMember *tgm = bs->opaque; >> + throttle_group_co_io_limits_intercept(tgm, bytes, true); >> + >> + return bdrv_co_preadv(bs->file, offset, bytes, qiov, flags); >> +} > >I think we want to set BlockDriver.supported_write_flags so that passing >down flags is actually of any use. Can you explain what you mean? Do you mean doing this in throttle_open(): bs->supported_write_flags = bs->file->bs->supported_write_flags; bs->supported_zero_flags = bs->file->bs->supported_zero_flags; > >> + >> +static int coroutine_fn throttle_co_pwrite_zeroes(BlockDriverState *bs, >> + int64_t offset, int bytes, BdrvRequestFlags flags) >> +{ >> + ThrottleGroupMember *tgm = bs->opaque; >> + throttle_group_co_io_limits_intercept(tgm, bytes, true); >> + >> + return bdrv_co_pwrite_zeroes(bs->file, offset, bytes, flags); >> +} > >The same is true for BlockDriver.supported_zero_flags. > >> +static int coroutine_fn throttle_co_pdiscard(BlockDriverState *bs, >> + int64_t offset, int bytes) >> +{ >> + ThrottleGroupMember *tgm = bs->opaque; >> + throttle_group_co_io_limits_intercept(tgm, bytes, true); >> + >> + return bdrv_co_pdiscard(bs->file->bs, offset, bytes); >> +} >> + >> +static int throttle_co_flush(BlockDriverState *bs) >> +{ >> + return bdrv_co_flush(bs->file->bs); >> +} >> + >> +static void throttle_detach_aio_context(BlockDriverState *bs) >> +{ >> + ThrottleGroupMember *tgm = bs->opaque; >> + throttle_group_detach_aio_context(tgm); >> +} >> + >> +static void throttle_attach_aio_context(BlockDriverState *bs, >> + AioContext *new_context) > >Indentation is off here... > >> +{ >> + ThrottleGroupMember *tgm = bs->opaque; >> + throttle_group_attach_aio_context(tgm, new_context); >> +} >> + >> +static int throttle_reopen_prepare(BDRVReopenState *reopen_state, >> + BlockReopenQueue *queue, Error **errp) > >...and here. > >> +{ >> + ThrottleGroupMember *tgm = NULL; >> + >> + assert(reopen_state != NULL); >> + assert(reopen_state->bs != NULL); >> + >> + reopen_state->opaque = g_new0(ThrottleGroupMember, 1); >> + tgm = reopen_state->opaque; >> + >> + return throttle_configure_tgm(reopen_state->bs, tgm, reopen_state->options, >> + errp); >> +} >> + >> +static void throttle_reopen_commit(BDRVReopenState *state) >> +{ >> + ThrottleGroupMember *tgm = state->bs->opaque; >> + >> + throttle_group_unregister_tgm(tgm); >> + g_free(state->bs->opaque); >> + state->bs->opaque = state->opaque; >> + state->opaque = NULL; >> +} >> + >> +static void throttle_reopen_abort(BDRVReopenState *state) >> +{ >> + ThrottleGroupMember *tgm = state->opaque; >> + >> + throttle_group_unregister_tgm(tgm); >> + g_free(state->opaque); >> + state->opaque = NULL; >> +} >> + >> +static bool throttle_recurse_is_first_non_filter(BlockDriverState *bs, >> + BlockDriverState *candidate) >> +{ >> + return bdrv_recurse_is_first_non_filter(bs->file->bs, candidate); >> +} >> + >> +static BlockDriver bdrv_throttle = { >> + .format_name = "throttle", >> + .protocol_name = "throttle", >> + .instance_size = sizeof(ThrottleGroupMember), >> + >> + .bdrv_file_open = throttle_open, >> + .bdrv_close = throttle_close, >> + .bdrv_co_flush = throttle_co_flush, >> + >> + .bdrv_child_perm = bdrv_filter_default_perms, >> + >> + .bdrv_getlength = throttle_getlength, >> + >> + .bdrv_co_preadv = throttle_co_preadv, >> + .bdrv_co_pwritev = throttle_co_pwritev, >> + >> + .bdrv_co_pwrite_zeroes = throttle_co_pwrite_zeroes, >> + .bdrv_co_pdiscard = throttle_co_pdiscard, >> + >> + .bdrv_recurse_is_first_non_filter = throttle_recurse_is_first_non_filter, >> + >> + .bdrv_attach_aio_context = throttle_attach_aio_context, >> + .bdrv_detach_aio_context = throttle_detach_aio_context, >> + >> + .bdrv_reopen_prepare = throttle_reopen_prepare, >> + .bdrv_reopen_commit = throttle_reopen_commit, >> + .bdrv_reopen_abort = throttle_reopen_abort, >> + >> + .is_filter = true, >> +}; > >What about .bdrv_co_get_block_status? We should use the default implementation here that you merged into your 2.11 tree, thanks for noticing and for all the comments. > >> +static void bdrv_throttle_init(void) >> +{ >> + bdrv_register(&bdrv_throttle); >> +} >> + >> +block_init(bdrv_throttle_init); >> diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h >> index 182b7896e1..3528a8f4a2 100644 >> --- a/include/qemu/throttle-options.h >> +++ b/include/qemu/throttle-options.h >> @@ -29,6 +29,7 @@ >> #define QEMU_OPT_BPS_WRITE_MAX "bps-write-max" >> #define QEMU_OPT_BPS_WRITE_MAX_LENGTH "bps-write-max-length" >> #define QEMU_OPT_IOPS_SIZE "iops-size" >> +#define QEMU_OPT_THROTTLE_GROUP_NAME "throttle-group" >> >> #define THROTTLE_OPT_PREFIX "throttling." >> #define THROTTLE_OPTS \ > >Kevin >
On 08/03/2017 03:07 AM, Kevin Wolf wrote: > Am 31.07.2017 um 11:54 hat Manos Pitsidianakis geschrieben: >> block/throttle.c uses existing I/O throttle infrastructure inside a >> block filter driver. I/O operations are intercepted in the filter's >> read/write coroutines, and referred to block/throttle-groups.c >> >> The driver can be used with the syntax >> -drive driver=throttle,file.filename=foo.qcow2, \ >> limits.iops-total=...,throttle-group=bar >> >> The configuration flags and their semantics are identical to the >> hardcoded throttling ones. >> >> A node can be created referring to an existing group, and will overwrite >> its limits if any are specified, otherwise they are retained. >> >> Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr> >> --- >> + >> + .is_filter = true, >> +}; > > What about .bdrv_co_get_block_status? And if so, do you want my byte-based block status to go in first? (Our two series conflict, so we need to pick who needs to rebase on top of the other).
Am 03.08.2017 um 13:48 hat Manos Pitsidianakis geschrieben: > On Thu, Aug 03, 2017 at 10:07:41AM +0200, Kevin Wolf wrote: > > Am 31.07.2017 um 11:54 hat Manos Pitsidianakis geschrieben: > > > +/* Extract ThrottleConfig options. Assumes cfg is initialized and will be > > > + * checked for validity. > > > + */ > > > +static int throttle_extract_options(QemuOpts *opts, ThrottleConfig *cfg, > > > + Error **errp) > > > +{ > > > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL)) { > > > + cfg->buckets[THROTTLE_BPS_TOTAL].avg = > > > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL, > > > + 0); > > > + } > > > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ)) { > > > + cfg->buckets[THROTTLE_BPS_READ].avg = > > > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ, > > > + 0); > > > + } > > > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE)) { > > > + cfg->buckets[THROTTLE_BPS_WRITE].avg = > > > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE, > > > + 0); > > > + } > > > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL)) { > > > + cfg->buckets[THROTTLE_OPS_TOTAL].avg = > > > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL, > > > + 0); > > > + } > > > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ)) { > > > + cfg->buckets[THROTTLE_OPS_READ].avg = > > > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ, > > > + 0); > > > + } > > > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE)) { > > > + cfg->buckets[THROTTLE_OPS_WRITE].avg = > > > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE, > > > + 0); > > > + } > > > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL_MAX)) { > > > + cfg->buckets[THROTTLE_BPS_TOTAL].max = > > > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > > > + QEMU_OPT_BPS_TOTAL_MAX, 0); > > > + } > > > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ_MAX)) { > > > + cfg->buckets[THROTTLE_BPS_READ].max = > > > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > > > + QEMU_OPT_BPS_READ_MAX, 0); > > > + } > > > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE_MAX)) { > > > + cfg->buckets[THROTTLE_BPS_WRITE].max = > > > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > > > + QEMU_OPT_BPS_WRITE_MAX, 0); > > > + } > > > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL_MAX)) { > > > + cfg->buckets[THROTTLE_OPS_TOTAL].max = > > > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > > > + QEMU_OPT_IOPS_TOTAL_MAX, 0); > > > + } > > > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ_MAX)) { > > > + cfg->buckets[THROTTLE_OPS_READ].max = > > > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > > > + QEMU_OPT_IOPS_READ_MAX, 0); > > > + } > > > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE_MAX)) { > > > + cfg->buckets[THROTTLE_OPS_WRITE].max = > > > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > > > + QEMU_OPT_IOPS_WRITE_MAX, 0); > > > + } > > > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL_MAX_LENGTH)) { > > > + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > > > + QEMU_OPT_BPS_TOTAL_MAX_LENGTH, 1) > UINT_MAX) { > > > + error_setg(errp, "%s value must be in the range [0, %u]", > > > + THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL_MAX_LENGTH, > > > + UINT_MAX); > > > + return -1; > > > + } > > > + cfg->buckets[THROTTLE_BPS_TOTAL].burst_length = > > > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > > > + QEMU_OPT_BPS_TOTAL_MAX_LENGTH, 1); > > > + } > > > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ_MAX_LENGTH)) { > > > + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > > > + QEMU_OPT_BPS_READ_MAX_LENGTH, 1) > UINT_MAX) { > > > + error_setg(errp, "%s must be in the range [0, %u]", > > > + THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ_MAX_LENGTH, > > > + UINT_MAX); > > > + return -1; > > > + } > > > + cfg->buckets[THROTTLE_BPS_READ].burst_length = > > > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > > > + QEMU_OPT_BPS_READ_MAX_LENGTH, 1); > > > + } > > > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE_MAX_LENGTH)) { > > > + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > > > + QEMU_OPT_BPS_WRITE_MAX_LENGTH, 1) > UINT_MAX) { > > > + error_setg(errp, "%s must be in the range [0, %u]", > > > + THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE_MAX_LENGTH, > > > + UINT_MAX); > > > + return -1; > > > + } > > > + cfg->buckets[THROTTLE_BPS_WRITE].burst_length = > > > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > > > + QEMU_OPT_BPS_WRITE_MAX_LENGTH, 1); > > > + } > > > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL_MAX_LENGTH)) { > > > + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > > > + QEMU_OPT_IOPS_TOTAL_MAX_LENGTH, 1) > UINT_MAX) { > > > + error_setg(errp, "%s must be in the range [0, %u]", > > > + THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL_MAX_LENGTH, > > > + UINT_MAX); > > > + return -1; > > > + } > > > + cfg->buckets[THROTTLE_OPS_TOTAL].burst_length = > > > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > > > + QEMU_OPT_IOPS_TOTAL_MAX_LENGTH, 1); > > > + } > > > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ_MAX_LENGTH)) { > > > + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > > > + QEMU_OPT_IOPS_READ_MAX_LENGTH, 1) > UINT_MAX) { > > > + error_setg(errp, "%s must be in the range [0, %u]", > > > + THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ_MAX_LENGTH, > > > + UINT_MAX); > > > + return -1; > > > + } > > > + cfg->buckets[THROTTLE_OPS_READ].burst_length = > > > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > > > + QEMU_OPT_IOPS_READ_MAX_LENGTH, 1); > > > + } > > > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE_MAX_LENGTH)) { > > > + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > > > + QEMU_OPT_IOPS_WRITE_MAX_LENGTH, 1) > UINT_MAX) { > > > + error_setg(errp, "%s must be in the range [0, %u]", > > > + THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE_MAX_LENGTH, > > > + UINT_MAX); > > > + return -1; > > > + } > > > + cfg->buckets[THROTTLE_OPS_WRITE].burst_length = > > > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX > > > + QEMU_OPT_IOPS_WRITE_MAX_LENGTH, 1); > > > + } > > > + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_SIZE)) { > > > + cfg->op_size = > > > + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_SIZE, > > > + 0); > > > + } > > > + return 0; > > > +} > > > > This function is very repetitive, but each block is long enough that > > you have to look closely to review whether the right constants are used > > everywhere. > > > > Maybe this could become a bit more readable with a macro or two? > > How about this? > > > # #define IF_OPT_SET(rvalue,opt_name) \ > # if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX opt_name)) { \ > # rvalue = qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX opt_name, > 0); } > > # IF_OPT_SET(cfg->buckets[THROTTLE_BPS_TOTAL].avg, QEMU_OPT_BPS_TOTAL); > # IF_OPT_SET(cfg->buckets[THROTTLE_BPS_READ].avg, QEMU_OPT_BPS_READ); > [...] Looks a lot more readable to me. :-) If nobody objects, I'd suggest to go this way. > > > +static int coroutine_fn throttle_co_pwritev(BlockDriverState *bs, > > > + uint64_t offset, uint64_t bytes, > > > + QEMUIOVector *qiov, int flags) > > > +{ > > > + ThrottleGroupMember *tgm = bs->opaque; > > > + throttle_group_co_io_limits_intercept(tgm, bytes, true); > > > + > > > + return bdrv_co_preadv(bs->file, offset, bytes, qiov, flags); > > > +} > > > > I think we want to set BlockDriver.supported_write_flags so that passing > > down flags is actually of any use. > > Can you explain what you mean? Do you mean doing this in throttle_open(): > > bs->supported_write_flags = bs->file->bs->supported_write_flags; > bs->supported_zero_flags = bs->file->bs->supported_zero_flags; Yes, I think that should do the trick. Kevin
On Thu, Aug 03, 2017 at 06:58:30AM -0500, Eric Blake wrote: >On 08/03/2017 03:07 AM, Kevin Wolf wrote: >> Am 31.07.2017 um 11:54 hat Manos Pitsidianakis geschrieben: >>> block/throttle.c uses existing I/O throttle infrastructure inside a >>> block filter driver. I/O operations are intercepted in the filter's >>> read/write coroutines, and referred to block/throttle-groups.c >>> >>> The driver can be used with the syntax >>> -drive driver=throttle,file.filename=foo.qcow2, \ >>> limits.iops-total=...,throttle-group=bar >>> >>> The configuration flags and their semantics are identical to the >>> hardcoded throttling ones. >>> >>> A node can be created referring to an existing group, and will overwrite >>> its limits if any are specified, otherwise they are retained. >>> >>> Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr> >>> --- > >>> + >>> + .is_filter = true, >>> +}; >> >> What about .bdrv_co_get_block_status? > >And if so, do you want my byte-based block status to go in first? (Our >two series conflict, so we need to pick who needs to rebase on top of >the other). No problem. My patch is in Kevin's branch for 2.11. Feel free to merge first if needed, I can rebase my patch if you do.
On Mon 31 Jul 2017 11:54:41 AM CEST, Manos Pitsidianakis wrote: > block/throttle.c uses existing I/O throttle infrastructure inside a > block filter driver. I/O operations are intercepted in the filter's > read/write coroutines, and referred to block/throttle-groups.c > > The driver can be used with the syntax > -drive driver=throttle,file.filename=foo.qcow2, \ > limits.iops-total=...,throttle-group=bar Sorry for not having noticed this earlier, but can't you define the throttling group (and its limits) using -object throttle-group ... as shown in the previous patch, and simply reference it here? Or would we have two alternative ways of setting the throttling limits? What happens if you have many -drive lines each one with a different set of limits but with the same throttling group? Berto
On Tue, Aug 08, 2017 at 03:13:36PM +0200, Alberto Garcia wrote: >On Mon 31 Jul 2017 11:54:41 AM CEST, Manos Pitsidianakis wrote: >> block/throttle.c uses existing I/O throttle infrastructure inside a >> block filter driver. I/O operations are intercepted in the filter's >> read/write coroutines, and referred to block/throttle-groups.c >> >> The driver can be used with the syntax >> -drive driver=throttle,file.filename=foo.qcow2, \ >> limits.iops-total=...,throttle-group=bar > >Sorry for not having noticed this earlier, but can't you define the >throttling group (and its limits) using -object throttle-group ... as >shown in the previous patch, and simply reference it here? Or would we >have two alternative ways of setting the throttling limits? > >What happens if you have many -drive lines each one with a different set >of limits but with the same throttling group? The limits of the last one to be processed will win. Quoting a reply I made to Kevin on the interface test patch: >> You're right, I missed this. The test result shows that this command >> succeeds. Do we really want to allow other nodes to be affected with a >> blockdev-add? Wouldn't it be cleaner to just forbid the combination of >> limits and throtte-group? > >So basically only anonymous, immutable groups can be created through the >driver then. All other shared group configurations must be explicitly >created with an -object / object-add syntax. I think this is a neat >separation and compromise if we allow anonymous groups. If not, we can >ignore limits on the throttle driver. So basically if we have anonymous groups, we accept limits in the driver options but only without a group-name. Without anonymous groups, we remove limits from the driver options and only use the object-add/-object commands to create throttle groups. Does this sound like a good idea? It will be more verbose for the human user. One advantage: all throttle groups can then be managed through qom-set/qom-get since they are owned by the qom tree.
On Tue 08 Aug 2017 03:45:44 PM CEST, Manos Pitsidianakis wrote: > On Tue, Aug 08, 2017 at 03:13:36PM +0200, Alberto Garcia wrote: >>On Mon 31 Jul 2017 11:54:41 AM CEST, Manos Pitsidianakis wrote: >>> block/throttle.c uses existing I/O throttle infrastructure inside a >>> block filter driver. I/O operations are intercepted in the filter's >>> read/write coroutines, and referred to block/throttle-groups.c >>> >>> The driver can be used with the syntax >>> -drive driver=throttle,file.filename=foo.qcow2, \ >>> limits.iops-total=...,throttle-group=bar >> >>Sorry for not having noticed this earlier, but can't you define the >>throttling group (and its limits) using -object throttle-group ... as >>shown in the previous patch, and simply reference it here? Or would we >>have two alternative ways of setting the throttling limits? >> >>What happens if you have many -drive lines each one with a different set >>of limits but with the same throttling group? > > The limits of the last one to be processed will win. That's what the current throttling API does, and I tend to agree that it's not completely straightforward (a few people have asked me the same question since this feature is available). If we're going to add a new API we could eliminate this ambiguity. After all the previous -drive throttling.iops-total=... would still be available, wouldn't it? > So basically if we have anonymous groups, we accept limits in the > driver options but only without a group-name. In the commit message you do however have limits and a group name, is that a mistake? -drive driver=throttle,file.filename=foo.qcow2, \ limits.iops-total=...,throttle-group=bar Berto
On Tue, Aug 08, 2017 at 04:53:08PM +0200, Alberto Garcia wrote: >On Tue 08 Aug 2017 03:45:44 PM CEST, Manos Pitsidianakis wrote: >> On Tue, Aug 08, 2017 at 03:13:36PM +0200, Alberto Garcia wrote: >>>On Mon 31 Jul 2017 11:54:41 AM CEST, Manos Pitsidianakis wrote: >>>> block/throttle.c uses existing I/O throttle infrastructure inside a >>>> block filter driver. I/O operations are intercepted in the filter's >>>> read/write coroutines, and referred to block/throttle-groups.c >>>> >>>> The driver can be used with the syntax >>>> -drive driver=throttle,file.filename=foo.qcow2, \ >>>> limits.iops-total=...,throttle-group=bar >>> >>>Sorry for not having noticed this earlier, but can't you define the >>>throttling group (and its limits) using -object throttle-group ... as >>>shown in the previous patch, and simply reference it here? Or would we >>>have two alternative ways of setting the throttling limits? >>> >>>What happens if you have many -drive lines each one with a different set >>>of limits but with the same throttling group? >> >> The limits of the last one to be processed will win. > >That's what the current throttling API does, and I tend to agree that >it's not completely straightforward (a few people have asked me the same >question since this feature is available). > >If we're going to add a new API we could eliminate this ambiguity. After >all the previous -drive throttling.iops-total=... would still be >available, wouldn't it? Indeed, it already is. > >> So basically if we have anonymous groups, we accept limits in the >> driver options but only without a group-name. > >In the commit message you do however have limits and a group name, is >that a mistake? > > -drive driver=throttle,file.filename=foo.qcow2, \ > limits.iops-total=...,throttle-group=bar > Sorry this wasn't clear, I'm actually proposing to remove limits from the throttle driver options and only create/config throttle groups via -object/object-add.
On Tue 08 Aug 2017 04:56:20 PM CEST, Manos Pitsidianakis wrote: >>> So basically if we have anonymous groups, we accept limits in the >>> driver options but only without a group-name. >> >>In the commit message you do however have limits and a group name, is >>that a mistake? >> >> -drive driver=throttle,file.filename=foo.qcow2, \ >> limits.iops-total=...,throttle-group=bar > > Sorry this wasn't clear, I'm actually proposing to remove limits from > the throttle driver options and only create/config throttle groups via > -object/object-add. Sorry I think it was me who misunderstood :-) Anyway in the new command-line API I would be more inclined to have limits defined using "-object throttle-group" and -drive would only reference the group id. I understand that this implies that it wouldn't be possible to create anonymous groups (at least not from the command line), is that a problem? Berto
On Tue, Aug 08, 2017 at 05:04:48PM +0200, Alberto Garcia wrote: >On Tue 08 Aug 2017 04:56:20 PM CEST, Manos Pitsidianakis wrote: >>>> So basically if we have anonymous groups, we accept limits in the >>>> driver options but only without a group-name. >>> >>>In the commit message you do however have limits and a group name, is >>>that a mistake? >>> >>> -drive driver=throttle,file.filename=foo.qcow2, \ >>> limits.iops-total=...,throttle-group=bar >> >> Sorry this wasn't clear, I'm actually proposing to remove limits from >> the throttle driver options and only create/config throttle groups via >> -object/object-add. > >Sorry I think it was me who misunderstood :-) Anyway in the new >command-line API I would be more inclined to have limits defined using >"-object throttle-group" and -drive would only reference the group id. > >I understand that this implies that it wouldn't be possible to create >anonymous groups (at least not from the command line), is that a >problem? > We can accept anonymous groups if a user specifies limits but not a group name in the throttle driver. (The only case where limits would be acccepted) Not creating eponymous throttle groups via the throttle driver means we don't need throttle_groups anymore, since even anonymous ones don't need to be accounted for in a list. I will send a new revision for this series but I can make this change in a later patch if everyone agrees.
On Wed 09 Aug 2017 11:36:12 AM CEST, Manos Pitsidianakis wrote: > On Tue, Aug 08, 2017 at 05:04:48PM +0200, Alberto Garcia wrote: >>On Tue 08 Aug 2017 04:56:20 PM CEST, Manos Pitsidianakis wrote: >>>>> So basically if we have anonymous groups, we accept limits in the >>>>> driver options but only without a group-name. >>>> >>>>In the commit message you do however have limits and a group name, is >>>>that a mistake? >>>> >>>> -drive driver=throttle,file.filename=foo.qcow2, \ >>>> limits.iops-total=...,throttle-group=bar >>> >>> Sorry this wasn't clear, I'm actually proposing to remove limits from >>> the throttle driver options and only create/config throttle groups via >>> -object/object-add. >> >>Sorry I think it was me who misunderstood :-) Anyway in the new >>command-line API I would be more inclined to have limits defined using >>"-object throttle-group" and -drive would only reference the group id. >> >>I understand that this implies that it wouldn't be possible to create >>anonymous groups (at least not from the command line), is that a >>problem? > > We can accept anonymous groups if a user specifies limits but not a > group name in the throttle driver. (The only case where limits would > be acccepted) Yeah but that's only if we have the limits.iops-total=... options in the throttle driver. If we "remove limits from the throttle driver options and only create/config throttle groups via -object/object-add" we cannot do that. > Not creating eponymous throttle groups via the throttle driver means > we don't need throttle_groups anymore, since even anonymous ones don't > need to be accounted for in a list. I don't follow you here, how else do you get a group by its name? Berto
On Wed, Aug 09, 2017 at 02:36:20PM +0200, Alberto Garcia wrote: >On Wed 09 Aug 2017 11:36:12 AM CEST, Manos Pitsidianakis wrote: >> On Tue, Aug 08, 2017 at 05:04:48PM +0200, Alberto Garcia wrote: >>>On Tue 08 Aug 2017 04:56:20 PM CEST, Manos Pitsidianakis wrote: >>>>>> So basically if we have anonymous groups, we accept limits in the >>>>>> driver options but only without a group-name. >>>>> >>>>>In the commit message you do however have limits and a group name, is >>>>>that a mistake? >>>>> >>>>> -drive driver=throttle,file.filename=foo.qcow2, \ >>>>> limits.iops-total=...,throttle-group=bar >>>> >>>> Sorry this wasn't clear, I'm actually proposing to remove limits from >>>> the throttle driver options and only create/config throttle groups via >>>> -object/object-add. >>> >>>Sorry I think it was me who misunderstood :-) Anyway in the new >>>command-line API I would be more inclined to have limits defined using >>>"-object throttle-group" and -drive would only reference the group id. >>> >>>I understand that this implies that it wouldn't be possible to create >>>anonymous groups (at least not from the command line), is that a >>>problem? >> >> We can accept anonymous groups if a user specifies limits but not a >> group name in the throttle driver. (The only case where limits would >> be acccepted) > >Yeah but that's only if we have the limits.iops-total=... options in the >throttle driver. If we "remove limits from the throttle driver options >and only create/config throttle groups via -object/object-add" we >cannot >do that. We can check that groups is not defined at the same time as limits, > >> Not creating eponymous throttle groups via the throttle driver means >> we don't need throttle_groups anymore, since even anonymous ones don't >> need to be accounted for in a list. > >I don't follow you here, how else do you get a group by its name? If all eponymous groups are managed by the QOM tree, we should be able to iterate over the object root container for all ThrottleGroups just like qmp_query_iothreads() in iothread.c
On Wed 09 Aug 2017 03:42:07 PM CEST, Manos Pitsidianakis wrote: > On Wed, Aug 09, 2017 at 02:36:20PM +0200, Alberto Garcia wrote: >>On Wed 09 Aug 2017 11:36:12 AM CEST, Manos Pitsidianakis wrote: >>> On Tue, Aug 08, 2017 at 05:04:48PM +0200, Alberto Garcia wrote: >>>>On Tue 08 Aug 2017 04:56:20 PM CEST, Manos Pitsidianakis wrote: >>>>>>> So basically if we have anonymous groups, we accept limits in the >>>>>>> driver options but only without a group-name. >>>>>> >>>>>>In the commit message you do however have limits and a group name, is >>>>>>that a mistake? >>>>>> >>>>>> -drive driver=throttle,file.filename=foo.qcow2, \ >>>>>> limits.iops-total=...,throttle-group=bar >>>>> >>>>> Sorry this wasn't clear, I'm actually proposing to remove limits from >>>>> the throttle driver options and only create/config throttle groups via >>>>> -object/object-add. >>>> >>>>Sorry I think it was me who misunderstood :-) Anyway in the new >>>>command-line API I would be more inclined to have limits defined using >>>>"-object throttle-group" and -drive would only reference the group id. >>>> >>>>I understand that this implies that it wouldn't be possible to create >>>>anonymous groups (at least not from the command line), is that a >>>>problem? >>> >>> We can accept anonymous groups if a user specifies limits but not a >>> group name in the throttle driver. (The only case where limits would >>> be acccepted) >> >>Yeah but that's only if we have the limits.iops-total=... options in the >>throttle driver. If we "remove limits from the throttle driver options >>and only create/config throttle groups via -object/object-add" we >>cannot >>do that. > > We can check that groups is not defined at the same time as limits, I'm not sure if I'm following the conversation anymore :-) let's try to recap: a) Groups are defined like this (with the current patches): -object throttle-group,id=foo,x-iops-total=100,x-.. b) Throttle nodes are defined like this: -drive driver=throttle,file.filename=foo.qcow2, \ limits.iops-total=...,throttle-group=bar c) Therefore limits can be defined either in (a) or (b) d) If we omit throttle-group=... in (b), we would create an anonymous group. e) The -drive syntax from (b) has the "problem" that it's possible to define several nodes with the same throttling group but different limits. The last one would win (as the legacy syntax does), but that's not something completely straightforward for the end user. f) The syntax from (b) also has the problem that there's one more place that needs code to parse throttling limits. g) We can solve (e) and (f) if we remove the limits.* options altogether from the throttling filter. In that case you would need to define a throttle-group object and use the throttle-group option of the filter node in all cases. h) If we remove the limits.* options we cannot have anonymous groups anymore (at least not using this API). My question is: is it a problem? What would we lose? What benefits do anonymous groups bring us? i) We can of course maintain the limits.* options, but disallow throttle-group when they are present. That way we would allow anonymous groups and we would solve the ambiguity problem described in (e). My question is: is it worth it? >>> Not creating eponymous throttle groups via the throttle driver means >>> we don't need throttle_groups anymore, since even anonymous ones >>> don't need to be accounted for in a list. >> >>I don't follow you here, how else do you get a group by its name? > > If all eponymous groups are managed by the QOM tree, we should be able > to iterate over the object root container for all ThrottleGroups just > like qmp_query_iothreads() in iothread.c Mmm... can't we actually use the root container now already? (even with anonymous groups I mean). Why do we need throttle_groups? Berto
Am 09.08.2017 um 16:45 hat Alberto Garcia geschrieben: > On Wed 09 Aug 2017 03:42:07 PM CEST, Manos Pitsidianakis wrote: > > On Wed, Aug 09, 2017 at 02:36:20PM +0200, Alberto Garcia wrote: > >>On Wed 09 Aug 2017 11:36:12 AM CEST, Manos Pitsidianakis wrote: > >>> On Tue, Aug 08, 2017 at 05:04:48PM +0200, Alberto Garcia wrote: > >>>>On Tue 08 Aug 2017 04:56:20 PM CEST, Manos Pitsidianakis wrote: > >>>>>>> So basically if we have anonymous groups, we accept limits in the > >>>>>>> driver options but only without a group-name. > >>>>>> > >>>>>>In the commit message you do however have limits and a group name, is > >>>>>>that a mistake? > >>>>>> > >>>>>> -drive driver=throttle,file.filename=foo.qcow2, \ > >>>>>> limits.iops-total=...,throttle-group=bar > >>>>> > >>>>> Sorry this wasn't clear, I'm actually proposing to remove limits from > >>>>> the throttle driver options and only create/config throttle groups via > >>>>> -object/object-add. > >>>> > >>>>Sorry I think it was me who misunderstood :-) Anyway in the new > >>>>command-line API I would be more inclined to have limits defined using > >>>>"-object throttle-group" and -drive would only reference the group id. > >>>> > >>>>I understand that this implies that it wouldn't be possible to create > >>>>anonymous groups (at least not from the command line), is that a > >>>>problem? > >>> > >>> We can accept anonymous groups if a user specifies limits but not a > >>> group name in the throttle driver. (The only case where limits would > >>> be acccepted) > >> > >>Yeah but that's only if we have the limits.iops-total=... options in the > >>throttle driver. If we "remove limits from the throttle driver options > >>and only create/config throttle groups via -object/object-add" we > >>cannot > >>do that. > > > > We can check that groups is not defined at the same time as limits, > > I'm not sure if I'm following the conversation anymore :-) let's try to > recap: > > a) Groups are defined like this (with the current patches): > > -object throttle-group,id=foo,x-iops-total=100,x-.. > > b) Throttle nodes are defined like this: > > -drive driver=throttle,file.filename=foo.qcow2, \ > limits.iops-total=...,throttle-group=bar > > c) Therefore limits can be defined either in (a) or (b) > > d) If we omit throttle-group=... in (b), we would create an anonymous > group. > > e) The -drive syntax from (b) has the "problem" that it's possible to > define several nodes with the same throttling group but different > limits. The last one would win (as the legacy syntax does), but > that's not something completely straightforward for the end user. > > f) The syntax from (b) also has the problem that there's one more > place that needs code to parse throttling limits. > > g) We can solve (e) and (f) if we remove the limits.* options > altogether from the throttling filter. In that case you would need > to define a throttle-group object and use the throttle-group option > of the filter node in all cases. > > h) If we remove the limits.* options we cannot have anonymous groups > anymore (at least not using this API). My question is: is it a > problem? What would we lose? What benefits do anonymous groups > bring us? As I understand it, basically only the convenience of begin able to specify a limit directly in -drive rather than having to create an -object and reference its ID. > i) We can of course maintain the limits.* options, but disallow > throttle-group when they are present. That way we would allow > anonymous groups and we would solve the ambiguity problem described > in (e). My question is: is it worth it? Maybe not. Depends on how important we consider the convenience feature. > >>> Not creating eponymous throttle groups via the throttle driver means > >>> we don't need throttle_groups anymore, since even anonymous ones > >>> don't need to be accounted for in a list. > >> > >>I don't follow you here, how else do you get a group by its name? > > > > If all eponymous groups are managed by the QOM tree, we should be able > > to iterate over the object root container for all ThrottleGroups just > > like qmp_query_iothreads() in iothread.c > > Mmm... can't we actually use the root container now already? (even with > anonymous groups I mean). Why do we need throttle_groups? Anonymous groups don't have a parent, so they aren't accessible through the root container. Kevin
On Wed, Aug 09, 2017 at 05:39:42PM +0200, Kevin Wolf wrote: >Am 09.08.2017 um 16:45 hat Alberto Garcia geschrieben: >> On Wed 09 Aug 2017 03:42:07 PM CEST, Manos Pitsidianakis wrote: >> > On Wed, Aug 09, 2017 at 02:36:20PM +0200, Alberto Garcia wrote: >> >>On Wed 09 Aug 2017 11:36:12 AM CEST, Manos Pitsidianakis wrote: >> >>> On Tue, Aug 08, 2017 at 05:04:48PM +0200, Alberto Garcia wrote: >> >>>>On Tue 08 Aug 2017 04:56:20 PM CEST, Manos Pitsidianakis wrote: >> >>>>>>> So basically if we have anonymous groups, we accept limits in the >> >>>>>>> driver options but only without a group-name. >> >>>>>> >> >>>>>>In the commit message you do however have limits and a group name, is >> >>>>>>that a mistake? >> >>>>>> >> >>>>>> -drive driver=throttle,file.filename=foo.qcow2, \ >> >>>>>> limits.iops-total=...,throttle-group=bar >> >>>>> >> >>>>> Sorry this wasn't clear, I'm actually proposing to remove limits from >> >>>>> the throttle driver options and only create/config throttle groups via >> >>>>> -object/object-add. >> >>>> >> >>>>Sorry I think it was me who misunderstood :-) Anyway in the new >> >>>>command-line API I would be more inclined to have limits defined using >> >>>>"-object throttle-group" and -drive would only reference the group id. >> >>>> >> >>>>I understand that this implies that it wouldn't be possible to create >> >>>>anonymous groups (at least not from the command line), is that a >> >>>>problem? >> >>> >> >>> We can accept anonymous groups if a user specifies limits but not a >> >>> group name in the throttle driver. (The only case where limits would >> >>> be acccepted) >> >> >> >>Yeah but that's only if we have the limits.iops-total=... options in the >> >>throttle driver. If we "remove limits from the throttle driver options >> >>and only create/config throttle groups via -object/object-add" we >> >>cannot >> >>do that. >> > >> > We can check that groups is not defined at the same time as limits, >> >> I'm not sure if I'm following the conversation anymore :-) let's try to >> recap: >> >> a) Groups are defined like this (with the current patches): >> >> -object throttle-group,id=foo,x-iops-total=100,x-.. >> >> b) Throttle nodes are defined like this: >> >> -drive driver=throttle,file.filename=foo.qcow2, \ >> limits.iops-total=...,throttle-group=bar >> >> c) Therefore limits can be defined either in (a) or (b) >> >> d) If we omit throttle-group=... in (b), we would create an anonymous >> group. >> >> e) The -drive syntax from (b) has the "problem" that it's possible to >> define several nodes with the same throttling group but different >> limits. The last one would win (as the legacy syntax does), but >> that's not something completely straightforward for the end user. >> >> f) The syntax from (b) also has the problem that there's one more >> place that needs code to parse throttling limits. >> >> g) We can solve (e) and (f) if we remove the limits.* options >> altogether from the throttling filter. In that case you would need >> to define a throttle-group object and use the throttle-group option >> of the filter node in all cases. >> >> h) If we remove the limits.* options we cannot have anonymous groups >> anymore (at least not using this API). My question is: is it a >> problem? What would we lose? What benefits do anonymous groups >> bring us? > >As I understand it, basically only the convenience of begin able to >specify a limit directly in -drive rather than having to create an >-object and reference its ID. > >> i) We can of course maintain the limits.* options, but disallow >> throttle-group when they are present. That way we would allow >> anonymous groups and we would solve the ambiguity problem described >> in (e). My question is: is it worth it? > >Maybe not. Depends on how important we consider the convenience feature. > >> >>> Not creating eponymous throttle groups via the throttle driver means >> >>> we don't need throttle_groups anymore, since even anonymous ones >> >>> don't need to be accounted for in a list. >> >> >> >>I don't follow you here, how else do you get a group by its name? >> > >> > If all eponymous groups are managed by the QOM tree, we should be able >> > to iterate over the object root container for all ThrottleGroups just >> > like qmp_query_iothreads() in iothread.c >> >> Mmm... can't we actually use the root container now already? (even with >> anonymous groups I mean). Why do we need throttle_groups? > >Anonymous groups don't have a parent, so they aren't accessible through >the root container. Anonymous groups wouldn't have to be in any kind of list, since they shouldn't be accessible by others and they don't have a name. They have a refcount of 1 and are owned by their filter node. (throttle_groups is used in throttle_group_incref() to fetch a tg with a name and increase its reference count). They will also not be accessible with qom-set and qom-get. I've prepared the root container change and will include it in the next revision. Do we go with anonymous groups and disallow limits.* with throttle-group, or do we drop anonymous groups altogether?
diff --git a/block/Makefile.objs b/block/Makefile.objs index 2aaede4ae1..6eaf78a046 100644 --- a/block/Makefile.objs +++ b/block/Makefile.objs @@ -25,6 +25,7 @@ block-obj-y += accounting.o dirty-bitmap.o block-obj-y += write-threshold.o block-obj-y += backup.o block-obj-$(CONFIG_REPLICATION) += replication.o +block-obj-y += throttle.o block-obj-y += crypto.o diff --git a/block/throttle.c b/block/throttle.c new file mode 100644 index 0000000000..f3395462fb --- /dev/null +++ b/block/throttle.c @@ -0,0 +1,395 @@ +/* + * QEMU block throttling filter driver infrastructure + * + * Copyright (c) 2017 Manos Pitsidianakis + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 or + * (at your option) version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, see <http://www.gnu.org/licenses/>. + */ + +#include "qemu/osdep.h" +#include "block/throttle-groups.h" +#include "qemu/throttle-options.h" +#include "qapi/error.h" + +#undef THROTTLE_OPT_PREFIX +#define THROTTLE_OPT_PREFIX "limits." +static QemuOptsList throttle_opts = { + .name = "throttle", + .head = QTAILQ_HEAD_INITIALIZER(throttle_opts.head), + .desc = { + THROTTLE_OPTS, + { + .name = QEMU_OPT_THROTTLE_GROUP_NAME, + .type = QEMU_OPT_STRING, + .help = "throttle group name", + }, + { /* end of list */ } + }, +}; + +/* Extract ThrottleConfig options. Assumes cfg is initialized and will be + * checked for validity. + */ +static int throttle_extract_options(QemuOpts *opts, ThrottleConfig *cfg, + Error **errp) +{ + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL)) { + cfg->buckets[THROTTLE_BPS_TOTAL].avg = + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL, + 0); + } + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ)) { + cfg->buckets[THROTTLE_BPS_READ].avg = + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ, + 0); + } + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE)) { + cfg->buckets[THROTTLE_BPS_WRITE].avg = + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE, + 0); + } + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL)) { + cfg->buckets[THROTTLE_OPS_TOTAL].avg = + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL, + 0); + } + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ)) { + cfg->buckets[THROTTLE_OPS_READ].avg = + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ, + 0); + } + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE)) { + cfg->buckets[THROTTLE_OPS_WRITE].avg = + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE, + 0); + } + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL_MAX)) { + cfg->buckets[THROTTLE_BPS_TOTAL].max = + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX + QEMU_OPT_BPS_TOTAL_MAX, 0); + } + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ_MAX)) { + cfg->buckets[THROTTLE_BPS_READ].max = + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX + QEMU_OPT_BPS_READ_MAX, 0); + } + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE_MAX)) { + cfg->buckets[THROTTLE_BPS_WRITE].max = + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX + QEMU_OPT_BPS_WRITE_MAX, 0); + } + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL_MAX)) { + cfg->buckets[THROTTLE_OPS_TOTAL].max = + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX + QEMU_OPT_IOPS_TOTAL_MAX, 0); + } + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ_MAX)) { + cfg->buckets[THROTTLE_OPS_READ].max = + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX + QEMU_OPT_IOPS_READ_MAX, 0); + } + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE_MAX)) { + cfg->buckets[THROTTLE_OPS_WRITE].max = + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX + QEMU_OPT_IOPS_WRITE_MAX, 0); + } + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL_MAX_LENGTH)) { + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX + QEMU_OPT_BPS_TOTAL_MAX_LENGTH, 1) > UINT_MAX) { + error_setg(errp, "%s value must be in the range [0, %u]", + THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL_MAX_LENGTH, + UINT_MAX); + return -1; + } + cfg->buckets[THROTTLE_BPS_TOTAL].burst_length = + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX + QEMU_OPT_BPS_TOTAL_MAX_LENGTH, 1); + } + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ_MAX_LENGTH)) { + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX + QEMU_OPT_BPS_READ_MAX_LENGTH, 1) > UINT_MAX) { + error_setg(errp, "%s must be in the range [0, %u]", + THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ_MAX_LENGTH, + UINT_MAX); + return -1; + } + cfg->buckets[THROTTLE_BPS_READ].burst_length = + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX + QEMU_OPT_BPS_READ_MAX_LENGTH, 1); + } + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE_MAX_LENGTH)) { + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX + QEMU_OPT_BPS_WRITE_MAX_LENGTH, 1) > UINT_MAX) { + error_setg(errp, "%s must be in the range [0, %u]", + THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE_MAX_LENGTH, + UINT_MAX); + return -1; + } + cfg->buckets[THROTTLE_BPS_WRITE].burst_length = + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX + QEMU_OPT_BPS_WRITE_MAX_LENGTH, 1); + } + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL_MAX_LENGTH)) { + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX + QEMU_OPT_IOPS_TOTAL_MAX_LENGTH, 1) > UINT_MAX) { + error_setg(errp, "%s must be in the range [0, %u]", + THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL_MAX_LENGTH, + UINT_MAX); + return -1; + } + cfg->buckets[THROTTLE_OPS_TOTAL].burst_length = + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX + QEMU_OPT_IOPS_TOTAL_MAX_LENGTH, 1); + } + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ_MAX_LENGTH)) { + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX + QEMU_OPT_IOPS_READ_MAX_LENGTH, 1) > UINT_MAX) { + error_setg(errp, "%s must be in the range [0, %u]", + THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ_MAX_LENGTH, + UINT_MAX); + return -1; + } + cfg->buckets[THROTTLE_OPS_READ].burst_length = + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX + QEMU_OPT_IOPS_READ_MAX_LENGTH, 1); + } + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE_MAX_LENGTH)) { + if (qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX + QEMU_OPT_IOPS_WRITE_MAX_LENGTH, 1) > UINT_MAX) { + error_setg(errp, "%s must be in the range [0, %u]", + THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE_MAX_LENGTH, + UINT_MAX); + return -1; + } + cfg->buckets[THROTTLE_OPS_WRITE].burst_length = + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX + QEMU_OPT_IOPS_WRITE_MAX_LENGTH, 1); + } + if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_SIZE)) { + cfg->op_size = + qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_SIZE, + 0); + } + return 0; +} + +static int throttle_configure_tgm(BlockDriverState *bs, + ThrottleGroupMember *tgm, + QDict *options, Error **errp) +{ + int ret; + ThrottleConfig cfg; + const char *group_name = NULL; + Error *local_err = NULL; + QemuOpts *opts = qemu_opts_create(&throttle_opts, NULL, 0, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return -EINVAL; + } + + qemu_opts_absorb_qdict(opts, options, &local_err); + if (local_err) { + error_propagate(errp, local_err); + goto err; + } + + /* If no name is specified, an anonymous group will be created */ + group_name = qemu_opt_get(opts, QEMU_OPT_THROTTLE_GROUP_NAME); + + /* Register membership to group with name group_name */ + throttle_group_register_tgm(tgm, group_name, bdrv_get_aio_context(bs)); + + /* Copy previous configuration */ + throttle_group_get_config(tgm, &cfg); + + /* Change limits if user has specified them */ + if (throttle_extract_options(opts, &cfg, errp) || + !throttle_is_valid(&cfg, errp)) { + throttle_group_unregister_tgm(tgm); + goto err; + } + /* Update group configuration */ + throttle_group_config(tgm, &cfg); + + ret = 0; + goto fin; + +err: + ret = -EINVAL; +fin: + qemu_opts_del(opts); + return ret; +} + +static int throttle_open(BlockDriverState *bs, QDict *options, + int flags, Error **errp) +{ + ThrottleGroupMember *tgm = bs->opaque; + + bs->file = bdrv_open_child(NULL, options, "file", + bs, &child_file, false, errp); + if (!bs->file) { + return -EINVAL; + } + + return throttle_configure_tgm(bs, tgm, options, errp); +} + +static void throttle_close(BlockDriverState *bs) +{ + ThrottleGroupMember *tgm = bs->opaque; + throttle_group_unregister_tgm(tgm); +} + + +static int64_t throttle_getlength(BlockDriverState *bs) +{ + return bdrv_getlength(bs->file->bs); +} + + +static int coroutine_fn throttle_co_preadv(BlockDriverState *bs, + uint64_t offset, uint64_t bytes, + QEMUIOVector *qiov, int flags) +{ + + ThrottleGroupMember *tgm = bs->opaque; + throttle_group_co_io_limits_intercept(tgm, bytes, false); + + return bdrv_co_preadv(bs->file, offset, bytes, qiov, flags); +} + +static int coroutine_fn throttle_co_pwritev(BlockDriverState *bs, + uint64_t offset, uint64_t bytes, + QEMUIOVector *qiov, int flags) +{ + ThrottleGroupMember *tgm = bs->opaque; + throttle_group_co_io_limits_intercept(tgm, bytes, true); + + return bdrv_co_preadv(bs->file, offset, bytes, qiov, flags); +} + +static int coroutine_fn throttle_co_pwrite_zeroes(BlockDriverState *bs, + int64_t offset, int bytes, BdrvRequestFlags flags) +{ + ThrottleGroupMember *tgm = bs->opaque; + throttle_group_co_io_limits_intercept(tgm, bytes, true); + + return bdrv_co_pwrite_zeroes(bs->file, offset, bytes, flags); +} + +static int coroutine_fn throttle_co_pdiscard(BlockDriverState *bs, + int64_t offset, int bytes) +{ + ThrottleGroupMember *tgm = bs->opaque; + throttle_group_co_io_limits_intercept(tgm, bytes, true); + + return bdrv_co_pdiscard(bs->file->bs, offset, bytes); +} + +static int throttle_co_flush(BlockDriverState *bs) +{ + return bdrv_co_flush(bs->file->bs); +} + +static void throttle_detach_aio_context(BlockDriverState *bs) +{ + ThrottleGroupMember *tgm = bs->opaque; + throttle_group_detach_aio_context(tgm); +} + +static void throttle_attach_aio_context(BlockDriverState *bs, + AioContext *new_context) +{ + ThrottleGroupMember *tgm = bs->opaque; + throttle_group_attach_aio_context(tgm, new_context); +} + +static int throttle_reopen_prepare(BDRVReopenState *reopen_state, + BlockReopenQueue *queue, Error **errp) +{ + ThrottleGroupMember *tgm = NULL; + + assert(reopen_state != NULL); + assert(reopen_state->bs != NULL); + + reopen_state->opaque = g_new0(ThrottleGroupMember, 1); + tgm = reopen_state->opaque; + + return throttle_configure_tgm(reopen_state->bs, tgm, reopen_state->options, + errp); +} + +static void throttle_reopen_commit(BDRVReopenState *state) +{ + ThrottleGroupMember *tgm = state->bs->opaque; + + throttle_group_unregister_tgm(tgm); + g_free(state->bs->opaque); + state->bs->opaque = state->opaque; + state->opaque = NULL; +} + +static void throttle_reopen_abort(BDRVReopenState *state) +{ + ThrottleGroupMember *tgm = state->opaque; + + throttle_group_unregister_tgm(tgm); + g_free(state->opaque); + state->opaque = NULL; +} + +static bool throttle_recurse_is_first_non_filter(BlockDriverState *bs, + BlockDriverState *candidate) +{ + return bdrv_recurse_is_first_non_filter(bs->file->bs, candidate); +} + +static BlockDriver bdrv_throttle = { + .format_name = "throttle", + .protocol_name = "throttle", + .instance_size = sizeof(ThrottleGroupMember), + + .bdrv_file_open = throttle_open, + .bdrv_close = throttle_close, + .bdrv_co_flush = throttle_co_flush, + + .bdrv_child_perm = bdrv_filter_default_perms, + + .bdrv_getlength = throttle_getlength, + + .bdrv_co_preadv = throttle_co_preadv, + .bdrv_co_pwritev = throttle_co_pwritev, + + .bdrv_co_pwrite_zeroes = throttle_co_pwrite_zeroes, + .bdrv_co_pdiscard = throttle_co_pdiscard, + + .bdrv_recurse_is_first_non_filter = throttle_recurse_is_first_non_filter, + + .bdrv_attach_aio_context = throttle_attach_aio_context, + .bdrv_detach_aio_context = throttle_detach_aio_context, + + .bdrv_reopen_prepare = throttle_reopen_prepare, + .bdrv_reopen_commit = throttle_reopen_commit, + .bdrv_reopen_abort = throttle_reopen_abort, + + .is_filter = true, +}; + +static void bdrv_throttle_init(void) +{ + bdrv_register(&bdrv_throttle); +} + +block_init(bdrv_throttle_init); diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h index 182b7896e1..3528a8f4a2 100644 --- a/include/qemu/throttle-options.h +++ b/include/qemu/throttle-options.h @@ -29,6 +29,7 @@ #define QEMU_OPT_BPS_WRITE_MAX "bps-write-max" #define QEMU_OPT_BPS_WRITE_MAX_LENGTH "bps-write-max-length" #define QEMU_OPT_IOPS_SIZE "iops-size" +#define QEMU_OPT_THROTTLE_GROUP_NAME "throttle-group" #define THROTTLE_OPT_PREFIX "throttling." #define THROTTLE_OPTS \
block/throttle.c uses existing I/O throttle infrastructure inside a block filter driver. I/O operations are intercepted in the filter's read/write coroutines, and referred to block/throttle-groups.c The driver can be used with the syntax -drive driver=throttle,file.filename=foo.qcow2, \ limits.iops-total=...,throttle-group=bar The configuration flags and their semantics are identical to the hardcoded throttling ones. A node can be created referring to an existing group, and will overwrite its limits if any are specified, otherwise they are retained. Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr> --- block/Makefile.objs | 1 + block/throttle.c | 395 ++++++++++++++++++++++++++++++++++++++++ include/qemu/throttle-options.h | 1 + 3 files changed, 397 insertions(+) create mode 100644 block/throttle.c