From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45222) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V2krE-00043H-Ho for qemu-devel@nongnu.org; Fri, 26 Jul 2013 12:27:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V2kr9-0007M6-Iw for qemu-devel@nongnu.org; Fri, 26 Jul 2013 12:27:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:30646) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V2kr9-0007M1-CH for qemu-devel@nongnu.org; Fri, 26 Jul 2013 12:27:03 -0400 Date: Fri, 26 Jul 2013 18:26:55 +0200 From: Kevin Wolf Message-ID: <20130726162655.GC18446@dhcp-200-207.str.redhat.com> References: <1374584606-5615-1-git-send-email-kwolf@redhat.com> <1374584606-5615-14-git-send-email-kwolf@redhat.com> <51F29F77.608@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <51F29F77.608@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 13/18] blockdev: Rename I/O throttling options for QMP List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: armbru@redhat.com, =?iso-8859-1?Q?Beno=EEt?= Canet , qemu-devel@nongnu.org, stefanha@redhat.com, lcapitulino@redhat.com 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. > >=20 > > Signed-off-by: Kevin Wolf > > --- > > blockdev.c | 52 +++++++++++++++++++++++++++++++++++++++------------- > > 1 file changed, 39 insertions(+), 13 deletions(-) > >=20 >=20 > This patch will probably conflict with Beno=EEt's work on leaky bucket > throttling; can the two of you decide which one should go in first? Ar= e > 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=EEt's series is for 1.7 as well, if I understood Stefan correctly. H= e 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, Block= InterfaceType block_default_type) > > =20 > > /* disk I/O throttling */ > > io_limits.bps[BLOCK_IO_LIMIT_TOTAL] =3D > > - qemu_opt_get_number(opts, "bps", 0); > > + qemu_opt_get_number(opts, "throttling.bps-total", 0); >=20 > I like the rename to bps-total, to make it more obvious why it is > provided in addition to bps-{read,write}. >=20 > > io_limits.bps[BLOCK_IO_LIMIT_READ] =3D > > - qemu_opt_get_number(opts, "bps_rd", 0); > > + qemu_opt_get_number(opts, "throttling.bps-read", 0); > > io_limits.bps[BLOCK_IO_LIMIT_WRITE] =3D > > - qemu_opt_get_number(opts, "bps_wr", 0); > > + qemu_opt_get_number(opts, "throttling.bps-write", 0); >=20 > Thank you for spelling out the names; QMP doesn't need the abbreviation= s > that the command line has. >=20 > > +DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_d= efault_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"); >=20 > 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