On Fri, 13 Jan 2012 14:53:19 -0600 Anthony Liguori <anth...@codemonkey.ws> wrote:
> On 01/09/2012 05:24 AM, Luiz Capitulino wrote: > > Signed-off-by: Luiz Capitulino<lcapitul...@redhat.com> > > --- > > blockdev.c | 47 ++++++++++++++--------------------------------- > > blockdev.h | 2 -- > > hmp-commands.hx | 3 +-- > > hmp.c | 14 ++++++++++++++ > > hmp.h | 1 + > > qapi-schema.json | 29 +++++++++++++++++++++++++++++ > > qmp-commands.hx | 5 +---- > > 7 files changed, 60 insertions(+), 41 deletions(-) > > > > diff --git a/blockdev.c b/blockdev.c > > index 8df78ce..5d16137 100644 > > --- a/blockdev.c > > +++ b/blockdev.c > > @@ -775,46 +775,29 @@ void qmp_change_blockdev(const char *device, const > > char *filename, > > } > > > > /* throttling disk I/O limits */ > > -int do_block_set_io_throttle(Monitor *mon, > > - const QDict *qdict, QObject **ret_data) > > +void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t > > bps_rd, > > + int64_t bps_wr, int64_t iops, int64_t > > iops_rd, > > + int64_t iops_wr, Error **errp) > > { > > BlockIOLimit io_limits; > > - const char *devname = qdict_get_str(qdict, "device"); > > BlockDriverState *bs; > > > > - io_limits.bps[BLOCK_IO_LIMIT_TOTAL] > > - = qdict_get_try_int(qdict, "bps", -1); > > - io_limits.bps[BLOCK_IO_LIMIT_READ] > > - = qdict_get_try_int(qdict, "bps_rd", -1); > > - io_limits.bps[BLOCK_IO_LIMIT_WRITE] > > - = qdict_get_try_int(qdict, "bps_wr", -1); > > - io_limits.iops[BLOCK_IO_LIMIT_TOTAL] > > - = qdict_get_try_int(qdict, "iops", -1); > > - io_limits.iops[BLOCK_IO_LIMIT_READ] > > - = qdict_get_try_int(qdict, "iops_rd", -1); > > - io_limits.iops[BLOCK_IO_LIMIT_WRITE] > > - = qdict_get_try_int(qdict, "iops_wr", -1); > > - > > - bs = bdrv_find(devname); > > + bs = bdrv_find(device); > > if (!bs) { > > - qerror_report(QERR_DEVICE_NOT_FOUND, devname); > > - return -1; > > + error_set(errp, QERR_DEVICE_NOT_FOUND, device); > > + return; > > } > > > > - if ((io_limits.bps[BLOCK_IO_LIMIT_TOTAL] == -1) > > - || (io_limits.bps[BLOCK_IO_LIMIT_READ] == -1) > > - || (io_limits.bps[BLOCK_IO_LIMIT_WRITE] == -1) > > - || (io_limits.iops[BLOCK_IO_LIMIT_TOTAL] == -1) > > - || (io_limits.iops[BLOCK_IO_LIMIT_READ] == -1) > > - || (io_limits.iops[BLOCK_IO_LIMIT_WRITE] == -1)) { > > - qerror_report(QERR_MISSING_PARAMETER, > > - "bps/bps_rd/bps_wr/iops/iops_rd/iops_wr"); > > - return -1; > > - } > > + io_limits.bps[BLOCK_IO_LIMIT_TOTAL] = bps; > > + io_limits.bps[BLOCK_IO_LIMIT_READ] = bps_rd; > > + io_limits.bps[BLOCK_IO_LIMIT_WRITE] = bps_wr; > > + io_limits.iops[BLOCK_IO_LIMIT_TOTAL]= iops; > > + io_limits.iops[BLOCK_IO_LIMIT_READ] = iops_rd; > > + io_limits.iops[BLOCK_IO_LIMIT_WRITE]= iops_wr; > > > > if (!do_check_io_limits(&io_limits)) { > > - qerror_report(QERR_INVALID_PARAMETER_COMBINATION); > > - return -1; > > + error_set(errp, QERR_INVALID_PARAMETER_COMBINATION); > > + return; > > } > > > > bs->io_limits = io_limits; > > @@ -829,8 +812,6 @@ int do_block_set_io_throttle(Monitor *mon, > > qemu_mod_timer(bs->block_timer, qemu_get_clock_ns(vm_clock)); > > } > > } > > - > > - return 0; > > } > > > > int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data) > > diff --git a/blockdev.h b/blockdev.h > > index b077449..260e16b 100644 > > --- a/blockdev.h > > +++ b/blockdev.h > > @@ -62,6 +62,4 @@ void qmp_change_blockdev(const char *device, const char > > *filename, > > bool has_format, const char *format, Error > > **errp); > > void do_commit(Monitor *mon, const QDict *qdict); > > int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data); > > -int do_block_set_io_throttle(Monitor *mon, > > - const QDict *qdict, QObject **ret_data); > > #endif > > diff --git a/hmp-commands.hx b/hmp-commands.hx > > index 9f44690..a5d8d33 100644 > > --- a/hmp-commands.hx > > +++ b/hmp-commands.hx > > @@ -1204,8 +1204,7 @@ ETEXI > > .args_type = > > "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l", > > .params = "device bps bps_rd bps_wr iops iops_rd iops_wr", > > .help = "change I/O throttle limits for a block drive", > > - .user_print = monitor_user_noop, > > - .mhandler.cmd_new = do_block_set_io_throttle, > > + .mhandler.cmd = hmp_block_set_io_throttle, > > }, > > > > STEXI > > diff --git a/hmp.c b/hmp.c > > index 89fa8e7..c88c93e 100644 > > --- a/hmp.c > > +++ b/hmp.c > > @@ -767,3 +767,17 @@ void hmp_change(Monitor *mon, const QDict *qdict) > > } > > hmp_handle_error(mon,&err); > > } > > + > > +void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict) > > +{ > > + Error *err = NULL; > > + > > + qmp_block_set_io_throttle(qdict_get_str(qdict, "device"), > > + qdict_get_int(qdict, "bps"), > > + qdict_get_int(qdict, "bps_rd"), > > + qdict_get_int(qdict, "bps_wr"), > > + qdict_get_int(qdict, "iops"), > > + qdict_get_int(qdict, "iops_rd"), > > + qdict_get_int(qdict, "iops_wr"),&err); > > + hmp_handle_error(mon,&err); > > +} > > diff --git a/hmp.h b/hmp.h > > index 621bdc2..aab0b1f 100644 > > --- a/hmp.h > > +++ b/hmp.h > > @@ -53,5 +53,6 @@ void hmp_set_password(Monitor *mon, const QDict *qdict); > > void hmp_expire_password(Monitor *mon, const QDict *qdict); > > void hmp_eject(Monitor *mon, const QDict *qdict); > > void hmp_change(Monitor *mon, const QDict *qdict); > > +void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict); > > > > #endif > > diff --git a/qapi-schema.json b/qapi-schema.json > > index 36fd156..1e1f128 100644 > > --- a/qapi-schema.json > > +++ b/qapi-schema.json > > @@ -1403,3 +1403,32 @@ > > ## > > { 'command': 'change', > > 'data': {'device': 'str', 'target': 'str', '*arg': 'str'} } > > + > > +## > > +# @block_set_io_throttle: > > +# > > +# Change I/O throttle limits for a block drive. > > +# > > +# @device: The name of the device > > +# > > +# @bps: total throughput limit in bytes per second > > +# > > +# @bps_rd: read throughput limit in bytes per second > > +# > > +# @bps_wr: read throughput limit in bytes per second > > write throughput. Fixed. > > +# > > +# @iops: total I/O operations per second > > +# > > +# @ops_rd: read I/O operations per second > > +# > > +# @iops_wr: write I/O operations per second > > +# > > +# Returns: Nothing on success > > +# If @device is not a valid block device, DeviceNotFound > > +# If the argument combination is invalid, > > InvalidParameterCombination > > +# > > +# Since: 1.1 > > > Do all of these fields have to be specified? It looks like they were all > optional in the previous form of the command so they should probably be > optional > here too. They were required, as it's necessary to use '?' for optionals in the args_type entry: > > --- a/qmp-commands.hx > > +++ b/qmp-commands.hx > > @@ -807,10 +807,7 @@ EQMP > > { > > .name = "block_set_io_throttle", > > .args_type = > > "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l", The implementation also seems to imply they are required because there's an explicit check for it (which can be dropped in the new version...). Can the author/block guys confirm these parameters are intended to be required?