[Top][All Lists]
[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: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change |
Date: |
Mon, 01 Aug 2011 17:44:19 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:5.0) Gecko/20110707 Thunderbird/5.0 |
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...
Kevin
- Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change, Stefan Hajnoczi, 2011/08/01
- Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change, Anthony Liguori, 2011/08/01
- Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change, Stefan Hajnoczi, 2011/08/01
- Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change,
Kevin Wolf <=
- Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change, Anthony Liguori, 2011/08/01
- Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change, Supriya Kannery, 2011/08/04
- Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change, Stefan Hajnoczi, 2011/08/04
- Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change, Supriya Kannery, 2011/08/04
- Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change, Supriya Kannery, 2011/08/04