Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change

[Supriya Kannery]

On 07/27/2011 09:32 PM, Anthony Liguori wrote:
> On 07/27/2011 09:31 AM, Stefan Hajnoczi wrote:
> > On Wed, Jul 27, 2011 at 1:58 PM, Anthony
> > Liguori<address@hidden> wrote:
> > > > Index: qemu/hmp-commands.hx
> > > > ===================================================================
> > > > --- qemu.orig/hmp-commands.hx
> > > > +++ qemu/hmp-commands.hx
> > > > @@ -70,6 +70,20 @@ but should be used with extreme caution.
> > > > resizes image files, it can not resize block devices like LVM volumes.
> > > > ETEXI
> > > >
> > > > + {
> > > > + .name = "block_set",
> > > > + .args_type = "device:B,device:O",
> > > > + .params = "device [prop=value][,...]",
> > > > + .help = "Change block device parameters
> > > > [hostcache=on/off]",
> > > > + .user_print = monitor_user_noop,
> > > > + .mhandler.cmd_new = do_block_set,
> > > > + },
> > > > +STEXI
> > > > address@hidden block_set @var{config}
> > > > address@hidden block_set
> > > > +Change block device parameters (eg: hostcache=on/off) while guest is
> > > > running.
> > > > +ETEXI
> > > > +
> > >
> > > block_set_hostcache() please.
> > >
> > > Multiplexing commands is generally a bad idea. It weakens typing. In 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.
>
> Think about it from a client perspective. How do I determine which
> properties are supported by this version of QEMU? I have no way to
> identify programmatically what arguments are valid for block_set.

   User can programmatically find out valid parameters for
block_set. Internally, validation of prop=value is done against -drive
parameter list and then, only the valid/implemented commands (for now, 
hostcache) are accepted from that list. Please find execution output 
attached:
========================================================================
(qemu) info block
ide0-hd0: removable=0 hostcache=1 file=../rhel6-32.qcow2 ro=0 drv=qcow2 
encrypted=0
floppy0: removable=1 locked=0 hostcache=0 file=test.img ro=0 drv=raw 
encrypted=0
ide1-cd0: removable=1 locked=0 hostcache=1 [not inserted]
sd0: removable=1 locked=0 hostcache=1 [not inserted]
(qemu) block
block_resize  block_set     block_passwd
(qemu) block_set
block_set: block device name expected
(qemu) block
block_resize  block_set     block_passwd
(qemu) help block_set
block_set device [prop=value][,...] -- Change block device parameters 
[hostcache=on/off]
(qemu) block_set ide
Device 'ide' not found
(qemu) block_set ide0-hd0
Usage: block_set device [prop=value][,...]
(qemu) block_set ide0-hd0 cache
Invalid parameter 'cache'
(qemu) block_set ide0-hd0 cache=on
Parameter 'hostcache' expects on/off
(qemu) block_set ide0-hd0 hostcache=on
(qemu) block_set ide0-hd0 hostcache=off
(qemu) info block
ide0-hd0: removable=0 hostcache=0 file=../rhel6-32.qcow2 ro=0 drv=qcow2 
encrypted=0
floppy0: removable=1 locked=0 hostcache=0 file=test.img ro=0 drv=raw 
encrypted=0
ide1-cd0: removable=1 locked=0 hostcache=1 [not inserted]
sd0: removable=1 locked=0 hostcache=1 [not inserted]
========================================================================

 When we add further parameters, "usage" string can be enhanced to
include those parameters for informing user.

- Thanks, Supriya

> OTOH, if you have strong types like block_set_hostcache, query-commands
> tells me exactly what's supported.
>
> Regards,
>
> Anthony Liguori
>
> > Stefan