[RFC v4][PATCH 2/3] Monitor command 'info trace'

2010-06-24 Thread Prerna Saxena 
Monitor command 'info trace' to display contents of trace buffer

Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com
---
 configure   |3 +++
 monitor.c   |   12 
 qemu-monitor.hx |4 
 simpletrace.c   |   12 
 tracetool   |1 +
 5 files changed, 32 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..8b60830 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
@@ -2780,6 +2783,15 @@ static const mon_cmd_t info_cmds[] = {
 .help   = show roms,
 .mhandler.info = do_info_roms,
 },
+#if defined(CONFIG_SIMPLE_TRACE)
+{
+.name   = trace,
+.args_type  = ,
+.params = ,
+.help   = show current contents of trace buffer,
+.mhandler.info = do_info_trace,
+},
+#endif
 {
 .name   = NULL,
 },
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index b6e3467..766c30f 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -114,6 +114,10 @@ show migration status
 show balloon information
 @item info qtree
 show device tree
+#ifdef CONFIG_SIMPLE_TRACE
+...@item info trace
+show contents of trace buffer
+#endif
 @end table
 ETEXI
 
diff --git a/simpletrace.c b/simpletrace.c
index b488380..5734d15 100644
--- a/simpletrace.c
+++ b/simpletrace.c
@@ -1,5 +1,6 @@
 #include stdlib.h
 #include stdio.h
+#include monitor.h
 #include trace.h
 
 typedef struct {
@@ -62,3 +63,14 @@ void trace4(TraceEventID event, unsigned long x1, unsigned 
long x2, unsigned lon
 void trace5(TraceEventID 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)
+{
+unsigned int i;
+
+for (i = 0; i  trace_idx ; i++) {
+monitor_printf(mon, Event %lu : %lx %lx %lx %lx %lx\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);
+}
+}
diff --git a/tracetool b/tracetool
index 0dc7f1b..6f8f8a9 100755
--- a/tracetool
+++ b/tracetool
@@ -130,6 +130,7 @@ void trace2(TraceEventID event, unsigned long x1, unsigned 
long x2);
 void trace3(TraceEventID event, unsigned long x1, unsigned long x2, unsigned 
long x3);
 void trace4(TraceEventID event, unsigned long x1, unsigned long x2, unsigned 
long x3, unsigned long x4);
 void trace5(TraceEventID 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
-- 
1.6.2.5



-- 
Prerna Saxena

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

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] Monitor command 'info trace'

2010-06-18 Thread Prerna Saxena 

Hi Stefan, Jan,
Thanks for taking a look.

On 06/17/2010 08:38 PM, Stefan Hajnoczi wrote:

On Wed, Jun 16, 2010 at 06:12:06PM +0530, Prerna Saxena wrote:

diff --git a/simpletrace.c b/simpletrace.c
index 2fec4d3..239ae3f 100644
--- a/simpletrace.c
+++ b/simpletrace.c
@@ -62,3 +62,16 @@ 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)
+{
+unsigned int i, max_idx;
+
+max_idx = trace_idx ? trace_idx : TRACE_BUF_LEN;


trace_idx is always in the range [0, TRACE_BUF_LEN).  There is no need
to perform this test.


I only display the logged contents in the trace buffer (till trace_idx) 
, and not the entire trace buffer. Only when the index is full that the 
entire buffer is displayed.





+
+for (i=0; imax_idx ;i++) {


Whitespace i=0; imax_idx ;i++.  i = 0; i  max_idx; i++ is pretty
common across QEMU.



I'll fix this.


+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);


Getting only numeric output is the limitation of a binary trace.  It
would probably be possible to pretty-print without much additional code
by using the format strings from the trace-events file.

I think the numeric dump is good for now though.  Hex is more compact
than decimal and would make pointers easier to spot.  Want to change
this?



I agree, but we can let this be a todo till after the first prototype 
goes upstream ?



+}
+}
diff --git a/tracetool b/tracetool
index 9ea9c08..2c73bab 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,7 @@ tracetoh()
  #define TRACE_H

  #include qemu-common.h
+#include monitor.h


qemu-common.h forward-declares Monitor, I don't think you need
monitor.h.



Right.


Stefan


I'll post patches by Monday that addresses your suggestions, and try to 
get it integrated with QMP.


--
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] Monitor command 'info trace'

2010-06-18 Thread Stefan Hajnoczi 
On Fri, Jun 18, 2010 at 12:58 PM, Prerna Saxena
pre...@linux.vnet.ibm.com wrote:
 Hi Stefan, Jan,
 Thanks for taking a look.

 On 06/17/2010 08:38 PM, Stefan Hajnoczi wrote:

 On Wed, Jun 16, 2010 at 06:12:06PM +0530, Prerna Saxena wrote:

 diff --git a/simpletrace.c b/simpletrace.c
 index 2fec4d3..239ae3f 100644
 --- a/simpletrace.c
 +++ b/simpletrace.c
 @@ -62,3 +62,16 @@ 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)
 +{
 +    unsigned int i, max_idx;
 +
 +    max_idx = trace_idx ? trace_idx : TRACE_BUF_LEN;

 trace_idx is always in the range [0, TRACE_BUF_LEN).  There is no need
 to perform this test.

 I only display the logged contents in the trace buffer (till trace_idx) ,
 and not the entire trace buffer. Only when the index is full that the entire
 buffer is displayed.

Thanks for explaining, I understand what you are doing now.  Due to
this special case, the code will dump out the empty trace buffer if
used before anything has been traced (trace_idx=0).

 +        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);

 Getting only numeric output is the limitation of a binary trace.  It
 would probably be possible to pretty-print without much additional code
 by using the format strings from the trace-events file.

 I think the numeric dump is good for now though.  Hex is more compact
 than decimal and would make pointers easier to spot.  Want to change
 this?


 I agree, but we can let this be a todo till after the first prototype goes
 upstream ?

