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 <ebl...@redhat.com> > ---
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
