[PATCH v7 26/47] block: Improve get_allocated_file_size's default

Max Reitz posted 47 patches 5 years, 4 months ago
There is a newer version of this series
[PATCH v7 26/47] block: Improve get_allocated_file_size's default
Posted by Max Reitz 5 years, 4 months ago
There are two practical problems with bdrv_get_allocated_file_size()'s
default right now:
(1) For drivers with children, we should generally sum all their sizes
    instead of just passing the request through to bs->file.  The latter
    is good for filters, but not so much for format drivers.

(2) Filters need not have bs->file, so we should actually go to the
    filtered child instead of hard-coding bs->file.

And we can make the whole default implementation more idiomatic by using
the three generic functions added by the previous patch.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index fc01ce90b3..a19f243997 100644
--- a/block.c
+++ b/block.c
@@ -4997,10 +4997,21 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)
     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);
+
+    if (drv->bdrv_file_open) {
+        /*
+         * Protocol drivers default to -ENOTSUP (most of their data is
+         * not stored in any of their children (if they even have any),
+         * so there is no generic way to figure it out).
+         */
+        return bdrv_notsup_allocated_file_size(bs);
+    } else if (drv->is_filter) {
+        /* Filter drivers default to the size of their primary child */
+        return bdrv_primary_allocated_file_size(bs);
+    } else {
+        /* Other drivers default to summing their children's sizes */
+        return bdrv_sum_allocated_file_size(bs);
     }
-    return -ENOTSUP;
 }
 
 /**
-- 
2.26.2


Re: [PATCH v7 26/47] block: Improve get_allocated_file_size's default
Posted by Andrey Shinkevich 5 years, 3 months ago
On 25.06.2020 18:21, Max Reitz wrote:
> There are two practical problems with bdrv_get_allocated_file_size()'s
> default right now:
> (1) For drivers with children, we should generally sum all their sizes
>      instead of just passing the request through to bs->file.  The latter
>      is good for filters, but not so much for format drivers.
>
> (2) Filters need not have bs->file, so we should actually go to the
>      filtered child instead of hard-coding bs->file.
>
> And we can make the whole default implementation more idiomatic by using
> the three generic functions added by the previous patch.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>   block.c | 17 ++++++++++++++---
>   1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/block.c b/block.c
> index fc01ce90b3..a19f243997 100644
> --- a/block.c
> +++ b/block.c
> @@ -4997,10 +4997,21 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)
>       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);
> +
> +    if (drv->bdrv_file_open) {
> +        /*
> +         * Protocol drivers default to -ENOTSUP (most of their data is
> +         * not stored in any of their children (if they even have any),
> +         * so there is no generic way to figure it out).
> +         */
> +        return bdrv_notsup_allocated_file_size(bs);
> +    } else if (drv->is_filter) {
> +        /* Filter drivers default to the size of their primary child */
> +        return bdrv_primary_allocated_file_size(bs);
> +    } else {
> +        /* Other drivers default to summing their children's sizes */
> +        return bdrv_sum_allocated_file_size(bs);
>       }
> -    return -ENOTSUP;
>   }
>   
>   /**


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