Re: [Qemu-devel] [PATCH v5 10/42] block: Use CAF in bdrv_is_encrypted()

[Vladimir Sementsov-Ogievskiy]

13.06.2019 1:09, Max Reitz wrote:
> bdrv_is_encrypted() should not only check the BDS's backing child, but
> any filtered child: If a filter's child is encrypted, the filter node
> itself naturally is encrypted, too.  Furthermore, we need to recurse
> down the chain.
> 
> (CAF means child access function.)

Hmm, so, if only one node in the backing chain is encrypted, all overlays,
filters or not are considered encrypted too? Even if all the data is in top
node and is not encrypted?

Checked that the function is used only for reporting through
bdrv_query_image_info, which is called from bdrv_block_device_info() (which
loops through backings), and from collect_image_info_list(), which loops through
backings if @chain=true.

And collect_image_info_list() is used only in img_info(), @chain is a mirrored
--backing-chain parameter..

So, isn't it more correct to return exactly bs->encrypted in this function? It 
will
give more correct and informative results for queries for the whole chain.


> 
> Signed-off-by: Max Reitz <address@hidden>
> ---
>   block.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 45882a3470..567a0f82c8 100644
> --- a/block.c
> +++ b/block.c
> @@ -4574,10 +4574,14 @@ bool bdrv_is_sg(BlockDriverState *bs)
>   
>   bool bdrv_is_encrypted(BlockDriverState *bs)
>   {
> -    if (bs->backing && bs->backing->bs->encrypted) {
> +    BlockDriverState *filtered = bdrv_filtered_bs(bs);
> +    if (bs->encrypted) {
>           return true;
>       }
> -    return bs->encrypted;
> +    if (filtered && bdrv_is_encrypted(filtered)) {
> +        return true;
> +    }
> +    return false;
>   }
>   
>   const char *bdrv_get_format_name(BlockDriverState *bs)
> 


-- 
Best regards,
Vladimir