[Top][All Lists]

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

Re: [Qemu-devel] [PATCH v6 19/25] block: Add BlockDriver.bdrv_gather_chi

From: Max Reitz
Subject: Re: [Qemu-devel] [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)...


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

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

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