On Tue, Jul 25, 2017 at 11:17 PM, Eduardo Habkost <ehabk...@redhat.com> wrote:
> 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 <apa...@redhat.com>
>> ---
>> 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.
Ok, matter of preference. I'm ok with both.
>
>> 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.
Ok.
>
> 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).
I'd prefer to keep the original exceptions and make this a debug message.
>
>
>> 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
