Re: [Qemu-devel] [PATCH v5 3/6] qemu.py: cleanup message on negative exit code

[Eduardo Habkost]

On Thu, Jul 27, 2017 at 10:21:22AM +0200, Amador Pahim wrote:
> On Tue, Jul 25, 2017 at 9:51 PM, Eduardo Habkost <address@hidden> wrote:
> > On Tue, Jul 25, 2017 at 07:10:11PM +0200, Amador Pahim wrote:
> >> The message contains the self._args, which has only part of the
> >> options used in the qemu command line and is not representative
> >> enough to figure out what happened to the process.
> >>
> >> This patch drops the self._args part of the message.
> >>
> >> Signed-off-by: Amador Pahim <address@hidden>
> >
> > I actually think it is a very useful debugging message as is,
> > because the command-line arguments are often all we need to
> > reproduce a QEMU crash.
> 
> The message currently contains only part of the args, not all
> (base_args are not included). Let's include the full command then.
> 
> >
> > That said, sys.stderr.write doesn't belong to the QEMUMachine
> > code, as callers should decide if/when/how/where to print
> > information about a QEMU crash.
> >
> > I think a QEMUCrashed exception class would be the best way to
> > report that to callers.  Including the full QEMU command-line on
> > the exception __str__ method would make it helpful when debugging
> > crashes: existing code that doesn't catch launch() exceptions
> > will crash with a more helpful stack trace, and code that already
> > catches exceptions is probably going to print exception info
> > somewhere.
> 
> I agree using sys.stderr.write should be avoided, but I'm not
> convinced this message should raise an exception. [...]

No problem, we can discuss later when/how to raise exceptions to
indicate specific error cases.

We could make the log message conditional on self._debug by now,
but I don't think it will be a problem if we keep it
unconditional (as QEMU crashes are not supposed to happen under
normal circumstances).

>                                             [...] I think it's time to
> improve the logging capabilities here.

What about using the Python logging module?

-- 
Eduardo