Re: [PATCH v4 8/9] iotests: Filter testfiles out in img_info_log() - Vladimir Sementsov-Ogievskiy

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: Kevin Wolf <kwolf@redhat.com>, qemu-block@nongnu.org
Cc: berto@igalia.com, qemu-devel@nongnu.org, mreitz@redhat.com
Subject: Re: [PATCH v4 8/9] iotests: Filter testfiles out in img_info_log()
Date: Tue, 21 Apr 2020 14:34:09 +0300	[thread overview]
Message-ID: <19cee1b2-6a00-2fad-cbf0-7fa08b7ce0a6@virtuozzo.com> (raw)
In-Reply-To: <20200420133214.28921-9-kwolf@redhat.com>

20.04.2020 16:32, Kevin Wolf wrote:
> This changes the output of some files where instead of filter_img_info()
> now filter_testfiles() takes precedence, so update the reference output.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   tests/qemu-iotests/iotests.py |  5 ++++-
>   tests/qemu-iotests/206.out    | 12 ++++++------
>   tests/qemu-iotests/242.out    | 12 ++++++------
>   3 files changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 7bc4934cd2..3eaf29411b 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -152,7 +152,10 @@ def img_info_log(filename, filter_path=None, imgopts=False, extra_args=[]):
>       output = qemu_img_pipe(*args)
>       if not filter_path:
>           filter_path = filename
> -    log(filter_img_info(output, filter_path))
> +
> +    output = filter_testfiles(output)
> +    output = filter_img_info(output, filter_path)
> +    log(output)

Looks weird for me, because filter_img_info is used only in img_info_log, and seems to be the function, which specifically prepare img_info output for printing. So, isn't it better to improve filter_img_info itself?

Hmm, qemu_io_log do the same thing, it uses filter_testfiles, and filter_qemu_io.. So, at least you are not the first.

Still, for applying several filters, seems better to use filters=[] argument of log, like in qemu_io_log.

>   
>   def qemu_io(*args):
>       '''Run qemu-io and return the stdout data'''
> diff --git a/tests/qemu-iotests/206.out b/tests/qemu-iotests/206.out
> index 61e7241e0b..adde82f9b8 100644
> --- a/tests/qemu-iotests/206.out
> +++ b/tests/qemu-iotests/206.out
> @@ -12,7 +12,7 @@
>   {"execute": "job-dismiss", "arguments": {"id": "job0"}}
>   {"return": {}}
>   
> -image: TEST_IMG
> +image: TEST_DIR/PID-t.IMGFMT

Honestly, doesn't look as improvement.. But not a problem of course.

>   file format: IMGFMT
>   virtual size: 128 MiB (134217728 bytes)
>   cluster_size: 65536
> @@ -34,7 +34,7 @@ Format specific information:
>   {"execute": "job-dismiss", "arguments": {"id": "job0"}}
>   {"return": {}}
>   
> -image: TEST_IMG
> +image: TEST_DIR/PID-t.IMGFMT
>   file format: IMGFMT
>   virtual size: 64 MiB (67108864 bytes)
>   cluster_size: 65536
> @@ -56,7 +56,7 @@ Format specific information:
>   {"execute": "job-dismiss", "arguments": {"id": "job0"}}
>   {"return": {}}
>   
> -image: TEST_IMG
> +image: TEST_DIR/PID-t.IMGFMT
>   file format: IMGFMT
>   virtual size: 32 MiB (33554432 bytes)
>   cluster_size: 2097152
> @@ -78,11 +78,11 @@ Format specific information:
>   {"execute": "job-dismiss", "arguments": {"id": "job0"}}
>   {"return": {}}
>   
> -image: TEST_IMG
> +image: TEST_DIR/PID-t.IMGFMT
>   file format: IMGFMT
>   virtual size: 32 MiB (33554432 bytes)
>   cluster_size: 512
> -backing file: TEST_IMG.base
> +backing file: TEST_DIR/PID-t.IMGFMT.base
>   backing file format: IMGFMT
>   Format specific information:
>       compat: 0.10
> @@ -95,7 +95,7 @@ Format specific information:
>   {"execute": "job-dismiss", "arguments": {"id": "job0"}}
>   {"return": {}}
>   
> -image: TEST_IMG
> +image: TEST_DIR/PID-t.IMGFMT
>   file format: IMGFMT
>   virtual size: 32 MiB (33554432 bytes)
>   encrypted: yes
> diff --git a/tests/qemu-iotests/242.out b/tests/qemu-iotests/242.out
> index 7ac8404d11..0cc3a78bb2 100644
> --- a/tests/qemu-iotests/242.out
> +++ b/tests/qemu-iotests/242.out
> @@ -6,7 +6,7 @@ wrote 262144/262144 bytes at offset 0
>   
>   qemu-img info dump:
>   
> -image: TEST_IMG
> +image: TEST_DIR/PID-disk
>   file format: IMGFMT
>   virtual size: 1 MiB (1048576 bytes)
>   cluster_size: 65536
> @@ -26,7 +26,7 @@ wrote 262144/262144 bytes at offset 262144
>   
>   qemu-img info dump:
>   
> -image: TEST_IMG
> +image: TEST_DIR/PID-disk
>   file format: IMGFMT
>   virtual size: 1 MiB (1048576 bytes)
>   cluster_size: 65536
> @@ -58,7 +58,7 @@ wrote 262144/262144 bytes at offset 524288
>   
>   qemu-img info dump:
>   
> -image: TEST_IMG
> +image: TEST_DIR/PID-disk
>   file format: IMGFMT
>   virtual size: 1 MiB (1048576 bytes)
>   cluster_size: 65536
> @@ -98,7 +98,7 @@ Test 4
>   Checking "in-use" flag...
>   qemu-img info dump:
>   
> -image: TEST_IMG
> +image: TEST_DIR/PID-disk
>   file format: IMGFMT
>   virtual size: 1 MiB (1048576 bytes)
>   cluster_size: 65536
> @@ -143,11 +143,11 @@ Test 5
>   {"execute": "block-dirty-bitmap-add", "arguments": {"disabled": false, "granularity": 16384, "name": "bitmap-0", "node": "drive0", "persistent": true}}
>   {"return": {}}
>   Write an unknown bitmap flag '0x4' into a new QCOW2 image at offset 327695
> -qemu-img: Could not open 'TEST_IMG': Bitmap 'bitmap-0' doesn't satisfy the constraints
> +qemu-img: Could not open 'TEST_DIR/PID-disk': Bitmap 'bitmap-0' doesn't satisfy the constraints
>   
>   Unset the unknown bitmap flag '0x4' in the bitmap directory entry:
>   
> -image: TEST_IMG
> +image: TEST_DIR/PID-disk
>   file format: IMGFMT
>   virtual size: 1 MiB (1048576 bytes)
>   cluster_size: 65536
> 

-- 
Best regards,
Vladimir