| Message ID | 1312432489-19101-1-git-send-email-wuzhy@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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
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 --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" },
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(-)