Re: [Qemu-devel] [PATCH 1/9] IDE: replace DEBUG_IDE with tracing system

[Eric Blake]

On 08/08/2017 01:32 PM, John Snow wrote:
> Out with the old, in with the new.
> 
> Signed-off-by: John Snow <address@hidden>
> ---

>  hw/ide/piix.c             | 11 ++++----
>  hw/ide/trace-events       | 33 ++++++++++++++++++++++++
>  hw/ide/via.c              | 10 +++-----

Hmm - should we tweak scripts/git.orderfile to prioritize trace-events
over .c files? Then again, right now it prioritizes all .c files before
anything that didn't match, so that things like trace-events will at
least avoid falling in the middle of a patch if you use the project's
orderfile.

> +++ b/hw/ide/cmd646.c
> @@ -32,6 +32,7 @@
>  #include "sysemu/dma.h"
>  
>  #include "hw/ide/pci.h"
> +#include "trace.h"
>  
>  /* CMD646 specific */
>  #define CFR          0x50
> @@ -195,9 +196,8 @@ static uint64_t bmdma_read(void *opaque, hwaddr addr,
>          val = 0xff;
>          break;
>      }
> -#ifdef DEBUG_IDE
> -    printf("bmdma: readb " TARGET_FMT_plx " : 0x%02x\n", addr, val);
> -#endif

Yay for killing code prone to bitrot.

> +++ b/hw/ide/core.c

> @@ -2054,18 +2044,18 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>      }

>      hob = 0;
> -    switch(addr) {
> +    switch(reg_num) {

Worth fixing the style to put space after switch while touching this?

> +++ b/hw/ide/trace-events
> @@ -0,0 +1,33 @@
> +# See docs/devel/tracing.txt for syntax documentation.
> +
> +# hw/ide/core.c

> +
> +# hw/ide/pci.c
> +bmdma_reset(void) ""

An empty trace? Do all the backends support it?

> +# hw/ide/cmd646.c

Worth sorting the sections by filename?

Whether or not you tweak based on my nits,

Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature