Re: [PATCH 06/14] iotest 302: use img_info_log() helper

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

16.07.2021 14:39, Max Reitz wrote:
> On 05.07.21 11:15, Vladimir Sementsov-Ogievskiy wrote:
>> Instead of qemu_img_log("info", ..) use generic helper img_info_log().
>>
>> img_info_log() has smarter logic. For example it use filter_img_info()
>> to filter output, which in turns filter a compression type. So it will
>> help us in future when we implement a possibility to use zstd
>> compression by default (with help of some runtime config file or maybe
>> build option). For now to test you should recompile qemu with a small
>> patch:
>>
>>      --- a/block/qcow2.c
>>      +++ b/block/qcow2.c
>>      @@ -3540,6 +3540,11 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
>>               }
>>           }
>>
>>      +    if (!qcow2_opts->has_compression_type && version >= 3) {
>>      +        qcow2_opts->has_compression_type = true;
>>      +        qcow2_opts->compression_type = QCOW2_COMPRESSION_TYPE_ZSTD;
>>      +    }
>>      +
>>           if (qcow2_opts->has_compression_type &&
>>               qcow2_opts->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB) {
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/302     | 3 ++-
>>   tests/qemu-iotests/302.out | 7 +++----
>>   2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/302 b/tests/qemu-iotests/302
>> index 5695af4914..2180dbc896 100755
>> --- a/tests/qemu-iotests/302
>> +++ b/tests/qemu-iotests/302
>> @@ -34,6 +34,7 @@ from iotests import (
>>       qemu_img_measure,
>>       qemu_io,
>>       qemu_nbd_popen,
>> +    img_info_log,
>>   )
>>   iotests.script_initialize(supported_fmts=["qcow2"])
>> @@ -99,7 +100,7 @@ with tarfile.open(tar_file, "w") as tar:
>>               nbd_uri)
>>           iotests.log("=== Converted image info ===")
>> -        qemu_img_log("info", nbd_uri)
>> +        img_info_log(nbd_uri)
> 
> There’s another `qemu_img_log("info", nbd_uri)` call above this place. We can’t use `img_info_log()` there, because in that case, the image is not in qcow2 format (which is the test’s image format), but `img_info_log()` enforces “-f {imgfmt}”.  It would have been nice to have a comment on that somewhere, though.

I'll add a comment. Actually, I only fixed places which breaks when I set zstd by default. That's why some things may be not covered.

> 
> But, well.
> 
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> 
> (And speaking in principle, I don’t think I like the broad img_info_log() very much anyway, because I feel like tests should rather only have the actually relevant bits in their reference outputs…)

I agree, extra useless information in test outputs is always a pain.. We should pay more attention to it when add new tests.

> 
>>           iotests.log("=== Converted image check ===")
>>           qemu_img_log("check", nbd_uri)
>> diff --git a/tests/qemu-iotests/302.out b/tests/qemu-iotests/302.out
>> index e2f6077e83..3e7c281b91 100644
>> --- a/tests/qemu-iotests/302.out
>> +++ b/tests/qemu-iotests/302.out
>> @@ -6,14 +6,13 @@ virtual size: 448 KiB (458752 bytes)
>>   disk size: unavailable
>>   === Converted image info ===
>> -image: nbd+unix:///exp?socket=SOCK_DIR/PID-nbd-sock
>> -file format: qcow2
>> +image: TEST_IMG
>> +file format: IMGFMT
>>   virtual size: 1 GiB (1073741824 bytes)
>> -disk size: unavailable
>>   cluster_size: 65536
>>   Format specific information:
>>       compat: 1.1
>> -    compression type: zlib
>> +    compression type: COMPRESSION_TYPE
>>       lazy refcounts: false
>>       refcount bits: 16
>>       corrupt: false
> 


-- 
Best regards,
Vladimir