Re: [Qemu-devel] [PATCH] block.c, block/vmdk.c: Fixed major bug in VMDK WRITE and READ handling - FIXES DATA CORRUPTION

[Stefan Hajnoczi <stefanha@gmail.com>]

On Tue, Nov 13, 2012 at 01:27:57PM +0100, Kevin Wolf wrote:
> Am 10.11.2012 10:59, schrieb Gerhard Wiesinger:
> > On 10.11.2012 09:55, Paolo Bonzini wrote:
> >> Il 10/11/2012 09:30, Gerhard Wiesinger ha scritto:
> >>>>> 2.) Added debug code to block.c and to block/vmdk.c to verify
> >>>>> correctness
> >>>> Same here.  Also, please use the tracing infrastructure---a lot of the
> >>>> debug
> >>>> messages you're adding, though not all, are in fact already available
> >>>> (not
> >>>> saying the others aren't useful!)
> >>> Any chance that the patch with debug code only (after some cleaning)
> >>> would be accepted (other modules do debug logging, too)?
> >>> I  don't like to do useless work.
> >>> Tracing infrastructure is quite limited to function calls only (as far
> >>> as I saw).
> >> No, tracing infrastructure uses function calls for tracing (messages go
> >> into trace-events) but you can apply it to everything you want.  Use the
> >> stderr backend to debug it.
> > 
> > Tracing is a good thing for normal behavior but the major limitation is 
> > that a function call must be involved. But for deep debugging one needs 
> > a lot of more messages than function calls are available.
> > 
> > Of course every DPRINTF line could be made in a function call but IHMO 
> > this introduces unnecessary overhead in performance.
> > 
> > So how to proceed further, some options:

QEMU tracing has a solution for this, use the *_ENABLED macro to
determine at compile-time whether tracing code should be compiled in or
not:

if (TRACE_MY_EXPENSIVE_THING_ENABLED) {
    const char *pi = calculate_pi_to_10000000000_decimal_places();
    trace_my_expensive_thing(pi);
}

If the trace event is marked with "disable" in trace-events, then
TRACE_MY_EXPENSIVE_THING_ENABLED will be 0 and the compiler will drop
the dead code.  This way you can put expensive debug-only tracing into
QEMU without affecting production builds - you do lose the advantage of
being able to toggle "disabled" trace events at runtime, but it still
beats DPRINTF().

Stefan