[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v6 19/25] block: Add BlockDriver.bdrv_gather_chi
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH v6 19/25] block: Add BlockDriver.bdrv_gather_child_options |
Date: |
Wed, 8 Nov 2017 19:52:35 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 |
On 2017-11-08 18:18, Kevin Wolf wrote:
> Am 29.09.2017 um 18:53 hat Max Reitz geschrieben:
>> Some follow-up patches will rework the way bs->full_open_options is
>> refreshed in bdrv_refresh_filename(). The new implementation will remove
>> the need for the block drivers' bdrv_refresh_filename() implementations
>> to set bs->full_open_options; instead, it will be generic and use static
>> information from each block driver.
>>
>> However, by implementing bdrv_gather_child_options(), block drivers will
>> still be able to override the way the full_open_options of their
>> children are incorporated into their own.
>>
>> We need to implement this function for VMDK because we have to prevent
>> the generic implementation from gathering the options of all children:
>> It is not possible to specify options for the extents through the
>> runtime options.
>
> Sounds more like a bug than a feature.
Want to fix it? :-)
>> For quorum, the child names that would be used by the generic
>> implementation and the ones that we actually want to use differ. See
>> quorum_gather_child_options() for more information.
>
> :-/
>
> What was the conclusion of our discussion at KVM Forum again? I just
> remember that this caused problems in other contexts like dynamic
> reconfiguration, too.
I might have just winced a little.
The short conclusion was that it's all broken.
The long conclusion... Is that I might try to wrestle with quorum
before this series? :-/
Let's see... The conclusion was that the user needs to specify a unique
name for each quorum child. Then, this name would be equivalent to the
runtime name. Sounds good so far? Well, I might have just screamed a
little when I remembered another issue:
The generic implementation gathers the options like so:
{ /* node options */
"child0": { /* child0 options */ },
"child1": { /* child1 options */ },
... }
However, specifying child options like so doesn't work easily with QAPI
and quorum, for two reasons:
(1) Specifying arbitrary keys with specific values would need new QAPI
infrastructure. Who is going to implement this? :-)
(2) Quorum needs a fixed order for its children, at least for FIFO mode.
A JSON object does not have inherent ordering.
The simple way to fix this is to make the children list a list of
objects which contain the child name and the options:
{ /* quorum node options */
"children": [
{ "child-name": "child0",
"options": { /* child0 options */ } },
{ "child-name": "child1",
"options": { /* child1 options */ } },
... ] }
But this would still require bdrv_gather_child_options() to generate.
The other way would be to add the QAPI infrastructure to make the
generic thing work, and add a "children-order" list or something which
gives order to the children. Then the generic implementation should
work... As long as the quorum driver makes sure to update this list
whenever a child is added or removed.
Maybe the latter is actually the better choice? :-/
But it definitely means more work (because of the QAPI modifications)...
Max
>> Signed-off-by: Max Reitz <address@hidden>
>> ---
>> include/block/block_int.h | 13 +++++++++++++
>> block/quorum.c | 30 ++++++++++++++++++++++++++++++
>> block/vmdk.c | 13 +++++++++++++
>> 3 files changed, 56 insertions(+)
>
> Kevin
>
signature.asc
Description: OpenPGP digital signature