[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 13/18] blockdev: Rename I/O throttling options f
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 13/18] blockdev: Rename I/O throttling options for QMP |
Date: |
Fri, 26 Jul 2013 18:26:55 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 26.07.2013 um 18:10 hat Eric Blake geschrieben:
> 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 <address@hidden>
> > ---
> > 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?
If you complete the review before I leave today, I might still send a
pull request, but as I'm going to disable blockdev-add and the new
options once again for 1.6, it doesn't really matter that much.
Benoît's series is for 1.7 as well, if I understood Stefan correctly. He
said he was going to merge a bug fix part of it for 1.6 and leave the
rest for 1.7. (I haven't been following the throttling series myself,
that's why I can't comment in much more detail.)
> > @@ -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?
Yes, of course the command line will accept the new spellings as well.
I'll update the commit message.
Kevin
- [Qemu-devel] [PATCH 09/18] qapi.py: Maintain a list of union types, (continued)
- [Qemu-devel] [PATCH 09/18] qapi.py: Maintain a list of union types, Kevin Wolf, 2013/07/23
- [Qemu-devel] [PATCH 08/18] qapi: Add consume argument to qmp_input_get_object(), Kevin Wolf, 2013/07/23
- [Qemu-devel] [PATCH 10/18] qapi: Anonymous unions, Kevin Wolf, 2013/07/23
- [Qemu-devel] [PATCH 11/18] block: Allow "driver" option on the top level, Kevin Wolf, 2013/07/23
- [Qemu-devel] [PATCH 13/18] blockdev: Rename I/O throttling options for QMP, Kevin Wolf, 2013/07/23
- Re: [Qemu-devel] [PATCH 13/18] blockdev: Rename I/O throttling options for QMP, Benoît Canet, 2013/07/26
- Re: [Qemu-devel] [PATCH 13/18] blockdev: Rename I/O throttling options for QMP, Eric Blake, 2013/07/26
[Qemu-devel] [PATCH 12/18] QemuOpts: Add qemu_opt_unset(), Kevin Wolf, 2013/07/23
[Qemu-devel] [PATCH 15/18] blockdev: Rename 'readonly' option to 'read-only', Kevin Wolf, 2013/07/23
[Qemu-devel] [PATCH 14/18] qcow2: Use dashes instead of underscores in options, Kevin Wolf, 2013/07/23