On Fri, Feb 15, 2013 at 10:50:16AM +0100, Andreas Färber wrote: > 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.
Redefining debug functions in every file is nasty. I am also not a fan of modifying a #define, it always need to be reverted before committing changes. > > 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. Not sure I understand. You don't need to come up with new trace events in code you didn't write. DPRINTF() can be converted mechanically to trace_foo(arg1, arg). Then we get rid of all the DPRINTF() definitions. The ./trace-events list is informal and can change at any time. We don't need to coordinate between different subsystems or targets in QEMU. Stefan
