From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39360) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XqHlO-0002hJ-Tu for qemu-devel@nongnu.org; Mon, 17 Nov 2014 03:34:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XqHlI-0003fb-L8 for qemu-devel@nongnu.org; Mon, 17 Nov 2014 03:34:22 -0500 Received: from mx1.redhat.com ([209.132.183.28]:45028) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XqHlI-0003f7-EH for qemu-devel@nongnu.org; Mon, 17 Nov 2014 03:34:16 -0500 Message-ID: <5469B300.9080903@redhat.com> Date: Mon, 17 Nov 2014 09:34:08 +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> In-Reply-To: <5467682E.1030604@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-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. I'll see whether I can add a test for increasing the size of the original reftable during amendment, too. Max