From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Subject: Re: [PATCH 2/3] Monitor command 'info trace'
Date: Thu, 17 Jun 2010 16:08:11 +0100
Message-ID: <20100617150810.GA15845@stefan-thinkpad.transitives.com>
References: <20100616180542.474535e3@zephyr>
 <20100616181206.65186bc4@zephyr>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: qemu-devel@nongnu.org, Anthony Liguori <aliguori@us.ibm.com>,
	kvm@vger.kernel.org, Stefan@us.ibm.com,
	Luiz Capitulino <lcapitulino@redhat.com>,
	Maneesh Soni <maneesh@linux.vnet.ibm.com>
To: Prerna Saxena <prerna@linux.vnet.ibm.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mtagate7.uk.ibm.com ([194.196.100.167]:38071 "EHLO
	mtagate7.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1755890Ab0FQPIV (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 17 Jun 2010 11:08:21 -0400
Received: from d06nrmr1407.portsmouth.uk.ibm.com (d06nrmr1407.portsmouth.uk.ibm.com [9.149.38.185])
	by mtagate7.uk.ibm.com (8.13.1/8.13.1) with ESMTP id o5HF8DF1028269
	for <kvm@vger.kernel.org>; Thu, 17 Jun 2010 15:08:13 GMT
Received: from d06av02.portsmouth.uk.ibm.com (d06av02.portsmouth.uk.ibm.com [9.149.37.228])
	by d06nrmr1407.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id o5HF8CR9585946
	for <kvm@vger.kernel.org>; Thu, 17 Jun 2010 16:08:12 +0100
Received: from d06av02.portsmouth.uk.ibm.com (loopback [127.0.0.1])
	by d06av02.portsmouth.uk.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id o5HF8Cf3020809
	for <kvm@vger.kernel.org>; Thu, 17 Jun 2010 16:08:12 +0100
Content-Disposition: inline
In-Reply-To: <20100616181206.65186bc4@zephyr>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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; i<max_idx ;i++) {

Whitespace "i=0; i<max_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