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

[Max Reitz]

On 2017-11-09 15:11, Eric Blake wrote:
> 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"

Although ":>" looks funny, I guess I'd rather stay with the echo...
Yes, it may look stupid to you, but I would have to look up what exactly
that will do, whereas the echo is clear.

And in the end, both work equally fine.

>>      $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")

Oh, nice!  Will do, thanks.

Max

signature.asc
Description: OpenPGP digital signature