[PATCH v4 1/2] qcow2: Report BDRV_BLOCK_ZERO more accurately in bdrv_co_block_status()

Alberto Garcia posted 2 patches 5 years, 4 months ago
Maintainers: Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>, Fam Zheng <fam@euphon.net>, Stefan Hajnoczi <stefanha@redhat.com>
There is a newer version of this series
[PATCH v4 1/2] qcow2: Report BDRV_BLOCK_ZERO more accurately in bdrv_co_block_status()
Posted by Alberto Garcia 5 years, 4 months ago
If a BlockDriverState supports backing files but has none then any
unallocated area reads back as zeroes.

bdrv_co_block_status() is only reporting this is if want_zero is true,
but this is an inexpensive test and there is no reason not to do it in
all cases.

Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 block/io.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/io.c b/block/io.c
index a2389bb38c..ef1ea806e8 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2391,17 +2391,17 @@ static int coroutine_fn bdrv_co_block_status(BlockDriverState *bs,
 
     if (ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ZERO)) {
         ret |= BDRV_BLOCK_ALLOCATED;
-    } else if (want_zero && bs->drv->supports_backing) {
+    } else if (bs->drv->supports_backing) {
         BlockDriverState *cow_bs = bdrv_cow_bs(bs);
 
-        if (cow_bs) {
+        if (!cow_bs) {
+            ret |= BDRV_BLOCK_ZERO;
+        } else if (want_zero) {
             int64_t size2 = bdrv_getlength(cow_bs);
 
             if (size2 >= 0 && offset >= size2) {
                 ret |= BDRV_BLOCK_ZERO;
             }
-        } else {
-            ret |= BDRV_BLOCK_ZERO;
         }
     }
 
-- 
2.20.1


Re: [PATCH v4 1/2] qcow2: Report BDRV_BLOCK_ZERO more accurately in bdrv_co_block_status()
Posted by Vladimir Sementsov-Ogievskiy 5 years, 3 months ago
21.09.2020 17:30, Alberto Garcia wrote:
> If a BlockDriverState supports backing files but has none then any
> unallocated area reads back as zeroes.
> 
> bdrv_co_block_status() is only reporting this is if want_zero is true,
> but this is an inexpensive test and there is no reason not to do it in
> all cases.
> 
> Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Signed-off-by: Alberto Garcia <berto@igalia.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

> ---
>   block/io.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index a2389bb38c..ef1ea806e8 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -2391,17 +2391,17 @@ static int coroutine_fn bdrv_co_block_status(BlockDriverState *bs,
>   
>       if (ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ZERO)) {
>           ret |= BDRV_BLOCK_ALLOCATED;
> -    } else if (want_zero && bs->drv->supports_backing) {
> +    } else if (bs->drv->supports_backing) {
>           BlockDriverState *cow_bs = bdrv_cow_bs(bs);
>   
> -        if (cow_bs) {
> +        if (!cow_bs) {
> +            ret |= BDRV_BLOCK_ZERO;
> +        } else if (want_zero) {
>               int64_t size2 = bdrv_getlength(cow_bs);
>   
>               if (size2 >= 0 && offset >= size2) {
>                   ret |= BDRV_BLOCK_ZERO;
>               }
> -        } else {
> -            ret |= BDRV_BLOCK_ZERO;
>           }
>       }
>   
> 


-- 
Best regards,
Vladimir