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

[Kevin Wolf <kwolf@redhat.com>]

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:
> 1.) Add additional function calls for each DPRINTF statement?
> 2.) Add just plain DPRINTF statements?
> 3.) Or a mixture of both: on function call boundaries use Tracing, in 
> function debug info use DPRINTF?
> 4.) Refactor code that always function calls are involved?
> 
> Example:
> static void traceing_func(int mul)
> {
>      // Do nothing here
> }
> 
> // Just some dummy useless function doing illustration
> static int addandmultiply(int arg1, int arg2)
> {
>      int mul = 0;
>      int sum = arg1 + arg2;
>      DPRINTF("....", arg1, arg2); // this one can be handled by tracing 
> infrastructure
>      DPRINTF("....", sum); // this one can't be done with tracing

What's the problem? It would usually turn into something like

  trace_addandmultiply_sum(sum);

where trace_addandmultiply_sum() is a generated static inline function
in trace.h, which is empty and has zero overhead with disabled tracing.

Kevin