Re: [Qemu-devel] [RFC] Introduce qemu_abort?

[Markus Armbruster]

Peter Maydell peter.mayd...@linaro.org writes:

 On 26 October 2011 19:34, Stefan Weil s...@weilnetz.de wrote:
 Adding the infrastructure (macros / implementation) could be done
 faster. I suggest these interfaces in qemu-common.h:

 qemu_abort() - abort QEMU with a message containing function name,
 file name and line (macro, see message text in my previous mail cited above)

 qemu_fatal(formatstring, ...) - abort QEMU with a printf like message
 (function, prints QEMU aborted,  and the text according to the parameters)

 We already have a couple of print a message and abort functions:
   hw_error()  (prints qemu: hardware error message, dumps cpu state
for all cores and abort()s)
   cpu_abort() (prints qemu: fatal: message, dumps cpu state for one
core and abort()s)
   tcg_abort() (takes no arguments, acts like your proposed qemu_abort())

 ...perhaps we should try to straighten out the naming inconsistencies
 here and document which ones to use when.

Yes, that would be useful.

 I think the qemu_fatal() might be better addressed as part of a set
 of functions for handling fatal and other errors in a more consistent
 way. At the moment it's a bit random whether a device model will
 abort, warn but continue or silently continue if a guest does something
 that tickles unimplemented behaviour or does something that's probably
 a guest error (eg accessing nonexistent registers). It would be good
 to have some of this be user configurable (some people want to see
 my guest OS is doing something that's probably a guest OS bug warnings,
 some don't, for instance).

Yes.

assert() and abort() are strictly for programming errors: the programmer
screwed up, program is in disorder, and can't continue.  They're not
appropriate ways to handle environmental conditions that can be expected
(out of memory, network kaputt, ...), or bad input.  For QEMU, input
includes guest actions.

I generally don't bother to provide nice ways to abort on programming
error.  assert() is standard, and it's just fine.  To do anything about
it, I'll need a debugger anyway.  If you don't like abort() not telling
you function and line, use assert().

That said, if you want to wrap around yet another standard function, go
right ahead, one more won't make a difference.

 Regarding whether to put this into qemu 1.0 or not -- my preference
 would be for 'not' because any change touching a lot of files has
 the potential to cause pending patches/pullreqs people are trying
 to get in before the feature freeze deadline to no longer apply.

Agree.

[Qemu-devel] [RFC] Introduce qemu_abort? (was: Fix compiler warning (always return a value))

[Stefan Weil]

Am 26.10.2011 19:49, schrieb Blue Swirl:

On Wed, Oct 26, 2011 at 16:35, Stefan Weil s...@weilnetz.de wrote:

...
I personally don't like abort() because it does not show the
reason for the failure.

Most users don't know how to get a core dump or how to
use gdb. And even for those who know, a crash caused
by an abort() which cannot be reproduced usually happens
on a system were ulimit disables core dumps...

I'd like to have a qemu_abort() macro in qemu-common.h which
replaces all abort() calls used today:


Also assert(0) calls.


#define qemu_abort() \
 do { \
   fprintf(stderr, QEMU aborted in %s, %s:%u\n, __func__, __FILE__,
__LINE__);
   abort();
 } while (0)

(The macro could also call a function which handles fprintf and abort).


There could be also a version with additional error message parameter.


Replacing abort() and assert(0) by qemu_abort() touches a lot of files.
Do you think this can be a change for QEMU 1.0, or is it better
to wait?

Adding the infrastructure (macros / implementation) could be done
faster. I suggest these interfaces in qemu-common.h:

qemu_abort() - abort QEMU with a message containing function name,
file name and line (macro, see message text in my previous mail cited above)

qemu_fatal(formatstring, ...) - abort QEMU with a printf like message
(function, prints QEMU aborted,  and the text according to the parameters)

Regards,
Stefan W.

Re: [Qemu-devel] [RFC] Introduce qemu_abort? (was: Fix compiler warning (always return a value))

[Blue Swirl]

On Wed, Oct 26, 2011 at 18:34, Stefan Weil s...@weilnetz.de wrote:
 Am 26.10.2011 19:49, schrieb Blue Swirl:

 On Wed, Oct 26, 2011 at 16:35, Stefan Weil s...@weilnetz.de wrote:

 ...
 I personally don't like abort() because it does not show the
 reason for the failure.

 Most users don't know how to get a core dump or how to
 use gdb. And even for those who know, a crash caused
 by an abort() which cannot be reproduced usually happens
 on a system were ulimit disables core dumps...

 I'd like to have a qemu_abort() macro in qemu-common.h which
 replaces all abort() calls used today:

 Also assert(0) calls.

 #define qemu_abort() \
  do { \
   fprintf(stderr, QEMU aborted in %s, %s:%u\n, __func__, __FILE__,
 __LINE__);
   abort();
  } while (0)

 (The macro could also call a function which handles fprintf and abort).

 There could be also a version with additional error message parameter.

 Replacing abort() and assert(0) by qemu_abort() touches a lot of files.
 Do you think this can be a change for QEMU 1.0, or is it better
 to wait?

It shouldn't destabilize anything since we are going to abort anyway.

 Adding the infrastructure (macros / implementation) could be done
 faster. I suggest these interfaces in qemu-common.h:

 qemu_abort() - abort QEMU with a message containing function name,
 file name and line (macro, see message text in my previous mail cited above)

 qemu_fatal(formatstring, ...) - abort QEMU with a printf like message
 (function, prints QEMU aborted,  and the text according to the parameters)

This function could also be added, though I'd leave actual conversions
to 1.1, in case printfs turn out dangerous.

Re: [Qemu-devel] [RFC] Introduce qemu_abort? (was: Fix compiler warning (always return a value))

[Peter Maydell]

On 26 October 2011 19:34, Stefan Weil s...@weilnetz.de wrote:
 Adding the infrastructure (macros / implementation) could be done
 faster. I suggest these interfaces in qemu-common.h:

 qemu_abort() - abort QEMU with a message containing function name,
 file name and line (macro, see message text in my previous mail cited above)

 qemu_fatal(formatstring, ...) - abort QEMU with a printf like message
 (function, prints QEMU aborted,  and the text according to the parameters)

We already have a couple of print a message and abort functions:
  hw_error()  (prints qemu: hardware error message, dumps cpu state
   for all cores and abort()s)
  cpu_abort() (prints qemu: fatal: message, dumps cpu state for one
   core and abort()s)
  tcg_abort() (takes no arguments, acts like your proposed qemu_abort())

...perhaps we should try to straighten out the naming inconsistencies
here and document which ones to use when.

I think the qemu_fatal() might be better addressed as part of a set
of functions for handling fatal and other errors in a more consistent
way. At the moment it's a bit random whether a device model will
abort, warn but continue or silently continue if a guest does something
that tickles unimplemented behaviour or does something that's probably
a guest error (eg accessing nonexistent registers). It would be good
to have some of this be user configurable (some people want to see
my guest OS is doing something that's probably a guest OS bug warnings,
some don't, for instance).

Regarding whether to put this into qemu 1.0 or not -- my preference
would be for 'not' because any change touching a lot of files has
the potential to cause pending patches/pullreqs people are trying
to get in before the feature freeze deadline to no longer apply.

-- PMM