[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v1 02/13] qcow2: is_zero_sectors(): return true
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v1 02/13] qcow2: is_zero_sectors(): return true if area is outside of backing file |
Date: |
Mon, 22 May 2017 14:12:24 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 |
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 <address@hidden>
> ---
> 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
- Re: [Qemu-devel] [PATCH v1 01/13] qcow2: alloc space for COW in one chunk, (continued)
[Qemu-devel] [PATCH v1 02/13] qcow2: is_zero_sectors(): return true if area is outside of backing file, Anton Nefedov, 2017/05/19
- Re: [Qemu-devel] [PATCH v1 02/13] qcow2: is_zero_sectors(): return true if area is outside of backing file,
Eric Blake <=
[Qemu-devel] [PATCH v1 03/13] qcow2: do not COW the empty areas, Anton Nefedov, 2017/05/19
[Qemu-devel] [PATCH v1 05/13] qcow2: set inactive flag, Anton Nefedov, 2017/05/19
[Qemu-devel] [PATCH v1 04/13] qcow2: preallocation at image expand, Anton Nefedov, 2017/05/19