From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:46891) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QnuVp-0000d2-Cp for qemu-devel@nongnu.org; Mon, 01 Aug 2011 11:34:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QnuVo-0005MJ-C7 for qemu-devel@nongnu.org; Mon, 01 Aug 2011 11:34:37 -0400 Received: from mail-yw0-f45.google.com ([209.85.213.45]:59631) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QnuVo-0005MD-4g for qemu-devel@nongnu.org; Mon, 01 Aug 2011 11:34:36 -0400 Received: by ywb3 with SMTP id 3so917694ywb.4 for ; Mon, 01 Aug 2011 08:34:35 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <4E36C608.2030107@codemonkey.ws> References: <20110727113000.25109.16204.sendpatchset@skannery> <20110727113045.25109.54866.sendpatchset@skannery> <4E300B8E.2020509@codemonkey.ws> <4E3036AA.8030604@codemonkey.ws> <4E36C608.2030107@codemonkey.ws> Date: Mon, 1 Aug 2011 16:34:35 +0100 Message-ID: From: Stefan Hajnoczi Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Kevin Wolf , Supriya Kannery , qemu-devel@nongnu.org, Christoph Hellwig On Mon, Aug 1, 2011 at 4:28 PM, Anthony Liguori wro= te: > On 08/01/2011 10:22 AM, Stefan Hajnoczi wrote: >> >> On Thu, Jul 28, 2011 at 10:29 AM, Stefan Hajnoczi >> =A0wrote: >>> >>> On Wed, Jul 27, 2011 at 5:02 PM, Anthony Liguori >>> =A0wrote: >>>> >>>> On 07/27/2011 09:31 AM, Stefan Hajnoczi wrote: >>>>> >>>>> On Wed, Jul 27, 2011 at 1:58 PM, Anthony Liguori >>>>> =A0wrote: >>>>>>> >>>>>>> Index: qemu/hmp-commands.hx >>>>>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >>>>>>> --- qemu.orig/hmp-commands.hx >>>>>>> +++ qemu/hmp-commands.hx >>>>>>> @@ -70,6 +70,20 @@ but should be used with extreme caution. >>>>>>> =A0resizes image files, it can not resize block devices like LVM >>>>>>> volumes. >>>>>>> =A0ETEXI >>>>>>> >>>>>>> + =A0 =A0{ >>>>>>> + =A0 =A0 =A0 =A0.name =A0 =A0 =A0 =3D "block_set", >>>>>>> + =A0 =A0 =A0 =A0.args_type =A0=3D "device:B,device:O", >>>>>>> + =A0 =A0 =A0 =A0.params =A0 =A0 =3D "device [prop=3Dvalue][,...]", >>>>>>> + =A0 =A0 =A0 =A0.help =A0 =A0 =A0 =3D "Change block device paramet= ers >>>>>>> [hostcache=3Don/off]", >>>>>>> + =A0 =A0 =A0 =A0.user_print =3D monitor_user_noop, >>>>>>> + =A0 =A0 =A0 =A0.mhandler.cmd_new =3D do_block_set, >>>>>>> + =A0 =A0}, >>>>>>> +STEXI >>>>>>> +@item block_set @var{config} >>>>>>> +@findex block_set >>>>>>> +Change block device parameters (eg: hostcache=3Don/off) while gues= t is >>>>>>> running. >>>>>>> +ETEXI >>>>>>> + >>>>>> >>>>>> block_set_hostcache() please. >>>>>> >>>>>> Multiplexing commands is generally a bad idea. =A0It weakens typing.= =A0In >>>>>> the >>>>>> absence of a generic way to set block device properties, implementin= g >>>>>> properties as generic in the QMP layer seems like a bad idea to me. >>>>> >>>>> The idea behind block_set was to have a unified interface for changin= g >>>>> block device parameters at runtime. =A0This prevents us from reinvent= ing >>>>> new commands from scratch. =A0For example, block I/O throttling is >>>>> already queued up to add run-time parameters. >>>>> >>>>> Without a unified command we have a bulkier QMP/HMP interface, >>>>> duplicated code, and possibly inconsistencies in syntax between the >>>>> commands. =A0Isn't the best way to avoid these problems a unified >>>>> interface? >>>>> >>>>> I understand the lack of type safety concern but in this case we >>>>> already have to manually pull parsed arguments (i.e. cast to specific >>>>> types and deal with invalid input). =A0To me this is a reason *for* >>>>> using a unified interface like block_set. >>>> >>>> Think about it from a client perspective. =A0How do I determine which >>>> properties are supported by this version of QEMU? =A0I have no way to >>>> identify >>>> programmatically what arguments are valid for block_set. >>>> >>>> OTOH, if you have strong types like block_set_hostcache, query-command= s >>>> tells me exactly what's supported. >>> >>> Use query-block and see if 'hostcache' is there. =A0If yes, then the >>> hostcache parameter is available. =A0If we allow BlockDrivers to have >>> their own runtime parameters then query-commands does not tell you >>> anything because the specific BlockDriver may or may not support that >>> runtime parameter - you need to use query-block. >> >> Let's reach agreement here. =A0The choices are: >> >> 1. Top-level block_set command. =A0Supported parameters are discovered >> by looking query-block output. > > I'm strongly opposed to this. =A0There needs to be a single consistent wa= y to > determine supported operations with QMP. > > And that single mechanism already exists--query_commands. Okay, works for me. Supriya: this means that there should be a block_set_hostcache command and you don't need to worry about a generic block_set command. Stefan