Re: [Qemu-devel] [PATCH for-2.10] iotests: 109: Filter out "len" of failed jobs

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2516 bytes --]

On 04/18/2017 02:59 AM, Fam Zheng wrote:
> Mirror calculates job len from current I/O progress:
> 
>     s->common.len = s->common.offset +
>                     (cnt + s->sectors_in_flight) * BDRV_SECTOR_SIZE;
> 
> The final "len" of a failed mirror job in iotests 109 depends on the
> subtle timing of the completion of read and write issued in the first
> mirror iteration.  The second iteration may or may not have run when the
> I/O error happens, resulting in an undeterministic output of the

perhaps s/an undeterministic/non-deterministic/

> BLOCK_JOB_COMPLETED event text.

I can definitely reproduce hitting those issues (often with ./check -raw
109).

> 
> Similar to what was done in a752e4786, filter out the field to make the
> test robust.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  tests/qemu-iotests/109           |  3 ++-
>  tests/qemu-iotests/109.out       | 12 ++++++------
>  tests/qemu-iotests/common.filter |  6 ++++++
>  3 files changed, 14 insertions(+), 7 deletions(-)
> 

> diff --git a/tests/qemu-iotests/109 b/tests/qemu-iotests/109
> index 927151a..6d61cf1 100755
> --- a/tests/qemu-iotests/109
> +++ b/tests/qemu-iotests/109
> @@ -80,7 +80,8 @@ for fmt in qcow qcow2 qed vdi vmdk vpc; do
>  
>      # This first test should fail: The image format was probed, we may not
>      # write an image header at the start of the image
> -    run_qemu "$TEST_IMG" "$TEST_IMG.src" "" "BLOCK_JOB_ERROR"
> +    run_qemu "$TEST_IMG" "$TEST_IMG.src" "" "BLOCK_JOB_ERROR" | \

Technically, \ after | is not necessary; but it doesn't hurt to leave it
in (shell grammar is weird).

> +++ b/tests/qemu-iotests/common.filter
> @@ -92,6 +92,12 @@ _filter_block_job_offset()
>      sed -e 's/, "offset": [0-9]\+,/, "offset": OFFSET,/'
>  }
>  
> +# replace block job len
> +_filter_block_job_len()
> +{
> +    sed -e 's/, "len": [0-9]\+,/, "len": LEN,/'

Right now, we only have one "len" per line; but do you want to add the g
flag (s/.../.../g), for future robustness?

Use of \+ is not POSIX, but it is portable enough in practice (at any
rate, we already have several other tests using it), and sure beats the
more portable alternative of [0-9][0-9]* in conciseness.

So whether or not you add the 'g',

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

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]