[2/3] Monitor command 'trace'

This introduces the monitor command 'trace' to read current contents of 
trace buffer. 


Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
---
 configure       |    3 +++
 monitor.c       |    3 +++
 qemu-monitor.hx |   16 ++++++++++++++++
 simpletrace.c   |   15 +++++++++++++++
 tracetool       |    4 ++++
 5 files changed, 41 insertions(+), 0 deletions(-)

On Tue, 8 Jun 2010 12:34:37 +0530
Prerna Saxena <prerna@linux.vnet.ibm.com> wrote:

> This introduces the monitor command 'trace' to read current contents of 
> trace buffer. 
> 
> 
> Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
> ---
>  configure       |    3 +++
>  monitor.c       |    3 +++
>  qemu-monitor.hx |   16 ++++++++++++++++
>  simpletrace.c   |   15 +++++++++++++++
>  tracetool       |    4 ++++
>  5 files changed, 41 insertions(+), 0 deletions(-)
> 
> diff --git a/configure b/configure
> index 675d0fc..56af8dd 100755
> --- a/configure
> +++ b/configure
> @@ -2302,6 +2302,9 @@ bsd)
>  esac
>  
>  echo "TRACE_BACKEND=$trace_backend" >> $config_host_mak
> +if test "$trace_backend" = "simple"; then
> +  echo "CONFIG_SIMPLE_TRACE=y" >> $config_host_mak
> +fi
>  if test "$trace_backend" = "ust"; then
>    LIBS="-lust $LIBS"
>  fi
> diff --git a/monitor.c b/monitor.c
> index ad50f12..fda29aa 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -55,6 +55,9 @@
>  #include "json-streamer.h"
>  #include "json-parser.h"
>  #include "osdep.h"
> +#ifdef CONFIG_SIMPLE_TRACE
> +#include "trace.h"
> +#endif
>  
>  //#define DEBUG
>  //#define DEBUG_COMPLETION
> diff --git a/qemu-monitor.hx b/qemu-monitor.hx
> index b6e3467..c604aec 100644
> --- a/qemu-monitor.hx
> +++ b/qemu-monitor.hx
> @@ -221,6 +221,22 @@ STEXI
>  @item logfile @var{filename}
>  @findex logfile
>  Output logs to @var{filename}.
> +#ifdef CONFIG_SIMPLE_TRACE
> +ETEXI
> +
> +    {
> +        .name       = "trace",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "shows contents of trace buffer",
> +        .mhandler.cmd = do_info_trace,
> +    },
> +
> +STEXI
> +@item trace
> +@findex trace
> +show contents of trace buffer
> +#endif
>  ETEXI
>  
>      {
> diff --git a/simpletrace.c b/simpletrace.c
> index 2fec4d3..8f33a81 100644
> --- a/simpletrace.c
> +++ b/simpletrace.c
> @@ -62,3 +62,18 @@ void trace4(TraceEvent event, unsigned long x1, unsigned long x2, unsigned long
>  void trace5(TraceEvent event, unsigned long x1, unsigned long x2, unsigned long x3, unsigned long x4, unsigned long x5) {
>      trace(event, x1, x2, x3, x4, x5);
>  }
> +
> +void do_info_trace(Monitor *mon) {

 You sure this shouldn't be 'info trace'?

> +    static unsigned int i, max_idx;

 Why static?

> +
> +    if (trace_idx)
> +        max_idx = trace_idx;
> +    else
> +        max_idx = TRACE_BUF_LEN;

 max_idx = trace_idx ? trace_idx : TRACE_BUF_LEN;

> +
> +    for (i=0; i<max_idx ;i++)
> +        monitor_printf(mon, "Event %ld : %ld %ld %ld %ld %ld\n",
> +	trace_buf[i].event, trace_buf[i].x1, trace_buf[i].x2,
> +	trace_buf[i].x3, trace_buf[i].x4, trace_buf[i].x5);

 Style & indentation.

> +    return;

 Not needed.

> +}
> diff --git a/tracetool b/tracetool
> index 9ea9c08..6c15154 100755
> --- a/tracetool
> +++ b/tracetool
> @@ -130,6 +130,7 @@ void trace2(TraceEvent event, unsigned long x1, unsigned long x2);
>  void trace3(TraceEvent event, unsigned long x1, unsigned long x2, unsigned long x3);
>  void trace4(TraceEvent event, unsigned long x1, unsigned long x2, unsigned long x3, unsigned long x4);
>  void trace5(TraceEvent event, unsigned long x1, unsigned long x2, unsigned long x3, unsigned long x4, unsigned long x5);
> +void do_info_trace(Monitor *mon);
>  EOF
>  
>      simple_event_num=0
> @@ -289,6 +290,9 @@ tracetoh()
>  #define TRACE_H
>  
>  #include "qemu-common.h"
> +
> +extern void monitor_printf(Monitor *mon, const char *fmt, ...)
> +    __attribute__ ((__format__ (__printf__, 2, 3)));
>  EOF
>      convert h
>      echo "#endif /* TRACE_H */"

Hi Luiz,
Thanks for your feedback.

On 06/10/2010 02:07 AM, Luiz Capitulino wrote:
> On Tue, 8 Jun 2010 12:34:37 +0530
> Prerna Saxena<prerna@linux.vnet.ibm.com>  wrote:
>
>> This introduces the monitor command 'trace' to read current contents of
>> trace buffer.
>>
>> ...
>> diff --git a/simpletrace.c b/simpletrace.c
>> index 2fec4d3..8f33a81 100644
>> --- a/simpletrace.c
>> +++ b/simpletrace.c
>> @@ -62,3 +62,18 @@ void trace4(TraceEvent event, unsigned long x1, unsigned long x2, unsigned long
>>   void trace5(TraceEvent event, unsigned long x1, unsigned long x2, unsigned long x3, unsigned long x4, unsigned long x5) {
>>       trace(event, x1, x2, x3, x4, x5);
>>   }
>> +
>> +void do_info_trace(Monitor *mon) {
>
>   You sure this shouldn't be 'info trace'?

In this set, I had a direct monitor command 'trace' to display trace 
buffer contents.
In v2, I have introduced an 'info trace' command to do the same, since 
it intuitively made more sense to use an 'info' command to see state of 
trace buffer. For this implementation, the present handler name makes 
more sense.(do_info_trace())

>
>> +    static unsigned int i, max_idx;
>
>   Why static?

This isnt needed. The next patch in this series removed it (This change 
should've been a part of this patch, but went into next)
Cleaned it up in v2.

>
>> +
>> +    if (trace_idx)
>> +        max_idx = trace_idx;
>> +    else
>> +        max_idx = TRACE_BUF_LEN;
>
>   max_idx = trace_idx ? trace_idx : TRACE_BUF_LEN;
>
>> +
>> +    for (i=0; i<max_idx ;i++)
>> +        monitor_printf(mon, "Event %ld : %ld %ld %ld %ld %ld\n",
>> +	trace_buf[i].event, trace_buf[i].x1, trace_buf[i].x2,
>> +	trace_buf[i].x3, trace_buf[i].x4, trace_buf[i].x5);
>
>   Style&  indentation.

Changed in v2.

>
>> +    return;
>
>   Not needed.

Removed in v2.

>
>> +}
>> diff --git a/tracetool b/tracetool
>> ....
>
>