| Message ID | 1326108257-13042-11-git-send-email-lcapitulino@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 01/09/2012 05:24 AM, Luiz Capitulino wrote: > Signed-off-by: Luiz Capitulino<lcapitulino@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. > +# > +# @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. Regards, Anthony Liguori > +## > +{ 'command': 'block_set_io_throttle', > + 'data': { 'device': 'str', 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int', > + 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int' } } > diff --git a/qmp-commands.hx b/qmp-commands.hx > index bfae81f..799e655 100644 > --- 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", > - .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_new = qmp_marshal_input_block_set_io_throttle, > }, > > SQMP
On Fri, 13 Jan 2012 14:53:19 -0600 Anthony Liguori <anthony@codemonkey.ws> wrote: > On 01/09/2012 05:24 AM, Luiz Capitulino wrote: > > Signed-off-by: Luiz Capitulino<lcapitulino@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?
On Tue, Jan 17, 2012 at 3:18 PM, Luiz Capitulino <lcapitulino@redhat.com> wrote: > On Fri, 13 Jan 2012 14:53:19 -0600 > Anthony Liguori <anthony@codemonkey.ws> wrote: > >> On 01/09/2012 05:24 AM, Luiz Capitulino wrote: >> > Signed-off-by: Luiz Capitulino<lcapitulino@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? For hmp, they are required, while for qmp, they are optional in theory. In order to two interfaces consistent, these parameters are all required. >
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 +# +# @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 +## +{ 'command': 'block_set_io_throttle', + 'data': { 'device': 'str', 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int', + 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int' } } diff --git a/qmp-commands.hx b/qmp-commands.hx index bfae81f..799e655 100644 --- 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", - .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_new = qmp_marshal_input_block_set_io_throttle, }, SQMP
Signed-off-by: Luiz Capitulino <lcapitulino@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(-)