[Top][All Lists]

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

Re: [Qemu-block] [PATCH v7 for-2.12 05/25] block: Respect backing bs in

From: Max Reitz
Subject: Re: [Qemu-block] [PATCH v7 for-2.12 05/25] block: Respect backing bs in bdrv_refresh_filename
Date: Fri, 8 Dec 2017 14:59:35 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0

On 2017-12-05 14:27, Alberto Garcia wrote:
> On Mon 20 Nov 2017 09:09:44 PM CET, Max Reitz wrote:
>> @@ -5016,6 +5016,7 @@ void bdrv_refresh_filename(BlockDriverState *bs)
>>          opts = qdict_new();
>>          has_open_options = append_open_options(opts, bs);
>> +        has_open_options |= bs->backing_overridden;
>>          /* If no specific options have been given for this BDS, the 
>> filename of
>>           * the underlying file should suffice for this one as well */
>> @@ -5027,11 +5028,20 @@ void bdrv_refresh_filename(BlockDriverState *bs)
>>           * file BDS. The full options QDict of that file BDS should somehow
>>           * contain a representation of the filename, therefore the following
>>           * suffices without querying the (exact_)filename of this BDS. */
>> -        if (bs->file->bs->full_open_options) {
>> +        if (bs->file->bs->full_open_options &&
>> +            (!bs->backing || bs->backing->bs->full_open_options))
>> +        {
> Does this mean that both file. and backing. have to be overriden?

The bs->file->bs->full_open_options is not a check whether anything has
been overridden but just whether we have a QDict of runtime options for
file (and we should always have one).  After this series, that check
therefore disappears.

> Shouldn't that be a || instead of a && ??

The block under this if condition automatically constructs
bs->full_open_options -- but it can only do so if all of the relevant
children's options are known (because their options need to be put into

Previously, that was only the file child.  This patch adds support for
the backing child: Whenever there is a backing child node, we need to
include its options under the "backing" key.

So that's where the condition comes from: For all children that this
node has, we need their full_open_options or we cannot construct this
node's full_open_options.

We do have bs->file (because that is the condition for the block this
condition is in), so we need bs->file->bs->full_open_options.  And if we
have bs->backing, we also need bs->backing->bs->full_open_options.

>>              qdict_put_str(opts, "driver", drv->format_name);
>>              QINCREF(bs->file->bs->full_open_options);
>>              qdict_put(opts, "file", bs->file->bs->full_open_options);
>> +            if (bs->backing) {
>> +                QINCREF(bs->backing->bs->full_open_options);
>> +                qdict_put(opts, "backing", 
>> bs->backing->bs->full_open_options);
>> +            } else if (bs->backing_overridden && !bs->backing) {
>> +                qdict_put(opts, "backing", qstring_new());
>> +            }
> You don't need the !bs->backing in the second if, it's implied from the
> previous one.

Maybe that was intentional (to be more explicit, and because after this
series only this branch remains), maybe it wasn't.  I don't know
anymore, so I'll just drop it (even though I'll have to re-add it later,
because as I said, only this else if branch will remain).


Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

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