Re: [Qemu-devel] [PATCH v2 21/21] iotests: Add test for different refcount widths

[Max Reitz <mreitz@redhat.com>]

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

I'm currently thinking about a way to test the old reftable reallocation 
issue, and I can't find any. So, for the old reftable to require a 
reallocation it must grow. For it to grow we need some allocation beyond 
what it can currently represent. For this to happen during the refblock 
allocation walk, this allocation must be the allocation of a new refblock.

If the refblock is allocated beyond the current reftable's limit, this 
means that either all clusters between free_cluster_index and that point 
are already taken. If the reftable is then reallocated, it will 
therefore *always* be allocated behind that refblock, which is beyond 
its old limit. Therefore, that walk through the old reftable will never 
miss that new allocation.

So the issue can only occur if the old reftable is resized after the 
walk through it, that is, when allocating the new reftable. That is 
indeed an issue but I think it manifests itself basically like the issue 
I'm testing here: There is now an area in the old refcount structures 
which was free before but has is used now, and the allocation causing 
that was the allocation of the new reftable. The only difference is 
whether the it's the old or the new reftable that resides in the 
previously free area. Thus, I think I'll leave it at this test – but if 
you can describe to me how to create an image for a different "rewalk" 
path, I'm all ears.

Max