From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49741) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XqK4p-00018a-HM for qemu-devel@nongnu.org; Mon, 17 Nov 2014 06:02:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XqK4j-0000bQ-B8 for qemu-devel@nongnu.org; Mon, 17 Nov 2014 06:02:35 -0500 Received: from mx1.redhat.com ([209.132.183.28]:44068) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XqK4j-0000bK-43 for qemu-devel@nongnu.org; Mon, 17 Nov 2014 06:02:29 -0500 Message-ID: <5469D5BA.3080800@redhat.com> Date: Mon, 17 Nov 2014 12:02:18 +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> <5469D012.4030404@redhat.com> In-Reply-To: <5469D012.4030404@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 11:38, Max Reitz wrote: > 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... Problem "solved": If there will be more walks than originally thought (3+1 instead of 2+1), progress will regress at one point. I'll just grep for that point and that should be enough (progress jumping from 66.67 % to 50.00 %). Max