From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:41068) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qnufp-0007vD-GE for qemu-devel@nongnu.org; Mon, 01 Aug 2011 11:44:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qnufo-0007TL-DS for qemu-devel@nongnu.org; Mon, 01 Aug 2011 11:44:57 -0400 Received: from mail-yi0-f45.google.com ([209.85.218.45]:56415) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qnufo-0007TA-4m for qemu-devel@nongnu.org; Mon, 01 Aug 2011 11:44:56 -0400 Received: by yia25 with SMTP id 25so4209005yia.4 for ; Mon, 01 Aug 2011 08:44:55 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <4E36C9D3.4020803@redhat.com> References: <20110727113000.25109.16204.sendpatchset@skannery> <20110727113045.25109.54866.sendpatchset@skannery> <4E300B8E.2020509@codemonkey.ws> <4E3036AA.8030604@codemonkey.ws> <4E36C608.2030107@codemonkey.ws> <4E36C9D3.4020803@redhat.com> Date: Mon, 1 Aug 2011 16:44:55 +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: Kevin Wolf Cc: Supriya Kannery , qemu-devel@nongnu.org, Christoph Hellwig On Mon, Aug 1, 2011 at 4:44 PM, Kevin Wolf wrote: > Am 01.08.2011 17:28, schrieb Anthony Liguori: >> 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 >>>>>> =A0 wrote: >>>>>>>> >>>>>>>> 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. >>>>>>>> =A0 resizes image files, it can not resize block devices like LVM = volumes. >>>>>>>> =A0 ETEXI >>>>>>>> >>>>>>>> + =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 parame= ters >>>>>>>> [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 gue= st 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, implementi= ng >>>>>>> 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 changi= ng >>>>>> block device parameters at runtime. =A0This prevents us from reinven= ting >>>>>> 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 specifi= c >>>>>> 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-comman= ds >>>>> 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 w= ay >> to determine supported operations with QMP. >> >> And that single mechanism already exists--query_commands. >> >>> 2. Top-level command for each parameter (e.g. block_set_hostcache). >>> Supported parameters are easily discoverable via query-commands. =A0If >>> individual block devices support different sets of parameters then >>> they may have to return -ENOTSUPP. >>> >>> I like the block_set approach. >>> >>> Anthony, Kevin, Supriya: Any thoughts? >> >> For the sake of overall QMP sanity, I think block_set_hostcache is >> really our only option. > > Ideally we should have blockdev_add, and blockdev_set would just take > the same arguments and update the given driver. > > But we don't have blockdev_add today, so whatever works for your as a > temporary solution... Anthony's point is that blockdev_set does not fit with QMP command discoverability. Stefan