Re: [Qemu-devel] [PATCH v5 6/6] qemu.py: include qemu command line and output on launch error

[Eduardo Habkost]

On Tue, Jul 25, 2017 at 07:10:14PM +0200, Amador Pahim wrote:
> When launching a VM, if an exception happens and the VM is not
> initiated, it is useful to see the qemu command line that was executed
> and the output of that command.
> 
> Signed-off-by: Amador Pahim <address@hidden>
> ---
>  scripts/qemu.py | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index e18e8ec657..abfa3eebe9 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -131,7 +131,8 @@ class QEMUMachine(object):
>  
>      def launch(self):
>          '''
> -        Try to launch the VM and make sure we cleanup on exception.
> +        Try to launch the VM and make sure we cleanup and expose the
> +        command line/output in case of exception.
>          '''
>          if self.is_running():
>              return
> @@ -142,18 +143,24 @@ class QEMUMachine(object):
>              self.shutdown()
>  
>          try:
> -            self._launch()
> +            args = None
> +            args = self._wrapper + [self._binary] + self._base_args() + 
> self._args
> +            self._launch(args)

I think it would be easier to set a self._full_qemu_args
attribute instead of passing data around through local variables
and method arguments.

>              self._shutdown_pending = True
>          except:
>              self.shutdown()
> +            sys.stderr.write('Error launching VM.\n%s%s' %
> +                             ('Command:\n%s\n' %
> +                              ' '.join(args) if args else '',
> +                              'Output:\n%s\n' %
> +                              ''.join(self._iolog) if self._iolog else ''))

I suggest making this conditional on self._debug.

We could also introduce a QEMULaunchError exception class
containing this info, but I'm unsure if we should raise it on
every exception, or raise more specific custom exceptions on
specific cases (e.g. one exception for QMP timeout, another for
QEMU binary not found, another for QEMU killed by a signal, etc).


>              raise
>  
> -    def _launch(self):
> +    def _launch(self, args):
>          '''Launch the VM and establish a QMP connection.'''
>          devnull = open('/dev/null', 'rb')
>          qemulog = open(self._qemu_log_path, 'wb')
>          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._post_launch()
> -- 
> 2.13.3
> 

-- 
Eduardo