Re: [PATCH v7 01/47] block: Add child access functions

[Max Reitz]

On 13.07.20 11:06, 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 <mreitz@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);

Hm, why not.

> +            found = c;
>          }
>      }
>  
> -    return NULL;
> +    return c;
>  }
> 
> 
> with or without:
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

In any case, thanks for reviewing!

Max

signature.asc
Description: OpenPGP digital signature