Re: [Qemu-devel] [PATCH v5 22/42] block: Use CAFs in bdrv_get_allocated_file_size()

[Vladimir Sementsov-Ogievskiy]

13.06.2019 1:09, Max Reitz wrote:
> Signed-off-by: Max Reitz <address@hidden>
> ---
>   block.c | 26 ++++++++++++++++++++++++--
>   1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 11b7ba8cf6..856d9b58be 100644
> --- a/block.c
> +++ b/block.c
> @@ -4511,15 +4511,37 @@ exit:
>   int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)
>   {
>       BlockDriver *drv = bs->drv;
> +    BlockDriverState *storage_bs, *metadata_bs;
> +
>       if (!drv) {
>           return -ENOMEDIUM;
>       }
> +
>       if (drv->bdrv_get_allocated_file_size) {
>           return drv->bdrv_get_allocated_file_size(bs);
>       }
> -    if (bs->file) {
> -        return bdrv_get_allocated_file_size(bs->file->bs);
> +
> +    storage_bs = bdrv_storage_bs(bs);
> +    metadata_bs = bdrv_metadata_bs(bs);
> +
> +    if (storage_bs) {
> +        int64_t data_size, metadata_size = 0;
> +
> +        data_size = bdrv_get_allocated_file_size(storage_bs);
> +        if (data_size < 0) {
> +            return data_size;
> +        }
> +
> +        if (storage_bs != metadata_bs) {
> +            metadata_size = bdrv_get_allocated_file_size(metadata_bs);
> +            if (metadata_size < 0) {
> +                return metadata_size;
> +            }
> +        }
> +
> +        return data_size + metadata_size;
>       }
> +
>       return -ENOTSUP;
>   }
>   
> 

Again, I dislike nailing down new fresh feature about separate metadata and 
storage child
to the generic block layer, as it's simple to imagine a driver which needs 
three or more
children to store all its data and metadata..

Isn't it better by default loop through all children and sum all their 
allocated sizes?

Hmm, but we want exclude backing, yes? Still we may ignore it while iterating.

-- 
Best regards,
Vladimir