On Thu, Jul 27, 2017 at 10:21:22AM +0200, Amador Pahim wrote: > On Tue, Jul 25, 2017 at 9:51 PM, Eduardo Habkost <ehabk...@redhat.com> 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 <apa...@redhat.com> > > > > 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
