[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-next v3 2/5] tmp105: Add debug output
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH for-next v3 2/5] tmp105: Add debug output |
Date: |
Fri, 15 Feb 2013 13:51:00 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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
- [Qemu-devel] [PATCH for-next v3 0/5] qtest: tmp105 cleanups and MMIO support, Andreas Färber, 2013/02/11
- [Qemu-devel] [PATCH for-next v3 2/5] tmp105: Add debug output, Andreas Färber, 2013/02/11
- Re: [Qemu-devel] [PATCH for-next v3 2/5] tmp105: Add debug output, Andreas Färber, 2013/02/14
- Re: [Qemu-devel] [PATCH for-next v3 2/5] tmp105: Add debug output, Stefan Hajnoczi, 2013/02/15
- Re: [Qemu-devel] [PATCH for-next v3 2/5] tmp105: Add debug output, Andreas Färber, 2013/02/15
- Re: [Qemu-devel] [PATCH for-next v3 2/5] tmp105: Add debug output,
Stefan Hajnoczi <=
- Re: [Qemu-devel] [PATCH for-next v3 2/5] tmp105: Add debug output, Andreas Färber, 2013/02/15
- Re: [Qemu-devel] [PATCH for-next v3 2/5] tmp105: Add debug output, Alexander Graf, 2013/02/15
- Re: [Qemu-devel] [PATCH for-next v3 2/5] tmp105: Add debug output, Andreas Färber, 2013/02/16
- Re: [Qemu-devel] [PATCH for-next v3 2/5] tmp105: Add debug output, Alexander Graf, 2013/02/25
- Re: [Qemu-devel] [PATCH for-next v3 2/5] tmp105: Add debug output, Paolo Bonzini, 2013/02/17
[Qemu-devel] [PATCH for-next v3 4/5] libqtest: Introduce qtest_qmpv() and convert remaining macro, Andreas Färber, 2013/02/11
[Qemu-devel] [PATCH for-next v3 3/5] libqtest: Convert macros to functions and clean up documentation, Andreas Färber, 2013/02/11
[Qemu-devel] [PATCH for-next v3 1/5] tmp105-test: Combine assertions of 16-bit responses, Andreas Färber, 2013/02/11
[Qemu-devel] [PATCH for-next v3 5/5] qtest: Add MMIO support, Andreas Färber, 2013/02/11