Re: [Qemu-devel] [PATCH for-next v3 2/5] tmp105: Add debug output

[Andreas Färber]

Am 15.02.2013 10:06, schrieb Stefan Hajnoczi:
> In iPXE they use a clever compile-time debug macro:
> https://git.ipxe.org/ipxe.git/blob/HEAD:/src/include/compiler.h#l204
> 
> Basically you do DBG("hello world\n") and it gets compiled out by
> default using:
>   if (DBG_LOG) {
>       printf("hello world\n");
>   }
> 
> Here's the really cool part, debugging can be toggled on a per-object
> file basis at compile-time:
> 
> make DEBUG=e1000  # build QEMU with debug printfs only in e1000.o
> 
> The iPXE implementation is fancier than this.  It supports different log
> levels, hex dumping, MD5 hashing, etc.
> 
> But the core idea is:
> 1. Debug printfs are always if (0 or 1) { printf(...); } so that the
>    compiler syntax checks them - no more bitrot in debug printfs.
> 2. A single debug.h header included from qemu-common.h with Makefile
>    support that lets you say "make DEBUG=e1000,rtl8139,net".
> 
> It would be a big step up from the mess we have today.

Thanks for that feedback. If you look at the previous discussion, that's
part of what I did in my RFC, and it was rejected in favor of keeping
#ifdef'able defines. Using inline functions to avoid some nasty
macro-is-not-function issues, there seemed to be no need to combine both
approaches due to the format string being checked one level above.

> Personally, I think we should use tracing instead of debug printfs
> though.  Tracing allows you to use not just fprintf(stderr) but also
> DTrace, if you like.  You can mark debug trace events "disabled" in
> ./trace-events so they will not be compiled in by default - zero
> performance overhead.

This series or patch is not against tracing. It might be an option to
use them for tmp105. But not being the author of the targets and all of
the devices my CPU refactorings potentially touch, it is infeasable for
me to determine an appropriate set of tracepoints everywhere. We'd also
need to decide whether we want per-target tracepoints or per-aspect
tracepoints, since it's a global list. Independent of that question,
some kind of include mechanism (like for the default-configs) would be
nice to decouple adding tracepoints in different areas.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg