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

[Anthony Liguori <anthony@codemonkey.ws>]

On 07/28/2011 05:13 AM, Supriya Kannery wrote:
> 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<anthony@codemonkey.ws> 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
>>>>> +@item block_set @var{config}
>>>>> +@findex 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

That's HMP btw, not QMP.

> 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]

Parsing help text is not introspection!

Regards,

Anthony Liguori

> (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
>>>
>>
>>
>
>