[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
bs->full_open_options).
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).
Max
signature.asc
Description: OpenPGP digital signature