Re: [Qemu-block] [PATCH v5 05/42] block: Add chain helper functions

2019-06-13 Thread Vladimir Sementsov-Ogievskiy
13.06.2019 15:33, Max Reitz wrote:
> On 13.06.19 14:26, Vladimir Sementsov-Ogievskiy wrote:
>> 13.06.2019 1:09, Max Reitz wrote:
>>> Add some helper functions for skipping filters in a chain of block
>>> nodes.
>>>
>>> Signed-off-by: Max Reitz 
>>> ---
>>>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 7ce71623f8..875a33f255 100644
>>> --- a/include/block/block_int.h
>>> +++ b/include/block/block_int.h
>>> @@ -1264,6 +1264,9 @@ BdrvChild *bdrv_filtered_child(BlockDriverState *bs);
>>>BdrvChild *bdrv_metadata_child(BlockDriverState *bs);
>>>BdrvChild *bdrv_storage_child(BlockDriverState *bs);
>>>BdrvChild *bdrv_primary_child(BlockDriverState *bs);
>>> +BlockDriverState *bdrv_skip_implicit_filters(BlockDriverState *bs);
>>> +BlockDriverState *bdrv_skip_rw_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 724d8889a6..be18130944 100644
>>> --- a/block.c
>>> +++ b/block.c
>>> @@ -6494,3 +6494,58 @@ BdrvChild *bdrv_primary_child(BlockDriverState *bs)
>>>{
>>>return bdrv_filtered_rw_child(bs) ?: bs->file;
>>>}
>>> +
>>> +static BlockDriverState *bdrv_skip_filters(BlockDriverState *bs,
>>> +   bool stop_on_explicit_filter)
>>> +{
>>> +BdrvChild *filtered;
>>> +
>>> +if (!bs) {
>>> +return NULL;
>>> +}
>>> +
>>> +while (!(stop_on_explicit_filter && !bs->implicit)) {
>>> +filtered = bdrv_filtered_rw_child(bs);
>>> +if (!filtered) {
>>> +break;
>>> +}
>>> +bs = filtered->bs;
>>> +}
>>> +/*
>>> + * Note that this treats nodes with bs->drv == NULL
>>
>> as well as filters without filtered_rw child
> 
> A filter always must have a filtered_rw child, though.  So I don’t quite
> understand what you mean here...
> 
> Max
> 
>>as not being
>>> + * R/W filters (bs->drv == NULL should be replaced by something
>>> + * else anyway).
>>> + * The advantage of this behavior is that this function will thus
>>> + * always return a non-NULL value (given a non-NULL @bs).
>>
>> and this is the advantage of what I've written, not about bs->drv.

I mean, that advantage seems unrelated to the reason about bs->drv == NULL,
as even with bs->drv == NULL we can go to bs->backing or bs->file..

But I don't  really care, my r-b stays here anyway

>>
>>> + */
>>> +
>>> +return bs;
>>> +}
>>> +
>>> +/*
>>> + * Return the first BDS that has not been added implicitly or that
>>> + * does not have an RW-filtered child down the chain starting from @bs
>>> + * (including @bs itself).
>>> + */
>>> +BlockDriverState *bdrv_skip_implicit_filters(BlockDriverState *bs)
>>> +{
>>> +return bdrv_skip_filters(bs, true);
>>> +}
>>> +
>>> +/*
>>> + * Return the first BDS that does not have an RW-filtered child down
>>> + * the chain starting from @bs (including @bs itself).
>>> + */
>>> +BlockDriverState *bdrv_skip_rw_filters(BlockDriverState *bs)
>>> +{
>>> +return bdrv_skip_filters(bs, false);
>>> +}
>>> +
>>> +/*
>>> + * For a backing chain, return the first non-filter backing image of
>>> + * the first non-filter image.
>>> + */
>>> +BlockDriverState *bdrv_backing_chain_next(BlockDriverState *bs)
>>> +{
>>> +return 
>>> bdrv_skip_rw_filters(bdrv_filtered_cow_bs(bdrv_skip_rw_filters(bs)));
>>> +}
>>>
>>
>>
>> Reviewed-by: Vladimir Sementsov-Ogievskiy 
>>
> 
> 


-- 
Best regards,
Vladimir


Re: [Qemu-block] [PATCH v5 05/42] block: Add chain helper functions

