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 <apahim@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)
+ 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 '')
self._load_io_log()
self._post_shutdown()
--
2.13.3
Amador Pahim <apahim@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 <apahim@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?
> + 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.
> 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().
On Tue, Aug 15, 2017 at 10:26 AM, Markus Armbruster <armbru@redhat.com> wrote:
> Amador Pahim <apahim@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 <apahim@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.
Amador Pahim <apahim@redhat.com> writes:
> On Tue, Aug 15, 2017 at 10:26 AM, Markus Armbruster <armbru@redhat.com> wrote:
>> Amador Pahim <apahim@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 <apahim@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.
Ignorant question: why aren't we cleaning it up?
>>> + 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.
On Fri, Aug 18, 2017 at 3:57 PM, Markus Armbruster <armbru@redhat.com> wrote:
> Amador Pahim <apahim@redhat.com> writes:
>
>> On Tue, Aug 15, 2017 at 10:26 AM, Markus Armbruster <armbru@redhat.com> wrote:
>>> Amador Pahim <apahim@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 <apahim@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.
>
> Ignorant question: why aren't we cleaning it up?
The "self._qemu_full_args = None" is the cleanup. Case the next
instruction fails, exception will have "None" instead of arguments
from a previous launch().
>
>>>> + 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.
© 2016 - 2026 Red Hat, Inc.