[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 05/15] qcow2: do not COW the empty areas
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2 05/15] qcow2: do not COW the empty areas |
Date: |
Mon, 5 Jun 2017 09:40:06 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 |
On 06/01/2017 10:14 AM, Anton Nefedov wrote:
> If COW area of the newly allocated cluster is zeroes, there is no reason
> to write zero sectors in perform_cow() again now as whole clusters are
> zeroed out in single chunks by handle_alloc_space().
>
> Introduce QCowL2Meta field "reduced", since the existing fields
> (offset and nb_bytes) still has to keep other write requests from
> simultaneous writing in the area
>
> iotest 060:
> write to the discarded cluster does not trigger COW anymore.
> so, break on write_aio event instead, will work for the test
> (but write won't fail anymore, so update reference output)
>
> iotest 066:
> cluster-alignment areas that were not really COWed are now detected
> as zeroes, hence the initial write has to be exactly the same size for
> the maps to match
> +++ b/block/qcow2.c
> @@ -64,6 +64,9 @@ typedef struct {
> #define QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
> #define QCOW2_EXT_MAGIC_FEATURE_TABLE 0x6803f857
>
> +static bool is_zero_sectors(BlockDriverState *bs, int64_t start,
> + uint32_t count);
> +
I've already posted patches that redo this function to be byte-based;
I'm not sure which of our series will land first, but it is something to
keep in mind.
I'm also not a big fan of forward declarations of leaf static functions
(they are essential for mutually recursive functions, but when there is
no recursion, the better solution is to just hoist the implementation of
the function to occur where it is first needed, rather than using a
forward declaration).
> @@ -1588,8 +1611,13 @@ static void handle_alloc_space(BlockDriverState *bs,
> QCowL2Meta *l2meta)
> }
>
> /* try to alloc host space in one chunk for better locality */
> - bdrv_co_pwrite_zeroes(bs->file, m->alloc_offset, bytes,
> - BDRV_REQ_ALLOCATE);
> + ret = bdrv_co_pwrite_zeroes(bs->file, m->alloc_offset, bytes,
> + BDRV_REQ_ALLOCATE);
> + if (ret != 0) {
> + continue;
> + }
> +
> + handle_cow_reduce(bs, m);
Does this look any better as:
if (bdrv_co_pwrite_zeroes(...) == 0) {
handle_cow_reduce(bs, m);
}
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v2 02/15] file-posix: support BDRV_REQ_ALLOCATE, (continued)
- [Qemu-devel] [PATCH v2 01/15] block: introduce BDRV_REQ_ALLOCATE flag, Anton Nefedov, 2017/06/01
- [Qemu-devel] [PATCH v2 04/15] qcow2: alloc space for COW in one chunk, Anton Nefedov, 2017/06/01
- [Qemu-devel] [PATCH v2 05/15] qcow2: do not COW the empty areas, Anton Nefedov, 2017/06/01
- Re: [Qemu-devel] [PATCH v2 05/15] qcow2: do not COW the empty areas,
Eric Blake <=
- [Qemu-devel] [PATCH v2 03/15] blkdebug: support BDRV_REQ_ALLOCATE, Anton Nefedov, 2017/06/01
- [Qemu-devel] [PATCH v2 07/15] qcow2: set inactive flag, Anton Nefedov, 2017/06/01
- [Qemu-devel] [PATCH v2 10/15] qcow2: handle_prealloc(): find out if area zeroed by earlier preallocation, Anton Nefedov, 2017/06/01
- [Qemu-devel] [PATCH v2 06/15] qcow2: preallocation at image expand, Anton Nefedov, 2017/06/01
- [Qemu-devel] [PATCH v2 08/15] qcow2: truncate preallocated space, Anton Nefedov, 2017/06/01
- [Qemu-devel] [PATCH v2 09/15] qcow2: check space leak at the end of the image, Anton Nefedov, 2017/06/01
- [Qemu-devel] [PATCH v2 11/15] qcow2: fix misleading comment about L2 linking, Anton Nefedov, 2017/06/01
- [Qemu-devel] [PATCH v2 12/15] qcow2-cluster: slightly refactor handle_dependencies(), Anton Nefedov, 2017/06/01