[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.8] qcow2: Don't strand clusters near 2G in
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH for-2.8] qcow2: Don't strand clusters near 2G intervals during commit |
Date: |
Mon, 5 Dec 2016 09:43:44 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 |
On 12/03/2016 11:34 AM, Eric Blake wrote:
> The qcow2_make_empty() function is reached during 'qemu-img commit',
> in order to clear out ALL clusters of an image. However, if the
> image cannot use the fast code path (true if the image is format
> 0.10, or if the image contains a snapshot), the cluster size is
> larger than 512, and the image is larger than 2G in size, then our
> choice of sector_step causes problems. Since it is not cluster
> aligned, but qcow2_discard_clusters() silently ignores an unaligned
> head or tail, we are leaving clusters allocated.
>
> Enhance the testsuite to expose the flaw, and patch the problem by
> ensuring our step size is aligned.
>
> [qcow2_discard_clusters() is a GROSS interface: it takes a mix of
> byte offset and sector count to perform cluster operations. But
> fixing it to use a saner byte/byte rather than byte/sector interface,
> and/or asserting that the counts are now aligned thanks to both
> this patch and commit 3482b9b, is material for another day.]
>
> Signed-off-by: Eric Blake <address@hidden>
> ---
> block/qcow2.c | 3 +-
> tests/qemu-iotests/097 | 41 +++++---
> tests/qemu-iotests/097.out | 249
> +++++++++++++++++++++++++++++++++------------
> 3 files changed, 210 insertions(+), 83 deletions(-)
> +++ b/block/qcow2.c
> @@ -2808,7 +2808,8 @@ static int qcow2_make_empty(BlockDriverState *bs)
> {
> BDRVQcow2State *s = bs->opaque;
> uint64_t start_sector;
> - int sector_step = INT_MAX / BDRV_SECTOR_SIZE;
> + int sector_step = QEMU_ALIGN_DOWN(INT_MAX / BDRV_SECTOR_SIZE,
> + s->cluster_size);
Oh shoot. I got the units wrong, and made the slow path do more loop
iterations than necessary (rounding sectors to cluster size in bytes is
inappropriate - either the rounding has to occur before division, or the
rounding needs to be by secotrs instead of bytes). I'll send a v2 that
gets the math right.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature