From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=59265 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1ON1lK-0004wl-4H for qemu-devel@nongnu.org; Fri, 11 Jun 2010 06:46:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1ON1lG-0001bM-1Z for qemu-devel@nongnu.org; Fri, 11 Jun 2010 06:46:57 -0400 Received: from e23smtp07.au.ibm.com ([202.81.31.140]:53570) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1ON1lF-0001UI-GP for qemu-devel@nongnu.org; Fri, 11 Jun 2010 06:46:54 -0400 Received: from d23relay04.au.ibm.com (d23relay04.au.ibm.com [202.81.31.246]) by e23smtp07.au.ibm.com (8.14.4/8.13.1) with ESMTP id o5BAkpOL010285 for ; Fri, 11 Jun 2010 20:46:51 +1000 Received: from d23av04.au.ibm.com (d23av04.au.ibm.com [9.190.235.139]) by d23relay04.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id o5BAklAZ1593586 for ; Fri, 11 Jun 2010 20:46:47 +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 o5BAkl5w023608 for ; Fri, 11 Jun 2010 20:46:47 +1000 Message-ID: <4C121414.7040507@linux.vnet.ibm.com> Date: Fri, 11 Jun 2010 16:16:44 +0530 From: Prerna Saxena MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 2/3] Monitor command 'trace' References: <20100608122803.6a684a5e@zephyr> <20100608123437.5f87d045@zephyr> <20100609173753.359e081e@redhat.com> In-Reply-To: <20100609173753.359e081e@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: Anthony Liguori , Hajnoczi , kvm@vger.kernel.org, Kiszka , qemu-devel@nongnu.org, maneesh@linux.vnet.ibm.com, ananth@linux.vnet.ibm.com, Stefan@us.ibm.com 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 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> + 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