Re: [Qemu-block] [Qemu-devel] [PATCH 4/5] iotests: Make 083 less flaky

[Eric Blake]

On 11/08/2017 07:38 PM, Max Reitz wrote:
> 083 has (at least) two issues:

I think I hit one of them intermittently yesterday; thanks for
diagnosing these (and like you say, there may be more lurking, but we'll
whack them separately if we can reproduce and identify them).

> 
> 1. By launching the nbd-fault-injector in background, it may not be
>    scheduled until the first grep on its output file is executed.
>    However, until then, that file may not have been created yet -- so it
>    either does not exist yet (thus making the grep emit an error), or it
>    does exist but contains stale data (thus making the rest of the test
>    case work connect to a wrong address).
>    Fix this by explicitly overwriting the output file before executing
>    nbd-fault-injector.
> 
> 2. The nbd-fault-injector prints things other than "Listening on...".
>    It also prints a "Closing connection" message from time to time.  We
>    currently invoke sed on the whole file in the hope of it only
>    containing the "Listening on..." line yet.  That hope is sometimes
>    shattered by the brutal reality of race conditions, so invoke grep
>    before sed.

Invoking 'grep | sed' is almost always a waste of a process; sed can do
the job alone.

> 
> Signed-off-by: Max Reitz <address@hidden>
> ---
>  tests/qemu-iotests/083 | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/083 b/tests/qemu-iotests/083
> index 0306f112da..2f6444eeb9 100755
> --- a/tests/qemu-iotests/083
> +++ b/tests/qemu-iotests/083
> @@ -86,6 +86,7 @@ EOF
>  
>       rm -f "$TEST_DIR/nbd.sock"
>  
> +        echo > "$TEST_DIR/nbd-fault-injector.out"

This makes the file contain a blank line.  Would it be any better as a
truly empty file, as in:

: > "$TEST_DIR/nbd-fault-injector.out"

>       $PYTHON nbd-fault-injector.py $extra_args "$nbd_addr" 
> "$TEST_DIR/nbd-fault-injector.conf" >"$TEST_DIR/nbd-fault-injector.out" 2>&1 &
>  
>       # Wait for server to be ready
> @@ -94,7 +95,7 @@ EOF
>       done
>  
>       # Extract the final address (port number has now been assigned in tcp 
> case)
> -     nbd_addr=$(sed 's/Listening on \(.*\)$/\1/' 
> "$TEST_DIR/nbd-fault-injector.out")
> +        nbd_addr=$(grep 'Listening on ' "$TEST_DIR/nbd-fault-injector.out" | 
> sed 's/Listening on \(.*\)$/\1/')

Fixing TAB damage while at it - nice.

Here's how to do it using just sed, and with less typing:

    nbd_addr=$(sed -n 's/^Listening on //p' \
               "$TEST_DIR/nbd-fault-injector.out")

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

signature.asc
Description: OpenPGP digital signature