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

[Lukáš Doktor]

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

signature.asc
Description: OpenPGP digital signature