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

[Amador Pahim]

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. I think it's time to
improve the logging capabilities here.

>
>
>> ---
>>  scripts/qemu.py | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/qemu.py b/scripts/qemu.py
>> index f37e2fe58e..56142ed59b 100644
>> --- a/scripts/qemu.py
>> +++ b/scripts/qemu.py
>> @@ -154,7 +154,8 @@ class QEMUMachine(object):
>>
>>              exitcode = self._popen.wait()
>>              if exitcode < 0:
>> -                sys.stderr.write('qemu received signal %i: %s\n' % 
>> (-exitcode, ' '.join(self.args)))
>> +                sys.stderr.write('qemu received signal %i\n' % -exitcode)
>> +
>>              self._load_io_log()
>>              self._post_shutdown()
>>
>> --
>> 2.13.3
>>
>
> --
> Eduardo