Re: [Qemu-devel] [PATCH 13/18] blockdev: Rename I/O throttling options for QMP

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2440 bytes --]

On 07/23/2013 07:03 AM, Kevin Wolf wrote:
> In QMP, we want to use dashes instead of underscores in QMP argument
> names, and use nested options for throttling. Convert them for -drive
> before calling into the code that will be shared between -drive and
> blockdev-add.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  blockdev.c | 52 +++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 39 insertions(+), 13 deletions(-)
> 

This patch will probably conflict with Benoît's work on leaky bucket
throttling; can the two of you decide which one should go in first?  Are
we trying to target both this series and leaky bucket throttling for 1.6?

> @@ -485,17 +486,17 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
>  
>      /* disk I/O throttling */
>      io_limits.bps[BLOCK_IO_LIMIT_TOTAL]  =
> -                           qemu_opt_get_number(opts, "bps", 0);
> +        qemu_opt_get_number(opts, "throttling.bps-total", 0);

I like the rename to bps-total, to make it more obvious why it is
provided in addition to bps-{read,write}.

>      io_limits.bps[BLOCK_IO_LIMIT_READ]   =
> -                           qemu_opt_get_number(opts, "bps_rd", 0);
> +        qemu_opt_get_number(opts, "throttling.bps-read", 0);
>      io_limits.bps[BLOCK_IO_LIMIT_WRITE]  =
> -                           qemu_opt_get_number(opts, "bps_wr", 0);
> +        qemu_opt_get_number(opts, "throttling.bps-write", 0);

Thank you for spelling out the names; QMP doesn't need the abbreviations
that the command line has.

> +DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
> +{
> +    /* Change legacy command line options into QMP ones */
> +    qemu_opt_rename(all_opts, "iops", "throttling.iops-total");
> +    qemu_opt_rename(all_opts, "iops_rd", "throttling.iops-read");
> +    qemu_opt_rename(all_opts, "iops_wr", "throttling.iops-write");

Of course, the intent here is to preserve back-compat (the old spelling
continues to work, even if it differs from the QMP spelling).  But will
this patch also allow the command line to learn support for the new
spellings?  If so, is that worth mentioning as an intentional side
effect in the commit message?

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]