From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:54547)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1Qm59R-00069r-9F
	for qemu-devel@nongnu.org; Wed, 27 Jul 2011 10:31:58 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1Qm59P-0006gT-62
	for qemu-devel@nongnu.org; Wed, 27 Jul 2011 10:31:57 -0400
Received: from mail-vw0-f45.google.com ([209.85.212.45]:54620)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1Qm59P-0006gL-3i
	for qemu-devel@nongnu.org; Wed, 27 Jul 2011 10:31:55 -0400
Received: by vws17 with SMTP id 17so1304820vws.4
	for <qemu-devel@nongnu.org>; Wed, 27 Jul 2011 07:31:54 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <4E300B8E.2020509@codemonkey.ws>
References: <20110727113000.25109.16204.sendpatchset@skannery>
	<20110727113045.25109.54866.sendpatchset@skannery>
	<4E300B8E.2020509@codemonkey.ws>
Date: Wed, 27 Jul 2011 15:31:54 +0100
Message-ID: <CAJSP0QVsa30NWu++QdEMmLpa0Qaabmr+EuRcZuF6Fuuj7Co1hA@mail.gmail.com>
From: Stefan Hajnoczi <stefanha@gmail.com>
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: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Anthony Liguori <anthony@codemonkey.ws>
Cc: Kevin Wolf <kwolf@redhat.com>, Supriya Kannery <supriyak@linux.vnet.ibm.com>, qemu-devel@nongnu.org, Christoph Hellwig <hch@lst.de>

On Wed, Jul 27, 2011 at 1:58 PM, Anthony Liguori <anthony@codemonkey.ws> wr=
ote:
>> 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 parameters
>> [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 guest is
>> running.
>> +ETEXI
>> +
>
> block_set_hostcache() please.
>
> Multiplexing commands is generally a bad idea. =A0It weakens typing. =A0I=
n the
> absence of a generic way to set block device properties, implementing
> 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 changing
block device parameters at runtime.  This prevents us from reinventing
new commands from scratch.  For 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.  Isn'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).  To me this is a reason *for*
using a unified interface like block_set.

Stefan