From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44753) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XqJhS-0000ka-Vh for qemu-devel@nongnu.org; Mon, 17 Nov 2014 05:38:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XqJhL-00013M-3M for qemu-devel@nongnu.org; Mon, 17 Nov 2014 05:38:26 -0500 Received: from mx1.redhat.com ([209.132.183.28]:52132) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XqJhK-00013H-Sx for qemu-devel@nongnu.org; Mon, 17 Nov 2014 05:38:19 -0500 Message-ID: <5469D012.4030404@redhat.com> Date: Mon, 17 Nov 2014 11:38:10 +0100 From: Max Reitz MIME-Version: 1.0 References: <1415970374-16811-1-git-send-email-mreitz@redhat.com> <1415970374-16811-22-git-send-email-mreitz@redhat.com> <5467682E.1030604@redhat.com> <5469B300.9080903@redhat.com> In-Reply-To: <5469B300.9080903@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 21/21] iotests: Add test for different refcount widths List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org Cc: Kevin Wolf , Peter Lieven , Stefan Hajnoczi On 2014-11-17 at 09:34, Max Reitz wrote: > On 2014-11-15 at 15:50, Eric Blake wrote: >> On 11/14/2014 06:06 AM, Max Reitz wrote: >>> Add a test for conversion between different refcount widths and errors >>> specific to certain widths (i.e. snapshots with refcount_width=1). >>> >>> Signed-off-by: Max Reitz >>> --- >>> tests/qemu-iotests/112 | 252 >>> +++++++++++++++++++++++++++++++++++++++++++++ >>> tests/qemu-iotests/112.out | 131 +++++++++++++++++++++++ >>> tests/qemu-iotests/group | 1 + >>> 3 files changed, 384 insertions(+) >>> create mode 100755 tests/qemu-iotests/112 >>> create mode 100644 tests/qemu-iotests/112.out >>> >>> +echo >>> +echo '=== Testing too many references for check ===' >>> +echo >>> + >>> +IMGOPTS="$IMGOPTS,refcount_width=1" _make_test_img 64M >>> +print_refcount_width >>> + >>> +# This cluster should be created at 0x50000 >>> +$QEMU_IO -c 'write 0 64k' "$TEST_IMG" | _filter_qemu_io >>> +# Now make the second L2 entry (the L2 table should be at 0x40000) >>> point to that >>> +# cluster, so we have two references >>> +poke_file "$TEST_IMG" $((0x40008)) "\x80\x00\x00\x00\x00\x05\x00\x00" >>> + >>> +# This should say "please use amend" >>> +_check_test_img -r all >>> + >>> +# So we do that >>> +$QEMU_IMG amend -o refcount_width=2 "$TEST_IMG" >>> +print_refcount_width >>> + >>> +# And try again >>> +_check_test_img -r all >> I think this section also deserves a test that fuzzes an image with >> width=64 to intentionally set the most significant bit of one of the >> refcounts, and make sure that we gracefully diagnose it as invalid. >> >>> + >>> +echo >>> +echo '=== Multiple walks necessary during amend ===' >>> +echo >>> + >>> +IMGOPTS="$IMGOPTS,refcount_width=1,cluster_size=512" _make_test_img >>> 64k >>> + >>> +# Cluster 0 is the image header, clusters 1 to 4 are used by the L1 >>> table, a >>> +# single L2 table, the reftable and a single refblock. This creates >>> 58 data >>> +# clusters (actually, the L2 table is created here, too), so in >>> total there are >>> +# then 63 used clusters in the image. With a refcount width of 64, >>> one refblock >>> +# describes 64 clusters (512 bytes / 64 bits/entry = 64 entries), >>> so this will >>> +# make the first target refblock have exactly one free entry. >>> +$QEMU_IO -c "write 0 $((58 * 512))" "$TEST_IMG" | _filter_qemu_io >>> + >>> +# Now change the refcount width; since the first target refblock >>> has exactly one >>> +# free entry, that entry will be used to store its own reference. >>> No other >>> +# refblocks are needed, so then the new reftable will be allocated; >>> since the >>> +# first target refblock is completely filled up, this will require >>> a new >>> +# refblock which is why the refcount width changing function will >>> need to run >>> +# through everything one more time until the allocations are stable. >>> +$QEMU_IMG amend -o refcount_width=64 "$TEST_IMG" >>> +print_refcount_width >> Umm, that sounds backwards from what you document. It's a good test of >> the _new_ reftable needing a second round of allocations. So keep it >> with corrected comments. But I think you _intended_ to write a test >> that starts with a refcount_width=64 image and resize to a >> refcount_width=1, where the _old_ reftable then suffers a reallocation >> as part of allocating refblocks for the new table. > > That's what you intended, but that's harder to test, so I settled for > this (and the comments are appropriate (note that "target refblock" > refers to the refblocks after amendment); note that this does indeed > fail with v1 of this series. > >> It may even help if >> you add a tracepoint for every iteration through the walk function >> callback, to prove we are indeed executing it 3 times instead of the >> usual 2, for these test cases. > > That's a good idea! I thought about adding some info, but totally > forgot about trace points. ...On second thought, trace doesn't work so well with qemu-img. My best bet would be blkdebug, but that seems kind of ugly to me... Max > I'll see whether I can add a test for increasing the size of the > original reftable during amendment, too. > > Max