| Message ID | 1494405683-20877-4-git-send-email-pradeep.jagadeesh@huawei.com |
|---|---|
| State | New |
| Headers | show |
On 05/10/2017 03:41 AM, Pradeep Jagadeesh wrote: > This patch factor out the duplicate qmp throttle interface code > that was present in both block and fsdev device files. > > Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> > --- > blockdev.c | 53 +++------------------------------------- > hmp.c | 21 +++++++++++----- > include/qemu/throttle-options.h | 2 ++ > util/throttle-options.c | 54 +++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 74 insertions(+), 56 deletions(-) > > +++ b/include/qemu/throttle-options.h > @@ -94,4 +94,6 @@ > > void parse_io_throttle_options(ThrottleConfig *, QemuOpts *); > > +void qmp_set_io_throttle(ThrottleConfig *, IOThrottle *); I don't think this is the right name for this function. You are not adding a new QMP command (you didn't modify a .json file to add 'set-io-throttle'). As mentioned in patch 1/4, you can probably reuse the existing util/throttle.c instead of throttle-options.c, and name this something starting with throttle_ rather than with qmp_.
On Wed, 10 May 2017 04:41:22 -0400 Pradeep Jagadeesh <pradeepkiruvale@gmail.com> wrote: > This patch factor out the duplicate qmp throttle interface code > that was present in both block and fsdev device files. > There isn't any duplicated code at this stage since you have changed the patch ordering. Also, this patch is factoring out several things: - factoring code out of qmp_block_set_io_throttle() - factoring code out of hmp_block_set_io_throttle() So I'm not sure the patch title is quite right either. This seems to be the same in patch 4: it only mentions QMP even though it adds QMP and HMP interfaces. Maybe you should change this part of the series. A patch to introduce the full QMP interface, including the associated code refactoring (whose need would then be obvious). And a similar patch for the HMP interface. > Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> > --- > blockdev.c | 53 +++------------------------------------- > hmp.c | 21 +++++++++++----- > include/qemu/throttle-options.h | 2 ++ > util/throttle-options.c | 54 +++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 74 insertions(+), 56 deletions(-) > > diff --git a/blockdev.c b/blockdev.c > index a0eb2ed..a55f6be 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -2593,6 +2593,7 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp) > BlockDriverState *bs; > BlockBackend *blk; > AioContext *aio_context; > + IOThrottle *iothrottle; > > blk = qmp_get_blk(arg->has_device ? arg->device : NULL, > arg->has_id ? arg->id : NULL, > @@ -2610,56 +2611,8 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp) > goto out; > } > > - throttle_config_init(&cfg); > - cfg.buckets[THROTTLE_BPS_TOTAL].avg = arg->bps; > - cfg.buckets[THROTTLE_BPS_READ].avg = arg->bps_rd; > - cfg.buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr; > - > - cfg.buckets[THROTTLE_OPS_TOTAL].avg = arg->iops; > - cfg.buckets[THROTTLE_OPS_READ].avg = arg->iops_rd; > - cfg.buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr; > - > - if (arg->has_bps_max) { > - cfg.buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max; > - } > - if (arg->has_bps_rd_max) { > - cfg.buckets[THROTTLE_BPS_READ].max = arg->bps_rd_max; > - } > - if (arg->has_bps_wr_max) { > - cfg.buckets[THROTTLE_BPS_WRITE].max = arg->bps_wr_max; > - } > - if (arg->has_iops_max) { > - cfg.buckets[THROTTLE_OPS_TOTAL].max = arg->iops_max; > - } > - if (arg->has_iops_rd_max) { > - cfg.buckets[THROTTLE_OPS_READ].max = arg->iops_rd_max; > - } > - if (arg->has_iops_wr_max) { > - cfg.buckets[THROTTLE_OPS_WRITE].max = arg->iops_wr_max; > - } > - > - if (arg->has_bps_max_length) { > - cfg.buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_max_length; > - } > - if (arg->has_bps_rd_max_length) { > - cfg.buckets[THROTTLE_BPS_READ].burst_length = arg->bps_rd_max_length; > - } > - if (arg->has_bps_wr_max_length) { > - cfg.buckets[THROTTLE_BPS_WRITE].burst_length = arg->bps_wr_max_length; > - } > - if (arg->has_iops_max_length) { > - cfg.buckets[THROTTLE_OPS_TOTAL].burst_length = arg->iops_max_length; > - } > - if (arg->has_iops_rd_max_length) { > - cfg.buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length; > - } > - if (arg->has_iops_wr_max_length) { > - cfg.buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_wr_max_length; > - } > - > - if (arg->has_iops_size) { > - cfg.op_size = arg->iops_size; > - } > + iothrottle = qapi_BlockIOThrottle_base(arg); > + qmp_set_io_throttle(&cfg, iothrottle); > > if (!throttle_is_valid(&cfg, errp)) { > goto out; > diff --git a/hmp.c b/hmp.c > index ab407d6..9660373 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -1552,20 +1552,29 @@ void hmp_change(Monitor *mon, const QDict *qdict) > hmp_handle_error(mon, &err); > } > > +static void hmp_initialize_io_throttle(IOThrottle *iot, const QDict *qdict) > +{ > + iot->has_id = true; > + iot->id = (char *) qdict_get_str(qdict, "id"); > + iot->bps = qdict_get_int(qdict, "bps"); > + iot->bps_rd = qdict_get_int(qdict, "bps_rd"); > + iot->bps_wr = qdict_get_int(qdict, "bps_wr"); > + iot->iops = qdict_get_int(qdict, "iops"); > + iot->iops_rd = qdict_get_int(qdict, "iops_rd"); > + iot->iops_wr = qdict_get_int(qdict, "iops_wr"); > +} > + > void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict) > { > Error *err = NULL; > + IOThrottle *iothrottle; > BlockIOThrottle throttle = { > .has_device = true, > .device = (char *) qdict_get_str(qdict, "device"), > - .bps = qdict_get_int(qdict, "bps"), > - .bps_rd = qdict_get_int(qdict, "bps_rd"), > - .bps_wr = qdict_get_int(qdict, "bps_wr"), > - .iops = qdict_get_int(qdict, "iops"), > - .iops_rd = qdict_get_int(qdict, "iops_rd"), > - .iops_wr = qdict_get_int(qdict, "iops_wr"), > }; > > + iothrottle = qapi_BlockIOThrottle_base(&throttle); > + hmp_initialize_io_throttle(iothrottle, qdict); > qmp_block_set_io_throttle(&throttle, &err); > hmp_handle_error(mon, &err); > } > diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h > index 9b68eb8..ccf10cc 100644 > --- a/include/qemu/throttle-options.h > +++ b/include/qemu/throttle-options.h > @@ -94,4 +94,6 @@ > > void parse_io_throttle_options(ThrottleConfig *, QemuOpts *); > > +void qmp_set_io_throttle(ThrottleConfig *, IOThrottle *); > + > #endif > diff --git a/util/throttle-options.c b/util/throttle-options.c > index 02b26b8..ded57bc 100644 > --- a/util/throttle-options.c > +++ b/util/throttle-options.c > @@ -49,3 +49,57 @@ void parse_io_throttle_options(ThrottleConfig *throttle_cfg, QemuOpts *opts) > qemu_opt_get_number(opts, "throttling.iops-size", 0); > > } > + > +void qmp_set_io_throttle(ThrottleConfig *cfg, IOThrottle *arg) > +{ > + throttle_config_init(cfg); > + cfg->buckets[THROTTLE_BPS_TOTAL].avg = arg->bps; > + cfg->buckets[THROTTLE_BPS_READ].avg = arg->bps_rd; > + cfg->buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr; > + > + cfg->buckets[THROTTLE_OPS_TOTAL].avg = arg->iops; > + cfg->buckets[THROTTLE_OPS_READ].avg = arg->iops_rd; > + cfg->buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr; > + > + if (arg->has_bps_max) { > + cfg->buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max; > + } > + if (arg->has_bps_rd_max) { > + cfg->buckets[THROTTLE_BPS_READ].max = arg->bps_rd_max; > + } > + if (arg->has_bps_wr_max) { > + cfg->buckets[THROTTLE_BPS_WRITE].max = arg->bps_wr_max; > + } > + if (arg->has_iops_max) { > + cfg->buckets[THROTTLE_OPS_TOTAL].max = arg->iops_max; > + } > + if (arg->has_iops_rd_max) { > + cfg->buckets[THROTTLE_OPS_READ].max = arg->iops_rd_max; > + } > + if (arg->has_iops_wr_max) { > + cfg->buckets[THROTTLE_OPS_WRITE].max = arg->iops_wr_max; > + } > + > + if (arg->has_bps_max_length) { > + cfg->buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_max_length; > + } > + if (arg->has_bps_rd_max_length) { > + cfg->buckets[THROTTLE_BPS_READ].burst_length = arg->bps_rd_max_length; > + } > + if (arg->has_bps_wr_max_length) { > + cfg->buckets[THROTTLE_BPS_WRITE].burst_length = arg->bps_wr_max_length; > + } > + if (arg->has_iops_max_length) { > + cfg->buckets[THROTTLE_OPS_TOTAL].burst_length = arg->iops_max_length; > + } > + if (arg->has_iops_rd_max_length) { > + cfg->buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length; > + } > + if (arg->has_iops_wr_max_length) { > + cfg->buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_wr_max_length; > + } > + > + if (arg->has_iops_size) { > + cfg->op_size = arg->iops_size; > + } > +}
On 5/10/2017 9:56 PM, Eric Blake wrote: > On 05/10/2017 03:41 AM, Pradeep Jagadeesh wrote: >> This patch factor out the duplicate qmp throttle interface code >> that was present in both block and fsdev device files. >> >> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> >> --- >> blockdev.c | 53 +++------------------------------------- >> hmp.c | 21 +++++++++++----- >> include/qemu/throttle-options.h | 2 ++ >> util/throttle-options.c | 54 +++++++++++++++++++++++++++++++++++++++++ >> 4 files changed, 74 insertions(+), 56 deletions(-) >> > >> +++ b/include/qemu/throttle-options.h >> @@ -94,4 +94,6 @@ >> >> void parse_io_throttle_options(ThrottleConfig *, QemuOpts *); >> >> +void qmp_set_io_throttle(ThrottleConfig *, IOThrottle *); > > I don't think this is the right name for this function. You are not > adding a new QMP command (you didn't modify a .json file to add > 'set-io-throttle'). As mentioned in patch 1/4, you can probably reuse > the existing util/throttle.c instead of throttle-options.c, and name > this something starting with throttle_ rather than with qmp_. > OK -Pradeep >
On 5/11/2017 9:50 AM, Greg Kurz wrote: > On Wed, 10 May 2017 04:41:22 -0400 > Pradeep Jagadeesh <pradeepkiruvale@gmail.com> wrote: > >> This patch factor out the duplicate qmp throttle interface code >> that was present in both block and fsdev device files. >> > > There isn't any duplicated code at this stage since you have changed the > patch ordering. > > Also, this patch is factoring out several things: > - factoring code out of qmp_block_set_io_throttle() > - factoring code out of hmp_block_set_io_throttle() > So I'm not sure the patch title is quite right either. > > This seems to be the same in patch 4: it only mentions QMP even though > it adds QMP and HMP interfaces. Maybe you should change this part of > the series. > > A patch to introduce the full QMP interface, including the associated > code refactoring (whose need would then be obvious). And a similar patch > for the HMP interface. But without hmp functionalities the qmp does not work. So, I just kept both together. > >> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> >> --- >> blockdev.c | 53 +++------------------------------------- >> hmp.c | 21 +++++++++++----- >> include/qemu/throttle-options.h | 2 ++ >> util/throttle-options.c | 54 +++++++++++++++++++++++++++++++++++++++++ >> 4 files changed, 74 insertions(+), 56 deletions(-) >> >> diff --git a/blockdev.c b/blockdev.c >> index a0eb2ed..a55f6be 100644 >> --- a/blockdev.c >> +++ b/blockdev.c >> @@ -2593,6 +2593,7 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp) >> BlockDriverState *bs; >> BlockBackend *blk; >> AioContext *aio_context; >> + IOThrottle *iothrottle; >> >> blk = qmp_get_blk(arg->has_device ? arg->device : NULL, >> arg->has_id ? arg->id : NULL, >> @@ -2610,56 +2611,8 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp) >> goto out; >> } >> >> - throttle_config_init(&cfg); >> - cfg.buckets[THROTTLE_BPS_TOTAL].avg = arg->bps; >> - cfg.buckets[THROTTLE_BPS_READ].avg = arg->bps_rd; >> - cfg.buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr; >> - >> - cfg.buckets[THROTTLE_OPS_TOTAL].avg = arg->iops; >> - cfg.buckets[THROTTLE_OPS_READ].avg = arg->iops_rd; >> - cfg.buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr; >> - >> - if (arg->has_bps_max) { >> - cfg.buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max; >> - } >> - if (arg->has_bps_rd_max) { >> - cfg.buckets[THROTTLE_BPS_READ].max = arg->bps_rd_max; >> - } >> - if (arg->has_bps_wr_max) { >> - cfg.buckets[THROTTLE_BPS_WRITE].max = arg->bps_wr_max; >> - } >> - if (arg->has_iops_max) { >> - cfg.buckets[THROTTLE_OPS_TOTAL].max = arg->iops_max; >> - } >> - if (arg->has_iops_rd_max) { >> - cfg.buckets[THROTTLE_OPS_READ].max = arg->iops_rd_max; >> - } >> - if (arg->has_iops_wr_max) { >> - cfg.buckets[THROTTLE_OPS_WRITE].max = arg->iops_wr_max; >> - } >> - >> - if (arg->has_bps_max_length) { >> - cfg.buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_max_length; >> - } >> - if (arg->has_bps_rd_max_length) { >> - cfg.buckets[THROTTLE_BPS_READ].burst_length = arg->bps_rd_max_length; >> - } >> - if (arg->has_bps_wr_max_length) { >> - cfg.buckets[THROTTLE_BPS_WRITE].burst_length = arg->bps_wr_max_length; >> - } >> - if (arg->has_iops_max_length) { >> - cfg.buckets[THROTTLE_OPS_TOTAL].burst_length = arg->iops_max_length; >> - } >> - if (arg->has_iops_rd_max_length) { >> - cfg.buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length; >> - } >> - if (arg->has_iops_wr_max_length) { >> - cfg.buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_wr_max_length; >> - } >> - >> - if (arg->has_iops_size) { >> - cfg.op_size = arg->iops_size; >> - } >> + iothrottle = qapi_BlockIOThrottle_base(arg); >> + qmp_set_io_throttle(&cfg, iothrottle); >> >> if (!throttle_is_valid(&cfg, errp)) { >> goto out; >> diff --git a/hmp.c b/hmp.c >> index ab407d6..9660373 100644 >> --- a/hmp.c >> +++ b/hmp.c >> @@ -1552,20 +1552,29 @@ void hmp_change(Monitor *mon, const QDict *qdict) >> hmp_handle_error(mon, &err); >> } >> >> +static void hmp_initialize_io_throttle(IOThrottle *iot, const QDict *qdict) >> +{ >> + iot->has_id = true; >> + iot->id = (char *) qdict_get_str(qdict, "id"); >> + iot->bps = qdict_get_int(qdict, "bps"); >> + iot->bps_rd = qdict_get_int(qdict, "bps_rd"); >> + iot->bps_wr = qdict_get_int(qdict, "bps_wr"); >> + iot->iops = qdict_get_int(qdict, "iops"); >> + iot->iops_rd = qdict_get_int(qdict, "iops_rd"); >> + iot->iops_wr = qdict_get_int(qdict, "iops_wr"); >> +} >> + >> void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict) >> { >> Error *err = NULL; >> + IOThrottle *iothrottle; >> BlockIOThrottle throttle = { >> .has_device = true, >> .device = (char *) qdict_get_str(qdict, "device"), >> - .bps = qdict_get_int(qdict, "bps"), >> - .bps_rd = qdict_get_int(qdict, "bps_rd"), >> - .bps_wr = qdict_get_int(qdict, "bps_wr"), >> - .iops = qdict_get_int(qdict, "iops"), >> - .iops_rd = qdict_get_int(qdict, "iops_rd"), >> - .iops_wr = qdict_get_int(qdict, "iops_wr"), >> }; >> >> + iothrottle = qapi_BlockIOThrottle_base(&throttle); >> + hmp_initialize_io_throttle(iothrottle, qdict); >> qmp_block_set_io_throttle(&throttle, &err); >> hmp_handle_error(mon, &err); >> } >> diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h >> index 9b68eb8..ccf10cc 100644 >> --- a/include/qemu/throttle-options.h >> +++ b/include/qemu/throttle-options.h >> @@ -94,4 +94,6 @@ >> >> void parse_io_throttle_options(ThrottleConfig *, QemuOpts *); >> >> +void qmp_set_io_throttle(ThrottleConfig *, IOThrottle *); >> + >> #endif >> diff --git a/util/throttle-options.c b/util/throttle-options.c >> index 02b26b8..ded57bc 100644 >> --- a/util/throttle-options.c >> +++ b/util/throttle-options.c >> @@ -49,3 +49,57 @@ void parse_io_throttle_options(ThrottleConfig *throttle_cfg, QemuOpts *opts) >> qemu_opt_get_number(opts, "throttling.iops-size", 0); >> >> } >> + >> +void qmp_set_io_throttle(ThrottleConfig *cfg, IOThrottle *arg) >> +{ >> + throttle_config_init(cfg); >> + cfg->buckets[THROTTLE_BPS_TOTAL].avg = arg->bps; >> + cfg->buckets[THROTTLE_BPS_READ].avg = arg->bps_rd; >> + cfg->buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr; >> + >> + cfg->buckets[THROTTLE_OPS_TOTAL].avg = arg->iops; >> + cfg->buckets[THROTTLE_OPS_READ].avg = arg->iops_rd; >> + cfg->buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr; >> + >> + if (arg->has_bps_max) { >> + cfg->buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max; >> + } >> + if (arg->has_bps_rd_max) { >> + cfg->buckets[THROTTLE_BPS_READ].max = arg->bps_rd_max; >> + } >> + if (arg->has_bps_wr_max) { >> + cfg->buckets[THROTTLE_BPS_WRITE].max = arg->bps_wr_max; >> + } >> + if (arg->has_iops_max) { >> + cfg->buckets[THROTTLE_OPS_TOTAL].max = arg->iops_max; >> + } >> + if (arg->has_iops_rd_max) { >> + cfg->buckets[THROTTLE_OPS_READ].max = arg->iops_rd_max; >> + } >> + if (arg->has_iops_wr_max) { >> + cfg->buckets[THROTTLE_OPS_WRITE].max = arg->iops_wr_max; >> + } >> + >> + if (arg->has_bps_max_length) { >> + cfg->buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_max_length; >> + } >> + if (arg->has_bps_rd_max_length) { >> + cfg->buckets[THROTTLE_BPS_READ].burst_length = arg->bps_rd_max_length; >> + } >> + if (arg->has_bps_wr_max_length) { >> + cfg->buckets[THROTTLE_BPS_WRITE].burst_length = arg->bps_wr_max_length; >> + } >> + if (arg->has_iops_max_length) { >> + cfg->buckets[THROTTLE_OPS_TOTAL].burst_length = arg->iops_max_length; >> + } >> + if (arg->has_iops_rd_max_length) { >> + cfg->buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length; >> + } >> + if (arg->has_iops_wr_max_length) { >> + cfg->buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_wr_max_length; >> + } >> + >> + if (arg->has_iops_size) { >> + cfg->op_size = arg->iops_size; >> + } >> +}
On Fri, 12 May 2017 10:03:02 +0200 Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> wrote: > On 5/11/2017 9:50 AM, Greg Kurz wrote: > > On Wed, 10 May 2017 04:41:22 -0400 > > Pradeep Jagadeesh <pradeepkiruvale@gmail.com> wrote: > > > >> This patch factor out the duplicate qmp throttle interface code > >> that was present in both block and fsdev device files. > >> > > > > There isn't any duplicated code at this stage since you have changed the > > patch ordering. > > > > Also, this patch is factoring out several things: > > - factoring code out of qmp_block_set_io_throttle() > > - factoring code out of hmp_block_set_io_throttle() > > So I'm not sure the patch title is quite right either. > > > > This seems to be the same in patch 4: it only mentions QMP even though > > it adds QMP and HMP interfaces. Maybe you should change this part of > > the series. > > > > A patch to introduce the full QMP interface, including the associated > > code refactoring (whose need would then be obvious). And a similar patch > > for the HMP interface. > But without hmp functionalities the qmp does not work. So, I just kept > both together. This sounds weird... I would rather expect HMP to depend on QMP, not the contrary. Anyway, looking at this patch I still see two different things which deserve their own patch and appropriate description. > > > >> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> > >> --- > >> blockdev.c | 53 +++------------------------------------- > >> hmp.c | 21 +++++++++++----- > >> include/qemu/throttle-options.h | 2 ++ > >> util/throttle-options.c | 54 +++++++++++++++++++++++++++++++++++++++++ > >> 4 files changed, 74 insertions(+), 56 deletions(-) > >> > >> diff --git a/blockdev.c b/blockdev.c > >> index a0eb2ed..a55f6be 100644 > >> --- a/blockdev.c > >> +++ b/blockdev.c > >> @@ -2593,6 +2593,7 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp) > >> BlockDriverState *bs; > >> BlockBackend *blk; > >> AioContext *aio_context; > >> + IOThrottle *iothrottle; > >> > >> blk = qmp_get_blk(arg->has_device ? arg->device : NULL, > >> arg->has_id ? arg->id : NULL, > >> @@ -2610,56 +2611,8 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp) > >> goto out; > >> } > >> > >> - throttle_config_init(&cfg); > >> - cfg.buckets[THROTTLE_BPS_TOTAL].avg = arg->bps; > >> - cfg.buckets[THROTTLE_BPS_READ].avg = arg->bps_rd; > >> - cfg.buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr; > >> - > >> - cfg.buckets[THROTTLE_OPS_TOTAL].avg = arg->iops; > >> - cfg.buckets[THROTTLE_OPS_READ].avg = arg->iops_rd; > >> - cfg.buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr; > >> - > >> - if (arg->has_bps_max) { > >> - cfg.buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max; > >> - } > >> - if (arg->has_bps_rd_max) { > >> - cfg.buckets[THROTTLE_BPS_READ].max = arg->bps_rd_max; > >> - } > >> - if (arg->has_bps_wr_max) { > >> - cfg.buckets[THROTTLE_BPS_WRITE].max = arg->bps_wr_max; > >> - } > >> - if (arg->has_iops_max) { > >> - cfg.buckets[THROTTLE_OPS_TOTAL].max = arg->iops_max; > >> - } > >> - if (arg->has_iops_rd_max) { > >> - cfg.buckets[THROTTLE_OPS_READ].max = arg->iops_rd_max; > >> - } > >> - if (arg->has_iops_wr_max) { > >> - cfg.buckets[THROTTLE_OPS_WRITE].max = arg->iops_wr_max; > >> - } > >> - > >> - if (arg->has_bps_max_length) { > >> - cfg.buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_max_length; > >> - } > >> - if (arg->has_bps_rd_max_length) { > >> - cfg.buckets[THROTTLE_BPS_READ].burst_length = arg->bps_rd_max_length; > >> - } > >> - if (arg->has_bps_wr_max_length) { > >> - cfg.buckets[THROTTLE_BPS_WRITE].burst_length = arg->bps_wr_max_length; > >> - } > >> - if (arg->has_iops_max_length) { > >> - cfg.buckets[THROTTLE_OPS_TOTAL].burst_length = arg->iops_max_length; > >> - } > >> - if (arg->has_iops_rd_max_length) { > >> - cfg.buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length; > >> - } > >> - if (arg->has_iops_wr_max_length) { > >> - cfg.buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_wr_max_length; > >> - } > >> - > >> - if (arg->has_iops_size) { > >> - cfg.op_size = arg->iops_size; > >> - } > >> + iothrottle = qapi_BlockIOThrottle_base(arg); > >> + qmp_set_io_throttle(&cfg, iothrottle); > >> > >> if (!throttle_is_valid(&cfg, errp)) { > >> goto out; > >> diff --git a/hmp.c b/hmp.c > >> index ab407d6..9660373 100644 > >> --- a/hmp.c > >> +++ b/hmp.c > >> @@ -1552,20 +1552,29 @@ void hmp_change(Monitor *mon, const QDict *qdict) > >> hmp_handle_error(mon, &err); > >> } > >> > >> +static void hmp_initialize_io_throttle(IOThrottle *iot, const QDict *qdict) > >> +{ > >> + iot->has_id = true; > >> + iot->id = (char *) qdict_get_str(qdict, "id"); > >> + iot->bps = qdict_get_int(qdict, "bps"); > >> + iot->bps_rd = qdict_get_int(qdict, "bps_rd"); > >> + iot->bps_wr = qdict_get_int(qdict, "bps_wr"); > >> + iot->iops = qdict_get_int(qdict, "iops"); > >> + iot->iops_rd = qdict_get_int(qdict, "iops_rd"); > >> + iot->iops_wr = qdict_get_int(qdict, "iops_wr"); > >> +} > >> + > >> void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict) > >> { > >> Error *err = NULL; > >> + IOThrottle *iothrottle; > >> BlockIOThrottle throttle = { > >> .has_device = true, > >> .device = (char *) qdict_get_str(qdict, "device"), > >> - .bps = qdict_get_int(qdict, "bps"), > >> - .bps_rd = qdict_get_int(qdict, "bps_rd"), > >> - .bps_wr = qdict_get_int(qdict, "bps_wr"), > >> - .iops = qdict_get_int(qdict, "iops"), > >> - .iops_rd = qdict_get_int(qdict, "iops_rd"), > >> - .iops_wr = qdict_get_int(qdict, "iops_wr"), > >> }; > >> > >> + iothrottle = qapi_BlockIOThrottle_base(&throttle); > >> + hmp_initialize_io_throttle(iothrottle, qdict); > >> qmp_block_set_io_throttle(&throttle, &err); > >> hmp_handle_error(mon, &err); > >> } > >> diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h > >> index 9b68eb8..ccf10cc 100644 > >> --- a/include/qemu/throttle-options.h > >> +++ b/include/qemu/throttle-options.h > >> @@ -94,4 +94,6 @@ > >> > >> void parse_io_throttle_options(ThrottleConfig *, QemuOpts *); > >> > >> +void qmp_set_io_throttle(ThrottleConfig *, IOThrottle *); > >> + > >> #endif > >> diff --git a/util/throttle-options.c b/util/throttle-options.c > >> index 02b26b8..ded57bc 100644 > >> --- a/util/throttle-options.c > >> +++ b/util/throttle-options.c > >> @@ -49,3 +49,57 @@ void parse_io_throttle_options(ThrottleConfig *throttle_cfg, QemuOpts *opts) > >> qemu_opt_get_number(opts, "throttling.iops-size", 0); > >> > >> } > >> + > >> +void qmp_set_io_throttle(ThrottleConfig *cfg, IOThrottle *arg) > >> +{ > >> + throttle_config_init(cfg); > >> + cfg->buckets[THROTTLE_BPS_TOTAL].avg = arg->bps; > >> + cfg->buckets[THROTTLE_BPS_READ].avg = arg->bps_rd; > >> + cfg->buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr; > >> + > >> + cfg->buckets[THROTTLE_OPS_TOTAL].avg = arg->iops; > >> + cfg->buckets[THROTTLE_OPS_READ].avg = arg->iops_rd; > >> + cfg->buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr; > >> + > >> + if (arg->has_bps_max) { > >> + cfg->buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max; > >> + } > >> + if (arg->has_bps_rd_max) { > >> + cfg->buckets[THROTTLE_BPS_READ].max = arg->bps_rd_max; > >> + } > >> + if (arg->has_bps_wr_max) { > >> + cfg->buckets[THROTTLE_BPS_WRITE].max = arg->bps_wr_max; > >> + } > >> + if (arg->has_iops_max) { > >> + cfg->buckets[THROTTLE_OPS_TOTAL].max = arg->iops_max; > >> + } > >> + if (arg->has_iops_rd_max) { > >> + cfg->buckets[THROTTLE_OPS_READ].max = arg->iops_rd_max; > >> + } > >> + if (arg->has_iops_wr_max) { > >> + cfg->buckets[THROTTLE_OPS_WRITE].max = arg->iops_wr_max; > >> + } > >> + > >> + if (arg->has_bps_max_length) { > >> + cfg->buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_max_length; > >> + } > >> + if (arg->has_bps_rd_max_length) { > >> + cfg->buckets[THROTTLE_BPS_READ].burst_length = arg->bps_rd_max_length; > >> + } > >> + if (arg->has_bps_wr_max_length) { > >> + cfg->buckets[THROTTLE_BPS_WRITE].burst_length = arg->bps_wr_max_length; > >> + } > >> + if (arg->has_iops_max_length) { > >> + cfg->buckets[THROTTLE_OPS_TOTAL].burst_length = arg->iops_max_length; > >> + } > >> + if (arg->has_iops_rd_max_length) { > >> + cfg->buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length; > >> + } > >> + if (arg->has_iops_wr_max_length) { > >> + cfg->buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_wr_max_length; > >> + } > >> + > >> + if (arg->has_iops_size) { > >> + cfg->op_size = arg->iops_size; > >> + } > >> +} > >
On 6/8/2017 4:20 PM, Greg Kurz wrote: > On Fri, 12 May 2017 10:03:02 +0200 > Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> wrote: > >> On 5/11/2017 9:50 AM, Greg Kurz wrote: >>> On Wed, 10 May 2017 04:41:22 -0400 >>> Pradeep Jagadeesh <pradeepkiruvale@gmail.com> wrote: >>> >>>> This patch factor out the duplicate qmp throttle interface code >>>> that was present in both block and fsdev device files. >>>> >>> >>> There isn't any duplicated code at this stage since you have changed the >>> patch ordering. >>> >>> Also, this patch is factoring out several things: >>> - factoring code out of qmp_block_set_io_throttle() >>> - factoring code out of hmp_block_set_io_throttle() >>> So I'm not sure the patch title is quite right either. >>> >>> This seems to be the same in patch 4: it only mentions QMP even though >>> it adds QMP and HMP interfaces. Maybe you should change this part of >>> the series. >>> >>> A patch to introduce the full QMP interface, including the associated >>> code refactoring (whose need would then be obvious). And a similar patch >>> for the HMP interface. >> But without hmp functionalities the qmp does not work. So, I just kept >> both together. > > This sounds weird... I would rather expect HMP to depend on QMP, not the > contrary. > > Anyway, looking at this patch I still see two different things which > deserve their own patch and appropriate description. OK, I can create different patches. Regards, Pradeep > >>> >>>> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> >>>> --- >>>> blockdev.c | 53 +++------------------------------------- >>>> hmp.c | 21 +++++++++++----- >>>> include/qemu/throttle-options.h | 2 ++ >>>> util/throttle-options.c | 54 +++++++++++++++++++++++++++++++++++++++++ >>>> 4 files changed, 74 insertions(+), 56 deletions(-) >>>> >>>> diff --git a/blockdev.c b/blockdev.c >>>> index a0eb2ed..a55f6be 100644 >>>> --- a/blockdev.c >>>> +++ b/blockdev.c >>>> @@ -2593,6 +2593,7 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp) >>>> BlockDriverState *bs; >>>> BlockBackend *blk; >>>> AioContext *aio_context; >>>> + IOThrottle *iothrottle; >>>> >>>> blk = qmp_get_blk(arg->has_device ? arg->device : NULL, >>>> arg->has_id ? arg->id : NULL, >>>> @@ -2610,56 +2611,8 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp) >>>> goto out; >>>> } >>>> >>>> - throttle_config_init(&cfg); >>>> - cfg.buckets[THROTTLE_BPS_TOTAL].avg = arg->bps; >>>> - cfg.buckets[THROTTLE_BPS_READ].avg = arg->bps_rd; >>>> - cfg.buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr; >>>> - >>>> - cfg.buckets[THROTTLE_OPS_TOTAL].avg = arg->iops; >>>> - cfg.buckets[THROTTLE_OPS_READ].avg = arg->iops_rd; >>>> - cfg.buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr; >>>> - >>>> - if (arg->has_bps_max) { >>>> - cfg.buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max; >>>> - } >>>> - if (arg->has_bps_rd_max) { >>>> - cfg.buckets[THROTTLE_BPS_READ].max = arg->bps_rd_max; >>>> - } >>>> - if (arg->has_bps_wr_max) { >>>> - cfg.buckets[THROTTLE_BPS_WRITE].max = arg->bps_wr_max; >>>> - } >>>> - if (arg->has_iops_max) { >>>> - cfg.buckets[THROTTLE_OPS_TOTAL].max = arg->iops_max; >>>> - } >>>> - if (arg->has_iops_rd_max) { >>>> - cfg.buckets[THROTTLE_OPS_READ].max = arg->iops_rd_max; >>>> - } >>>> - if (arg->has_iops_wr_max) { >>>> - cfg.buckets[THROTTLE_OPS_WRITE].max = arg->iops_wr_max; >>>> - } >>>> - >>>> - if (arg->has_bps_max_length) { >>>> - cfg.buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_max_length; >>>> - } >>>> - if (arg->has_bps_rd_max_length) { >>>> - cfg.buckets[THROTTLE_BPS_READ].burst_length = arg->bps_rd_max_length; >>>> - } >>>> - if (arg->has_bps_wr_max_length) { >>>> - cfg.buckets[THROTTLE_BPS_WRITE].burst_length = arg->bps_wr_max_length; >>>> - } >>>> - if (arg->has_iops_max_length) { >>>> - cfg.buckets[THROTTLE_OPS_TOTAL].burst_length = arg->iops_max_length; >>>> - } >>>> - if (arg->has_iops_rd_max_length) { >>>> - cfg.buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length; >>>> - } >>>> - if (arg->has_iops_wr_max_length) { >>>> - cfg.buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_wr_max_length; >>>> - } >>>> - >>>> - if (arg->has_iops_size) { >>>> - cfg.op_size = arg->iops_size; >>>> - } >>>> + iothrottle = qapi_BlockIOThrottle_base(arg); >>>> + qmp_set_io_throttle(&cfg, iothrottle); >>>> >>>> if (!throttle_is_valid(&cfg, errp)) { >>>> goto out; >>>> diff --git a/hmp.c b/hmp.c >>>> index ab407d6..9660373 100644 >>>> --- a/hmp.c >>>> +++ b/hmp.c >>>> @@ -1552,20 +1552,29 @@ void hmp_change(Monitor *mon, const QDict *qdict) >>>> hmp_handle_error(mon, &err); >>>> } >>>> >>>> +static void hmp_initialize_io_throttle(IOThrottle *iot, const QDict *qdict) >>>> +{ >>>> + iot->has_id = true; >>>> + iot->id = (char *) qdict_get_str(qdict, "id"); >>>> + iot->bps = qdict_get_int(qdict, "bps"); >>>> + iot->bps_rd = qdict_get_int(qdict, "bps_rd"); >>>> + iot->bps_wr = qdict_get_int(qdict, "bps_wr"); >>>> + iot->iops = qdict_get_int(qdict, "iops"); >>>> + iot->iops_rd = qdict_get_int(qdict, "iops_rd"); >>>> + iot->iops_wr = qdict_get_int(qdict, "iops_wr"); >>>> +} >>>> + >>>> void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict) >>>> { >>>> Error *err = NULL; >>>> + IOThrottle *iothrottle; >>>> BlockIOThrottle throttle = { >>>> .has_device = true, >>>> .device = (char *) qdict_get_str(qdict, "device"), >>>> - .bps = qdict_get_int(qdict, "bps"), >>>> - .bps_rd = qdict_get_int(qdict, "bps_rd"), >>>> - .bps_wr = qdict_get_int(qdict, "bps_wr"), >>>> - .iops = qdict_get_int(qdict, "iops"), >>>> - .iops_rd = qdict_get_int(qdict, "iops_rd"), >>>> - .iops_wr = qdict_get_int(qdict, "iops_wr"), >>>> }; >>>> >>>> + iothrottle = qapi_BlockIOThrottle_base(&throttle); >>>> + hmp_initialize_io_throttle(iothrottle, qdict); >>>> qmp_block_set_io_throttle(&throttle, &err); >>>> hmp_handle_error(mon, &err); >>>> } >>>> diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h >>>> index 9b68eb8..ccf10cc 100644 >>>> --- a/include/qemu/throttle-options.h >>>> +++ b/include/qemu/throttle-options.h >>>> @@ -94,4 +94,6 @@ >>>> >>>> void parse_io_throttle_options(ThrottleConfig *, QemuOpts *); >>>> >>>> +void qmp_set_io_throttle(ThrottleConfig *, IOThrottle *); >>>> + >>>> #endif >>>> diff --git a/util/throttle-options.c b/util/throttle-options.c >>>> index 02b26b8..ded57bc 100644 >>>> --- a/util/throttle-options.c >>>> +++ b/util/throttle-options.c >>>> @@ -49,3 +49,57 @@ void parse_io_throttle_options(ThrottleConfig *throttle_cfg, QemuOpts *opts) >>>> qemu_opt_get_number(opts, "throttling.iops-size", 0); >>>> >>>> } >>>> + >>>> +void qmp_set_io_throttle(ThrottleConfig *cfg, IOThrottle *arg) >>>> +{ >>>> + throttle_config_init(cfg); >>>> + cfg->buckets[THROTTLE_BPS_TOTAL].avg = arg->bps; >>>> + cfg->buckets[THROTTLE_BPS_READ].avg = arg->bps_rd; >>>> + cfg->buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr; >>>> + >>>> + cfg->buckets[THROTTLE_OPS_TOTAL].avg = arg->iops; >>>> + cfg->buckets[THROTTLE_OPS_READ].avg = arg->iops_rd; >>>> + cfg->buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr; >>>> + >>>> + if (arg->has_bps_max) { >>>> + cfg->buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max; >>>> + } >>>> + if (arg->has_bps_rd_max) { >>>> + cfg->buckets[THROTTLE_BPS_READ].max = arg->bps_rd_max; >>>> + } >>>> + if (arg->has_bps_wr_max) { >>>> + cfg->buckets[THROTTLE_BPS_WRITE].max = arg->bps_wr_max; >>>> + } >>>> + if (arg->has_iops_max) { >>>> + cfg->buckets[THROTTLE_OPS_TOTAL].max = arg->iops_max; >>>> + } >>>> + if (arg->has_iops_rd_max) { >>>> + cfg->buckets[THROTTLE_OPS_READ].max = arg->iops_rd_max; >>>> + } >>>> + if (arg->has_iops_wr_max) { >>>> + cfg->buckets[THROTTLE_OPS_WRITE].max = arg->iops_wr_max; >>>> + } >>>> + >>>> + if (arg->has_bps_max_length) { >>>> + cfg->buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_max_length; >>>> + } >>>> + if (arg->has_bps_rd_max_length) { >>>> + cfg->buckets[THROTTLE_BPS_READ].burst_length = arg->bps_rd_max_length; >>>> + } >>>> + if (arg->has_bps_wr_max_length) { >>>> + cfg->buckets[THROTTLE_BPS_WRITE].burst_length = arg->bps_wr_max_length; >>>> + } >>>> + if (arg->has_iops_max_length) { >>>> + cfg->buckets[THROTTLE_OPS_TOTAL].burst_length = arg->iops_max_length; >>>> + } >>>> + if (arg->has_iops_rd_max_length) { >>>> + cfg->buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length; >>>> + } >>>> + if (arg->has_iops_wr_max_length) { >>>> + cfg->buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_wr_max_length; >>>> + } >>>> + >>>> + if (arg->has_iops_size) { >>>> + cfg->op_size = arg->iops_size; >>>> + } >>>> +} >> >> >
diff --git a/blockdev.c b/blockdev.c index a0eb2ed..a55f6be 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2593,6 +2593,7 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp) BlockDriverState *bs; BlockBackend *blk; AioContext *aio_context; + IOThrottle *iothrottle; blk = qmp_get_blk(arg->has_device ? arg->device : NULL, arg->has_id ? arg->id : NULL, @@ -2610,56 +2611,8 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp) goto out; } - throttle_config_init(&cfg); - cfg.buckets[THROTTLE_BPS_TOTAL].avg = arg->bps; - cfg.buckets[THROTTLE_BPS_READ].avg = arg->bps_rd; - cfg.buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr; - - cfg.buckets[THROTTLE_OPS_TOTAL].avg = arg->iops; - cfg.buckets[THROTTLE_OPS_READ].avg = arg->iops_rd; - cfg.buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr; - - if (arg->has_bps_max) { - cfg.buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max; - } - if (arg->has_bps_rd_max) { - cfg.buckets[THROTTLE_BPS_READ].max = arg->bps_rd_max; - } - if (arg->has_bps_wr_max) { - cfg.buckets[THROTTLE_BPS_WRITE].max = arg->bps_wr_max; - } - if (arg->has_iops_max) { - cfg.buckets[THROTTLE_OPS_TOTAL].max = arg->iops_max; - } - if (arg->has_iops_rd_max) { - cfg.buckets[THROTTLE_OPS_READ].max = arg->iops_rd_max; - } - if (arg->has_iops_wr_max) { - cfg.buckets[THROTTLE_OPS_WRITE].max = arg->iops_wr_max; - } - - if (arg->has_bps_max_length) { - cfg.buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_max_length; - } - if (arg->has_bps_rd_max_length) { - cfg.buckets[THROTTLE_BPS_READ].burst_length = arg->bps_rd_max_length; - } - if (arg->has_bps_wr_max_length) { - cfg.buckets[THROTTLE_BPS_WRITE].burst_length = arg->bps_wr_max_length; - } - if (arg->has_iops_max_length) { - cfg.buckets[THROTTLE_OPS_TOTAL].burst_length = arg->iops_max_length; - } - if (arg->has_iops_rd_max_length) { - cfg.buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length; - } - if (arg->has_iops_wr_max_length) { - cfg.buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_wr_max_length; - } - - if (arg->has_iops_size) { - cfg.op_size = arg->iops_size; - } + iothrottle = qapi_BlockIOThrottle_base(arg); + qmp_set_io_throttle(&cfg, iothrottle); if (!throttle_is_valid(&cfg, errp)) { goto out; diff --git a/hmp.c b/hmp.c index ab407d6..9660373 100644 --- a/hmp.c +++ b/hmp.c @@ -1552,20 +1552,29 @@ void hmp_change(Monitor *mon, const QDict *qdict) hmp_handle_error(mon, &err); } +static void hmp_initialize_io_throttle(IOThrottle *iot, const QDict *qdict) +{ + iot->has_id = true; + iot->id = (char *) qdict_get_str(qdict, "id"); + iot->bps = qdict_get_int(qdict, "bps"); + iot->bps_rd = qdict_get_int(qdict, "bps_rd"); + iot->bps_wr = qdict_get_int(qdict, "bps_wr"); + iot->iops = qdict_get_int(qdict, "iops"); + iot->iops_rd = qdict_get_int(qdict, "iops_rd"); + iot->iops_wr = qdict_get_int(qdict, "iops_wr"); +} + void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict) { Error *err = NULL; + IOThrottle *iothrottle; BlockIOThrottle throttle = { .has_device = true, .device = (char *) qdict_get_str(qdict, "device"), - .bps = qdict_get_int(qdict, "bps"), - .bps_rd = qdict_get_int(qdict, "bps_rd"), - .bps_wr = qdict_get_int(qdict, "bps_wr"), - .iops = qdict_get_int(qdict, "iops"), - .iops_rd = qdict_get_int(qdict, "iops_rd"), - .iops_wr = qdict_get_int(qdict, "iops_wr"), }; + iothrottle = qapi_BlockIOThrottle_base(&throttle); + hmp_initialize_io_throttle(iothrottle, qdict); qmp_block_set_io_throttle(&throttle, &err); hmp_handle_error(mon, &err); } diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h index 9b68eb8..ccf10cc 100644 --- a/include/qemu/throttle-options.h +++ b/include/qemu/throttle-options.h @@ -94,4 +94,6 @@ void parse_io_throttle_options(ThrottleConfig *, QemuOpts *); +void qmp_set_io_throttle(ThrottleConfig *, IOThrottle *); + #endif diff --git a/util/throttle-options.c b/util/throttle-options.c index 02b26b8..ded57bc 100644 --- a/util/throttle-options.c +++ b/util/throttle-options.c @@ -49,3 +49,57 @@ void parse_io_throttle_options(ThrottleConfig *throttle_cfg, QemuOpts *opts) qemu_opt_get_number(opts, "throttling.iops-size", 0); } + +void qmp_set_io_throttle(ThrottleConfig *cfg, IOThrottle *arg) +{ + throttle_config_init(cfg); + cfg->buckets[THROTTLE_BPS_TOTAL].avg = arg->bps; + cfg->buckets[THROTTLE_BPS_READ].avg = arg->bps_rd; + cfg->buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr; + + cfg->buckets[THROTTLE_OPS_TOTAL].avg = arg->iops; + cfg->buckets[THROTTLE_OPS_READ].avg = arg->iops_rd; + cfg->buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr; + + if (arg->has_bps_max) { + cfg->buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max; + } + if (arg->has_bps_rd_max) { + cfg->buckets[THROTTLE_BPS_READ].max = arg->bps_rd_max; + } + if (arg->has_bps_wr_max) { + cfg->buckets[THROTTLE_BPS_WRITE].max = arg->bps_wr_max; + } + if (arg->has_iops_max) { + cfg->buckets[THROTTLE_OPS_TOTAL].max = arg->iops_max; + } + if (arg->has_iops_rd_max) { + cfg->buckets[THROTTLE_OPS_READ].max = arg->iops_rd_max; + } + if (arg->has_iops_wr_max) { + cfg->buckets[THROTTLE_OPS_WRITE].max = arg->iops_wr_max; + } + + if (arg->has_bps_max_length) { + cfg->buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_max_length; + } + if (arg->has_bps_rd_max_length) { + cfg->buckets[THROTTLE_BPS_READ].burst_length = arg->bps_rd_max_length; + } + if (arg->has_bps_wr_max_length) { + cfg->buckets[THROTTLE_BPS_WRITE].burst_length = arg->bps_wr_max_length; + } + if (arg->has_iops_max_length) { + cfg->buckets[THROTTLE_OPS_TOTAL].burst_length = arg->iops_max_length; + } + if (arg->has_iops_rd_max_length) { + cfg->buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length; + } + if (arg->has_iops_wr_max_length) { + cfg->buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_wr_max_length; + } + + if (arg->has_iops_size) { + cfg->op_size = arg->iops_size; + } +}
This patch factor out the duplicate qmp throttle interface code that was present in both block and fsdev device files. Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> --- blockdev.c | 53 +++------------------------------------- hmp.c | 21 +++++++++++----- include/qemu/throttle-options.h | 2 ++ util/throttle-options.c | 54 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 74 insertions(+), 56 deletions(-)