From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:52914) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rlo7v-000889-GV for qemu-devel@nongnu.org; Fri, 13 Jan 2012 15:53:32 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Rlo7t-0005YQ-OS for qemu-devel@nongnu.org; Fri, 13 Jan 2012 15:53:31 -0500 Received: from mail-iy0-f173.google.com ([209.85.210.173]:34399) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rlo7t-0005YK-HD for qemu-devel@nongnu.org; Fri, 13 Jan 2012 15:53:29 -0500 Received: by iaeo4 with SMTP id o4so3934982iae.4 for ; Fri, 13 Jan 2012 12:53:25 -0800 (PST) Message-ID: <4F1099BF.6060607@codemonkey.ws> Date: Fri, 13 Jan 2012 14:53:19 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <1326108257-13042-1-git-send-email-lcapitulino@redhat.com> <1326108257-13042-11-git-send-email-lcapitulino@redhat.com> In-Reply-To: <1326108257-13042-11-git-send-email-lcapitulino@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 10/10] qapi: Convert block_set_io_throttle List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: kwolf@redhat.com, aliguori@us.ibm.com, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com On 01/09/2012 05:24 AM, Luiz Capitulino wrote: > Signed-off-by: Luiz Capitulino > --- > 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