Re: [Qemu-devel] [PATCH v7 07/11] qemu.py: include debug information on launch error

["Lukáš Doktor" <ldoktor@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2381 bytes --]

Dne 18.8.2017 v 19:05 Amador Pahim napsal(a):
> When launching a VM, if an exception happens and the VM is not
> initiated, it might be useful to see the qemu command line and
> the qemu command output.
> 
> This patch creates that message. Notice that self._iolog needs to be
> cleaned up in the beginning of the launch() to make sure we will not
> expose the qemu log from a previous launch if the current one fails.
> 
> Signed-off-by: Amador Pahim <apahim@redhat.com>
> ---
>  scripts/qemu.py | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index 0bcec4b3b1..29fd2469f9 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -147,6 +147,7 @@ class QEMUMachine(object):
>  
>      def launch(self):
>          '''Launch the VM and establish a QMP connection'''
> +        self._iolog = None
>          self._qemu_full_args = None
>          devnull = open(os.path.devnull, 'rb')
>          qemulog = open(self._qemu_log_path, 'wb')
> @@ -162,6 +163,13 @@ class QEMUMachine(object):
>              self._post_launch()
>          except:
>              self.shutdown()
> +
> +            LOG.debug('Error launching VM')
> +            if self._qemu_full_args:
> +                LOG.debug('Command: %r', ' '.join(self._qemu_full_args))
> +            if self._iolog:
> +                LOG.debug('Output: %r', self._iolog)
> +
>              raise
>  

I don't want to open the cave, but anybody else would actually prefer custom Exception with the qemu_full_args, iolog and details about the original exception to this? Users who don't care would see/log all the details from the `exception.__str__` and those who are interested in the original exception would query for it eg. via `exception.original_exception`.

Anyway even this change is IMO step into the right direction and we can (and should) refine the use of exceptions in following patches (as Markus spotted, pure Exceptions and other ugliness lives in those waters).

As for the debug vs. error I'd actually stick to "debug", because in negative testing the logged message is only an expected debug and in positive testing it expands the knowledge of the flow, but the actual error message should come from the exception. Not a strong opinion, though...

Regards,
Lukáš

>      def shutdown(self):
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 502 bytes --]