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

[Kevin Wolf]

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),
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?

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.

> +/*
> + * 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).

Kevin