Re: [PATCH v7 02/47] block: Add chain helper functions

[Max Reitz]

On 08.07.20 19:20, Andrey Shinkevich wrote:
> On 25.06.2020 18:21, Max Reitz wrote:
>> Add some helper functions for skipping filters in a chain of block
>> nodes.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   include/block/block_int.h |  3 +++
>>   block.c                   | 55 +++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 58 insertions(+)
>>
>> diff --git a/include/block/block_int.h b/include/block/block_int.h
>> index bb3457c5e8..5da793bfc3 100644
>> --- a/include/block/block_int.h
>> +++ b/include/block/block_int.h
>> @@ -1382,6 +1382,9 @@ BdrvChild *bdrv_cow_child(BlockDriverState *bs);
>>   BdrvChild *bdrv_filter_child(BlockDriverState *bs);
>>   BdrvChild *bdrv_filter_or_cow_child(BlockDriverState *bs);
>>   BdrvChild *bdrv_primary_child(BlockDriverState *bs);
>> +BlockDriverState *bdrv_skip_implicit_filters(BlockDriverState *bs);
>> +BlockDriverState *bdrv_skip_filters(BlockDriverState *bs);
>> +BlockDriverState *bdrv_backing_chain_next(BlockDriverState *bs);
>>     static inline BlockDriverState *child_bs(BdrvChild *child)
>>   {
>> diff --git a/block.c b/block.c
>> index 5a42ef49fd..0a0b855261 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -7008,3 +7008,58 @@ BdrvChild *bdrv_primary_child(BlockDriverState
>> *bs)
>>         return NULL;
>>   }
>> +
>> +static BlockDriverState *bdrv_do_skip_filters(BlockDriverState *bs,
>> +                                              bool
>> stop_on_explicit_filter)
>> +{
>> +    BdrvChild *c;
>> +
>> +    if (!bs) {
>> +        return NULL;
>> +    }
>> +
>> +    while (!(stop_on_explicit_filter && !bs->implicit)) {
>> +        c = bdrv_filter_child(bs);
>> +        if (!c) {
>> +            break;
>> +        }
>> +        bs = c->bs;
> 
> Could it be child_bs(bs) ?

Well, in a sense, but not really.  We need to check whether there is a
child before overwriting @bs (because @bs must stay a non-NULL pointer),
so we wouldn’t have fewer lines of code if we replaced “BdrvChild *c” by
“BlockDriverState *child_bs”, and then used bdrv_child() to set child_bs.

(And because we have to check whether @c is NULL anyway, there is no
real reason to use child_bs(c) instead of c->bs afterwards.)

>> +    }
> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>

Thanks a lot for reviewing!

signature.asc
Description: OpenPGP digital signature