diff mbox

[v1] qmp/hmp: add block_set_io_throttle and enhance query_block

Message ID 1312432489-19101-1-git-send-email-wuzhy@linux.vnet.ibm.com
State New
Headers show

Commit Message

Zhi Yong Wu Aug. 4, 2011, 4:34 a.m. UTC
The brief intro:
This patch is created based on the code V4 for block I/O throttling. It mainly adds a new qmp/hmp command block_set_io_throttle, and enhances query_block(qmp) and info block(hmp) to display current I/O throttling settings.

Welcome to all kinds of comments from you.

Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
---
 block.c         |  155 ++++++++++++++++++++++++++++++++++++++----------------
 block.h         |    5 ++
 block_int.h     |    2 +
 blockdev.c      |   86 ++++++++++++++++++++++++++++++-
 blockdev.h      |    2 +
 hmp-commands.hx |   15 +++++
 qerror.c        |    8 +++
 qerror.h        |    6 ++
 qmp-commands.hx |   52 ++++++++++++++++++-
 9 files changed, 283 insertions(+), 48 deletions(-)

Comments

Stefan Hajnoczi Aug. 4, 2011, 1:07 p.m. UTC | #1
On Thu, Aug 4, 2011 at 5:34 AM, Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> wrote:
> @@ -1387,6 +1422,11 @@ void bdrv_set_io_limits(BlockDriverState *bs,
>  {
>     memset(&bs->io_limits, 0, sizeof(BlockIOLimit));
>     bs->io_limits = *io_limits;
> +    if (bdrv_io_limits_enabled(bs)) {
> +        bs->io_limits_enabled = true;
> +    } else {
> +        bs->io_limits_enabled = false;
> +    }

Or in one line:
bs->io_limits_enabled = bdrv_io_limits_enabled(bs);

>  }
>
>  /* Recognize floppy formats */
> @@ -1621,6 +1661,7 @@ BlockDriverState *bdrv_find(const char *name)
>     return NULL;
>  }
>
> +/* disk I/O throttling */

Looks like this should not be here.

