[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v9 01/13] qcow2: Unallocate unmapped zero cluste
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v9 01/13] qcow2: Unallocate unmapped zero clusters if no backing file |
Date: |
Wed, 12 Apr 2017 11:49:09 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 11.04.2017 um 03:17 hat Eric Blake geschrieben:
> 'qemu-img map' already coalesces information about unallocated
> clusters (those with status 0) and pure zero clusters (those
> with status BDRV_BLOCK_ZERO and no offset). Furthermore, all
> qcow2 images with no backing file already report all unallocated
> clusters (in the preallocation sense of clusters with no offset)
> as BDRV_BLOCK_ZERO, regardless of whether the QCOW_OFLAG_ZERO was
> set in that L2 entry (QCOW_OFLAG_ZERO also implies a return of
> BDRV_BLOCK_ALLOCATED, but we intentionally do not expose that bit
> to external users), thanks to generic block layer code in
> bdrv_co_get_block_status().
>
> So, for an image with no backing file, having bdrv_pwrite_zeroes
> mark clusters as unallocated (defer to backing file) rather than
> reads-as-zero (regardless of backing file) makes no difference
> to normal behavior, but may potentially allow for fewer writes to
> the L2 table of a freshly-created image where the L2 table is
> initially written to all-zeroes (although I don't actually know
> if we skip an L2 update and flush when re-writing the same
> contents as already present).
I don't get this. Allocating a cluster always involves an L2 update, no
matter whether it was previously unallocated or a zero cluster.
> Furthermore, this matches the behavior of discard_single_l2(), in
> favoring an unallocated cluster over a zero cluster when full
> discard is requested.
The only use for "full discard" is qcow2_make_empty(). It explicitly
requests that the backing file becomes visible again. This is a
completely different case.
In other words, in order to stay consistent between discard and
write_zeroes from a guest POV, we need to leave this code alone.
> Meanwhile, version 2 qcow2 files (compat=0.10) lack support for an
> explicit zero cluster. This minor tweak therefore allows us to turn
> write zeroes with unmap into an actual unallocation on those files,
> where they used to return -ENOTSUP and cause an allocation due to
> the fallback to explicitly written zeroes.
Okay, this is true.
But I doubt that making write_zeroes more efficient on v2 images without
a backing file is really worth any extra complexity at this point...
Kevin
- [Qemu-devel] [PATCH for-2.10 v9 00/13] add blkdebug tests, Eric Blake, 2017/04/10
- [Qemu-devel] [PATCH v9 04/13] qemu-io: Switch 'map' output to byte-based reporting, Eric Blake, 2017/04/10
- [Qemu-devel] [PATCH v9 02/13] iotests: Add test 179 to cover write zeroes with unmap, Eric Blake, 2017/04/10
- [Qemu-devel] [PATCH v9 06/13] qcow2: Assert that cluster operations are aligned, Eric Blake, 2017/04/10
- [Qemu-devel] [PATCH v9 09/13] blkdebug: Refactor error injection, Eric Blake, 2017/04/10
- [Qemu-devel] [PATCH v9 03/13] qemu-io: Switch 'alloc' command to byte-based length, Eric Blake, 2017/04/10
- [Qemu-devel] [PATCH v9 01/13] qcow2: Unallocate unmapped zero clusters if no backing file, Eric Blake, 2017/04/10
- Re: [Qemu-devel] [PATCH v9 01/13] qcow2: Unallocate unmapped zero clusters if no backing file,
Kevin Wolf <=
- [Qemu-devel] [PATCH v9 05/13] qcow2: Optimize write zero of unaligned tail cluster, Eric Blake, 2017/04/10
- [Qemu-devel] [PATCH v9 07/13] qcow2: Discard/zero clusters by byte count, Eric Blake, 2017/04/10
- [Qemu-devel] [PATCH v9 08/13] blkdebug: Sanity check block layer guarantees, Eric Blake, 2017/04/10
- [Qemu-devel] [PATCH v9 10/13] blkdebug: Add pass-through write_zero and discard support, Eric Blake, 2017/04/10
- [Qemu-devel] [PATCH v9 12/13] blkdebug: Add ability to override unmap geometries, Eric Blake, 2017/04/10
- [Qemu-devel] [PATCH v9 11/13] blkdebug: Simplify override logic, Eric Blake, 2017/04/10