From nobody Sun May 19 02:26:39 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1491529105575375.16120720538015; Thu, 6 Apr 2017 18:38:25 -0700 (PDT) Received: from localhost ([::1]:48354 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cwIr2-0002PG-9s for importer@patchew.org; Thu, 06 Apr 2017 21:38:24 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48641) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cwIq3-00021c-42 for qemu-devel@nongnu.org; Thu, 06 Apr 2017 21:37:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cwIq1-0008Kw-Vu for qemu-devel@nongnu.org; Thu, 06 Apr 2017 21:37:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50064) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cwIpx-0008JN-2L; Thu, 06 Apr 2017 21:37:17 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E9ED03D954; Fri, 7 Apr 2017 01:37:15 +0000 (UTC) Received: from red.redhat.com (ovpn-124-8.rdu2.redhat.com [10.10.124.8]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1AE0D9F7B1; Fri, 7 Apr 2017 01:37:14 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com E9ED03D954 Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eblake@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com E9ED03D954 From: Eric Blake To: qemu-devel@nongnu.org Date: Thu, 6 Apr 2017 20:37:09 -0500 Message-Id: <20170407013709.18440-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Fri, 07 Apr 2017 01:37:16 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH for-2.9?] qcow2: Allow discard of final unaligned cluster X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, "open list:qcow2" , mreitz@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" 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 --- I can't convince myself whether we strongly rely on aligned discards to guarantee that we read back zeroes on qcow2 (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. 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. 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. block/qcow2.c | 7 ++++++- tests/qemu-iotests/066 | 12 +++++++----- tests/qemu-iotests/066.out | 12 ++++++++---- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index 6a92d2e..863d889 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2515,7 +2515,12 @@ static coroutine_fn int qcow2_co_pdiscard(BlockDrive= rState *bs, if (!QEMU_IS_ALIGNED(offset | count, s->cluster_size)) { assert(count < s->cluster_size); - return -ENOTSUP; + /* Ignore partial clusters, except for the special case of the + * complete partial cluster at the end of an unaligned file */ + if (!QEMU_IS_ALIGNED(offset, s->cluster_size) || + offset + count !=3D bs->total_sectors * BDRV_SECTOR_SIZE) { + return -ENOTSUP; + } } qemu_co_mutex_lock(&s->lock); diff --git a/tests/qemu-iotests/066 b/tests/qemu-iotests/066 index 364166d..c2116a3 100755 --- a/tests/qemu-iotests/066 +++ b/tests/qemu-iotests/066 @@ -42,16 +42,18 @@ _supported_fmt qcow2 _supported_proto generic _supported_os Linux +# Intentionally create an unaligned image IMGOPTS=3D"compat=3D1.1" -IMG_SIZE=3D64M +IMG_SIZE=3D$((64 * 1024 * 1024 + 512)) echo -echo "=3D=3D=3D Testing snapshotting an image with zero clusters =3D=3D=3D" +echo "=3D=3D=3D Testing cluster discards =3D=3D=3D" echo _make_test_img $IMG_SIZE -# Write some normal clusters, zero them (creating preallocated zero cluste= rs) -# and discard those -$QEMU_IO -c "write 0 256k" -c "write -z 0 256k" -c "discard 0 256k" "$TEST= _IMG" \ +# Write some normal clusters, zero some of them (creating preallocated +# zero clusters) and discard everything. Everything should now read as 0. +$QEMU_IO -c "write 0 256k" -c "write -z 0 256k" -c "write 64M 512" \ + -c "discard 0 $IMG_SIZE" -c "read -P 0 0 $IMG_SIZE" "$TEST_IMG" \ | _filter_qemu_io # Check the image (there shouldn't be any leaks) _check_test_img diff --git a/tests/qemu-iotests/066.out b/tests/qemu-iotests/066.out index 7bc9a10..7c1f31a 100644 --- a/tests/qemu-iotests/066.out +++ b/tests/qemu-iotests/066.out @@ -1,13 +1,17 @@ QA output created by 066 -=3D=3D=3D Testing snapshotting an image with zero clusters =3D=3D=3D +=3D=3D=3D Testing cluster discards =3D=3D=3D -Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D67108864 +Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D67109376 wrote 262144/262144 bytes at offset 0 256 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) wrote 262144/262144 bytes at offset 0 256 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -discard 262144/262144 bytes at offset 0 -256 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 512/512 bytes at offset 67108864 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +discard 67109376/67109376 bytes at offset 0 +64 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 67109376/67109376 bytes at offset 0 +64 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) No errors were found on the image. *** done --=20 2.9.3