Re: [Qemu-devel] [PATCH v8 04/13] qemu.py: improve message on negative exit code

[Kevin Wolf]

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 <address@hidden>
> > ---
> >  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 <address@hidden>

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