On 05/19/2017 04:34 AM, Anton Nefedov wrote: > in such case, bdrv_get_block_status() shall return 0, *nr == 0
Sounds better as s/shall/will/
>
> iotest 154 updated accordingly: write-zeroes tail alignment can be detected
> as zeroes now, so pwrite_zeroes succeeds
>
> Signed-off-by: Anton Nefedov <anton.nefe...@virtuozzo.com>
> ---
> block/qcow2.c | 6 ++++--
> tests/qemu-iotests/154.out | 4 ++--
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
You'll want to also patch test 154 proper to remove these two TODO comments:
# TODO: Note that this forces an allocation, because we aren't yet able to
# quickly detect that reads beyond EOF of the backing file are always zero
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 2e6a0ec..b885dfc 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -2482,7 +2482,7 @@ static bool is_zero_sectors(BlockDriverState *bs,
> int64_t start,
> int64_t res;
>
> if (start + count > bs->total_sectors) {
> - count = bs->total_sectors - start;
> + count = start < bs->total_sectors ? bs->total_sectors - start : 0;
> }
>
> if (!count) {
> @@ -2490,7 +2490,9 @@ static bool is_zero_sectors(BlockDriverState *bs,
> int64_t start,
> }
> res = bdrv_get_block_status_above(bs, NULL, start, count,
> &nr, &file);
> - return res >= 0 && (res & BDRV_BLOCK_ZERO) && nr == count;
> + return res >= 0
> + && (((res & BDRV_BLOCK_ZERO) && nr == count)
> + || nr == 0);
The logic makes sense to me, although the formatting is unusual (we tend
to split && and || with the operator at the tail of the previous line,
not the head of the new line).
This quick check may make me revisit whether it is worth my my RFC
series about adding BDRV_BLOCK_EOF for more quickly tracking reads
beyond EOF (my solution in that series was able to make the same change
to test 154, but by doing it at the block layer instead of the qcow2.c
code). https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg01127.html
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
