On Tue, Aug 15, 2017 at 10:26 AM, Markus Armbruster <arm...@redhat.com> wrote:
> Amador Pahim <apa...@redhat.com> writes:
>
>> The current message shows 'self._args', which contains only part of the
>> options used in the qemu command line.
>>
>> This patch makes the qemu full args list an instance variable and then
>> uses it in the negative exit code message.
>>
>> Signed-off-by: Amador Pahim <apa...@redhat.com>
>> ---
>> scripts/qemu.py | 17 ++++++++++++-----
>> 1 file changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/scripts/qemu.py b/scripts/qemu.py
>> index e3ea534ec4..9434ccc30b 100644
>> --- a/scripts/qemu.py
>> +++ b/scripts/qemu.py
>> @@ -48,6 +48,7 @@ class QEMUMachine(object):
>> self._iolog = None
>> self._socket_scm_helper = socket_scm_helper
>> self._debug = debug
>> + self._qemu_full_args = None
>>
>> # This can be used to add an unused monitor instance.
>> def add_monitor_telnet(self, ip, port):
>> @@ -140,9 +141,14 @@ class QEMUMachine(object):
>> qemulog = open(self._qemu_log_path, 'wb')
>> try:
>> self._pre_launch()
>> - args = self._wrapper + [self._binary] + self._base_args() +
>> self._args
>> - self._popen = subprocess.Popen(args, stdin=devnull,
>> stdout=qemulog,
>> - stderr=subprocess.STDOUT,
>> shell=False)
>> + self._qemu_full_args = None
>> + self._qemu_full_args = (self._wrapper + [self._binary] +
>> + self._base_args() + self._args)
>
> Why set self._qemu_full_args twice?
If it's not cleaned up and an exception happens in
"self._qemu_full_args = (self._wrapper ...", the message logged in the
"except" will expose an outdated version of it.
>
>> + self._popen = subprocess.Popen(self._qemu_full_args,
>> + stdin=devnull,
>> + stdout=qemulog,
>> + stderr=subprocess.STDOUT,
>> + shell=False)
>> self._post_launch()
>> except:
>> if self.is_running():
>> @@ -163,8 +169,9 @@ class QEMUMachine(object):
>>
>> exitcode = self._popen.wait()
>> if exitcode < 0:
>> - LOG.error('qemu received signal %i: %s', -exitcode,
>> - ' '.join(self._args))
>> + LOG.error('qemu received signal %i:%s', -exitcode,
>> + ' Command: %r.' % ' '.join(self._qemu_full_args)
>> + if self._qemu_full_args else '')
>
> The last argument is hard to read.
Ok, I'm making it easier.
>
>> self._load_io_log()
>> self._post_shutdown()
>
> The subprocess module appears to keep track of the full command with
> methods check_call() and check_output(): its in exception
> CalledProcessError right when you need it. Sadly, it doesn't appear to
> be tracked with method Popen().
Indeed.
