Re: [Qemu-devel] [PATCH 1/2] block: Fix dst total_sectors after copy offloading

[Kevin Wolf]

Am 04.07.2018 um 08:13 hat Fam Zheng geschrieben:
> This was noticed by the new image fleecing tests case 222. The issue is
> apparent and we should just do the same right things as in
> bdrv_aligned_pwritev.
> 
> Reported-by: John Snow <address@hidden>
> Signed-off-by: Fam Zheng <address@hidden>

> --- a/block/io.c
> +++ b/block/io.c
> @@ -2945,6 +2945,10 @@ static int coroutine_fn 
> bdrv_co_copy_range_internal(BdrvChild *src,
>                                                    dst, dst_offset,
>                                                    bytes, flags);
>      }
> +    if (ret == 0) {
> +        int64_t end_sector = DIV_ROUND_UP(dst_offset + bytes, 
> BDRV_SECTOR_SIZE);
> +        dst->bs->total_sectors = MAX(dst->bs->total_sectors, end_sector);
> +    }

I think it's time to extract this stuff to a common function. I was
already aware that a write request that extends the image isn't behaving
exactly the same as bdrv_co_truncate(), and this one is bound to diverge
from the other two instances as well.

This is what bdrv_aligned_pwritev() does after the write:

    atomic_inc(&bs->write_gen);
    bdrv_set_dirty(bs, offset, bytes);

    stat64_max(&bs->wr_highest_offset, offset + bytes);

    if (ret >= 0) {
        bs->total_sectors = MAX(bs->total_sectors, end_sector);
        ret = 0;
    }

Before the write, it also calls bs->before_write_notifiers.

This is what bdrv_co_truncate() does after truncating the image:

    ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
    if (ret < 0) {
        error_setg_errno(errp, -ret, "Could not refresh total sector count");
    } else {
        offset = bs->total_sectors * BDRV_SECTOR_SIZE;
    }
    bdrv_dirty_bitmap_truncate(bs, offset);
    bdrv_parent_cb_resize(bs);
    atomic_inc(&bs->write_gen);

This means that we probably have at least the following bugs in
bdrv_co_copy_range_internal():

1. bs->write_gen is not increased, a following flush is ignored
2. Dirty bitmaps are not dirtied
3. Dirty bitmaps are not resized when extending the image
4. bs->wr_highest_offset is not adjusted correctly
5. bs->total_sectors is not resized (the bug this patch fixes)
6. The parent node isn't notified about an image size change

Of these, 3. and 6. also seem to be bugs in bdrv_aligned_pwritev().

Kevin