[Qemu-devel] Re: [PATCH][Tracing] Fix build errors for target i386-linux-user

[Stefan Hajnoczi]

On Wed, Jun 30, 2010 at 09:11:45PM +0530, Prerna Saxena wrote:
> [PATCH 1/1] Move definitions of monitor command handlers (do_info_trace, 
> do_info_all_trace_events) to monitor.c. This removes build errors for 
> user targets such as i386-linux-user, which are not linked with monitor.
> 
> The export of trace_buf and trace_idx is an unfortunate side effect, 
> since these are needed by do_info_trace which dumps buffer 
> contents.

"git grep monitor_printf" suggests that other source files do their own
monitor printing.  Did you look into alternatives that don't expose the
trace buffer internals?

I feel #ifndef CONFIG_USER_ONLY in simpletrace.c would be would be nicer
than exposing the internals.  (CONFIG_USER_ONLY is just a guess, I
haven't investigated which is the right thing to test for.)

Stefan

> 
> Signed-off-by: Prerna Saxena <address@hidden>
> ---
>  monitor.c     |   21 +++++++++++++++++++++
>  simpletrace.c |   39 ++-------------------------------------
>  tracetool     |   16 ++++++++++++++++
>  3 files changed, 39 insertions(+), 37 deletions(-)
> 
> diff --git a/monitor.c b/monitor.c
> index 433a3ec..9b5d65a 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -540,6 +540,27 @@ static void do_change_trace_event_state(Monitor *mon, 
> const QDict *qdict)
>      bool new_state = qdict_get_bool(qdict, "option");
>      change_trace_event_state(tp_name, new_state);
>  }
> +
> +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);
> +    }
> +}
> +
> +void do_info_all_trace_events(Monitor *mon)
> +{
> +    unsigned int i;
> +
> +    for (i = 0; i < NR_TRACE_EVENTS; i++) {
> +        monitor_printf(mon, "%s [Event ID %u] : state %u\n",
> +                                trace_list[i].tp_name, i, 
> trace_list[i].state);
> +    }
> +}
>  #endif
> 
>  static void user_monitor_complete(void *opaque, QObject *ret_data)
> diff --git a/simpletrace.c b/simpletrace.c
> index 57c41fc..834b4c1 100644
> --- a/simpletrace.c
> +++ b/simpletrace.c
> @@ -1,23 +1,9 @@
>  #include <stdlib.h>
>  #include <stdio.h>
> -#include "monitor.h"
>  #include "trace.h"
> 
> -typedef struct {
> -    unsigned long event;
> -    unsigned long x1;
> -    unsigned long x2;
> -    unsigned long x3;
> -    unsigned long x4;
> -    unsigned long x5;
> -} TraceRecord;
> -
> -enum {
> -    TRACE_BUF_LEN = 64 * 1024 / sizeof(TraceRecord),
> -};
> -
> -static TraceRecord trace_buf[TRACE_BUF_LEN];
> -static unsigned int trace_idx;
> +TraceRecord trace_buf[TRACE_BUF_LEN];
> +unsigned int trace_idx;
>  static FILE *trace_fp;
> 
>  static void trace(TraceEventID event, unsigned long x1,
> @@ -69,27 +55,6 @@ void trace5(TraceEventID event, unsigned long x1, unsigned 
> long x2, unsigned lon
>      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);
> -    }
> -}
> -
> -void do_info_all_trace_events(Monitor *mon)
> -{
> -    unsigned int i;
> -
> -    for (i = 0; i < NR_TRACE_EVENTS; i++) {
> -        monitor_printf(mon, "%s [Event ID %u] : state %u\n",
> -                                trace_list[i].tp_name, i, 
> trace_list[i].state);
> -    }
> -}
> -
>  static TraceEvent* find_trace_event_by_name(const char *tname)
>  {
>      unsigned int i;
> diff --git a/tracetool b/tracetool
> index c77280d..c7a690d 100755
> --- a/tracetool
> +++ b/tracetool
> @@ -125,6 +125,22 @@ typedef struct {
>      bool state;
>  } TraceEvent;
> 
> +typedef struct {
> +    unsigned long event;
> +    unsigned long x1;
> +    unsigned long x2;
> +    unsigned long x3;
> +    unsigned long x4;
> +    unsigned long x5;
> +} TraceRecord;
> +
> +enum {
> +    TRACE_BUF_LEN = 64 * 1024 / sizeof(TraceRecord),
> +};
> +
> +extern TraceRecord trace_buf[TRACE_BUF_LEN];
> +extern unsigned int trace_idx;
> +
>  void trace1(TraceEventID event, unsigned long x1);
>  void trace2(TraceEventID event, unsigned long x1, unsigned long x2);
>  void trace3(TraceEventID event, unsigned long x1, unsigned long x2, unsigned 
> long x3);
> -- 
> 1.6.2.5
> 
> 
> 
> -- 
> Prerna Saxena
> 
> Linux Technology Centre,
> IBM Systems and Technology Lab,
> Bangalore, India
>