[PATCH 0/3] Monitor commands for 'simple' trace backend

[PATCH 0/3] Monitor commands for 'simple' trace backend

[Prerna Saxena]

This patchset is based on Stefan's trace framework:
( http://lists.gnu.org/archive/html/qemu-devel/2010-05/msg02407.html )

This adds the following monitor commands for the 'simple' backend:
- trace 		: to view current contents of the trace buffer
- info tracepoints 	: to view all available tracepoints and their 
			  state.
- tracepoint NAME on|off: to enable/disable the logging of data from 
			  tracepoint 'NAME'.

-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India

[PATCH 1/3] export tdb_hash()

[Prerna Saxena]

This exports tdb_hash() for use by tracing framework.

Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
---
 qdict.c |    2 +-
 qdict.h |    2 ++
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/qdict.c b/qdict.c
index 175bc17..5261872 100644
--- a/qdict.c
+++ b/qdict.c
@@ -56,7 +56,7 @@ QDict *qobject_to_qdict(const QObject *obj)
  * tdb_hash(): based on the hash agorithm from gdbm, via tdb
  * (from module-init-tools)
  */
-static unsigned int tdb_hash(const char *name)
+unsigned int tdb_hash(const char *name)
 {
     unsigned value;	/* Used to compute the hash value.  */
     unsigned   i;	/* Used to cycle through random values. */
diff --git a/qdict.h b/qdict.h
index 5e5902c..d221c18 100644
--- a/qdict.h
+++ b/qdict.h
@@ -59,4 +59,6 @@ int64_t qdict_get_try_int(const QDict *qdict, const char *key,
                           int64_t err_value);
 const char *qdict_get_try_str(const QDict *qdict, const char *key);
 
+/* Export tdb_hash() for use by trace framework */
+unsigned int tdb_hash(const char *name);
 #endif /* QDICT_H */
-- 
1.6.2.5



-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India

Re: [Qemu-devel] [PATCH 1/3] export tdb_hash()

[Luiz Capitulino]

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

> This exports tdb_hash() for use by tracing framework.

 Suggest to rename it (eg. qemu_hash()) and move it to a better location, qdict
is not the best module to export such service.

> 
> Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
> ---
>  qdict.c |    2 +-
>  qdict.h |    2 ++
>  2 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/qdict.c b/qdict.c
> index 175bc17..5261872 100644
> --- a/qdict.c
> +++ b/qdict.c
> @@ -56,7 +56,7 @@ QDict *qobject_to_qdict(const QObject *obj)
>   * tdb_hash(): based on the hash agorithm from gdbm, via tdb
>   * (from module-init-tools)
>   */
> -static unsigned int tdb_hash(const char *name)
> +unsigned int tdb_hash(const char *name)
>  {
>      unsigned value;	/* Used to compute the hash value.  */
>      unsigned   i;	/* Used to cycle through random values. */
> diff --git a/qdict.h b/qdict.h
> index 5e5902c..d221c18 100644
> --- a/qdict.h
> +++ b/qdict.h
> @@ -59,4 +59,6 @@ int64_t qdict_get_try_int(const QDict *qdict, const char *key,
>                            int64_t err_value);
>  const char *qdict_get_try_str(const QDict *qdict, const char *key);
>  
> +/* Export tdb_hash() for use by trace framework */
> +unsigned int tdb_hash(const char *name);
>  #endif /* QDICT_H */

Re: [Qemu-devel] [PATCH 1/3] export tdb_hash()

[Prerna Saxena]

On 06/10/2010 02:05 AM, Luiz Capitulino wrote:
> On Tue, 8 Jun 2010 12:31:38 +0530
> Prerna Saxena<prerna@linux.vnet.ibm.com>  wrote:
>
>> This exports tdb_hash() for use by tracing framework.
>
>   Suggest to rename it (eg. qemu_hash()) and move it to a better location, qdict
> is not the best module to export such service.
>

Would 'qemu-common.h' be a good place to export it ? If not, any 
suggestions on where else I can export it ?
I havent renamed it in v2 of patches that I sent out. I'll send out a 
cumulative patch that renames and exports tdb_hash() from an appropriate 
location.

Regards,
-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India

[PATCH 2/3] Monitor command 'trace'

[Prerna Saxena]

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) {
+    static unsigned int i, max_idx;
+
+    if (trace_idx)
+        max_idx = trace_idx;
+    else
+        max_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);
+    return;
+}
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 */"
-- 
1.6.2.5



-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India

Re: [Qemu-devel] [PATCH 2/3] Monitor command 'trace'

[Luiz Capitulino]

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 */"

Re: [Qemu-devel] [PATCH 2/3] Monitor command 'trace'

[Prerna Saxena]

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
>> ....
>
>


-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India

[PATCH 3/3] Toggle tracepoint state

[Prerna Saxena]

This patch adds support for dynamically enabling/disabling of tracepoints.
Monitor commands added :
1) info tracepoints 		: to view all available tracepoints and 
				  their state.
2) tracepoint NAME on|off 	: to enable/disable data logging from a 
				  given tracepoint.
				  Eg, tracepoint paio_submit off 
				  	disables logging of data when 
					paio_submit is hit.

