Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] iotests: Warn if python subprocess is killed

[John Snow <jsnow@redhat.com>]

On 09/08/2015 05:38 PM, Max Reitz wrote:
> On 08.09.2015 23:37, John Snow wrote:
>> 
>> 
>> On 09/08/2015 05:29 PM, Max Reitz wrote:
>>> On 08.09.2015 23:25, John Snow wrote:
>>>> 
>>>> 
>>>> On 08/31/2015 03:05 PM, Max Reitz wrote:
>>>>> Currently, if a subprocess of a python test (i.e. qemu-io, 
>>>>> qemu-img, or qemu) receives a signal and is subsequently 
>>>>> aborted, this is not logged.
>>>>> 
>>>>> This patch makes python tests always check the exit code
>>>>> of these subprocesses, and emit a message if they have
>>>>> been killed.
>>>>> 
>>>>> Signed-off-by: Max Reitz <mreitz@redhat.com> --- 
>>>>> tests/qemu-iotests/iotests.py | 26
>>>>> +++++++++++++++++++++----- 1 file changed, 21
>>>>> insertions(+), 5 deletions(-)
>>>>> 
>>>>> diff --git a/tests/qemu-iotests/iotests.py 
>>>>> b/tests/qemu-iotests/iotests.py index 927c74a..d082597
>>>>> 100644 --- a/tests/qemu-iotests/iotests.py +++ 
>>>>> b/tests/qemu-iotests/iotests.py @@ -49,20 +49,34 @@ 
>>>>> socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 
>>>>> 'socket_scm_helper') def qemu_img(*args): '''Run qemu-img
>>>>> and return the exit code''' devnull = open('/dev/null',
>>>>> 'r+') - return subprocess.call(qemu_img_args + list(args), 
>>>>> stdin=devnull, stdout=devnull) +    exitcode = 
>>>>> subprocess.call(qemu_img_args + list(args), stdin=devnull, 
>>>>> stdout=devnull) +    if exitcode < 0: + 
>>>>> sys.stderr.write('qemu-img received signal %i: %s\n' % 
>>>>> (-exitcode, ' '.join(qemu_img_args + list(args)))) +
>>>>> return exitcode
>>>>> 
>>>> 
>>>> Why tack on the arguments after the retcode for the print?
>>>> The format makes it look like it should be a description for
>>>> the signal received.
>>> 
>>> qemu_img_args contains the qemu-img filename as well, so it
>>> should be obvious that that is the command line.
>>> 
>>> For the "why": I hope it will make debugging easier by
>>> providing the exact command line so you can reproduce the
>>> problem outside of the test.
>>> 
>>> Max
>>> 
>> 
>> Sorry for being captain bikeshed: can this be something like:
>> 
>> "%s received signal %i; args: %s"
>> 
>> to avoid the string looking like it was meant to answer the
>> question of what the signal was? I won't insist on it, though:
>> 
>> Reviewed-by: John Snow <jsnow@redhat.com>
> 
> Well, feel free to fix it since it's already in master. :-)
> 
> (also, compare how bash does it:
>> $FILE: line $LINE: $PID $SIGNAL_NAME (core dumped) ($ARGS)
> )
> 
> Max
> 

ugh! pfeh! didn't see the v2, sorry. ignore this garbage.

--js