qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change
Date: Mon, 1 Aug 2011 16:44:55 +0100

On Mon, Aug 1, 2011 at 4:44 PM, Kevin Wolf <address@hidden> 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<address@hidden>  wrote:
>>>> On Wed, Jul 27, 2011 at 5:02 PM, Anthony Liguori<address@hidden>  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.
>>>>>
>>>>> OTOH, if you have strong types like block_set_hostcache, query-commands
>>>>> tells me exactly what's supported.
>>>>
>>>> Use query-block and see if 'hostcache' is there.  If yes, then the
>>>> hostcache parameter is available.  If 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.  The choices are:
>>>
>>> 1. Top-level block_set command.  Supported parameters are discovered
>>> by looking query-block output.
>>
>> I'm strongly opposed to this.  There needs to be a single consistent way
>> 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.  If
>>> 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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]