On 2020-07-13 at 11:06 CEST, Vladimir Sementsov-Ogievskiy wrote...
> 25.06.2020 18:21, Max Reitz wrote:
>> 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 a filtered child; bs->file can be a
>> 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 functions 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 in a meaningful way.
>>
>> Signed-off-by: Max Reitz <mre...@redhat.com>
>> ---
>
> [..]
>
>> +/*
>> + * Return the primary child of this node: For filters, that is the
>> + * filtered child. For other nodes, that is usually the child storing
>> + * metadata.
>> + * (A generally more helpful description is that this is (usually) the
>> + * child that has the same filename as @bs.)
>> + *
>> + * Drivers do not necessarily have a primary child; for example quorum
>> + * does not.
>> + */
>> +BdrvChild *bdrv_primary_child(BlockDriverState *bs)
>> +{
>> + BdrvChild *c;
>> +
>> + QLIST_FOREACH(c, &bs->children, next) {
>> + if (c->role & BDRV_CHILD_PRIMARY) {
>> + return c;
>> + }
>> + }
>> +
>> + return NULL;
>> +}
>>
>
> Suggest squash-in to also assert that not more than one primary child:
> --- a/block.c
> +++ b/block.c
> @@ -6998,13 +6998,14 @@ BdrvChild *bdrv_filter_or_cow_child(BlockDriverState
> *bs)
> */
> BdrvChild *bdrv_primary_child(BlockDriverState *bs)
> {
> - BdrvChild *c;
> + BdrvChild *c, *found = NULL;
>
> QLIST_FOREACH(c, &bs->children, next) {
> if (c->role & BDRV_CHILD_PRIMARY) {
> - return c;
> + assert(!found);
> + found = c;
> }
> }
>
> - return NULL;
> + return c;
Shouldn't that be "return found"?
> }
>
>
> with or without:
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
--
Cheers,
Christophe de Dinechin (IRC c3d)
