Re: [PATCH 7/7] iotests/271: check disk usage on subcluster-based discard/unmap

[Hanna Czenczek <hreitz@redhat.com>]

On 03.11.23 16:51, Hanna Czenczek wrote:
> On 20.10.23 23:56, Andrey Drobyshev wrote: 

[...]

>> @@ -528,6 +543,14 @@ for use_backing_file in yes no; do
>>       else
>>           _make_test_img -o extended_l2=on 1M
>>       fi
>> +    # Write cluster #0 and discard its subclusters #0-#3
>> +    $QEMU_IO -c 'write -q 0 64k' "$TEST_IMG"
>> +    before=$(disk_usage "$TEST_IMG")
>> +    $QEMU_IO -c 'discard -q 0 8k' "$TEST_IMG"
>> +    after=$(disk_usage "$TEST_IMG")
>> +    _verify_du_delta $before $after 8192
>> +    alloc="$(seq 4 31)"; zero="$(seq 0 3)"
>> +    _verify_l2_bitmap 0
>>       # Write clusters #0-#2 and then discard them
>>       $QEMU_IO -c 'write -q 0 128k' "$TEST_IMG"
>>       $QEMU_IO -c 'discard -q 0 128k' "$TEST_IMG"
>
> Similarly to above, I think it would be good if we combined this 
> following case with the one you added, i.e. to write 128k from the 
> beginning, drop the write here, and change the discard to be “discard 
> -q 8k 120k”, i.e. skip the subclusters we have already discarded, to 
> see that this is still combined to discard the whole first cluster.
>
> ...Ah, see, and when I try this, the following assertion fails:
>
> qemu-io: ../block/qcow2-cache.c:156: qcow2_cache_destroy: Assertion 
> `c->entries[i].ref == 0' failed.
> ./common.rc: line 220: 128894 Aborted                 (core dumped) ( 
> VALGRIND_QEMU="${VALGRIND_QEMU_IO}" _qemu_proc_exec 
> "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>
> Looks like an L2 table is leaked somewhere.  That’s why SCRI should be 
> a g_auto()-able type.

Forgot to add: This single test case here is the only place where we 
test the added functionality.  I think there should be more cases. It 
doesn’t really make sense now that 271 has so many cases for writing 
zeroes, but so few for discarding, now that discarding works on 
subclusters.  Most of them should at least be considered whether we can 
run them for discard as well.

I didn’t want to push for such an extensive set of tests, but, well, now 
it turned out I overlooked a bug in patch 4, and only found it because I 
thought “this place might also make a nice test case for this series”.

Hanna