2019-06-13 Thread Max Reitz
On 13.06.19 14:26, Vladimir Sementsov-Ogievskiy wrote:
> 13.06.2019 1:09, Max Reitz wrote:
>> Add some helper functions for skipping filters in a chain of block
>> nodes.
>>
>> Signed-off-by: Max Reitz 
>> ---
>>   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 7ce71623f8..875a33f255 100644
>> --- a/include/block/block_int.h
>> +++ b/include/block/block_int.h
>> @@ -1264,6 +1264,9 @@ BdrvChild *bdrv_filtered_child(BlockDriverState *bs);
>>   BdrvChild *bdrv_metadata_child(BlockDriverState *bs);
>>   BdrvChild *bdrv_storage_child(BlockDriverState *bs);
>>   BdrvChild *bdrv_primary_child(BlockDriverState *bs);
>> +BlockDriverState *bdrv_skip_implicit_filters(BlockDriverState *bs);
>> +BlockDriverState *bdrv_skip_rw_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 724d8889a6..be18130944 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -6494,3 +6494,58 @@ BdrvChild *bdrv_primary_child(BlockDriverState *bs)
>>   {
>>   return bdrv_filtered_rw_child(bs) ?: bs->file;
>>   }
>> +
>> +static BlockDriverState *bdrv_skip_filters(BlockDriverState *bs,
>> +   bool stop_on_explicit_filter)
>> +{
>> +BdrvChild *filtered;
>> +
>> +if (!bs) {
>> +return NULL;
>> +}
>> +
>> +while (!(stop_on_explicit_filter && !bs->implicit)) {
>> +filtered = bdrv_filtered_rw_child(bs);
>> +if (!filtered) {
>> +break;
>> +}
>> +bs = filtered->bs;
>> +}
>> +/*
>> + * Note that this treats nodes with bs->drv == NULL
> 
> as well as filters without filtered_rw child

A filter always must have a filtered_rw child, though.  So I don’t quite
understand what you mean here...

Max

>   as not being
>> + * R/W filters (bs->drv == NULL should be replaced by something
>> + * else anyway).
>> + * The advantage of this behavior is that this function will thus
>> + * always return a non-NULL value (given a non-NULL @bs).
> 
> and this is the advantage of what I've written, not about bs->drv.
> 
>> + */
>> +
>> +return bs;
>> +}
>> +
>> +/*
>> + * Return the first BDS that has not been added implicitly or that
>> + * does not have an RW-filtered child down the chain starting from @bs
>> + * (including @bs itself).
>> + */
>> +BlockDriverState *bdrv_skip_implicit_filters(BlockDriverState *bs)
>> +{
>> +return bdrv_skip_filters(bs, true);
>> +}
>> +
>> +/*
>> + * Return the first BDS that does not have an RW-filtered child down
>> + * the chain starting from @bs (including @bs itself).
>> + */
>> +BlockDriverState *bdrv_skip_rw_filters(BlockDriverState *bs)
>> +{
>> +return bdrv_skip_filters(bs, false);
>> +}
>> +
>> +/*
>> + * For a backing chain, return the first non-filter backing image of
>> + * the first non-filter image.
>> + */
>> +BlockDriverState *bdrv_backing_chain_next(BlockDriverState *bs)
>> +{
>> +return 
>> bdrv_skip_rw_filters(bdrv_filtered_cow_bs(bdrv_skip_rw_filters(bs)));
>> +}
>>
> 
> 
> Reviewed-by: Vladimir Sementsov-Ogievskiy 
> 




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-block] [PATCH v5 05/42] block: Add chain helper functions

2019-06-13 Thread Vladimir Sementsov-Ogievskiy
13.06.2019 1:09, Max Reitz wrote:
> Add some helper functions for skipping filters in a chain of block
> nodes.
> 
> Signed-off-by: Max Reitz 
> ---
>   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 7ce71623f8..875a33f255 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -1264,6 +1264,9 @@ BdrvChild *bdrv_filtered_child(BlockDriverState *bs);
>   BdrvChild *bdrv_metadata_child(BlockDriverState *bs);
>   BdrvChild *bdrv_storage_child(BlockDriverState *bs);
>   BdrvChild *bdrv_primary_child(BlockDriverState *bs);
> +BlockDriverState *bdrv_skip_implicit_filters(BlockDriverState *bs);
> +BlockDriverState *bdrv_skip_rw_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 724d8889a6..be18130944 100644
> --- a/block.c
> +++ b/block.c
> @@ -6494,3 +6494,58 @@ BdrvChild *bdrv_primary_child(BlockDriverState *bs)
>   {
>   return bdrv_filtered_rw_child(bs) ?: bs->file;
>   }
> +
> +static BlockDriverState *bdrv_skip_filters(BlockDriverState *bs,
> +   bool stop_on_explicit_filter)
> +{
> +BdrvChild *filtered;
> +
> +if (!bs) {
> +return NULL;
> +}
> +
> +while (!(stop_on_explicit_filter && !bs->implicit)) {
> +filtered = bdrv_filtered_rw_child(bs);
> +if (!filtered) {
> +break;
> +}
> +bs = filtered->bs;
> +}
> +/*
> + * Note that this treats nodes with bs->drv == NULL

as well as filters without filtered_rw child

  as not being
> + * R/W filters (bs->drv == NULL should be replaced by something
> + * else anyway).
> + * The advantage of this behavior is that this function will thus
> + * always return a non-NULL value (given a non-NULL @bs).

and this is the advantage of what I've written, not about bs->drv.

> + */
> +
> +return bs;
> +}
> +
> +/*
> + * Return the first BDS that has not been added implicitly or that
> + * does not have an RW-filtered child down the chain starting from @bs
> + * (including @bs itself).
> + */
> +BlockDriverState *bdrv_skip_implicit_filters(BlockDriverState *bs)
> +{
> +return bdrv_skip_filters(bs, true);
> +}
> +
> +/*
> + * Return the first BDS that does not have an RW-filtered child down
> + * the chain starting from @bs (including @bs itself).
> + */
> +BlockDriverState *bdrv_skip_rw_filters(BlockDriverState *bs)
> +{
> +return bdrv_skip_filters(bs, false);
> +}
> +
> +/*
> + * For a backing chain, return the first non-filter backing image of
> + * the first non-filter image.
> + */
> +BlockDriverState *bdrv_backing_chain_next(BlockDriverState *bs)
> +{
> +return 
> bdrv_skip_rw_filters(bdrv_filtered_cow_bs(bdrv_skip_rw_filters(bs)));
> +}
> 


Reviewed-by: Vladimir Sementsov-Ogievskiy 

-- 
Best regards,
Vladimir