At present this is done with a simple comparison -- there is scope 
for optimizations that can be employed here to speed this up.

Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
---
 monitor.c       |   21 +++++++++++++++++++++
 qemu-monitor.hx |   16 ++++++++++++++++
 simpletrace.c   |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 tracetool       |   30 ++++++++++++++++++++++++++----
 vl.c            |    6 ++++++
 5 files changed, 123 insertions(+), 5 deletions(-)

diff --git a/monitor.c b/monitor.c
index fda29aa..1e05b38 100644
--- a/monitor.c
+++ b/monitor.c
@@ -547,7 +547,19 @@ static void do_commit(Monitor *mon, const QDict *qdict)
         bdrv_commit(dinfo->bdrv);
     }
 }
+#ifdef CONFIG_SIMPLE_TRACE
+static void do_change_tracepoint_state(Monitor *mon, const QDict *qdict)
+{
+    const char *tp_name = qdict_get_str(qdict, "name");
+    const char *tp_state = qdict_get_str(qdict, "option");
 
+    if (!strncmp(tp_state, "on", 2))
+	change_tracepoint_state(tp_name, 1);
+    if (!strncmp(tp_state, "off", 3))
+	change_tracepoint_state(tp_name, 0);
+    return;
+}
+#endif
 static void user_monitor_complete(void *opaque, QObject *ret_data)
 {
     MonitorCompletionData *data = (MonitorCompletionData *)opaque; 
@@ -2783,6 +2795,15 @@ static const mon_cmd_t info_cmds[] = {
         .help       = "show roms",
         .mhandler.info = do_info_roms,
     },
+#ifdef CONFIG_SIMPLE_TRACE
+    {
+        .name       = "tracepoints",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show available tracepoints & their state",
+        .mhandler.info = do_info_all_tracepoints,
+    },
+#endif
     {
         .name       = NULL,
     },
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index c604aec..36481ea 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -114,6 +114,8 @@ show migration status
 show balloon information
 @item info qtree
 show device tree
+@item info tracepoints
+show available tracepoints and their state
 @end table
 ETEXI
 
@@ -236,6 +238,20 @@ STEXI
 @item trace
 @findex trace
 show contents of trace buffer
+ETEXI
+
+    {
+        .name       = "tracepoint",
+        .args_type  = "name:s,option:s",
+        .params     = "name on|off",
+        .help       = "changes status of a specific tracepoint",
+        .mhandler.cmd = do_change_tracepoint_state,
+    },
+
+STEXI
+@item tracepoint
+@findex tracepoint
+changes status of a tracepoint
 #endif
 ETEXI
 
diff --git a/simpletrace.c b/simpletrace.c
index 8f33a81..75d2fca 100644
--- a/simpletrace.c
+++ b/simpletrace.c
@@ -3,6 +3,12 @@
 #include "trace.h"
 
 typedef struct {
+    char *tp_name;
+    bool state;
+    unsigned int hash;
+} Tracepoint;
+
+typedef struct {
     unsigned long event;
     unsigned long x1;
     unsigned long x2;
@@ -18,10 +24,24 @@ enum {
 static TraceRecord trace_buf[TRACE_BUF_LEN];
 static unsigned int trace_idx;
 static FILE *trace_fp;
+static Tracepoint trace_list[NR_TRACEPOINTS];
+
+void init_tracepoint(const char *tname, TraceEvent tevent) {
+    if (!tname || tevent > NR_TRACEPOINTS)
+	return;
+
+    trace_list[tevent].tp_name = (char*)qemu_malloc(strlen(tname)+1);
+    strncpy(trace_list[tevent].tp_name, tname, strlen(tname));
+    trace_list[tevent].hash = tdb_hash(tname);
+    trace_list[tevent].state = 1; /* Enable all by default */
+    return;
+}
 
 static void trace(TraceEvent event, unsigned long x1,
                   unsigned long x2, unsigned long x3,
                   unsigned long x4, unsigned long x5) {
+    if (!trace_list[event].state)
+	return;
     TraceRecord *rec = &trace_buf[trace_idx];
     rec->event = event;
     rec->x1 = x1;
@@ -64,7 +84,7 @@ void trace5(TraceEvent event, unsigned long x1, unsigned long x2, unsigned long
 }
 
 void do_info_trace(Monitor *mon) {
-    static unsigned int i, max_idx;
+    unsigned int i, max_idx;
 
     if (trace_idx)
         max_idx = trace_idx;
@@ -77,3 +97,36 @@ void do_info_trace(Monitor *mon) {
 	trace_buf[i].x3, trace_buf[i].x4, trace_buf[i].x5);
     return;
 }
+
+void do_info_all_tracepoints(Monitor *mon) {
+    unsigned int i;
+    for (i=0; i<NR_TRACEPOINTS; i++)
+	monitor_printf(mon, "%s [Event ID %u] : state %u\n",
+		trace_list[i].tp_name, i, trace_list[i].state);
+    return;
+}
+
+static int find_tracepoint_by_name(const char *tname) {
+    unsigned int i, name_hash;
+
+    if (!tname)
+	return -1;
+
+    name_hash = tdb_hash(tname);
+
+    for (i=0; i<NR_TRACEPOINTS; i++)
+	if (trace_list[i].hash == name_hash &&
+		 !strncmp(trace_list[i].tp_name, tname, strlen(tname)))
+	    return i;
+    return -1; /* indicates end of list reached without a match */
+}
+
+void change_tracepoint_state(const char *tname, bool tstate) {
+    int i;
+
+    i = find_tracepoint_by_name(tname);
+    /* TODO : This update might need to be protected by a memory barrier */
+    if (i >= 0)
+	trace_list[i].state = tstate;
+    return;
+}
diff --git a/tracetool b/tracetool
index 6c15154..cc2b22c 100755
--- a/tracetool
+++ b/tracetool
@@ -123,14 +123,20 @@ linetoc_end_nop()
 linetoh_begin_simple()
 {
     cat <<EOF
+#include <stdbool.h>
+
 typedef unsigned int TraceEvent;
 
+void init_tracepoint_table(void);
+void init_tracepoint(const char *tname, TraceEvent tevent);
 void trace1(TraceEvent event, unsigned long x1);
 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);
+void do_info_all_tracepoints(Monitor *mon);
+void change_tracepoint_state(const char *tname, bool tstate);
 EOF
 
     simple_event_num=0
@@ -163,22 +169,38 @@ EOF
 
 linetoh_end_simple()
 {
-    return
+    cat <<EOF
+#define NR_TRACEPOINTS $simple_event_num
+EOF
 }
 
 linetoc_begin_simple()
 {
-    return
+    cat <<EOF
+#include "trace.h"
+
+void init_tracepoint_table(void) {
+EOF
+    simple_event_num=0
+
 }
 
 linetoc_simple()
 {
-    return
+    local name
+    name=$(get_name "$1")
+    cat <<EOF
+init_tracepoint("$name", $simple_event_num);
+EOF
+    simple_event_num=$((simple_event_num + 1))
 }
 
 linetoc_end_simple()
 {
-    return
+    cat <<EOF
+return;
+}
+EOF
 }
 
 linetoh_begin_ust()
diff --git a/vl.c b/vl.c
index 328395e..dd07904 100644
--- a/vl.c
+++ b/vl.c
@@ -163,6 +163,9 @@ int main(int argc, char **argv)
 #include "cpus.h"
 #include "arch_init.h"
 
+#ifdef CONFIG_SIMPLE_TRACE
+#include "trace.h"
+#endif
 //#define DEBUG_NET
 //#define DEBUG_SLIRP
 
@@ -3604,6 +3607,9 @@ int main(int argc, char **argv, char **envp)
     if (net_init_clients() < 0) {
         exit(1);
     }
+#ifdef CONFIG_SIMPLE_TRACE
+    init_tracepoint_table();
+#endif
 
     /* init the bluetooth world */
     if (foreach_device_config(DEV_BT, bt_parse))
-- 
1.6.2.5



-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India

Re: [Qemu-devel] [PATCH 3/3] Toggle tracepoint state

[Luiz Capitulino]

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

> This patch adds support for dynamically enabling/disabling of tracepoints.
> Monitor commands added :
> 1) info tracepoints 		: to view all available tracepoints and 
> 				  their state.
> 2) tracepoint NAME on|off 	: to enable/disable data logging from a 
> 				  given tracepoint.
> 				  Eg, tracepoint paio_submit off 
> 				  	disables logging of data when 
> 					paio_submit is hit.
> 
> At present this is done with a simple comparison -- there is scope 
> for optimizations that can be employed here to speed this up.
> 
> Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
> ---
>  monitor.c       |   21 +++++++++++++++++++++
>  qemu-monitor.hx |   16 ++++++++++++++++
>  simpletrace.c   |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  tracetool       |   30 ++++++++++++++++++++++++++----
>  vl.c            |    6 ++++++
>  5 files changed, 123 insertions(+), 5 deletions(-)
> 
> diff --git a/monitor.c b/monitor.c
> index fda29aa..1e05b38 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -547,7 +547,19 @@ static void do_commit(Monitor *mon, const QDict *qdict)
>          bdrv_commit(dinfo->bdrv);
>      }
>  }
> +#ifdef CONFIG_SIMPLE_TRACE
> +static void do_change_tracepoint_state(Monitor *mon, const QDict *qdict)
> +{
> +    const char *tp_name = qdict_get_str(qdict, "name");
> +    const char *tp_state = qdict_get_str(qdict, "option");
>  
> +    if (!strncmp(tp_state, "on", 2))
> +	change_tracepoint_state(tp_name, 1);
> +    if (!strncmp(tp_state, "off", 3))
> +	change_tracepoint_state(tp_name, 0);

 Monitor has a bool type, please take a look in do_set_link().

> +    return;

 Not needed, also true for other functions in this patch, suggest reading
CODING_STYLE for other style related issues in this series.

> +}
> +#endif
>  static void user_monitor_complete(void *opaque, QObject *ret_data)
>  {
>      MonitorCompletionData *data = (MonitorCompletionData *)opaque; 
> @@ -2783,6 +2795,15 @@ static const mon_cmd_t info_cmds[] = {
>          .help       = "show roms",
>          .mhandler.info = do_info_roms,
>      },
> +#ifdef CONFIG_SIMPLE_TRACE
> +    {
> +        .name       = "tracepoints",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show available tracepoints & their state",
> +        .mhandler.info = do_info_all_tracepoints,
> +    },
> +#endif
>      {
>          .name       = NULL,
>      },
> diff --git a/qemu-monitor.hx b/qemu-monitor.hx
> index c604aec..36481ea 100644
> --- a/qemu-monitor.hx
> +++ b/qemu-monitor.hx
> @@ -114,6 +114,8 @@ show migration status
>  show balloon information
>  @item info qtree
>  show device tree
> +@item info tracepoints
> +show available tracepoints and their state
>  @end table
>  ETEXI
>  
> @@ -236,6 +238,20 @@ STEXI
>  @item trace
>  @findex trace
>  show contents of trace buffer
> +ETEXI
> +
> +    {
> +        .name       = "tracepoint",
> +        .args_type  = "name:s,option:s",
> +        .params     = "name on|off",
> +        .help       = "changes status of a specific tracepoint",
> +        .mhandler.cmd = do_change_tracepoint_state,
> +    },
> +
> +STEXI
> +@item tracepoint
> +@findex tracepoint
> +changes status of a tracepoint
>  #endif
>  ETEXI
>  
> diff --git a/simpletrace.c b/simpletrace.c
> index 8f33a81..75d2fca 100644
> --- a/simpletrace.c
> +++ b/simpletrace.c
> @@ -3,6 +3,12 @@
>  #include "trace.h"
>  
>  typedef struct {
> +    char *tp_name;
> +    bool state;
> +    unsigned int hash;
> +} Tracepoint;
> +
> +typedef struct {
>      unsigned long event;
>      unsigned long x1;
>      unsigned long x2;
> @@ -18,10 +24,24 @@ enum {
>  static TraceRecord trace_buf[TRACE_BUF_LEN];
>  static unsigned int trace_idx;
>  static FILE *trace_fp;
> +static Tracepoint trace_list[NR_TRACEPOINTS];
> +
> +void init_tracepoint(const char *tname, TraceEvent tevent) {
> +    if (!tname || tevent > NR_TRACEPOINTS)
> +	return;
> +
> +    trace_list[tevent].tp_name = (char*)qemu_malloc(strlen(tname)+1);
> +    strncpy(trace_list[tevent].tp_name, tname, strlen(tname));
> +    trace_list[tevent].hash = tdb_hash(tname);
> +    trace_list[tevent].state = 1; /* Enable all by default */
> +    return;
> +}
>  
>  static void trace(TraceEvent event, unsigned long x1,
>                    unsigned long x2, unsigned long x3,
>                    unsigned long x4, unsigned long x5) {
> +    if (!trace_list[event].state)
> +	return;
>      TraceRecord *rec = &trace_buf[trace_idx];
>      rec->event = event;
>      rec->x1 = x1;
> @@ -64,7 +84,7 @@ void trace5(TraceEvent event, unsigned long x1, unsigned long x2, unsigned long
>  }
>  
>  void do_info_trace(Monitor *mon) {
> -    static unsigned int i, max_idx;
> +    unsigned int i, max_idx;
>  
>      if (trace_idx)
>          max_idx = trace_idx;
> @@ -77,3 +97,36 @@ void do_info_trace(Monitor *mon) {
>  	trace_buf[i].x3, trace_buf[i].x4, trace_buf[i].x5);
>      return;
>  }
> +
> +void do_info_all_tracepoints(Monitor *mon) {
> +    unsigned int i;
> +    for (i=0; i<NR_TRACEPOINTS; i++)
> +	monitor_printf(mon, "%s [Event ID %u] : state %u\n",
> +		trace_list[i].tp_name, i, trace_list[i].state);
> +    return;
> +}
> +
> +static int find_tracepoint_by_name(const char *tname) {
> +    unsigned int i, name_hash;
> +
> +    if (!tname)
> +	return -1;
> +
> +    name_hash = tdb_hash(tname);
> +
> +    for (i=0; i<NR_TRACEPOINTS; i++)
> +	if (trace_list[i].hash == name_hash &&
> +		 !strncmp(trace_list[i].tp_name, tname, strlen(tname)))
> +	    return i;
> +    return -1; /* indicates end of list reached without a match */
> +}
> +
> +void change_tracepoint_state(const char *tname, bool tstate) {
> +    int i;
> +
> +    i = find_tracepoint_by_name(tname);
> +    /* TODO : This update might need to be protected by a memory barrier */
> +    if (i >= 0)
> +	trace_list[i].state = tstate;
> +    return;
> +}
> diff --git a/tracetool b/tracetool
> index 6c15154..cc2b22c 100755
> --- a/tracetool
> +++ b/tracetool
> @@ -123,14 +123,20 @@ linetoc_end_nop()
>  linetoh_begin_simple()
>  {
>      cat <<EOF
> +#include <stdbool.h>
> +
>  typedef unsigned int TraceEvent;
>  
> +void init_tracepoint_table(void);
> +void init_tracepoint(const char *tname, TraceEvent tevent);
>  void trace1(TraceEvent event, unsigned long x1);
>  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);
> +void do_info_all_tracepoints(Monitor *mon);
> +void change_tracepoint_state(const char *tname, bool tstate);
>  EOF
>  
>      simple_event_num=0
> @@ -163,22 +169,38 @@ EOF
>  
>  linetoh_end_simple()
>  {
> -    return
> +    cat <<EOF
> +#define NR_TRACEPOINTS $simple_event_num
> +EOF
>  }
>  
>  linetoc_begin_simple()
>  {
> -    return
> +    cat <<EOF
> +#include "trace.h"
> +
> +void init_tracepoint_table(void) {
> +EOF
> +    simple_event_num=0
> +
>  }
>  
>  linetoc_simple()
>  {
> -    return
> +    local name
> +    name=$(get_name "$1")
> +    cat <<EOF
> +init_tracepoint("$name", $simple_event_num);
> +EOF
> +    simple_event_num=$((simple_event_num + 1))
>  }
>  
>  linetoc_end_simple()
>  {
> -    return
> +    cat <<EOF
> +return;
> +}
> +EOF
>  }
>  
>  linetoh_begin_ust()
> diff --git a/vl.c b/vl.c
> index 328395e..dd07904 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -163,6 +163,9 @@ int main(int argc, char **argv)
>  #include "cpus.h"
>  #include "arch_init.h"
>  
> +#ifdef CONFIG_SIMPLE_TRACE
> +#include "trace.h"
> +#endif
>  //#define DEBUG_NET
>  //#define DEBUG_SLIRP
>  
> @@ -3604,6 +3607,9 @@ int main(int argc, char **argv, char **envp)
>      if (net_init_clients() < 0) {
>          exit(1);
>      }
> +#ifdef CONFIG_SIMPLE_TRACE
> +    init_tracepoint_table();
> +#endif
>  
>      /* init the bluetooth world */
>      if (foreach_device_config(DEV_BT, bt_parse))

Re: [Qemu-devel] [PATCH 3/3] Toggle tracepoint state

[Prerna Saxena]

Hi Luiz,
Thanks for taking time to review it.

On 06/10/2010 02:13 AM, Luiz Capitulino wrote:
> On Tue, 8 Jun 2010 12:38:58 +0530
> Prerna Saxena<prerna@linux.vnet.ibm.com>  wrote:
>
>> This patch adds support for dynamically enabling/disabling of tracepoints.
>> Monitor commands added :
>> ....
>>
>
>   Monitor has a bool type, please take a look in do_set_link().
>

Thanks for the pointer. Changed in v2.

>> +    return;
>
>   Not needed, also true for other functions in this patch, suggest reading
> CODING_STYLE for other style related issues in this series.

Done.

>
>>....
>>

Hope these commands would make it easy to visualize logged traces via 
the monitor. I'd appreciate feedback on how v2 of the patches posted 
can be enhanced.

-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India

[PATCH 0/3] Monitor support QEMU trace framework

[Prerna Saxena]

This is v3 of a set of patches to enable trace visualization & control 
via the QEMU monitor. It is based on trace infrastructure posted by 
Stefan :
( http://lists.gnu.org/archive/html/qemu-devel/2010-05/msg02407.html )


This patchset adds monitor commands :
- info trace : to view current contents of the trace buffer
- info tracepoints : to view all available tracepoints and their state.
- tracepoint NAME on|off : to enable/disable the logging of data
 from tracepoint 'NAME' to on/off.

Changelog :
- Clean-ups from v2, particularly relating to export of tdb_hash()

-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India

[PATCH 3/3] Toggle tracepoint state

[Prerna Saxena]

This patch adds support for dynamically enabling/disabling of tracepoints.
This is done by internally maintaining each tracepoint's state, and 
permitting logging of data from a tracepoint only if it is in an 
'active' state.

Monitor commands added :
1) info tracepoints 		: to view all available tracepoints and 
				  their state.
2) tracepoint NAME on|off 	: to enable/disable data logging from a 
				  given tracepoint.
				  Eg, tracepoint paio_submit off 
				  	disables logging of data when 
					paio_submit is hit.

Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
---

 monitor.c       |   16 ++++++++++++++
 qemu-monitor.hx |   18 ++++++++++++++++
 simpletrace.c   |   63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tracetool       |   30 +++++++++++++++++++++++---
 vl.c            |    6 +++++
 5 files changed, 129 insertions(+), 4 deletions(-)


diff --git a/monitor.c b/monitor.c
index 8b60830..238bdf0 100644
--- a/monitor.c
+++ b/monitor.c
@@ -548,6 +548,15 @@ static void do_commit(Monitor *mon, const QDict *qdict)
     }
 }
 
+#ifdef CONFIG_SIMPLE_TRACE
+static void do_change_tracepoint_state(Monitor *mon, const QDict *qdict)
+{
+    const char *tp_name = qdict_get_str(qdict, "name");
+    bool new_state = qdict_get_bool(qdict, "option");
+    change_tracepoint_state(tp_name, new_state);
+}
+#endif
+
 static void user_monitor_complete(void *opaque, QObject *ret_data)
 {
     MonitorCompletionData *data = (MonitorCompletionData *)opaque; 
@@ -2791,6 +2800,13 @@ static const mon_cmd_t info_cmds[] = {
         .help       = "show current contents of trace buffer",
         .mhandler.info = do_info_trace,
     },
+    {
+        .name       = "tracepoints",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show available tracepoints & their state",
+        .mhandler.info = do_info_all_tracepoints,
+    },
 #endif
     {
         .name       = NULL,
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 766c30f..8540b8f 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -117,6 +117,8 @@ show device tree
 #ifdef CONFIG_SIMPLE_TRACE
 @item info trace
 show contents of trace buffer
+@item info tracepoints
+show available tracepoints and their state
 #endif
 @end table
 ETEXI
@@ -225,6 +227,22 @@ STEXI
 @item logfile @var{filename}
 @findex logfile
 Output logs to @var{filename}.
+#ifdef CONFIG_SIMPLE_TRACE
+ETEXI
+
+    {
+        .name       = "tracepoint",
+        .args_type  = "name:s,option:b",
+        .params     = "name on|off",
+        .help       = "changes status of a specific tracepoint",
+        .mhandler.cmd = do_change_tracepoint_state,
+    },
+
+STEXI
+@item tracepoint
+@findex tracepoint
+changes status of a tracepoint
+#endif
 ETEXI
 
     {
diff --git a/simpletrace.c b/simpletrace.c
index 239ae3f..4221a8f 100644
--- a/simpletrace.c
+++ b/simpletrace.c
@@ -3,6 +3,12 @@
 #include "trace.h"
 
 typedef struct {
+    char *tp_name;
+    bool state;
+    unsigned int hash;
+} Tracepoint;
+
+typedef struct {
     unsigned long event;
     unsigned long x1;
     unsigned long x2;
@@ -18,11 +24,29 @@ enum {
 static TraceRecord trace_buf[TRACE_BUF_LEN];
 static unsigned int trace_idx;
 static FILE *trace_fp;
+static Tracepoint trace_list[NR_TRACEPOINTS];
+
+void init_tracepoint(const char *tname, TraceEvent tevent)
+{
+    if (!tname || tevent > NR_TRACEPOINTS) {
+        return;
+    }
+
+    trace_list[tevent].tp_name = (char*)qemu_malloc(strlen(tname)+1);
+    strncpy(trace_list[tevent].tp_name, tname, strlen(tname));
+    trace_list[tevent].hash = qemu_hash(tname);
+    trace_list[tevent].state = 1; /* Enable all by default */
+}
 
 static void trace(TraceEvent event, unsigned long x1,
                   unsigned long x2, unsigned long x3,
                   unsigned long x4, unsigned long x5) {
     TraceRecord *rec = &trace_buf[trace_idx];
+
+    if (!trace_list[event].state) {
+        return;
+    }
+
     rec->event = event;
     rec->x1 = x1;
     rec->x2 = x2;
@@ -75,3 +99,42 @@ void do_info_trace(Monitor *mon)
                             trace_buf[i].x3, trace_buf[i].x4, trace_buf[i].x5);
     }
 }
+
+void do_info_all_tracepoints(Monitor *mon)
+{
+    unsigned int i;
+
+    for (i=0; i<NR_TRACEPOINTS; i++) {
+        monitor_printf(mon, "%s [Event ID %u] : state %u\n",
+                                trace_list[i].tp_name, i, trace_list[i].state);
+        }
+}
+
+static Tracepoint* find_tracepoint_by_name(const char *tname)
+{
+    unsigned int i, name_hash;
+
+    if (!tname) {
+        return NULL;
+    }
+
+    name_hash = qemu_hash(tname);
+
+    for (i=0; i<NR_TRACEPOINTS; i++) {
+        if (trace_list[i].hash == name_hash &&
+                       !strncmp(trace_list[i].tp_name, tname, strlen(tname))) {
+            return &trace_list[i];
+        }
+    }
+    return NULL; /* indicates end of list reached without a match */
+}
+
+void change_tracepoint_state(const char *tname, bool tstate)
+{
+    Tracepoint *tp;
+
+    tp = find_tracepoint_by_name(tname);
+    if (tp) {
+        tp->state = tstate;
+    }
+}
diff --git a/tracetool b/tracetool
index 2c73bab..00af205 100755
--- a/tracetool
+++ b/tracetool
@@ -123,14 +123,20 @@ linetoc_end_nop()
 linetoh_begin_simple()
 {
     cat <<EOF
+#include <stdbool.h>
+
 typedef unsigned int TraceEvent;
 
+void init_tracepoint_table(void);
+void init_tracepoint(const char *tname, TraceEvent tevent);
 void trace1(TraceEvent event, unsigned long x1);
 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);
+void do_info_all_tracepoints(Monitor *mon);
+void change_tracepoint_state(const char *tname, bool tstate);
 EOF
 
     simple_event_num=0
@@ -163,22 +169,38 @@ EOF
 
 linetoh_end_simple()
 {
-    return
+    cat <<EOF
+#define NR_TRACEPOINTS $simple_event_num
+EOF
 }
 
 linetoc_begin_simple()
 {
-    return
+    cat <<EOF
+#include "trace.h"
+
+void init_tracepoint_table(void) {
+EOF
+    simple_event_num=0
+
 }
 
 linetoc_simple()
 {
-    return
+    local name
+    name=$(get_name "$1")
+    cat <<EOF
+init_tracepoint("$name", $simple_event_num);
+EOF
+    simple_event_num=$((simple_event_num + 1))
 }
 
 linetoc_end_simple()
 {
-    return
+    cat <<EOF
+return;
+}
+EOF
 }
 
 linetoh_begin_ust()
diff --git a/vl.c b/vl.c
index 328395e..dd07904 100644
--- a/vl.c
+++ b/vl.c
@@ -163,6 +163,9 @@ int main(int argc, char **argv)
 #include "cpus.h"
 #include "arch_init.h"
 
+#ifdef CONFIG_SIMPLE_TRACE
+#include "trace.h"
+#endif
 //#define DEBUG_NET
 //#define DEBUG_SLIRP
 
@@ -3604,6 +3607,9 @@ int main(int argc, char **argv, char **envp)
     if (net_init_clients() < 0) {
         exit(1);
     }
+#ifdef CONFIG_SIMPLE_TRACE
+    init_tracepoint_table();
+#endif
 
     /* init the bluetooth world */
     if (foreach_device_config(DEV_BT, bt_parse))


-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India

Re: [PATCH 3/3] Toggle tracepoint state

[Stefan Hajnoczi]

On Wed, Jun 16, 2010 at 06:14:35PM +0530, Prerna Saxena wrote:
> This patch adds support for dynamically enabling/disabling of tracepoints.
> This is done by internally maintaining each tracepoint's state, and 
> permitting logging of data from a tracepoint only if it is in an 
> 'active' state.
> 
> Monitor commands added :
> 1) info tracepoints 		: to view all available tracepoints and 
> 				  their state.
> 2) tracepoint NAME on|off 	: to enable/disable data logging from a 
> 				  given tracepoint.
> 				  Eg, tracepoint paio_submit off 
> 				  	disables logging of data when 
> 					paio_submit is hit.
> 
> Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
> ---
> 
>  monitor.c       |   16 ++++++++++++++
>  qemu-monitor.hx |   18 ++++++++++++++++
>  simpletrace.c   |   63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tracetool       |   30 +++++++++++++++++++++++---
>  vl.c            |    6 +++++
>  5 files changed, 129 insertions(+), 4 deletions(-)
> 
> 
> diff --git a/monitor.c b/monitor.c
> index 8b60830..238bdf0 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -548,6 +548,15 @@ static void do_commit(Monitor *mon, const QDict *qdict)
>      }
>  }
> 
> +#ifdef CONFIG_SIMPLE_TRACE
> +static void do_change_tracepoint_state(Monitor *mon, const QDict *qdict)
> +{
> +    const char *tp_name = qdict_get_str(qdict, "name");
> +    bool new_state = qdict_get_bool(qdict, "option");
> +    change_tracepoint_state(tp_name, new_state);
> +}
> +#endif
> +
>  static void user_monitor_complete(void *opaque, QObject *ret_data)
>  {
>      MonitorCompletionData *data = (MonitorCompletionData *)opaque; 
> @@ -2791,6 +2800,13 @@ static const mon_cmd_t info_cmds[] = {
>          .help       = "show current contents of trace buffer",
>          .mhandler.info = do_info_trace,
>      },
> +    {
> +        .name       = "tracepoints",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show available tracepoints & their state",
> +        .mhandler.info = do_info_all_tracepoints,
> +    },
>  #endif
>      {
>          .name       = NULL,
> diff --git a/qemu-monitor.hx b/qemu-monitor.hx
> index 766c30f..8540b8f 100644
> --- a/qemu-monitor.hx
> +++ b/qemu-monitor.hx
> @@ -117,6 +117,8 @@ show device tree
>  #ifdef CONFIG_SIMPLE_TRACE
>  @item info trace
>  show contents of trace buffer
> +@item info tracepoints
> +show available tracepoints and their state
>  #endif
>  @end table
>  ETEXI
> @@ -225,6 +227,22 @@ STEXI
>  @item logfile @var{filename}
>  @findex logfile
>  Output logs to @var{filename}.
> +#ifdef CONFIG_SIMPLE_TRACE
> +ETEXI
> +
> +    {
> +        .name       = "tracepoint",
> +        .args_type  = "name:s,option:b",
> +        .params     = "name on|off",
> +        .help       = "changes status of a specific tracepoint",
> +        .mhandler.cmd = do_change_tracepoint_state,
> +    },
> +
> +STEXI
> +@item tracepoint
> +@findex tracepoint
> +changes status of a tracepoint
> +#endif
>  ETEXI
> 
>      {
> diff --git a/simpletrace.c b/simpletrace.c
> index 239ae3f..4221a8f 100644
> --- a/simpletrace.c
> +++ b/simpletrace.c
> @@ -3,6 +3,12 @@
>  #include "trace.h"
> 
>  typedef struct {
> +    char *tp_name;
> +    bool state;
> +    unsigned int hash;
> +} Tracepoint;

The tracing infrastructure avoids using the name 'tracepoint'.  It calls
them trace events.  I didn't deliberately choose that name, but was
unaware at the time that Linux tracing calls them tracepoints.  Given
that 'trace event' is currently used, it would be nice to remain
consistent/reduce confusion.

How about:
typedef struct {
    const char *name;
    bool enabled;
    unsigned int hash;
} TraceEventState;

Or a nicer overall change might be to rename enum TraceEvent to
TraceEventID and Tracepoint to TraceEvent.

> +
> +typedef struct {
>      unsigned long event;
>      unsigned long x1;
>      unsigned long x2;
> @@ -18,11 +24,29 @@ enum {
>  static TraceRecord trace_buf[TRACE_BUF_LEN];
>  static unsigned int trace_idx;
>  static FILE *trace_fp;
> +static Tracepoint trace_list[NR_TRACEPOINTS];
> +
> +void init_tracepoint(const char *tname, TraceEvent tevent)
> +{
> +    if (!tname || tevent > NR_TRACEPOINTS) {
> +        return;
> +    }

I'd drop this check because only trace.c should use init_tracepoint()
and you have ensured it uses correct arguments.  Just a coding style
suggestion; having redundant checks makes the code more verbose, may
lead the reader to assume that this function really is called with junk
arguments, and silently returning will not help make the issue visible.

> +    trace_list[tevent].tp_name = (char*)qemu_malloc(strlen(tname)+1);
> +    strncpy(trace_list[tevent].tp_name, tname, strlen(tname));

Or use qemmu_strdup() but we don't really need to allocate memory at all
here.  Just hold the const char* to a string literal since the trace
event is a static object that is built into the binary.

> +    trace_list[tevent].hash = qemu_hash(tname);
> +    trace_list[tevent].state = 1; /* Enable all by default */
> +}
> 
>  static void trace(TraceEvent event, unsigned long x1,
>                    unsigned long x2, unsigned long x3,
>                    unsigned long x4, unsigned long x5) {
>      TraceRecord *rec = &trace_buf[trace_idx];
> +
> +    if (!trace_list[event].state) {
> +        return;
> +    }
> +
>      rec->event = event;
>      rec->x1 = x1;
>      rec->x2 = x2;
> @@ -75,3 +99,42 @@ void do_info_trace(Monitor *mon)
>                              trace_buf[i].x3, trace_buf[i].x4, trace_buf[i].x5);
>      }
>  }
> +
> +void do_info_all_tracepoints(Monitor *mon)
> +{
> +    unsigned int i;
> +
> +    for (i=0; i<NR_TRACEPOINTS; i++) {
> +        monitor_printf(mon, "%s [Event ID %u] : state %u\n",
> +                                trace_list[i].tp_name, i, trace_list[i].state);
> +        }

Whitespace and indentation.

> +}
> +
> +static Tracepoint* find_tracepoint_by_name(const char *tname)
> +{
> +    unsigned int i, name_hash;
> +
> +    if (!tname) {
> +        return NULL;
> +    }
> +
> +    name_hash = qemu_hash(tname);
> +
> +    for (i=0; i<NR_TRACEPOINTS; i++) {
> +        if (trace_list[i].hash == name_hash &&
> +                       !strncmp(trace_list[i].tp_name, tname, strlen(tname))) {
> +            return &trace_list[i];
> +        }
> +    }
> +    return NULL; /* indicates end of list reached without a match */

I don't see the need for hashing.  We don't actually have a hash table
and are doing a linear search.  There will be a smallish constant number
of trace events and only change_tracepoint_by_name() needs to do a
lookup.  There's no need to optimize that.

Is strncmp() used so looking up "foo" is like searching for foo*?  The
strlen(tname) should be outside the loop.  I think that could be useful
but we should document it or just use strcmp().

> +}
> +
> +void change_tracepoint_state(const char *tname, bool tstate)
> +{
> +    Tracepoint *tp;
> +
> +    tp = find_tracepoint_by_name(tname);
> +    if (tp) {
> +        tp->state = tstate;
> +    }
> +}
> diff --git a/tracetool b/tracetool
> index 2c73bab..00af205 100755
> --- a/tracetool
> +++ b/tracetool
> @@ -123,14 +123,20 @@ linetoc_end_nop()
>  linetoh_begin_simple()
>  {
>      cat <<EOF
> +#include <stdbool.h>
> +
>  typedef unsigned int TraceEvent;
> 
> +void init_tracepoint_table(void);
> +void init_tracepoint(const char *tname, TraceEvent tevent);
>  void trace1(TraceEvent event, unsigned long x1);
>  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);
> +void do_info_all_tracepoints(Monitor *mon);
> +void change_tracepoint_state(const char *tname, bool tstate);
>  EOF
> 
>      simple_event_num=0
> @@ -163,22 +169,38 @@ EOF
> 
>  linetoh_end_simple()
>  {
> -    return
> +    cat <<EOF
> +#define NR_TRACEPOINTS $simple_event_num
> +EOF
>  }
> 
>  linetoc_begin_simple()
>  {
> -    return
> +    cat <<EOF
> +#include "trace.h"
> +
> +void init_tracepoint_table(void) {
> +EOF

Have you looked at module.h?  Perhaps that provides a good solution for
initializing trace events.  It would allow the vl.c changes to be done
without an #ifdef.

> +    simple_event_num=0
> +
>  }
> 
>  linetoc_simple()
>  {
> -    return
> +    local name
> +    name=$(get_name "$1")
> +    cat <<EOF
> +init_tracepoint("$name", $simple_event_num);
> +EOF
> +    simple_event_num=$((simple_event_num + 1))
>  }
> 
>  linetoc_end_simple()
>  {
> -    return
> +    cat <<EOF
> +return;

Please don't use return at the end of a void function.  Coding style.

> +}
> +EOF
>  }
> 
>  linetoh_begin_ust()
> diff --git a/vl.c b/vl.c
> index 328395e..dd07904 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -163,6 +163,9 @@ int main(int argc, char **argv)
>  #include "cpus.h"
>  #include "arch_init.h"
> 
> +#ifdef CONFIG_SIMPLE_TRACE
> +#include "trace.h"
> +#endif
>  //#define DEBUG_NET
>  //#define DEBUG_SLIRP
> 
> @@ -3604,6 +3607,9 @@ int main(int argc, char **argv, char **envp)
>      if (net_init_clients() < 0) {
>          exit(1);
>      }
> +#ifdef CONFIG_SIMPLE_TRACE
> +    init_tracepoint_table();
> +#endif
> 
>      /* init the bluetooth world */
>      if (foreach_device_config(DEV_BT, bt_parse))
> 
> 
> -- 
> Prerna Saxena
> 
> Linux Technology Centre,
> IBM Systems and Technology Lab,
> Bangalore, India
>