From mboxrd@z Thu Jan 1 00:00:00 1970 From: Prerna Saxena Subject: Re: [PATCH 2/3] Monitor command 'info trace' Date: Fri, 18 Jun 2010 17:28:44 +0530 Message-ID: <4C1B5F74.8040609@linux.vnet.ibm.com> References: <20100616180542.474535e3@zephyr> <20100616181206.65186bc4@zephyr> <20100617150810.GA15845@stefan-thinkpad.transitives.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: qemu-devel@nongnu.org, Anthony Liguori , kvm@vger.kernel.org, Stefan@us.ibm.com, Luiz Capitulino , Maneesh Soni , Ananth To: Stefan Hajnoczi Return-path: Received: from e23smtp08.au.ibm.com ([202.81.31.141]:58809 "EHLO e23smtp08.au.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756645Ab0FRL6u (ORCPT ); Fri, 18 Jun 2010 07:58:50 -0400 Received: from d23relay05.au.ibm.com (d23relay05.au.ibm.com [202.81.31.247]) by e23smtp08.au.ibm.com (8.14.4/8.13.1) with ESMTP id o5IBwlv6021570 for ; Fri, 18 Jun 2010 21:58:47 +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 o5IBwm5d1327312 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 o5IBwlKE009704 for ; Fri, 18 Jun 2010 21:58:48 +1000 In-Reply-To: <20100617150810.GA15845@stefan-thinkpad.transitives.com> Sender: kvm-owner@vger.kernel.org List-ID: 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