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

[Max Reitz <mreitz@redhat.com>]

On 2014-12-04 at 01:03, Eric Blake wrote:
> On 12/03/2014 05:37 AM, Max Reitz wrote:
>> Add a test for conversion between different refcount widths and errors
>> specific to certain widths (i.e. snapshots with refcount_bits=1).
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   tests/qemu-iotests/112     | 296 +++++++++++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/112.out | 155 ++++++++++++++++++++++++
>>   tests/qemu-iotests/group   |   1 +
>>   3 files changed, 452 insertions(+)
>>   create mode 100755 tests/qemu-iotests/112
>>   create mode 100644 tests/qemu-iotests/112.out
>
>> +# This test will set refcount_bits on its own which would conflict with the
>> +# manual setting; compat will be overridden as well
>> +_unsupported_imgopts refcount_bits 'compat=0.10'
> Should this be 'compat' rather than 'compat=0.10' as the filter?  The
> reason I ask is that if I can pass an explicit 'compat=1.1'...
>
>> +echo
>> +echo '=== refcount_bits and compat=0.10 ==='
>> +echo
>> +
>> +# Should work
>> +IMGOPTS="$IMGOPTS,compat=0.10,refcount_bits=16" _make_test_img 64M
> ...is it going to conflict with this explicit compat=0.10?
>
> I didn't actually try this setup, though, so if the test passes with an
> explicit imgopt request for compat=1.1, then I'm fine with it as-is.

Yes, it works; the last option given counts (see iotest 082).

> Reviewed-by: Eric Blake <eblake@redhat.com>

Thank you!

> Side note:
>
> Now that we can produce MUCH smaller images where the reftable can
> easily require enough contiguous clusters to require the creation of at
> least one refblock that cannot be self-referential, it would probably be
> good to modify an existing test and/or add a new test to prove that we
> don't trip up when trying to create and read such an image.

Reading is rarely a problem because we don't even need to read the 
refcounts then. However, creation may indeed be (or better: it should 
not be), so I will see to add a test later on.

Max

> But I'm
> fine with doing that as a later patch, so don't hold up this series for
> it (unless you want to add a 27/26).  See my mail here where I calculate
> the minimum size required to guarantee that situation at just under 256
> megabytes with 512 byte clusters:
> https://lists.gnu.org/archive/html/qemu-devel/2014-11/msg03455.html
>
> But now that I looked that up, I just realized that that email did not
> calculate what it would take to get an L1 table to likewise occupy
> enough contiguous clusters to guarantee a refblock where every entry is
> pointing to clusters of the L1 table and therefore cannot be
> self-referential.  But as both L1 and L2 table entries are 64 bits, it
> looks like the math is the same as for 64-bit width refcounts, so it
> happens to be the same magic size of just under 256 megabytes - and in
> this case, the magic value is hit even without relying on this series'
> addition of 64-bit refcount widths.
>