| Message ID | 20220117162649.193501-2-hreitz@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | block/io: Update BSC only if want_zero is true | expand |
On Mon, Jan 17, 2022 at 6:26 PM Hanna Reitz <hreitz@redhat.com> 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: 0bc329fbb009f8601cec23bf2bc48ead0c5a5fa2 > ("block: block-status cache for data regions") > Signed-off-by: Hanna Reitz <hreitz@redhat.com> > --- > block/io.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > 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)) > { > /* > -- > 2.33.1 > ovirt-imageio tests pass with this change. Thanks for the quick fix! Reviewed-by: Nir Soffer <nsoffer@redhat.com>
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)) { /*
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: 0bc329fbb009f8601cec23bf2bc48ead0c5a5fa2 ("block: block-status cache for data regions") Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- block/io.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)