Re: [PATCH 15/18] iotests: Make 137 work with data_file

[Max Reitz]

On 29.09.19 18:38, Maxim Levitsky wrote:
> On Fri, 2019-09-27 at 11:42 +0200, Max Reitz wrote:
>> When using an external data file, there are no refcounts for data
>> clusters.  We thus have to adjust the corruption test in this patch to
>> not be based around a data cluster allocation, but the L2 table
>> allocation (L2 tables are still refcounted with external data files).
>>
>> Doing so means this test works both with and without external data
>> files.
>>
>> Signed-off-by: Max Reitz <address@hidden>
>> ---
>>  tests/qemu-iotests/137     | 10 ++++++----
>>  tests/qemu-iotests/137.out |  4 +---
>>  2 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/137 b/tests/qemu-iotests/137
>> index 6cf2997577..dd3484205e 100755
>> --- a/tests/qemu-iotests/137
>> +++ b/tests/qemu-iotests/137
>> @@ -138,14 +138,16 @@ $QEMU_IO \
>>      "$TEST_IMG" 2>&1 | _filter_qemu_io
>>  
>>  # The dirty bit must not be set
>> -$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
>> +# (Filter the external data file bit)
>> +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features \
>> +    | sed -e 's/0x4/0x0/'
> Maybe it is better to filter all the feature bits, but the dirty bit,
> since only it is needed here, so that when we start running tests with
> more features, we won't need to do this again?

I’d hate a filter s/[02468ace]$/no dirty bit/ though.

>>  
>>  # Similarly we can test whether corruption detection has been enabled:
>> -# Create L1/L2, overwrite first entry in refcount block, allocate something.
>> +# Create L1, overwrite refcounts, force allocation of L2 by writing
>> +# data.
>>  # Disabling the checks should fail, so the corruption must be detected.
>>  _make_test_img 64M
>> -$QEMU_IO -c "write 0 64k" "$TEST_IMG" | _filter_qemu_io
>> -poke_file "$TEST_IMG" "$((0x20000))" "\x00\x00"
>> +poke_file "$TEST_IMG" "$((0x20000))" "\x00\x00\x00\x00\x00\x00\x00\x00"
> 
> I am wondering if there is any better way to do this (regardless of this 
> patch),
> Basically the above code pokes into the 3rd cluster on the disk I *think*, 
> and I don't
> understand why it has to contain refcounts. Hmm...
> First cluster I can guess will have the header, 2nd cluster probably L1 
> table, and 3rd, refcounts?
> If so, the test should specify that it needs 64K clusters, because the day 
> will come when
> we will need to test this as well, but I guess in a separate patch,

When that day comes, a whole lot of other stuff will break, too.

Max

signature.asc
Description: OpenPGP digital signature