[Top][All Lists]

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

Re: [Qemu-block] [PATCH v6 04/42] block: Add child access functions

From: Max Reitz
Subject: Re: [Qemu-block] [PATCH v6 04/42] block: Add child access functions
Date: Mon, 9 Sep 2019 09:56:26 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0

On 04.09.19 18:16, Kevin Wolf wrote:
> Am 09.08.2019 um 18:13 hat Max Reitz geschrieben:
>> There are BDS children that the general block layer code can access,
>> namely bs->file and bs->backing.  Since the introduction of filters and
>> external data files, their meaning is not quite clear.  bs->backing can
>> be a COW source, or it can be an R/W-filtered child; bs->file can be an
>> R/W-filtered child, it can be data and metadata storage, or it can be
>> just metadata storage.
>> This overloading really is not helpful.  This patch adds function that
>> retrieve the correct child for each exact purpose.  Later patches in
>> this series will make use of them.  Doing so will allow us to handle
>> filter nodes and external data files in a meaningful way.
>> Signed-off-by: Max Reitz <address@hidden>
>> Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> Each time I look at this patch, I'm confused by the function names.
> Maybe I should just ask what the idea there was, or more specifically:
> What does the "filtered" in "filtered child" really mean?
> Apparently any child of a filter node is "filtered" (which makes sense),

It isn’t, filters can have non-filter children.  For example, backup-top
could have the source as a filtered child and the target as a non-filter

> but also bs->backing of a qcow2 image, while bs->file of qcow2 isn't.
> raw doesn't have any "filtered" child. What's the system behind this?

“filtered” means: If the parent node returns data from this child, it
won’t modify it, neither its content nor its position.  COW and R/W
filters differ in how they handle writes; R/W filters pass them through
to the filtered child, COW filters copy them off to some other child
node (and then the filtered child’s data will no longer be visible at
that location).

The main reason behind the common “filtered” name is for the generic
functions that work on both COW and true filter (R/W filters) chains.
We need such functionality sometimes.  I personally felt like the
concept of true (R/W) filters and COW children was similar enough to
share a common name base.

qcow2 has a COW child.  As such, it acts as a COW filter in the sense of
the function names.

raw has neither a COW child nor acts as an R/W filter.  As such, it has
no filtered child.  My opinion on this hasn’t changed.

(To reiterate, in practice I see no way anyone would ever use raw as an
R/W filter.
Either you use it without offset/size, in which case you simply use it
in lieu of a format node, so you precisely don’t want it to act as a
filter when it comes to allocation information and so on (even though it
can be classified a filter here).
Or you use it as kind of a filter with offset/size, but then it no
longer is a filter.

Filters are defined by “Every filter must fulfill these conditions: ...”
– not by “Everything that fulfills these conditions is a filter”.
Marking a driver as a filter has consequences, and I don’t see why we
would want those consequences for raw.)

> It looks like bdrv_filtered_child() is the right function to iterate
> along a backing file chain, but I just still fail to connect that and
> the name of the function in a meaningful way.

It‘s the right function to iterate along a filter chain.  This includes
COW backing children and R/W filtered children.

>> +/*
>> + * Return the child that @bs acts as an overlay for, and from which data 
>> may be
>> + * copied in COW or COR operations.  Usually this is the backing file.
>> + */
> Or NULL, if no such child exists.
> It's relatively obvious here, but for some of the functions further down
> it would be really good to describe in which cases NULL is expected (or
> that NULL is even a possible return value).

I’ll look into it.


Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

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