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: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>

>> 
>>> def qemu_img_verbose(*args): '''Run qemu-img without
>>> suppressing its output and return the exit code''' -    return
>>> subprocess.call(qemu_img_args + list(args)) +    exitcode =
>>> subprocess.call(qemu_img_args + list(args)) +    if exitcode <
>>> 0: +        sys.stderr.write('qemu-img received signal %i:
>>> %s\n' % (-exitcode, ' '.join(qemu_img_args + list(args)))) +
>>> return exitcode
>>> 
>>> def qemu_img_pipe(*args): '''Run qemu-img and return its
>>> output''' -    return subprocess.Popen(qemu_img_args +
>>> list(args), stdout=subprocess.PIPE).communicate()[0] +    subp
>>> = subprocess.Popen(qemu_img_args + list(args),
>>> stdout=subprocess.PIPE) +    exitcode = subp.wait() +    if
>>> exitcode < 0: +        sys.stderr.write('qemu-img received
>>> signal %i: %s\n' % (-exitcode, ' '.join(qemu_img_args +
>>> list(args)))) +    return subp.communicate()[0]
>>> 
>>> def qemu_io(*args): '''Run qemu-io and return the stdout
>>> data''' args = qemu_io_args + list(args) -    return
>>> subprocess.Popen(args,
>>> stdout=subprocess.PIPE).communicate()[0] +    subp =
>>> subprocess.Popen(args, stdout=subprocess.PIPE) +    exitcode =
>>> subp.wait() +    if exitcode < 0: +
>>> sys.stderr.write('qemu-io received signal %i: %s\n' %
>>> (-exitcode, ' '.join(args))) +    return subp.communicate()[0]
>>> 
>>> def compare_images(img1, img2): '''Return True if two image
>>> files are identical''' @@ -197,7 +211,9 @@ class VM(object): 
>>> '''Terminate the VM and clean up''' if not self._popen is
>>> None: self._qmp.cmd('quit') -            self._popen.wait() +
>>> exitcode = self._popen.wait() +            if exitcode < 0: +
>>> sys.stderr.write('qemu received signal %i: %s\n' % (-exitcode,
>>> ' '.join(self._args))) os.remove(self._monitor_path) 
>>> os.remove(self._qtest_path) os.remove(self._qemu_log_path)
>>> 
> 
>