From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=57874 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OPaDl-000593-Tp for qemu-devel@nongnu.org; Fri, 18 Jun 2010 07:58:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OPaDk-0001IH-Cq for qemu-devel@nongnu.org; Fri, 18 Jun 2010 07:58:53 -0400 Received: from e23smtp02.au.ibm.com ([202.81.31.144]:52873) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OPaDj-0001Hr-QU for qemu-devel@nongnu.org; Fri, 18 Jun 2010 07:58:52 -0400 Received: from d23relay05.au.ibm.com (d23relay05.au.ibm.com [202.81.31.247]) by e23smtp02.au.ibm.com (8.14.4/8.13.1) with ESMTP id o5IBt1f0031023 for ; Fri, 18 Jun 2010 21:55:01 +1000 Received: from d23av04.au.ibm.com (d23av04.au.ibm.com [9.190.235.139]) by d23relay05.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id o5IBwmvB999572 for ; Fri, 18 Jun 2010 21:58:48 +1000 Received: from d23av04.au.ibm.com (loopback [127.0.0.1]) by d23av04.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id o5IBwlK8009704 for ; Fri, 18 Jun 2010 21:58:47 +1000 Message-ID: <4C1B5F74.8040609@linux.vnet.ibm.com> Date: Fri, 18 Jun 2010 17:28:44 +0530 From: Prerna Saxena MIME-Version: 1.0 References: <20100616180542.474535e3@zephyr> <20100616181206.65186bc4@zephyr> <20100617150810.GA15845@stefan-thinkpad.transitives.com> In-Reply-To: <20100617150810.GA15845@stefan-thinkpad.transitives.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH 2/3] Monitor command 'info trace' List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Anthony Liguori , kvm@vger.kernel.org, qemu-devel@nongnu.org, Luiz Capitulino , Maneesh Soni , Ananth , Stefan@us.ibm.com 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; i > Whitespace "i=0; i 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