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

[Max Reitz <mreitz@redhat.com>]

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

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


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