Re: [Qemu-devel] [PATCH V5 5/5] block: Add iops_sector_count to do the iops accounting for a given io size.

[Benoît Canet]

Le Wednesday 14 Aug 2013 à 17:48:46 (+0800), Fam Zheng a écrit :
> On Mon, 08/12 18:53, Benoît Canet wrote:
> > This feature can be used in case where users are avoiding the iops limit by
> > doing jumbo I/Os hammering the storage backend.
> > 
> You are accounting io ops by the op size:
> (unit = size / iops_sector_count), which is equivelant to bps.  So I'm
> still not understanding why can't such jumbo IO be throttled by bps
> limits.

I am aiming at providing to the users an amazon like feature set.
http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/AmazonEBS.html

>From the provisioned volume section:

The read and write operations have a block size of 16 KB or less. If the I/O
increases above 16 KB, the IOPS delivered drop in proportion to the increase in
the size of the I/O. For example, a 1000 IOPS volume can deliver 1000 16 KB
writes per second, 500 32 KB writes per second, or 250 64 KB writes per second.

Best regards

Benoît

> 
> > Signed-off-by: Benoit Canet <address@hidden>
> > ---
> >  block/qapi.c     |    3 +++
> >  blockdev.c       |   22 +++++++++++++++++++---
> >  hmp.c            |    8 ++++++--
> >  qapi-schema.json |   10 ++++++++--
> >  qemu-options.hx  |    2 +-
> >  qmp-commands.hx  |    8 ++++++--
> >  6 files changed, 43 insertions(+), 10 deletions(-)
> > 
> > diff --git a/block/qapi.c b/block/qapi.c
> > index 5ba10f4..f98ff64 100644
> > --- a/block/qapi.c
> > +++ b/block/qapi.c
> > @@ -258,6 +258,9 @@ void bdrv_query_info(BlockDriverState *bs,
> >                  cfg.buckets[THROTTLE_OPS_WRITE].max;
> >              info->inserted->iops_wr_max     =
> >                  cfg.buckets[THROTTLE_OPS_WRITE].max;
> > +
> > +            info->inserted->has_iops_sector_count = cfg.op_size;
> > +            info->inserted->iops_sector_count = cfg.op_size;
> >          }
> >  
> >          bs0 = bs;
> > diff --git a/blockdev.c b/blockdev.c
> > index 22016a2..e2b0ee0 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -500,7 +500,7 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
> >          qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
> >  
> >      cfg.unit_size = BDRV_SECTOR_SIZE;
> > -    cfg.op_size = 0;
> > +    cfg.op_size = qemu_opt_get_number(opts, 
> > "throttling.iops-sector-count", 0);
> >  
> >      if (!check_throttle_config(&cfg, &error)) {
> >          error_report("%s", error_get_pretty(error));
> > @@ -786,6 +786,9 @@ DriveInfo *drive_init(QemuOpts *all_opts, 
> > BlockInterfaceType block_default_type)
> >      qemu_opt_rename(all_opts, "bps_rd_max", "throttling.bps-read-max");
> >      qemu_opt_rename(all_opts, "bps_wr_max", "throttling.bps-write-max");
> >  
> > +    qemu_opt_rename(all_opts,
> > +                    "iops_sector_count", "throttling.iops-sector-count");
> > +
> >      qemu_opt_rename(all_opts, "readonly", "read-only");
> >  
> >      value = qemu_opt_get(all_opts, "cache");
> > @@ -1285,7 +1288,9 @@ void qmp_block_set_io_throttle(const char *device, 
> > int64_t bps, int64_t bps_rd,
> >                                 bool has_iops_rd_max,
> >                                 int64_t iops_rd_max,
> >                                 bool has_iops_wr_max,
> > -                               int64_t iops_wr_max, Error **errp)
> > +                               int64_t iops_wr_max,
> > +                               bool has_iops_sector_count,
> > +                               int64_t iops_sector_count, Error **errp)
> >  {
> >      ThrottleConfig cfg;
> >      BlockDriverState *bs;
> > @@ -1325,7 +1330,10 @@ void qmp_block_set_io_throttle(const char *device, 
> > int64_t bps, int64_t bps_rd,
> >      }
> >  
> >      cfg.unit_size = BDRV_SECTOR_SIZE;
> > -    cfg.op_size = 0;
> > +
> > +    if (has_iops_sector_count) {
> > +        cfg.op_size = iops_sector_count;
> > +    }
> >  
> >      if (!check_throttle_config(&cfg, errp)) {
> >          return;
> > @@ -2051,6 +2059,10 @@ QemuOptsList qemu_common_drive_opts = {
> >              .type = QEMU_OPT_NUMBER,
> >              .help = "total bytes write burst",
> >          },{
> > +            .name = "throttling.iops-sector-count",
> > +            .type = QEMU_OPT_NUMBER,
> > +            .help = "when limiting by iops max size of an I/O in sector",
> > +        },{
> >              .name = "copy-on-read",
> >              .type = QEMU_OPT_BOOL,
> >              .help = "copy read data from backing file into image file",
> > @@ -2197,6 +2209,10 @@ QemuOptsList qemu_old_drive_opts = {
> >              .type = QEMU_OPT_NUMBER,
> >              .help = "total bytes write burst",
> >          },{
> > +            .name = "iops_sector_count",
> > +            .type = QEMU_OPT_NUMBER,
> > +            .help = "when limiting by iops max size of an I/O in sector",
> > +        },{
> >              .name = "copy-on-read",
> >              .type = QEMU_OPT_BOOL,
> >              .help = "copy read data from backing file into image file",
> > diff --git a/hmp.c b/hmp.c
> > index e0c387c..01f7685 100644
> > --- a/hmp.c
> > +++ b/hmp.c
> > @@ -351,7 +351,8 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
> >                              " iops_wr=%" PRId64
> >                              " iops_max=%" PRId64
> >                              " iops_rd_max=%" PRId64
> > -                            " iops_wr_max=%" PRId64 "\n",
> > +                            " iops_wr_max=%" PRId64
> > +                            " iops_sector_count=%" PRId64 "\n",
> >                              info->value->inserted->bps,
> >                              info->value->inserted->bps_rd,
> >                              info->value->inserted->bps_wr,
> > @@ -363,7 +364,8 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
> >                              info->value->inserted->iops_wr,
> >                              info->value->inserted->iops_max,
> >                              info->value->inserted->iops_rd_max,
> > -                            info->value->inserted->iops_wr_max);
> > +                            info->value->inserted->iops_wr_max,
> > +                            info->value->inserted->iops_sector_count);
> >          } else {
> >              monitor_printf(mon, " [not inserted]");
> >          }
> > @@ -1124,6 +1126,8 @@ void hmp_block_set_io_throttle(Monitor *mon, const 
> > QDict *qdict)
> >                                false,
> >                                0,
> >                                false,
> > +                              0,
> > +                              false, /* No default I/O size */
> >                                0, &err);
> >      hmp_handle_error(mon, &err);
> >  }
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 5e5461e..c7b1d5e 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -797,6 +797,8 @@
> >  #
> >  # @iops_wr_max: #optional write I/O operations max (Since 1.7)
> >  #
> > +# @iops_sector_count: #optional an I/O size in sector (Since 1.6)
> > +#
> >  # Since: 0.14.0
> >  #
> >  # Notes: This interface is only found in @BlockInfo.
> > @@ -810,7 +812,8 @@
> >              'image': 'ImageInfo',
> >              '*bps_max': 'int', '*bps_rd_max': 'int',
> >              '*bps_wr_max': 'int', '*iops_max': 'int',
> > -            '*iops_rd_max': 'int', '*iops_wr_max': 'int' }}
> > +            '*iops_rd_max': 'int', '*iops_wr_max': 'int',
> > +            '*iops_sector_count': 'int' } }
> >  
> >  ##
> >  # @BlockDeviceIoStatus:
> > @@ -2201,6 +2204,8 @@
> >  #
> >  # @iops_wr_max: #optional write I/O operations max (Since 1.7)
> >  #
> > +# @iops_sector_count: #optional an I/O size in sector (Since 1.6)
> > +#
> >  # Returns: Nothing on success
> >  #          If @device is not a valid block device, DeviceNotFound
> >  #
> > @@ -2211,7 +2216,8 @@
> >              'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
> >              '*bps_max': 'int', '*bps_rd_max': 'int',
> >              '*bps_wr_max': 'int', '*iops_max': 'int',
> > -            '*iops_rd_max': 'int', '*iops_wr_max': 'int' }}
> > +            '*iops_rd_max': 'int', '*iops_wr_max': 'int',
> > +            '*iops_sector_count': 'int' }}
> >  
> >  ##
> >  # @block-stream:
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 3aa8f9e..c539dd0 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -411,7 +411,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
> >      "       [,readonly=on|off][,copy-on-read=on|off]\n"
> >      "       [[,bps=b]|[[,bps_rd=r][,bps_wr=w]]][[,iops=i]|[[,iops_rd=r]"
> >      "        [,iops_wr=w][,bps_max=bt]|[[,bps_rd_max=rt][,bps_wr_max=wt]]]"
> > -    "       [[,iops_max=it]|[[,iops_rd_max=rt][,iops_wr_max=wt]]\n"
> > +    "       
> > [[,iops_max=it]|[[,iops_rd_max=rt][,iops_wr_max=wt][,iops_sector_count=cnt]]\n"
> >      "                use 'file' as a drive image\n", QEMU_ARCH_ALL)
> >  STEXI
> >  @item -drive @var{option}[,@var{option}[,@var{option}[,...]]]
> > diff --git a/qmp-commands.hx b/qmp-commands.hx
> > index 1610831..bcd255f 100644
> > --- a/qmp-commands.hx
> > +++ b/qmp-commands.hx
> > @@ -1389,7 +1389,7 @@ EQMP
> >  
> >      {
> >          .name       = "block_set_io_throttle",
> > -        .args_type  = 
> > "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l,bps_max:l?,bps_rd_max:l?,bps_wr_max:l?,iops_max:l?,iops_rd_max:l?,iops_wr_max:l?",
> > +        .args_type  = 
> > "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l,bps_max:l?,bps_rd_max:l?,bps_wr_max:l?,iops_max:l?,iops_rd_max:l?,iops_wr_max:l?,iops_sector_count:l?",
> >          .mhandler.cmd_new = qmp_marshal_input_block_set_io_throttle,
> >      },
> >  
> > @@ -1414,6 +1414,7 @@ Arguments:
> >  - "iops_max":  total I/O operations max(json-int)
> >  - "iops_rd_max":  read I/O operations max(json-int)
> >  - "iops_wr_max":  write I/O operations max(json-int)
> > +- "iops_sector_count":  I/O sector count when limiting(json-int)
> >  
> >  Example:
> >  
> > @@ -1429,7 +1430,8 @@ Example:
> >                                                 "bps_wr_max": "0",
> >                                                 "iops_max": "0",
> >                                                 "iops_rd_max": "0",
> > -                                               "iops_wr_max": "0" } }
> > +                                               "iops_wr_max": "0",
> > +                                               "iops_sector_count": "0" } }
> >  <- { "return": {} }
> >  
> >  EQMP
> > @@ -1776,6 +1778,7 @@ Each json-object contain the following:
> >           - "iops_max":  total I/O operations max(json-int)
> >           - "iops_rd_max":  read I/O operations max(json-int)
> >           - "iops_wr_max":  write I/O operations max(json-int)
> > +         - "iops_sector_count":  I/O sector count when limiting(json-int)
> >           - "image": the detail of the image, it is a json-object containing
> >              the following:
> >               - "filename": image file name (json-string)
> > @@ -1851,6 +1854,7 @@ Example:
> >                 "iops_max": "0",
> >                 "iops_rd_max": "0",
> >                 "iops_wr_max": "0",
> > +               "iops_sector_count": "0",
> >                 "image":{
> >                    "filename":"disks/test.qcow2",
> >                    "format":"qcow2",
> > -- 
> > 1.7.10.4
> > 
> > 
>