Re: [Qemu-devel] [PATCH v2 5/9] block: Fix handling of image enlarging write

[Kevin Wolf]

Am 05.07.2018 um 09:36 hat Fam Zheng geschrieben:
> Two problems exist when a write request that enlarges the image (i.e.
> write beyond EOF) finishes:
> 
> 1) parent is not notified about size change;
> 2) dirty bitmap is not resized although we try to set the dirty bits;
> 
> Fix them just like how bdrv_co_truncate works.
> 
> Reported-by: Kevin Wolf <address@hidden>
> Signed-off-by: Fam Zheng <address@hidden>
> ---
>  block/io.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index 03d9eb0a65..f06978dda0 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -40,6 +40,7 @@
>  
>  static AioWait drain_all_aio_wait;
>  
> +static void bdrv_parent_cb_resize(BlockDriverState *bs);
>  static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
>      int64_t offset, int bytes, BdrvRequestFlags flags);
>  
> @@ -1571,13 +1572,17 @@ bdrv_co_write_req_finish(BdrvChild *child, 
> BdrvTrackedRequest *req, int ret)
>      BlockDriverState *bs = child->bs;
>  
>      atomic_inc(&bs->write_gen);
> -    bdrv_set_dirty(bs, req->offset, req->bytes);
>  
>      stat64_max(&bs->wr_highest_offset, req->offset + req->bytes);
>  
> -    if (ret == 0) {
> -        bs->total_sectors = MAX(bs->total_sectors, end_sector);
> +    if (req->type != BDRV_TRACKED_DISCARD &&

This part of the condition belongs in the next patch really because
currently BDRV_TRACKED_DISCARD doesn't happen.

But then, I think we can only skip this part for BDRV_TRACKED_DISCARD
because discards can't extend the image. We already check this with
end_sector > bs->total_sectors, so that condition should be enough.

Maybe what we should do (in the next patch) is assert that this
condition is never true for discards, and then rely on it for the rest
of the code.

> +        ret == 0 &&
> +        end_sector > bs->total_sectors) {
> +        bs->total_sectors = end_sector;
> +        bdrv_parent_cb_resize(bs);
> +        bdrv_dirty_bitmap_truncate(bs, end_sector << BDRV_SECTOR_BITS);
>      }
> +    bdrv_set_dirty(bs, req->offset, req->bytes);
>  }

Kevin