Re: [Qemu-devel] [PULL 14/15] qemu.py: improve message on negative exit code

[Eduardo Habkost <ehabkost@redhat.com>]

On Mon, Sep 18, 2017 at 10:44:00AM +0100, Daniel P. Berrange wrote:
> On Fri, Sep 15, 2017 at 08:37:38PM -0300, Eduardo Habkost wrote:
> > From: Amador Pahim <apahim@redhat.com>
> > 
> > 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>
> > Message-Id: <20170901112829.2571-5-apahim@redhat.com>
> > [ehabkost: removed superfluous parenthesis]
> > Reviewed-by: Fam Zheng <famz@redhat.com>
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  scripts/qemu.py | 24 +++++++++++++++++-------
> >  1 file changed, 17 insertions(+), 7 deletions(-)
> > 
> > diff --git a/scripts/qemu.py b/scripts/qemu.py
> > index c9bcaafe41..9440261ac3 100644
> > --- a/scripts/qemu.py
> > +++ b/scripts/qemu.py
> > @@ -87,6 +87,7 @@ class QEMUMachine(object):
> >          self._socket_scm_helper = socket_scm_helper
> >          self._debug = debug
> >          self._qmp = None
> > +        self._qemu_full_args = None
> >  
> >      def __enter__(self):
> >          return self
> > @@ -186,13 +187,16 @@ 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,
> > +            self._qemu_full_args = self._wrapper + [self._binary] +
> > +                                    self._base_args() + self._args
> 
> FYI, this change causes a syntax error because you need either the ()
> brackets, or an explicit \ continuation. Unfortunately this wasn't
> caught by Peter's merge tests, since this code is only execised by
> the qemu iotests which aren't run from a 'make check'.
> 
> If someone has cycles, it would be great to write some unit tests
> directly targetting the qemu.py and qmp.py code, so that we get test
> coverage of it independantly of the iotest execution.

Oops, sorry!  I thought "make check" was exercising that code
path.

-- 
Eduardo