diff mbox series

[v2,1/2] block/io: Update BSC only if want_zero is true

Message ID 20220118170000.49423-2-hreitz@redhat.com (mailing list archive)
State New, archived
Headers show
Series block/io: Update BSC only if want_zero is true | expand

Commit Message

Hanna Czenczek Jan. 18, 2022, 4:59 p.m. UTC
We update the block-status cache whenever we get new information from a
bdrv_co_block_status() call to the block driver.  However, if we have
passed want_zero=false to that call, it may flag areas containing zeroes
as data, and so we would update the block-status cache with wrong
information.

Therefore, we should not update the cache with want_zero=false.

Reported-by: Nir Soffer <nsoffer@redhat.com>
Fixes: 0bc329fbb00 ("block: block-status cache for data regions")
Reviewed-by: Nir Soffer <nsoffer@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
---
 block/io.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Eric Blake Jan. 28, 2022, 8:46 p.m. UTC | #1
On Tue, Jan 18, 2022 at 05:59:59PM +0100, Hanna Reitz wrote:
> We update the block-status cache whenever we get new information from a
> bdrv_co_block_status() call to the block driver.  However, if we have
> passed want_zero=false to that call, it may flag areas containing zeroes
> as data, and so we would update the block-status cache with wrong
> information.
> 
> Therefore, we should not update the cache with want_zero=false.
> 
> Reported-by: Nir Soffer <nsoffer@redhat.com>
> Fixes: 0bc329fbb00 ("block: block-status cache for data regions")
> Reviewed-by: Nir Soffer <nsoffer@redhat.com>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Hanna Reitz <hreitz@redhat.com>
> ---
>  block/io.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>
diff mbox series

Patch

diff --git a/block/io.c b/block/io.c
index bb0a254def..4e4cb556c5 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2497,8 +2497,12 @@  static int coroutine_fn bdrv_co_block_status(BlockDriverState *bs,
              * non-protocol nodes, and then it is never used.  However, filling
              * the cache requires an RCU update, so double check here to avoid
              * such an update if possible.
+             *
+             * Check want_zero, because we only want to update the cache when we
+             * have accurate information about what is zero and what is data.
              */
-            if (ret == (BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID) &&
+            if (want_zero &&
+                ret == (BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID) &&
                 QLIST_EMPTY(&bs->children))
             {
                 /*