>  BlockDriverState *bdrv_next(BlockDriverState *bs)
>  {
>     if (!bs) {
> @@ -1784,6 +1825,16 @@ static void bdrv_print_dict(QObject *obj, void *opaque)
>                             qdict_get_bool(qdict, "ro"),
>                             qdict_get_str(qdict, "drv"),
>                             qdict_get_bool(qdict, "encrypted"));
> +
> +        monitor_printf(mon, " bps=%" PRId64 " bps_rd=%" PRId64
> +                            " bps_wr=%" PRId64 " iops=%" PRId64
> +                            " iops_rd=%" PRId64 " iops_wr=%" PRId64,
> +                            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"));
>     } else {
>         monitor_printf(mon, " [not inserted]");
>     }
> @@ -1816,10 +1867,22 @@ void bdrv_info(Monitor *mon, QObject **ret_data)
>             QDict *bs_dict = qobject_to_qdict(bs_obj);
>
>             obj = qobject_from_jsonf("{ 'file': %s, 'ro': %i, 'drv': %s, "
> -                                     "'encrypted': %i }",
> +                                     "'encrypted': %i, "
> +                                     "'bps': %" PRId64 ","
> +                                     "'bps_rd': %" PRId64 ","
> +                                     "'bps_wr': %" PRId64 ","
> +                                     "'iops': %" PRId64 ","
> +                                     "'iops_rd': %" PRId64 ","
> +                                     "'iops_wr': %" PRId64 "}",
>                                      bs->filename, bs->read_only,
>                                      bs->drv->format_name,
> -                                     bdrv_is_encrypted(bs));
> +                                     bdrv_is_encrypted(bs),
> +                                     bs->io_limits.bps[2],
> +                                     bs->io_limits.bps[0],
> +                                     bs->io_limits.bps[1],
> +                                     bs->io_limits.iops[2],
> +                                     bs->io_limits.iops[0],
> +                                     bs->io_limits.iops[1]);
>             if (bs->backing_file[0] != '\0') {
>                 QDict *qdict = qobject_to_qdict(obj);
>                 qdict_put(qdict, "backing_file",
> @@ -2307,7 +2370,7 @@ static bool bdrv_exceed_io_limits(BlockDriverState *bs, int nb_sectors,
>     }
>
>     /* If a limit was exceeded, immediately queue this request */
> -    if ((bs->req_from_queue == false)
> +    if (!bs->req_from_queue
>         && !QTAILQ_EMPTY(&bs->block_queue->requests)) {
>         if (bs->io_limits.bps[BLOCK_IO_LIMIT_TOTAL]
>             || bs->io_limits.bps[is_write] || bs->io_limits.iops[is_write]
> @@ -2362,14 +2425,14 @@ BlockDriverAIOCB *bdrv_aio_readv(BlockDriverState *bs, int64_t sector_num,
>     trace_bdrv_aio_readv(bs, sector_num, nb_sectors, opaque);
>
>     if (!drv || bdrv_check_request(bs, sector_num, nb_sectors)) {
> -        if (bdrv_io_limits_enable(&bs->io_limits)) {
> +        if (bs->io_limits_enabled) {
>             bs->req_from_queue = false;
>         }
>         return NULL;
>     }
>
>     /* throttling disk read I/O */
> -    if (bdrv_io_limits_enable(&bs->io_limits)) {
> +    if (bs->io_limits_enabled) {
>         if (bdrv_exceed_io_limits(bs, nb_sectors, false, &wait_time)) {
>             ret = qemu_block_queue_enqueue(bs->block_queue, bs, bdrv_aio_readv,
>                               sector_num, qiov, nb_sectors, cb, opaque);
> @@ -2388,14 +2451,14 @@ BlockDriverAIOCB *bdrv_aio_readv(BlockDriverState *bs, int64_t sector_num,
>        bs->rd_bytes += (unsigned) nb_sectors * BDRV_SECTOR_SIZE;
>        bs->rd_ops ++;
>
> -        if (bdrv_io_limits_enable(&bs->io_limits)) {
> +        if (bs->io_limits_enabled) {
>             bs->io_disps.bytes[BLOCK_IO_LIMIT_READ] +=
>                               (unsigned) nb_sectors * BDRV_SECTOR_SIZE;
>             bs->io_disps.ios[BLOCK_IO_LIMIT_READ]++;
>         }
>     }
>
> -    if (bdrv_io_limits_enable(&bs->io_limits)) {
> +    if (bs->io_limits_enabled) {
>         bs->req_from_queue = false;
>     }
>
> @@ -2451,7 +2514,7 @@ BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num,
>
>     if (!drv || bs->read_only
>         || bdrv_check_request(bs, sector_num, nb_sectors)) {
> -        if (bdrv_io_limits_enable(&bs->io_limits)) {
> +        if (bs->io_limits_enabled) {
>             bs->req_from_queue = false;
>         }
>
> @@ -2466,7 +2529,7 @@ BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num,
>     }
>
>     /* throttling disk write I/O */
> -    if (bdrv_io_limits_enable(&bs->io_limits)) {
> +    if (bs->io_limits_enabled) {
>         if (bdrv_exceed_io_limits(bs, nb_sectors, true, &wait_time)) {
>             ret = qemu_block_queue_enqueue(bs->block_queue, bs, bdrv_aio_writev,
>                                   sector_num, qiov, nb_sectors, cb, opaque);
> @@ -2488,14 +2551,14 @@ BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num,
>             bs->wr_highest_sector = sector_num + nb_sectors - 1;
>         }
>
> -        if (bdrv_io_limits_enable(&bs->io_limits)) {
> +        if (bs->io_limits_enabled) {
>             bs->io_disps.bytes[BLOCK_IO_LIMIT_WRITE] +=
>                                (unsigned) nb_sectors * BDRV_SECTOR_SIZE;
>             bs->io_disps.ios[BLOCK_IO_LIMIT_WRITE]++;
>         }
>     }
>
> -    if (bdrv_io_limits_enable(&bs->io_limits)) {
> +    if (bs->io_limits_enabled) {
>         bs->req_from_queue = false;
>     }
>
> diff --git a/block.h b/block.h
> index f0dac62..97d2177 100644
> --- a/block.h
> +++ b/block.h
> @@ -57,6 +57,11 @@ void bdrv_info(Monitor *mon, QObject **ret_data);
>  void bdrv_stats_print(Monitor *mon, const QObject *data);
>  void bdrv_info_stats(Monitor *mon, QObject **ret_data);
>
> +/* disk I/O throttling */
> +void bdrv_io_limits_res_init(BlockDriverState *bs);
> +void bdrv_io_limits_res_deinit(BlockDriverState *bs);
> +bool bdrv_io_limits_enabled(BlockDriverState *bs);
> +
>  void bdrv_init(void);
>  void bdrv_init_with_whitelist(void);
>  BlockDriver *bdrv_find_protocol(const char *filename);
> diff --git a/block_int.h b/block_int.h
> index 1ca826b..7c96094 100644
> --- a/block_int.h
> +++ b/block_int.h
> @@ -199,6 +199,8 @@ struct BlockDriverState {
>     BlockIODisp  io_disps;
>     BlockQueue   *block_queue;
>     QEMUTimer    *block_timer;
> +    bool         io_limits_enabled;
> +    bool         io_unlimit;
>     bool         req_from_queue;
>
>     /* I/O stats (display with "info blockstats"). */
> diff --git a/blockdev.c b/blockdev.c
> index aff6bb2..dc2e8a9 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -376,7 +376,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
>         return NULL;
>     }
>
> -    /* disk io limits */
> +    /* disk io throttling */
>     iol_flag = qemu_opt_io_limits_enable_flag(opts, iol_opts);
>     if (iol_flag) {
>         memset(&io_limits, 0, sizeof(BlockIOLimit));
> @@ -387,6 +387,14 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
>         io_limits.iops[2] = qemu_opt_get_number(opts, "iops", 0);
>         io_limits.iops[0] = qemu_opt_get_number(opts, "iops_rd", 0);
>         io_limits.iops[1] = qemu_opt_get_number(opts, "iops_wr", 0);
> +
> +        if (((io_limits.bps[2] != 0)
> +            && ((io_limits.bps[0] != 0) || (io_limits.bps[1] != 0)))
> +            || ((io_limits.iops[2] != 0)
> +            && ((io_limits.iops[0] != 0) || (io_limits.iops[1] != 0)))) {

Please define constants for read/write/total instead of using magic numbers.

> +            error_report("Some params for io throttling can not coexist");

This error message is vague.

"bps and bps_rd/bps_wr cannot be used at the same time"

> +            return NULL;
> +        }
>     }
>
>     on_write_error = BLOCK_ERR_STOP_ENOSPC;
> @@ -765,6 +773,82 @@ int do_change_block(Monitor *mon, const char *device,
>     return monitor_read_bdrv_key_start(mon, bs, NULL, NULL);
>  }
>
> +/* throttling disk I/O limits */
> +int do_block_set_io_throttle(Monitor *mon,
> +                       const QDict *qdict, QObject **ret_data)
> +{
> +    const char *devname = qdict_get_str(qdict, "device");
> +    uint64_t bps        = qdict_get_try_int(qdict, "bps", -1);
> +    uint64_t bps_rd     = qdict_get_try_int(qdict, "bps_rd", -1);
> +    uint64_t bps_wr     = qdict_get_try_int(qdict, "bps_wr", -1);
> +    uint64_t iops       = qdict_get_try_int(qdict, "iops", -1);
> +    uint64_t iops_rd    = qdict_get_try_int(qdict, "iops_rd", -1);
> +    uint64_t iops_wr    = qdict_get_try_int(qdict, "iops_wr", -1);
> +    BlockDriverState *bs;
> +
> +    bs = bdrv_find(devname);
> +    if (!bs) {
> +        qerror_report(QERR_DEVICE_NOT_FOUND, devname);
> +        return -1;
> +    }
> +
> +    if (devname && (bps == -1) && (bps_rd == -1) && (bps_wr == -1)
> +        && (iops == -1) && (iops_rd == -1) && (iops_wr == -1)) {

No need for the devname check.  It must be non-NULL since it is a
mandatory argument, also bdrv_find() would have crashed since devname
is used unchecked.

> +        qerror_report(QERR_IO_THROTTLE_NO_PARAM);
> +        return -1;
> +    }
> +
> +    if (((bps != -1) && ((bps_rd != -1) || (bps_wr != -1)))
> +        || ((iops != -1) && ((iops_rd != -1) || (iops_wr != -1)))) {
> +        qerror_report(QERR_IO_THROTTLE_PARAM_ERROR);
> +        return -1;
> +    }
> +
> +    if (bps != -1) {
> +        bs->io_limits.bps[2] = bps;
> +        bs->io_limits.bps[0] = 0;
> +        bs->io_limits.bps[1] = 0;
> +    }
> +
> +    if ((bps_rd != -1) || (bps_wr != -1)) {
> +        bs->io_limits.bps[0]  = (bps_rd == -1) ? bs->io_limits.bps[0] : bps_rd;
> +        bs->io_limits.bps[1]  = (bps_wr == -1) ? bs->io_limits.bps[1] : bps_wr;
> +        bs->io_limits.bps[2]  = 0;
> +    }
> +
> +    if (iops != -1) {
> +        bs->io_limits.iops[2] = iops;
> +        bs->io_limits.iops[0] = 0;
> +        bs->io_limits.iops[1] = 0;
> +    }
> +
> +    if ((iops_rd != -1) || (iops_wr != -1)) {
> +        bs->io_limits.iops[0] =
> +                      (iops_rd == -1) ? bs->io_limits.iops[0] : iops_rd;
> +        bs->io_limits.iops[1] =
> +                      (iops_wr == -1) ? bs->io_limits.iops[1] : iops_wr;
> +        bs->io_limits.iops[2] = 0;
> +    }
> +
> +    if (bdrv_io_limits_enabled(bs)) {
> +        if (!bs->io_limits_enabled) {
> +            bdrv_io_limits_res_init(bs);
> +        }
> +    } else {
> +        if (bs->io_limits_enabled) {
> +            BlockQueue *queue = bs->block_queue;
> +            if (!QTAILQ_EMPTY(&queue->requests)) {
> +                bs->io_unlimit = true;
> +                bs->io_limits_enabled = false;
> +            } else {
> +                bdrv_io_limits_res_deinit(bs);
> +            }
> +        }
> +    }

bdrv_io_limits_res_init() and bdrv_io_limits_res_deinit() are
basically enable/disable functions.  I would change this to:

if (!bs->io_limits_enabled && bdrv_io_limits_enabled(bs)) {
    bdrv_io_limits_enable(bs);
} else if (bs->io_limits_enabled && !bdrv_io_limits_enabled(bs)) {
    bdrv_io_limits_disable(bs);
}

And then remove the bs->io_unlimit field by immediately stopping I/O throttling:

void bdrv_io_limits_disable(BlockDriverState *bs)
{
    bs->io_limits_enabled = false;
    bs->req_from_queue   = false;

    if (bs->block_queue) {
        qemu_block_queue_flush(bs->block_queue); /* <--- dispatch
queued requests */
        qemu_del_block_queue(bs->block_queue);
    }

    if (bs->block_timer) {
        qemu_del_timer(bs->block_timer);
        qemu_free_timer(bs->block_timer);
    }

    bs->slice_start[0]   = 0;
    bs->slice_start[1]   = 0;

    bs->slice_end[0]     = 0;
    bs->slice_end[1]     = 0;
}

The queue flushing code from bdrv_block_timer() could be moved into
qemu_block_queue_flush().  bdrv_block_timer() would call that function
instead of looping over the queue itself.

> +
> +    return 0;
> +}
> +
>  int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
>  {
>     const char *id = qdict_get_str(qdict, "id");
> diff --git a/blockdev.h b/blockdev.h
> index 0a5144c..d0d0d77 100644
> --- a/blockdev.h
> +++ b/blockdev.h
> @@ -63,6 +63,8 @@ int do_block_set_passwd(Monitor *mon, const QDict *qdict, QObject **ret_data);
>  int do_change_block(Monitor *mon, const char *device,
>                     const char *filename, const char *fmt);
>  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);
>  int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data);
>  int do_block_resize(Monitor *mon, const QDict *qdict, QObject **ret_data);
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index aceba74..3ca496d 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1210,6 +1210,21 @@ ETEXI
>     },
>
>  STEXI
> +@item block_set_io_throttle @var{device} @var{bps} @var{bps_rd} @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
> +@findex block_set_io_throttle
> +Change I/O throttle limits for a block drive to @var{bps} @var{bps_rd} @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
> +ETEXI
> +
> +    {
> +        .name       = "block_set_io_throttle",
> +        .args_type  = "device:B,bps:i?,bps_rd:i?,bps_wr:i?,iops:i?,iops_rd:i?,iops_wr:i?",
> +        .params     = "device [bps] [bps_rd] [bps_wr] [iops] [iops_rd] [iops_wr]",
> +        .help       = "change I/O throttle limts for a block drive",
> +        .user_print = monitor_user_noop,
> +        .mhandler.cmd_new = do_block_set_io_throttle,
> +    },
> +
> +STEXI
>  @item block_passwd @var{device} @var{password}
>  @findex block_passwd
>  Set the encrypted device @var{device} password to @var{password}
> diff --git a/qerror.c b/qerror.c
> index d7fcd93..db04df7 100644
> --- a/qerror.c
> +++ b/qerror.c
> @@ -213,6 +213,14 @@ static const QErrorStringTable qerror_table[] = {
>         .error_fmt = QERR_VNC_SERVER_FAILED,
>         .desc      = "Could not start VNC server on %(target)",
>     },
> +    {
> +        .error_fmt = QERR_IO_THROTTLE_PARAM_ERROR,
> +        .desc      = "Some params for io throttling can not coexist",
> +    },
> +    {
> +        .error_fmt = QERR_IO_THROTTLE_NO_PARAM,
> +        .desc      = "No params for io throttling are specified",
> +    },
>     {}
>  };
>
> diff --git a/qerror.h b/qerror.h
> index 16c830d..892abbb 100644
> --- a/qerror.h
> +++ b/qerror.h
> @@ -181,4 +181,10 @@ QError *qobject_to_qerror(const QObject *obj);
>  #define QERR_FEATURE_DISABLED \
>     "{ 'class': 'FeatureDisabled', 'data': { 'name': %s } }"
>
> +#define QERR_IO_THROTTLE_PARAM_ERROR \
> +    "{ 'class': 'ParamsCoexistNotPermitted', 'data': {} }"

This error is not specific to I/O throttling.  Please add a generic error:

#define QERR_INVALID_PARAMETER_COMBINATION \
    "{ 'class': 'InvalidParameterCombination', 'data': { 'param1': %s,
'param2': %s } }"

> +#define QERR_IO_THROTTLE_NO_PARAM \
> +    "{ 'class': 'NoParamsSpecified', 'data': {} }"

QERR_MISSING_PARAMETER already does this.

> +
>  #endif /* QERROR_H */
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 92c5c3a..0aaeae1 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -828,6 +828,44 @@ Example:
>  EQMP
>
>     {
> +        .name       = "block_set_io_throttle",
> +        .args_type  = "device:B,bps:i?,bps_rd:i?,bps_wr:i?,iops:i?,iops_rd:i?,iops_wr:i?",
> +        .params     = "device [bps[ [bps_rd] [bps_wr] [iops] [iops_rd] [iops_wr]",

[bps]

> +        .help       = "change I/O throttle limts for a block drive",
> +        .user_print = monitor_user_noop,
> +        .mhandler.cmd_new = do_block_set_io_throttle,
> +    },
> +
> +SQMP
> +block_set_io_throttle
> +------------
> +
> +Change I/O throttle limts for a block drive.
> +
> +Arguments:
> +
> +- "device": device name (json-string)
> +- "bps":  total throughput value per second(json-int, optional)

Please document the units of bps ("bps" can mean bits per second or
bytes per second):

"total throughput limit in bytes per second (json-int, optional)"

> +- "bps_rd":  read throughput value per second(json-int, optional)
> +- "bps_wr":  read throughput value per second(json-int, optional)

Stefan
Zhiyong Wu Aug. 5, 2011, 2:03 a.m. UTC | #2
On Thu, Aug 4, 2011 at 9:07 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Thu, Aug 4, 2011 at 5:34 AM, Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> wrote:
>> @@ -1387,6 +1422,11 @@ void bdrv_set_io_limits(BlockDriverState *bs,
>>  {
>>     memset(&bs->io_limits, 0, sizeof(BlockIOLimit));
>>     bs->io_limits = *io_limits;
>> +    if (bdrv_io_limits_enabled(bs)) {
>> +        bs->io_limits_enabled = true;
>> +    } else {
>> +        bs->io_limits_enabled = false;
>> +    }
>
> Or in one line:
> bs->io_limits_enabled = bdrv_io_limits_enabled(bs);
nice.
>
>>  }
>>
>>  /* Recognize floppy formats */
>> @@ -1621,6 +1661,7 @@ BlockDriverState *bdrv_find(const char *name)
>>     return NULL;
>>  }
>>
>> +/* disk I/O throttling */
>
> Looks like this should not be here.\
Let me check when i will go to office next week.
>
>>  BlockDriverState *bdrv_next(BlockDriverState *bs)
>>  {
>>     if (!bs) {
>> @@ -1784,6 +1825,16 @@ static void bdrv_print_dict(QObject *obj, void *opaque)
>>                             qdict_get_bool(qdict, "ro"),
>>                             qdict_get_str(qdict, "drv"),
>>                             qdict_get_bool(qdict, "encrypted"));
>> +
>> +        monitor_printf(mon, " bps=%" PRId64 " bps_rd=%" PRId64
>> +                            " bps_wr=%" PRId64 " iops=%" PRId64
>> +                            " iops_rd=%" PRId64 " iops_wr=%" PRId64,
>> +                            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"));
>>     } else {
>>         monitor_printf(mon, " [not inserted]");
>>     }
>> @@ -1816,10 +1867,22 @@ void bdrv_info(Monitor *mon, QObject **ret_data)
>>             QDict *bs_dict = qobject_to_qdict(bs_obj);
>>
>>             obj = qobject_from_jsonf("{ 'file': %s, 'ro': %i, 'drv': %s, "
>> -                                     "'encrypted': %i }",
>> +                                     "'encrypted': %i, "
>> +                                     "'bps': %" PRId64 ","
>> +                                     "'bps_rd': %" PRId64 ","
>> +                                     "'bps_wr': %" PRId64 ","
>> +                                     "'iops': %" PRId64 ","
>> +                                     "'iops_rd': %" PRId64 ","
>> +                                     "'iops_wr': %" PRId64 "}",
>>                                      bs->filename, bs->read_only,
>>                                      bs->drv->format_name,
>> -                                     bdrv_is_encrypted(bs));
>> +                                     bdrv_is_encrypted(bs),
>> +                                     bs->io_limits.bps[2],
>> +                                     bs->io_limits.bps[0],
>> +                                     bs->io_limits.bps[1],
>> +                                     bs->io_limits.iops[2],
>> +                                     bs->io_limits.iops[0],
>> +                                     bs->io_limits.iops[1]);
>>             if (bs->backing_file[0] != '\0') {
>>                 QDict *qdict = qobject_to_qdict(obj);
>>                 qdict_put(qdict, "backing_file",
>> @@ -2307,7 +2370,7 @@ static bool bdrv_exceed_io_limits(BlockDriverState *bs, int nb_sectors,
>>     }
>>
>>     /* If a limit was exceeded, immediately queue this request */
>> -    if ((bs->req_from_queue == false)
>> +    if (!bs->req_from_queue
>>         && !QTAILQ_EMPTY(&bs->block_queue->requests)) {
>>         if (bs->io_limits.bps[BLOCK_IO_LIMIT_TOTAL]
>>             || bs->io_limits.bps[is_write] || bs->io_limits.iops[is_write]
>> @@ -2362,14 +2425,14 @@ BlockDriverAIOCB *bdrv_aio_readv(BlockDriverState *bs, int64_t sector_num,
>>     trace_bdrv_aio_readv(bs, sector_num, nb_sectors, opaque);
>>
>>     if (!drv || bdrv_check_request(bs, sector_num, nb_sectors)) {
>> -        if (bdrv_io_limits_enable(&bs->io_limits)) {
>> +        if (bs->io_limits_enabled) {
>>             bs->req_from_queue = false;
>>         }
>>         return NULL;
>>     }
>>
>>     /* throttling disk read I/O */
>> -    if (bdrv_io_limits_enable(&bs->io_limits)) {
>> +    if (bs->io_limits_enabled) {
>>         if (bdrv_exceed_io_limits(bs, nb_sectors, false, &wait_time)) {
>>             ret = qemu_block_queue_enqueue(bs->block_queue, bs, bdrv_aio_readv,
>>                               sector_num, qiov, nb_sectors, cb, opaque);
>> @@ -2388,14 +2451,14 @@ BlockDriverAIOCB *bdrv_aio_readv(BlockDriverState *bs, int64_t sector_num,
>>        bs->rd_bytes += (unsigned) nb_sectors * BDRV_SECTOR_SIZE;
>>        bs->rd_ops ++;
>>
>> -        if (bdrv_io_limits_enable(&bs->io_limits)) {
>> +        if (bs->io_limits_enabled) {
>>             bs->io_disps.bytes[BLOCK_IO_LIMIT_READ] +=
>>                               (unsigned) nb_sectors * BDRV_SECTOR_SIZE;
>>             bs->io_disps.ios[BLOCK_IO_LIMIT_READ]++;
>>         }
>>     }
>>
>> -    if (bdrv_io_limits_enable(&bs->io_limits)) {
>> +    if (bs->io_limits_enabled) {
>>         bs->req_from_queue = false;
>>     }
>>
>> @@ -2451,7 +2514,7 @@ BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num,
>>
>>     if (!drv || bs->read_only
>>         || bdrv_check_request(bs, sector_num, nb_sectors)) {
>> -        if (bdrv_io_limits_enable(&bs->io_limits)) {
>> +        if (bs->io_limits_enabled) {
>>             bs->req_from_queue = false;
>>         }
>>
>> @@ -2466,7 +2529,7 @@ BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num,
>>     }
>>
>>     /* throttling disk write I/O */
>> -    if (bdrv_io_limits_enable(&bs->io_limits)) {
>> +    if (bs->io_limits_enabled) {
>>         if (bdrv_exceed_io_limits(bs, nb_sectors, true, &wait_time)) {
>>             ret = qemu_block_queue_enqueue(bs->block_queue, bs, bdrv_aio_writev,
>>                                   sector_num, qiov, nb_sectors, cb, opaque);
>> @@ -2488,14 +2551,14 @@ BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num,
>>             bs->wr_highest_sector = sector_num + nb_sectors - 1;
>>         }
>>
>> -        if (bdrv_io_limits_enable(&bs->io_limits)) {
>> +        if (bs->io_limits_enabled) {
>>             bs->io_disps.bytes[BLOCK_IO_LIMIT_WRITE] +=
>>                                (unsigned) nb_sectors * BDRV_SECTOR_SIZE;
>>             bs->io_disps.ios[BLOCK_IO_LIMIT_WRITE]++;
>>         }
>>     }
>>
>> -    if (bdrv_io_limits_enable(&bs->io_limits)) {
>> +    if (bs->io_limits_enabled) {
>>         bs->req_from_queue = false;
>>     }
>>
>> diff --git a/block.h b/block.h
>> index f0dac62..97d2177 100644
>> --- a/block.h
>> +++ b/block.h
>> @@ -57,6 +57,11 @@ void bdrv_info(Monitor *mon, QObject **ret_data);
>>  void bdrv_stats_print(Monitor *mon, const QObject *data);
>>  void bdrv_info_stats(Monitor *mon, QObject **ret_data);
>>
>> +/* disk I/O throttling */
>> +void bdrv_io_limits_res_init(BlockDriverState *bs);
>> +void bdrv_io_limits_res_deinit(BlockDriverState *bs);
>> +bool bdrv_io_limits_enabled(BlockDriverState *bs);
>> +
>>  void bdrv_init(void);
>>  void bdrv_init_with_whitelist(void);
>>  BlockDriver *bdrv_find_protocol(const char *filename);
>> diff --git a/block_int.h b/block_int.h
>> index 1ca826b..7c96094 100644
>> --- a/block_int.h
>> +++ b/block_int.h
>> @@ -199,6 +199,8 @@ struct BlockDriverState {
>>     BlockIODisp  io_disps;
>>     BlockQueue   *block_queue;
>>     QEMUTimer    *block_timer;
>> +    bool         io_limits_enabled;
>> +    bool         io_unlimit;
>>     bool         req_from_queue;
>>
>>     /* I/O stats (display with "info blockstats"). */
>> diff --git a/blockdev.c b/blockdev.c
>> index aff6bb2..dc2e8a9 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -376,7 +376,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
>>         return NULL;
>>     }
>>
>> -    /* disk io limits */
>> +    /* disk io throttling */
>>     iol_flag = qemu_opt_io_limits_enable_flag(opts, iol_opts);
>>     if (iol_flag) {
>>         memset(&io_limits, 0, sizeof(BlockIOLimit));
>> @@ -387,6 +387,14 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
>>         io_limits.iops[2] = qemu_opt_get_number(opts, "iops", 0);
>>         io_limits.iops[0] = qemu_opt_get_number(opts, "iops_rd", 0);
>>         io_limits.iops[1] = qemu_opt_get_number(opts, "iops_wr", 0);
>> +
>> +        if (((io_limits.bps[2] != 0)
>> +            && ((io_limits.bps[0] != 0) || (io_limits.bps[1] != 0)))
>> +            || ((io_limits.iops[2] != 0)
>> +            && ((io_limits.iops[0] != 0) || (io_limits.iops[1] != 0)))) {
>
> Please define constants for read/write/total instead of using magic numbers.
They have been defined. OK. let me replace those magic numbers with them.
>
>> +            error_report("Some params for io throttling can not coexist");
>
> This error message is vague.
>
> "bps and bps_rd/bps_wr cannot be used at the same time"
>
>> +            return NULL;
>> +        }
>>     }
>>
>>     on_write_error = BLOCK_ERR_STOP_ENOSPC;
>> @@ -765,6 +773,82 @@ int do_change_block(Monitor *mon, const char *device,
>>     return monitor_read_bdrv_key_start(mon, bs, NULL, NULL);
>>  }
>>
>> +/* throttling disk I/O limits */
>> +int do_block_set_io_throttle(Monitor *mon,
>> +                       const QDict *qdict, QObject **ret_data)
>> +{
>> +    const char *devname = qdict_get_str(qdict, "device");
>> +    uint64_t bps        = qdict_get_try_int(qdict, "bps", -1);
>> +    uint64_t bps_rd     = qdict_get_try_int(qdict, "bps_rd", -1);
>> +    uint64_t bps_wr     = qdict_get_try_int(qdict, "bps_wr", -1);
>> +    uint64_t iops       = qdict_get_try_int(qdict, "iops", -1);
>> +    uint64_t iops_rd    = qdict_get_try_int(qdict, "iops_rd", -1);
>> +    uint64_t iops_wr    = qdict_get_try_int(qdict, "iops_wr", -1);
>> +    BlockDriverState *bs;
>> +
>> +    bs = bdrv_find(devname);
>> +    if (!bs) {
>> +        qerror_report(QERR_DEVICE_NOT_FOUND, devname);
>> +        return -1;
>> +    }
>> +
>> +    if (devname && (bps == -1) && (bps_rd == -1) && (bps_wr == -1)
>> +        && (iops == -1) && (iops_rd == -1) && (iops_wr == -1)) {
>
> No need for the devname check.  It must be non-NULL since it is a
> mandatory argument, also bdrv_find() would have crashed since devname
> is used unchecked.
OK. i will remove devname.
>
>> +        qerror_report(QERR_IO_THROTTLE_NO_PARAM);
>> +        return -1;
>> +    }
>> +
>> +    if (((bps != -1) && ((bps_rd != -1) || (bps_wr != -1)))
>> +        || ((iops != -1) && ((iops_rd != -1) || (iops_wr != -1)))) {
>> +        qerror_report(QERR_IO_THROTTLE_PARAM_ERROR);
>> +        return -1;
>> +    }
>> +
>> +    if (bps != -1) {
>> +        bs->io_limits.bps[2] = bps;
>> +        bs->io_limits.bps[0] = 0;
>> +        bs->io_limits.bps[1] = 0;
>> +    }
>> +
>> +    if ((bps_rd != -1) || (bps_wr != -1)) {
>> +        bs->io_limits.bps[0]  = (bps_rd == -1) ? bs->io_limits.bps[0] : bps_rd;
>> +        bs->io_limits.bps[1]  = (bps_wr == -1) ? bs->io_limits.bps[1] : bps_wr;
>> +        bs->io_limits.bps[2]  = 0;
>> +    }
>> +
>> +    if (iops != -1) {
>> +        bs->io_limits.iops[2] = iops;
>> +        bs->io_limits.iops[0] = 0;
>> +        bs->io_limits.iops[1] = 0;
>> +    }
>> +
>> +    if ((iops_rd != -1) || (iops_wr != -1)) {
>> +        bs->io_limits.iops[0] =
>> +                      (iops_rd == -1) ? bs->io_limits.iops[0] : iops_rd;
>> +        bs->io_limits.iops[1] =
>> +                      (iops_wr == -1) ? bs->io_limits.iops[1] : iops_wr;
>> +        bs->io_limits.iops[2] = 0;
>> +    }
>> +
>> +    if (bdrv_io_limits_enabled(bs)) {
>> +        if (!bs->io_limits_enabled) {
>> +            bdrv_io_limits_res_init(bs);
>> +        }
>> +    } else {
>> +        if (bs->io_limits_enabled) {
>> +            BlockQueue *queue = bs->block_queue;
>> +            if (!QTAILQ_EMPTY(&queue->requests)) {
>> +                bs->io_unlimit = true;
>> +                bs->io_limits_enabled = false;
>> +            } else {
>> +                bdrv_io_limits_res_deinit(bs);
>> +            }
>> +        }
>> +    }
>
> bdrv_io_limits_res_init() and bdrv_io_limits_res_deinit() are
> basically enable/disable functions.  I would change this to:
>
> if (!bs->io_limits_enabled && bdrv_io_limits_enabled(bs)) {
>    bdrv_io_limits_enable(bs);
> } else if (bs->io_limits_enabled && !bdrv_io_limits_enabled(bs)) {
>    bdrv_io_limits_disable(bs);
> }
Good point.
>
> And then remove the bs->io_unlimit field by immediately stopping I/O throttling:
>
> void bdrv_io_limits_disable(BlockDriverState *bs)
> {
>    bs->io_limits_enabled = false;
>    bs->req_from_queue   = false;
>
>     if (bs->block_queue) {
>        qemu_block_queue_flush(bs->block_queue); /* <--- dispatch
> queued requests */
>         qemu_del_block_queue(bs->block_queue);
>     }
>
>     if (bs->block_timer) {
>         qemu_del_timer(bs->block_timer);
>         qemu_free_timer(bs->block_timer);
>     }
>
>     bs->slice_start[0]   = 0;
>     bs->slice_start[1]   = 0;
>
>     bs->slice_end[0]     = 0;
>     bs->slice_end[1]     = 0;
> }
>
> The queue flushing code from bdrv_block_timer() could be moved into
> qemu_block_queue_flush().  bdrv_block_timer() would call that function
> instead of looping over the queue itself.
OK.
>
>> +
>> +    return 0;
>> +}
>> +
>>  int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
>>  {
>>     const char *id = qdict_get_str(qdict, "id");
>> diff --git a/blockdev.h b/blockdev.h
>> index 0a5144c..d0d0d77 100644
>> --- a/blockdev.h
>> +++ b/blockdev.h
>> @@ -63,6 +63,8 @@ int do_block_set_passwd(Monitor *mon, const QDict *qdict, QObject **ret_data);
>>  int do_change_block(Monitor *mon, const char *device,
>>                     const char *filename, const char *fmt);
>>  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);
>>  int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data);
>>  int do_block_resize(Monitor *mon, const QDict *qdict, QObject **ret_data);
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index aceba74..3ca496d 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -1210,6 +1210,21 @@ ETEXI
>>     },
>>
>>  STEXI
>> +@item block_set_io_throttle @var{device} @var{bps} @var{bps_rd} @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
>> +@findex block_set_io_throttle
>> +Change I/O throttle limits for a block drive to @var{bps} @var{bps_rd} @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
>> +ETEXI
>> +
>> +    {
>> +        .name       = "block_set_io_throttle",
>> +        .args_type  = "device:B,bps:i?,bps_rd:i?,bps_wr:i?,iops:i?,iops_rd:i?,iops_wr:i?",
>> +        .params     = "device [bps] [bps_rd] [bps_wr] [iops] [iops_rd] [iops_wr]",
>> +        .help       = "change I/O throttle limts for a block drive",
>> +        .user_print = monitor_user_noop,
>> +        .mhandler.cmd_new = do_block_set_io_throttle,
>> +    },
>> +
>> +STEXI
>>  @item block_passwd @var{device} @var{password}
>>  @findex block_passwd
>>  Set the encrypted device @var{device} password to @var{password}
>> diff --git a/qerror.c b/qerror.c
>> index d7fcd93..db04df7 100644
>> --- a/qerror.c
>> +++ b/qerror.c
>> @@ -213,6 +213,14 @@ static const QErrorStringTable qerror_table[] = {
>>         .error_fmt = QERR_VNC_SERVER_FAILED,
>>         .desc      = "Could not start VNC server on %(target)",
>>     },
>> +    {
>> +        .error_fmt = QERR_IO_THROTTLE_PARAM_ERROR,
>> +        .desc      = "Some params for io throttling can not coexist",
>> +    },
>> +    {
>> +        .error_fmt = QERR_IO_THROTTLE_NO_PARAM,
>> +        .desc      = "No params for io throttling are specified",
>> +    },
>>     {}
>>  };
>>
>> diff --git a/qerror.h b/qerror.h
>> index 16c830d..892abbb 100644
>> --- a/qerror.h
>> +++ b/qerror.h
>> @@ -181,4 +181,10 @@ QError *qobject_to_qerror(const QObject *obj);
>>  #define QERR_FEATURE_DISABLED \
>>     "{ 'class': 'FeatureDisabled', 'data': { 'name': %s } }"
>>
>> +#define QERR_IO_THROTTLE_PARAM_ERROR \
>> +    "{ 'class': 'ParamsCoexistNotPermitted', 'data': {} }"
>
> This error is not specific to I/O throttling.  Please add a generic error:
>
> #define QERR_INVALID_PARAMETER_COMBINATION \
>    "{ 'class': 'InvalidParameterCombination', 'data': { 'param1': %s,
> 'param2': %s } }"
>
>> +#define QERR_IO_THROTTLE_NO_PARAM \
>> +    "{ 'class': 'NoParamsSpecified', 'data': {} }"
>
> QERR_MISSING_PARAMETER already does this.
>
>> +
>>  #endif /* QERROR_H */
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index 92c5c3a..0aaeae1 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -828,6 +828,44 @@ Example:
>>  EQMP
>>
>>     {
>> +        .name       = "block_set_io_throttle",
>> +        .args_type  = "device:B,bps:i?,bps_rd:i?,bps_wr:i?,iops:i?,iops_rd:i?,iops_wr:i?",
>> +        .params     = "device [bps[ [bps_rd] [bps_wr] [iops] [iops_rd] [iops_wr]",
>
> [bps]
Good catch.
>
>> +        .help       = "change I/O throttle limts for a block drive",
>> +        .user_print = monitor_user_noop,
>> +        .mhandler.cmd_new = do_block_set_io_throttle,
>> +    },
>> +
>> +SQMP
>> +block_set_io_throttle
>> +------------
>> +
>> +Change I/O throttle limts for a block drive.
>> +
>> +Arguments:
>> +
>> +- "device": device name (json-string)
>> +- "bps":  total throughput value per second(json-int, optional)
>
> Please document the units of bps ("bps" can mean bits per second or
> bytes per second):
>
> "total throughput limit in bytes per second (json-int, optional)"
OK.
>
>> +- "bps_rd":  read throughput value per second(json-int, optional)
>> +- "bps_wr":  read throughput value per second(json-int, optional)
>
> Stefan
>

Stefan, thanks for your good comments and spent time.
diff mbox

Patch

diff --git a/block.c b/block.c
index 42763a3..8bdf31a 100644
--- a/block.c
+++ b/block.c
@@ -100,18 +100,83 @@  int is_windows_drive(const char *filename)
 }
 #endif
 
-static int bdrv_io_limits_enable(BlockIOLimit *io_limits)
+/* throttling disk I/O limits */
+void bdrv_io_limits_res_deinit(BlockDriverState *bs)
 {
+    bs->io_limits_enabled = false;
+    bs->req_from_queue   = false;
+    bs->io_unlimit       = false;
+
+    if (bs->block_queue) {
+        qemu_del_block_queue(bs->block_queue);
+    }
+
+    if (bs->block_timer) {
+        qemu_del_timer(bs->block_timer);
+        qemu_free_timer(bs->block_timer);
+    }
+
+    bs->slice_start[0]   = 0;
+    bs->slice_start[1]   = 0;
+
+    bs->slice_end[0]     = 0;
+    bs->slice_end[1]     = 0;
+}
+
+static void bdrv_block_timer(void *opaque)
+{
+    BlockDriverState *bs = opaque;
+    BlockQueue *queue = bs->block_queue;
+
+    while (!QTAILQ_EMPTY(&queue->requests)) {
+        BlockIORequest *request = NULL;
+        int ret = 0;
+
+        request = QTAILQ_FIRST(&queue->requests);
+        QTAILQ_REMOVE(&queue->requests, request, entry);
+
+        ret = qemu_block_queue_handler(request);
+        if (ret == 0) {
+            QTAILQ_INSERT_HEAD(&queue->requests, request, entry);
+            break;
+        }
+
+        qemu_free(request);
+    }
+
+    if (bs->io_unlimit) {
+        bdrv_io_limits_res_deinit(bs);
+    }
+}
+
+void bdrv_io_limits_res_init(BlockDriverState *bs)
+{
+    bs->req_from_queue = false;
+    bs->io_unlimit     = false;
+
+    bs->block_queue    = qemu_new_block_queue();
+    bs->block_timer    = qemu_new_timer_ns(vm_clock, bdrv_block_timer, bs);
+
+    bs->slice_start[0] = qemu_get_clock_ns(vm_clock);
+    bs->slice_start[1] = qemu_get_clock_ns(vm_clock);
+
+    bs->slice_end[0]   = qemu_get_clock_ns(vm_clock) + BLOCK_IO_SLICE_TIME;
+    bs->slice_end[1]   = qemu_get_clock_ns(vm_clock) + BLOCK_IO_SLICE_TIME;
+}
+
+bool bdrv_io_limits_enabled(BlockDriverState *bs)
+{
+    BlockIOLimit *io_limits = &bs->io_limits;
     if ((io_limits->bps[0] == 0)
          && (io_limits->bps[1] == 0)
          && (io_limits->bps[2] == 0)
          && (io_limits->iops[0] == 0)
          && (io_limits->iops[1] == 0)
          && (io_limits->iops[2] == 0)) {
-        return 0;
+        return false;
     }
 
-    return 1;
+    return true;
 }
 
 /* check if the path starts with "<protocol>:" */
@@ -191,28 +256,6 @@  void path_combine(char *dest, int dest_size,
     }
 }
 
-static void bdrv_block_timer(void *opaque)
-{
-    BlockDriverState *bs = opaque;
-    BlockQueue *queue = bs->block_queue;
-
-    while (!QTAILQ_EMPTY(&queue->requests)) {
-        BlockIORequest *request = NULL;
-        int ret = 0;
-
-        request = QTAILQ_FIRST(&queue->requests);
-        QTAILQ_REMOVE(&queue->requests, request, entry);
-
-        ret = qemu_block_queue_handler(request);
-        if (ret == 0) {
-            QTAILQ_INSERT_HEAD(&queue->requests, request, entry);
-            break;
-        }
-
-        qemu_free(request);
-    }
-}
-
 void bdrv_register(BlockDriver *bdrv)
 {
     if (!bdrv->bdrv_aio_readv) {
@@ -689,16 +732,8 @@  int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
     }
 
     /* throttling disk I/O limits */
-    if (bdrv_io_limits_enable(&bs->io_limits)) {
-        bs->req_from_queue = false;
-        bs->block_queue    = qemu_new_block_queue();
-        bs->block_timer    = qemu_new_timer_ns(vm_clock, bdrv_block_timer, bs);
-
-        bs->slice_start[0] = qemu_get_clock_ns(vm_clock);
-        bs->slice_start[1] = qemu_get_clock_ns(vm_clock);
-
-        bs->slice_end[0]   = qemu_get_clock_ns(vm_clock) + BLOCK_IO_SLICE_TIME;
-        bs->slice_end[1]   = qemu_get_clock_ns(vm_clock) + BLOCK_IO_SLICE_TIME;
+    if (bs->io_limits_enabled) {
+        bdrv_io_limits_res_init(bs);
     }
 
     return 0;
@@ -1387,6 +1422,11 @@  void bdrv_set_io_limits(BlockDriverState *bs,
 {
     memset(&bs->io_limits, 0, sizeof(BlockIOLimit));
     bs->io_limits = *io_limits;
+    if (bdrv_io_limits_enabled(bs)) {
+        bs->io_limits_enabled = true;
+    } else {
+        bs->io_limits_enabled = false;
+    }
 }
 
 /* Recognize floppy formats */
@@ -1621,6 +1661,7 @@  BlockDriverState *bdrv_find(const char *name)
     return NULL;
 }
 
+/* disk I/O throttling */
 BlockDriverState *bdrv_next(BlockDriverState *bs)
 {
     if (!bs) {
@@ -1784,6 +1825,16 @@  static void bdrv_print_dict(QObject *obj, void *opaque)
                             qdict_get_bool(qdict, "ro"),
                             qdict_get_str(qdict, "drv"),
                             qdict_get_bool(qdict, "encrypted"));
+
+        monitor_printf(mon, " bps=%" PRId64 " bps_rd=%" PRId64
+                            " bps_wr=%" PRId64 " iops=%" PRId64
+                            " iops_rd=%" PRId64 " iops_wr=%" PRId64,
+                            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"));
     } else {
         monitor_printf(mon, " [not inserted]");
     }
@@ -1816,10 +1867,22 @@  void bdrv_info(Monitor *mon, QObject **ret_data)
             QDict *bs_dict = qobject_to_qdict(bs_obj);
 
             obj = qobject_from_jsonf("{ 'file': %s, 'ro': %i, 'drv': %s, "
-                                     "'encrypted': %i }",
+                                     "'encrypted': %i, "
+                                     "'bps': %" PRId64 ","
+                                     "'bps_rd': %" PRId64 ","
+                                     "'bps_wr': %" PRId64 ","
+                                     "'iops': %" PRId64 ","
+                                     "'iops_rd': %" PRId64 ","
+                                     "'iops_wr': %" PRId64 "}",
                                      bs->filename, bs->read_only,
                                      bs->drv->format_name,
-                                     bdrv_is_encrypted(bs));
+                                     bdrv_is_encrypted(bs),
+                                     bs->io_limits.bps[2],
+                                     bs->io_limits.bps[0],
+                                     bs->io_limits.bps[1],
+                                     bs->io_limits.iops[2],
+                                     bs->io_limits.iops[0],
+                                     bs->io_limits.iops[1]);
             if (bs->backing_file[0] != '\0') {
                 QDict *qdict = qobject_to_qdict(obj);
                 qdict_put(qdict, "backing_file",
@@ -2307,7 +2370,7 @@  static bool bdrv_exceed_io_limits(BlockDriverState *bs, int nb_sectors,
     }
 
     /* If a limit was exceeded, immediately queue this request */
-    if ((bs->req_from_queue == false)
+    if (!bs->req_from_queue
         && !QTAILQ_EMPTY(&bs->block_queue->requests)) {
         if (bs->io_limits.bps[BLOCK_IO_LIMIT_TOTAL]
             || bs->io_limits.bps[is_write] || bs->io_limits.iops[is_write]
@@ -2362,14 +2425,14 @@  BlockDriverAIOCB *bdrv_aio_readv(BlockDriverState *bs, int64_t sector_num,
     trace_bdrv_aio_readv(bs, sector_num, nb_sectors, opaque);
 
     if (!drv || bdrv_check_request(bs, sector_num, nb_sectors)) {
-        if (bdrv_io_limits_enable(&bs->io_limits)) {
+        if (bs->io_limits_enabled) {
             bs->req_from_queue = false;
         }
         return NULL;
     }
 
     /* throttling disk read I/O */
-    if (bdrv_io_limits_enable(&bs->io_limits)) {
+    if (bs->io_limits_enabled) {
         if (bdrv_exceed_io_limits(bs, nb_sectors, false, &wait_time)) {
             ret = qemu_block_queue_enqueue(bs->block_queue, bs, bdrv_aio_readv,
                               sector_num, qiov, nb_sectors, cb, opaque);
@@ -2388,14 +2451,14 @@  BlockDriverAIOCB *bdrv_aio_readv(BlockDriverState *bs, int64_t sector_num,
 	bs->rd_bytes += (unsigned) nb_sectors * BDRV_SECTOR_SIZE;
 	bs->rd_ops ++;
 
-        if (bdrv_io_limits_enable(&bs->io_limits)) {
+        if (bs->io_limits_enabled) {
             bs->io_disps.bytes[BLOCK_IO_LIMIT_READ] +=
                               (unsigned) nb_sectors * BDRV_SECTOR_SIZE;
             bs->io_disps.ios[BLOCK_IO_LIMIT_READ]++;
         }
     }
 
-    if (bdrv_io_limits_enable(&bs->io_limits)) {
+    if (bs->io_limits_enabled) {
         bs->req_from_queue = false;
     }
 
@@ -2451,7 +2514,7 @@  BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num,
 
     if (!drv || bs->read_only
         || bdrv_check_request(bs, sector_num, nb_sectors)) {
-        if (bdrv_io_limits_enable(&bs->io_limits)) {
+        if (bs->io_limits_enabled) {
             bs->req_from_queue = false;
         }
 
@@ -2466,7 +2529,7 @@  BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num,
     }
 
     /* throttling disk write I/O */
-    if (bdrv_io_limits_enable(&bs->io_limits)) {
+    if (bs->io_limits_enabled) {
         if (bdrv_exceed_io_limits(bs, nb_sectors, true, &wait_time)) {
             ret = qemu_block_queue_enqueue(bs->block_queue, bs, bdrv_aio_writev,
                                   sector_num, qiov, nb_sectors, cb, opaque);
@@ -2488,14 +2551,14 @@  BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num,
             bs->wr_highest_sector = sector_num + nb_sectors - 1;
         }
 
-        if (bdrv_io_limits_enable(&bs->io_limits)) {
+        if (bs->io_limits_enabled) {
             bs->io_disps.bytes[BLOCK_IO_LIMIT_WRITE] +=
                                (unsigned) nb_sectors * BDRV_SECTOR_SIZE;
             bs->io_disps.ios[BLOCK_IO_LIMIT_WRITE]++;
         }
     }
 
-    if (bdrv_io_limits_enable(&bs->io_limits)) {
+    if (bs->io_limits_enabled) {
         bs->req_from_queue = false;
     }
 
diff --git a/block.h b/block.h
index f0dac62..97d2177 100644
--- a/block.h
+++ b/block.h
@@ -57,6 +57,11 @@  void bdrv_info(Monitor *mon, QObject **ret_data);
 void bdrv_stats_print(Monitor *mon, const QObject *data);
 void bdrv_info_stats(Monitor *mon, QObject **ret_data);
 
+/* disk I/O throttling */
+void bdrv_io_limits_res_init(BlockDriverState *bs);
+void bdrv_io_limits_res_deinit(BlockDriverState *bs);
+bool bdrv_io_limits_enabled(BlockDriverState *bs);
+
 void bdrv_init(void);
 void bdrv_init_with_whitelist(void);
 BlockDriver *bdrv_find_protocol(const char *filename);
diff --git a/block_int.h b/block_int.h
index 1ca826b..7c96094 100644
--- a/block_int.h
+++ b/block_int.h
@@ -199,6 +199,8 @@  struct BlockDriverState {
     BlockIODisp  io_disps;
     BlockQueue   *block_queue;
     QEMUTimer    *block_timer;
+    bool         io_limits_enabled;
+    bool         io_unlimit;
     bool         req_from_queue;
 
     /* I/O stats (display with "info blockstats"). */
diff --git a/blockdev.c b/blockdev.c
index aff6bb2..dc2e8a9 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -376,7 +376,7 @@  DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
         return NULL;
     }
 
-    /* disk io limits */
+    /* disk io throttling */
     iol_flag = qemu_opt_io_limits_enable_flag(opts, iol_opts);
     if (iol_flag) {
         memset(&io_limits, 0, sizeof(BlockIOLimit));
@@ -387,6 +387,14 @@  DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
         io_limits.iops[2] = qemu_opt_get_number(opts, "iops", 0);
         io_limits.iops[0] = qemu_opt_get_number(opts, "iops_rd", 0);
         io_limits.iops[1] = qemu_opt_get_number(opts, "iops_wr", 0);
+
+        if (((io_limits.bps[2] != 0)
+            && ((io_limits.bps[0] != 0) || (io_limits.bps[1] != 0)))
+            || ((io_limits.iops[2] != 0)
+            && ((io_limits.iops[0] != 0) || (io_limits.iops[1] != 0)))) {
+            error_report("Some params for io throttling can not coexist");
+            return NULL;
+        }
     }
 
     on_write_error = BLOCK_ERR_STOP_ENOSPC;
@@ -765,6 +773,82 @@  int do_change_block(Monitor *mon, const char *device,
     return monitor_read_bdrv_key_start(mon, bs, NULL, NULL);
 }
 
+/* throttling disk I/O limits */
+int do_block_set_io_throttle(Monitor *mon,
+                       const QDict *qdict, QObject **ret_data)
+{
+    const char *devname = qdict_get_str(qdict, "device");
+    uint64_t bps        = qdict_get_try_int(qdict, "bps", -1);
+    uint64_t bps_rd     = qdict_get_try_int(qdict, "bps_rd", -1);
+    uint64_t bps_wr     = qdict_get_try_int(qdict, "bps_wr", -1);
+    uint64_t iops       = qdict_get_try_int(qdict, "iops", -1);
+    uint64_t iops_rd    = qdict_get_try_int(qdict, "iops_rd", -1);
+    uint64_t iops_wr    = qdict_get_try_int(qdict, "iops_wr", -1);
+    BlockDriverState *bs;
+
+    bs = bdrv_find(devname);
+    if (!bs) {
+        qerror_report(QERR_DEVICE_NOT_FOUND, devname);
+        return -1;
+    }
+
+    if (devname && (bps == -1) && (bps_rd == -1) && (bps_wr == -1)
+        && (iops == -1) && (iops_rd == -1) && (iops_wr == -1)) {
+        qerror_report(QERR_IO_THROTTLE_NO_PARAM);
+        return -1;
+    }
+
+    if (((bps != -1) && ((bps_rd != -1) || (bps_wr != -1)))
+        || ((iops != -1) && ((iops_rd != -1) || (iops_wr != -1)))) {
+        qerror_report(QERR_IO_THROTTLE_PARAM_ERROR);
+        return -1;
+    }
+
+    if (bps != -1) {
+        bs->io_limits.bps[2] = bps;
+        bs->io_limits.bps[0] = 0;
+        bs->io_limits.bps[1] = 0;
+    }
+
+    if ((bps_rd != -1) || (bps_wr != -1)) {
+        bs->io_limits.bps[0]  = (bps_rd == -1) ? bs->io_limits.bps[0] : bps_rd;
+        bs->io_limits.bps[1]  = (bps_wr == -1) ? bs->io_limits.bps[1] : bps_wr;
+        bs->io_limits.bps[2]  = 0;
+    }
+
+    if (iops != -1) {
+        bs->io_limits.iops[2] = iops;
+        bs->io_limits.iops[0] = 0;
+        bs->io_limits.iops[1] = 0;
+    }
+
+    if ((iops_rd != -1) || (iops_wr != -1)) {
+        bs->io_limits.iops[0] =
+                      (iops_rd == -1) ? bs->io_limits.iops[0] : iops_rd;
+        bs->io_limits.iops[1] =
+                      (iops_wr == -1) ? bs->io_limits.iops[1] : iops_wr;
+        bs->io_limits.iops[2] = 0;
+    }
+
+    if (bdrv_io_limits_enabled(bs)) {
+        if (!bs->io_limits_enabled) {
+            bdrv_io_limits_res_init(bs);
+        }
+    } else {
+        if (bs->io_limits_enabled) {
+            BlockQueue *queue = bs->block_queue;
+            if (!QTAILQ_EMPTY(&queue->requests)) {
+                bs->io_unlimit = true;
+                bs->io_limits_enabled = false;
+            } else {
+                bdrv_io_limits_res_deinit(bs);
+            }
+        }
+    }
+
+    return 0;
+}
+
 int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     const char *id = qdict_get_str(qdict, "id");
diff --git a/blockdev.h b/blockdev.h
index 0a5144c..d0d0d77 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -63,6 +63,8 @@  int do_block_set_passwd(Monitor *mon, const QDict *qdict, QObject **ret_data);
 int do_change_block(Monitor *mon, const char *device,
                     const char *filename, const char *fmt);
 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);
 int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data);
 int do_block_resize(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
diff --git a/hmp-commands.hx b/hmp-commands.hx
index aceba74..3ca496d 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1210,6 +1210,21 @@  ETEXI
     },
 
 STEXI
+@item block_set_io_throttle @var{device} @var{bps} @var{bps_rd} @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
+@findex block_set_io_throttle
+Change I/O throttle limits for a block drive to @var{bps} @var{bps_rd} @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
+ETEXI
+
+    {
+        .name       = "block_set_io_throttle",
+        .args_type  = "device:B,bps:i?,bps_rd:i?,bps_wr:i?,iops:i?,iops_rd:i?,iops_wr:i?",
+        .params     = "device [bps] [bps_rd] [bps_wr] [iops] [iops_rd] [iops_wr]",
+        .help       = "change I/O throttle limts for a block drive",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_block_set_io_throttle,
+    },
+
+STEXI
 @item block_passwd @var{device} @var{password}
 @findex block_passwd
 Set the encrypted device @var{device} password to @var{password}
diff --git a/qerror.c b/qerror.c
index d7fcd93..db04df7 100644
--- a/qerror.c
+++ b/qerror.c
@@ -213,6 +213,14 @@  static const QErrorStringTable qerror_table[] = {
         .error_fmt = QERR_VNC_SERVER_FAILED,
         .desc      = "Could not start VNC server on %(target)",
     },
+    {
+        .error_fmt = QERR_IO_THROTTLE_PARAM_ERROR,
+        .desc      = "Some params for io throttling can not coexist",
+    },
+    {
+        .error_fmt = QERR_IO_THROTTLE_NO_PARAM,
+        .desc      = "No params for io throttling are specified",
+    },
     {}
 };
 
diff --git a/qerror.h b/qerror.h
index 16c830d..892abbb 100644
--- a/qerror.h
+++ b/qerror.h
@@ -181,4 +181,10 @@  QError *qobject_to_qerror(const QObject *obj);
 #define QERR_FEATURE_DISABLED \
     "{ 'class': 'FeatureDisabled', 'data': { 'name': %s } }"
 
+#define QERR_IO_THROTTLE_PARAM_ERROR \
+    "{ 'class': 'ParamsCoexistNotPermitted', 'data': {} }"
+
+#define QERR_IO_THROTTLE_NO_PARAM \
+    "{ 'class': 'NoParamsSpecified', 'data': {} }"
+
 #endif /* QERROR_H */
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 92c5c3a..0aaeae1 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -828,6 +828,44 @@  Example:
 EQMP
 
     {
+        .name       = "block_set_io_throttle",
+        .args_type  = "device:B,bps:i?,bps_rd:i?,bps_wr:i?,iops:i?,iops_rd:i?,iops_wr:i?",
+        .params     = "device [bps[ [bps_rd] [bps_wr] [iops] [iops_rd] [iops_wr]",
+        .help       = "change I/O throttle limts for a block drive",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_block_set_io_throttle,
+    },
+
+SQMP
+block_set_io_throttle
+------------
+
+Change I/O throttle limts for a block drive.
+
+Arguments:
+
+- "device": device name (json-string)
+- "bps":  total throughput value per second(json-int, optional)
+- "bps_rd":  read throughput value per second(json-int, optional)
+- "bps_wr":  read throughput value per second(json-int, optional)
+- "iops":  total I/O operations per second(json-int, optional)
+- "iops_rd":  read I/O operations per second(json-int, optional)
+- "iops_wr":  write I/O operations per second(json-int, optional)
+
+Example:
+
+-> { "execute": "block_set_io_throttle", "arguments": { "device": "virtio0",
+                                               "bps": "1000000",
+                                               "bps_rd": "0",
+                                               "bps_wr": "0",
+                                               "iops": "0",
+                                               "iops_rd": "0",
+                                               "iops_wr": "0" } }
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "set_password",
         .args_type  = "protocol:s,password:s,connected:s?",
         .params     = "protocol password action-if-connected",
@@ -1082,6 +1120,12 @@  Each json-object contain the following:
                                 "tftp", "vdi", "vmdk", "vpc", "vvfat"
          - "backing_file": backing file name (json-string, optional)
          - "encrypted": true if encrypted, false otherwise (json-bool)
+         - "bps": limit total bytes per second (json-int)
+         - "bps_rd": limit read bytes per second (json-int)
+         - "bps_wr": limit write bytes per second (json-int)
+         - "iops": limit total I/O operations per second (json-int)
+         - "iops_rd": limit read operations per second (json-int)
+         - "iops_wr": limit write operations per second (json-int)
 
 Example:
 
@@ -1096,7 +1140,13 @@  Example:
                "ro":false,
                "drv":"qcow2",
                "encrypted":false,
-               "file":"disks/test.img"
+               "file":"disks/test.img",
+               "bps":1000000,
+               "bps_rd":0,
+               "bps_wr":0,
+               "iops":1000000,
+               "iops_rd":0,
+               "iops_wr":0,
             },
             "type":"unknown"
          },