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.
Message was moved outside the 'if is_running' block to make sure it will
be logged if the VM finishes before the call to shutdown().
Signed-off-by: Amador Pahim <apahim@redhat.com>
---
scripts/qemu.py | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/scripts/qemu.py b/scripts/qemu.py
index a6e06291ea..670c048569 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -49,6 +49,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,13 +141,18 @@ class QEMUMachine(object):
def launch(self):
'''Launch the VM and establish a QMP connection'''
+ self._qemu_full_args = None
devnull = open(os.path.devnull, 'rb')
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 = (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():
@@ -164,14 +170,20 @@ class QEMUMachine(object):
self._qmp.close()
except:
self._popen.kill()
+ self._popen.wait()
- exitcode = self._popen.wait()
- if exitcode < 0:
- LOG.warn('qemu received signal %i: %s', -exitcode,
- ' '.join(self._args))
self._load_io_log()
self._post_shutdown()
+ exitcode = self.exitcode()
+ if exitcode is not None and exitcode < 0:
+ msg = 'qemu received signal %i: %s'
+ if self._qemu_full_args:
+ command = ' '.join(self._qemu_full_args)
+ else:
+ command = ''
+ LOG.warn(msg, exitcode, command)
+
underscore_to_dash = string.maketrans('_', '-')
def qmp(self, cmd, conv_keys=True, **args):
'''Invoke a QMP command and return the result dict'''
--
2.13.5
On Fri, 09/01 13:28, Amador Pahim wrote:
> 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.
>
> Message was moved outside the 'if is_running' block to make sure it will
> be logged if the VM finishes before the call to shutdown().
>
> Signed-off-by: Amador Pahim <apahim@redhat.com>
> ---
> scripts/qemu.py | 26 +++++++++++++++++++-------
> 1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index a6e06291ea..670c048569 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -49,6 +49,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,13 +141,18 @@ class QEMUMachine(object):
>
> def launch(self):
> '''Launch the VM and establish a QMP connection'''
> + self._qemu_full_args = None
> devnull = open(os.path.devnull, 'rb')
> 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 = (self._wrapper + [self._binary] +
> + self._base_args() + self._args)
The parentheses seem superfluous. With those removed:
Reviewed-by: Fam Zheng <famz@redhat.com>
> + 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():
> @@ -164,14 +170,20 @@ class QEMUMachine(object):
> self._qmp.close()
> except:
> self._popen.kill()
> + self._popen.wait()
>
> - exitcode = self._popen.wait()
> - if exitcode < 0:
> - LOG.warn('qemu received signal %i: %s', -exitcode,
> - ' '.join(self._args))
> self._load_io_log()
> self._post_shutdown()
>
> + exitcode = self.exitcode()
> + if exitcode is not None and exitcode < 0:
> + msg = 'qemu received signal %i: %s'
> + if self._qemu_full_args:
> + command = ' '.join(self._qemu_full_args)
> + else:
> + command = ''
> + LOG.warn(msg, exitcode, command)
> +
> underscore_to_dash = string.maketrans('_', '-')
> def qmp(self, cmd, conv_keys=True, **args):
> '''Invoke a QMP command and return the result dict'''
> --
> 2.13.5
>
>
Am 05.09.2017 um 05:02 hat Fam Zheng geschrieben: > On Fri, 09/01 13:28, Amador Pahim wrote: > > 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. > > > > Message was moved outside the 'if is_running' block to make sure it will > > be logged if the VM finishes before the call to shutdown(). > > > > Signed-off-by: Amador Pahim <apahim@redhat.com> > > --- > > scripts/qemu.py | 26 +++++++++++++++++++------- > > 1 file changed, 19 insertions(+), 7 deletions(-) > > > > diff --git a/scripts/qemu.py b/scripts/qemu.py > > index a6e06291ea..670c048569 100644 > > --- a/scripts/qemu.py > > +++ b/scripts/qemu.py > > @@ -49,6 +49,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,13 +141,18 @@ class QEMUMachine(object): > > > > def launch(self): > > '''Launch the VM and establish a QMP connection''' > > + self._qemu_full_args = None > > devnull = open(os.path.devnull, 'rb') > > 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 = (self._wrapper + [self._binary] + > > + self._base_args() + self._args) > > The parentheses seem superfluous. With those removed: > > Reviewed-by: Fam Zheng <famz@redhat.com> Congratulations, with this advice you just killed all Python-based qemu-iotests and filled up my inbox with CI failure messages. :-) +Traceback (most recent call last): + File "194", line 22, in <module> + import iotests + File "/home/kwolf/source/qemu/tests/qemu-iotests/iotests.py", line 27, in <module> + import qtest + File "/home/kwolf/source/qemu/tests/qemu-iotests/../../scripts/qtest.py", line 16, in <module> + import qemu + File "/home/kwolf/source/qemu/tests/qemu-iotests/../../scripts/qemu.py", line 196 + self._qemu_full_args = self._wrapper + [self._binary] + + ^ +SyntaxError: invalid syntax Kevin
On Mon, 09/18 07:18, Kevin Wolf wrote: > Am 05.09.2017 um 05:02 hat Fam Zheng geschrieben: > > On Fri, 09/01 13:28, Amador Pahim wrote: > > > 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. > > > > > > Message was moved outside the 'if is_running' block to make sure it will > > > be logged if the VM finishes before the call to shutdown(). > > > > > > Signed-off-by: Amador Pahim <apahim@redhat.com> > > > --- > > > scripts/qemu.py | 26 +++++++++++++++++++------- > > > 1 file changed, 19 insertions(+), 7 deletions(-) > > > > > > diff --git a/scripts/qemu.py b/scripts/qemu.py > > > index a6e06291ea..670c048569 100644 > > > --- a/scripts/qemu.py > > > +++ b/scripts/qemu.py > > > @@ -49,6 +49,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,13 +141,18 @@ class QEMUMachine(object): > > > > > > def launch(self): > > > '''Launch the VM and establish a QMP connection''' > > > + self._qemu_full_args = None > > > devnull = open(os.path.devnull, 'rb') > > > 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 = (self._wrapper + [self._binary] + > > > + self._base_args() + self._args) > > > > The parentheses seem superfluous. With those removed: > > > > Reviewed-by: Fam Zheng <famz@redhat.com> > > Congratulations, with this advice you just killed all Python-based > qemu-iotests and filled up my inbox with CI failure messages. :-) Oops. Why doesn't python understand such multi line expressions impllied by "+" just like in "[,]"? Iotests on patchew is not far away. (The docker test-block pull request was bounced a few times, unfortunately, for unrelated reasons) Fam
Am 18.09.2017 um 07:39 hat Fam Zheng geschrieben: > On Mon, 09/18 07:18, Kevin Wolf wrote: > > Am 05.09.2017 um 05:02 hat Fam Zheng geschrieben: > > > On Fri, 09/01 13:28, Amador Pahim wrote: > > > > 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. > > > > > > > > Message was moved outside the 'if is_running' block to make sure it will > > > > be logged if the VM finishes before the call to shutdown(). > > > > > > > > Signed-off-by: Amador Pahim <apahim@redhat.com> > > > > --- > > > > scripts/qemu.py | 26 +++++++++++++++++++------- > > > > 1 file changed, 19 insertions(+), 7 deletions(-) > > > > > > > > diff --git a/scripts/qemu.py b/scripts/qemu.py > > > > index a6e06291ea..670c048569 100644 > > > > --- a/scripts/qemu.py > > > > +++ b/scripts/qemu.py > > > > @@ -49,6 +49,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,13 +141,18 @@ class QEMUMachine(object): > > > > > > > > def launch(self): > > > > '''Launch the VM and establish a QMP connection''' > > > > + self._qemu_full_args = None > > > > devnull = open(os.path.devnull, 'rb') > > > > 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 = (self._wrapper + [self._binary] + > > > > + self._base_args() + self._args) > > > > > > The parentheses seem superfluous. With those removed: > > > > > > Reviewed-by: Fam Zheng <famz@redhat.com> > > > > Congratulations, with this advice you just killed all Python-based > > qemu-iotests and filled up my inbox with CI failure messages. :-) > > Oops. Why doesn't python understand such multi line expressions > impllied by "+" just like in "[,]"? To be honest, I didn't know that either until now. But apparently the different kinds of brackets (including those around function arguments) are the only things that allow multiline expressions in Python. > Iotests on patchew is not far away. (The docker test-block pull > request was bounced a few times, unfortunately, for unrelated reasons) Nice! Kevin
On Mon, 09/18 08:44, Kevin Wolf wrote: > > > > > + self._qemu_full_args = (self._wrapper + [self._binary] + > > > > > + self._base_args() + self._args) > > > > > > > > The parentheses seem superfluous. With those removed: > > > > > > > > Reviewed-by: Fam Zheng <famz@redhat.com> > > > > > > Congratulations, with this advice you just killed all Python-based > > > qemu-iotests and filled up my inbox with CI failure messages. :-) > > > > Oops. Why doesn't python understand such multi line expressions > > impllied by "+" just like in "[,]"? > > To be honest, I didn't know that either until now. But apparently the > different kinds of brackets (including those around function arguments) > are the only things that allow multiline expressions in Python. Yes, and now I read that PEP recommends using brackets for multilines compared to backslashes. I was totally wrong with this advice. Fam
© 2016 - 2025 Red Hat, Inc.