Am 09.09.2019 um 09:56 hat Max Reitz geschrieben:
> 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 <mreitz@redhat.com>
> >> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> > 
> > 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
> child.

Hm, okay, makes sense. I had a definition in mind that says that filter
nodes only have a single child node. Is it that a filter may have only a
single _filtered_ child node?

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

But there is no reason why a node couldn't fulfill this condition for
more than one child node. bdrv_filtered_child() isn't well-defined then.
Technically, the description "Return any filtered child" is correct
because "any" can be interpreted as "an arbitrary", but obviously that
makes the function useless.

Specficially, according to your definition, qcow2 filters both the
backing file (COW filter) and the external data file (R/W filter).

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

We generally call this concept a "backing chain".

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

Agreed with offset, but with only size, it matches your definition of 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.

qcow2 doesn't fulfill the conditions for begin a filter driver. Two of
its possible children fulfill the conditions for being a filtered child.
You can pick either approach, talking about a "filter chain" just
doesn't make sense there. Either the chain is broken by a non-filter
driver like qcow2, or it must become a filter tree.

What we're really interested in is iterating the backing chain even
across filter nodes, so your implementation achieves the right result.
It just feels completely arbitrary, counterintuitive and confusing to
call this a (or actually "the") "filter chain" and to pretend that the
name tells anyone what it really is.

Kevin