[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.9?] qcow2: Allow discard of final unaligne
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH for-2.9?] qcow2: Allow discard of final unaligned cluster |
Date: |
Fri, 7 Apr 2017 16:46:29 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 |
On 07.04.2017 03:37, Eric Blake wrote:
> As mentioned in commit 0c1bd46, we ignored requests to
> discard the trailing cluster of an unaligned image. While
> discard is an advisory operation from the guest standpoint,
> (and we are therefore free to ignore any request), our
> qcow2 implementation exploits the fact that a discarded
> cluster reads back as 0. As long as we discard on cluster
> boundaries, we are fine; but that means we could observe
> non-zero data leaked at the tail of an unaligned image.
>
> Enhance iotest 66 to cover this case, and fix the implementation
> to honor a discard request on the final partial cluster.
>
> Signed-off-by: Eric Blake <address@hidden>
> ---
Thanks!
> I can't convince myself whether we strongly rely on aligned discards
> to guarantee that we read back zeroes on qcow2
No, we don't.
> (it would be a
> stronger contract than what the block layer requires of pdiscard,
> since the guest cannot guarantee that a discard does anything). If
> we do, then this is a bug fix worthy of 2.9.
I'm not sure it would be, even if we "relied" on it. For instance,
intra-cluster discarding will be silently ignored (in contrast to
intra-cluster zero writes).
(Obviously one might argue that the desired behavior is to read back
zeroes because for compat=1.1 images we actually write zero clusters
instead of just completely discarding clusters, but...)
> If we don't, then the
> changes to test 66 rely on internal implementation (but the test is
> already specific to qcow2), and the patch can wait for 2.10.
If you want to write zeroes, use zero writing.
Note that discarding on compat=0.10 images will actually really discard
the clusters instead of creating zero clusters (because those don't
exist in compat=0.10). So if you have a backing file, afterwards you'll
see its contents in the discarded areas.
(I personally actually really like that discard behavior. If I had to
decide, I would like that behavior for compat=1.1 images, too, but I
don't, so it reads back as zero there.)
> At any rate, I do know that we don't want to make discard work on
> sub-cluster boundaries anywhere except at the tail of the image
> (that's what write-zeroes is for, and it may be slower when it
> has to do COW or read-modify-write). Any reliance that we (might)
> have on whole-cluster discards reading back as 0 is also relying
> on using aligned operations.
No, we don't, because discard doesn't give you guarantees about what
kind of data you'll read back.
Therefore: Applied to my block-next branch for 2.10:
https://github.com/XanClic/qemu/commits/block-next
Max
signature.asc
Description: OpenPGP digital signature