I still vote for hex instead of decimal :).  Since you're already
spinning a new patch it would be nice to put that change in, but no
worries.

 I'll post patches by Monday that addresses your suggestions, and try to get
 it integrated with QMP.

Excellent, thanks.  I'd like to put your patches onto my tracing
branch soon and test out the overall workflow of tracing QEMU.

Stefan
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] Monitor command 'info trace'

2010-06-17 Thread Stefan Hajnoczi 
On Wed, Jun 16, 2010 at 06:12:06PM +0530, Prerna Saxena wrote:
 diff --git a/simpletrace.c b/simpletrace.c
 index 2fec4d3..239ae3f 100644
 --- a/simpletrace.c
 +++ b/simpletrace.c
 @@ -62,3 +62,16 @@ 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)
 +{
 +unsigned int i, max_idx;
 +
 +max_idx = trace_idx ? trace_idx : TRACE_BUF_LEN;

trace_idx is always in the range [0, TRACE_BUF_LEN).  There is no need
to perform this test.

 +
 +for (i=0; imax_idx ;i++) {

Whitespace i=0; imax_idx ;i++.  i = 0; i  max_idx; i++ is pretty
common across QEMU.

 +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);

Getting only numeric output is the limitation of a binary trace.  It
would probably be possible to pretty-print without much additional code
by using the format strings from the trace-events file.

I think the numeric dump is good for now though.  Hex is more compact
than decimal and would make pointers easier to spot.  Want to change
this?

 +}
 +}
 diff --git a/tracetool b/tracetool
 index 9ea9c08..2c73bab 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,7 @@ tracetoh()
  #define TRACE_H
 
  #include qemu-common.h
 +#include monitor.h

qemu-common.h forward-declares Monitor, I don't think you need
monitor.h.

Stefan
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] Monitor command 'info trace'

2010-06-16 Thread Prerna Saxena 
Monitor command 'info trace' to display contents of trace buffer

Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com
---

 configure   |3 +++
 monitor.c   |   12 
 qemu-monitor.hx |4 
 simpletrace.c   |   13 +
 tracetool   |2 ++
 5 files changed, 34 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..8b60830 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
@@ -2780,6 +2783,15 @@ static const mon_cmd_t info_cmds[] = {
 .help   = show roms,
 .mhandler.info = do_info_roms,
 },
+#if defined(CONFIG_SIMPLE_TRACE)
+{
+.name   = trace,
+.args_type  = ,
+.params = ,
+.help   = show current contents of trace buffer,
+.mhandler.info = do_info_trace,
+},
+#endif
 {
 .name   = NULL,
 },
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index b6e3467..766c30f 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -114,6 +114,10 @@ show migration status
 show balloon information
 @item info qtree
 show device tree
+#ifdef CONFIG_SIMPLE_TRACE
+...@item info trace
+show contents of trace buffer
+#endif
 @end table
 ETEXI
 
diff --git a/simpletrace.c b/simpletrace.c
index 2fec4d3..239ae3f 100644
--- a/simpletrace.c
+++ b/simpletrace.c
@@ -62,3 +62,16 @@ 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)
+{
+unsigned int i, max_idx;
+
+max_idx = trace_idx ? trace_idx : TRACE_BUF_LEN;
+
+for (i=0; imax_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);
+}
+}
diff --git a/tracetool b/tracetool
index 9ea9c08..2c73bab 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,7 @@ tracetoh()
 #define TRACE_H
 
 #include qemu-common.h
+#include monitor.h
 EOF
 convert h
 echo #endif /* TRACE_H */


-- 
Prerna Saxena

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

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC v2] [PATCH 2/3] Monitor command 'info trace'

2010-06-10 Thread Prerna Saxena 
This introduces the monitor command 'info trace' to display current 
contents of trace buffer.

Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com
---
 configure   |3 +++
 monitor.c   |   12 
 qemu-monitor.hx |4 
 simpletrace.c   |   12 
 tracetool   |2 ++
 5 files changed, 33 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..8b60830 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
@@ -2780,6 +2783,15 @@ static const mon_cmd_t info_cmds[] = {
 .help   = show roms,
 .mhandler.info = do_info_roms,
 },
+#if defined(CONFIG_SIMPLE_TRACE)
+{
+.name   = trace,
+.args_type  = ,
+.params = ,
+.help   = show current contents of trace buffer,
+.mhandler.info = do_info_trace,
+},
+#endif
 {
 .name   = NULL,
 },
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index b6e3467..766c30f 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -114,6 +114,10 @@ show migration status
 show balloon information
 @item info qtree
 show device tree
+#ifdef CONFIG_SIMPLE_TRACE
+...@item info trace
+show contents of trace buffer
+#endif
 @end table
 ETEXI
 
diff --git a/simpletrace.c b/simpletrace.c
index 2fec4d3..00df45a 100644
--- a/simpletrace.c
+++ b/simpletrace.c
@@ -62,3 +62,15 @@ 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)
+{
+unsigned int i, max_idx;
+
+max_idx = trace_idx ? trace_idx : TRACE_BUF_LEN;
+
+for (i=0; imax_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);
+}
diff --git a/tracetool b/tracetool
index 9ea9c08..2c73bab 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,7 @@ tracetoh()
 #define TRACE_H
 
 #include qemu-common.h
+#include monitor.h
 EOF
 convert h
 echo #endif /* TRACE_H */
-- 
1.6.2.5



-- 
Prerna Saxena

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

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html