Patchwork [v5,04/10] trace-state: separate trace event control and query routines from the simple backend

login
register
mail settings
Submitter =?utf-8?Q?Llu=C3=ADs?=
Date June 28, 2011, 4:53 p.m.
Message ID <20110628165321.23482.80961.stgit@ginnungagap.bsc.es>
Download mbox | patch
Permalink /patch/102429/
State New
Headers show

Comments

=?utf-8?Q?Llu=C3=ADs?= - June 28, 2011, 4:53 p.m.
Move the 'st_print_trace_events' and 'st_change_trace_event_state' into
backend-agnostic 'trace_print_events' and 'trace_event_set_state' (respectively)
in the "trace/control.c" file.

By moving them, other backends will later be able to provide their own
implementation.

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 Makefile.objs   |    1 +
 hmp-commands.hx |    2 +-
 monitor.c       |   13 +++++++------
 trace/control.c |   36 ++++++++++++++++++++++++++++++++++++
 trace/control.h |   14 ++++++++++++++
 trace/simple.c  |   23 -----------------------
 trace/simple.h  |    2 --
 7 files changed, 59 insertions(+), 32 deletions(-)
 create mode 100644 trace/control.c
 create mode 100644 trace/control.h
Stefan Hajnoczi - July 15, 2011, 9:41 a.m.
On Tue, Jun 28, 2011 at 06:53:21PM +0200, Lluís wrote:
> Move the 'st_print_trace_events' and 'st_change_trace_event_state' into
> backend-agnostic 'trace_print_events' and 'trace_event_set_state' (respectively)
> in the "trace/control.c" file.

Renaming the functions makes sense but moving their implementations out
into control.c inside an #ifdef is not clean.  Why not keep their
implementations in simple.c?

Just renaming them to the generic name should be enough.  Let the linker
do the #ifdef (we only link in simple.o or we link in another trace
backend).  There's no need for control.c.

Stefan

Patch

diff --git a/Makefile.objs b/Makefile.objs
index 9a67374..fe22e8a 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -366,6 +366,7 @@  trace-obj-y += trace/simple.o
 user-obj-y += qemu-timer-common.o
 endif
 endif
+trace-obj-y += trace/control.o
 
 ######################################################################
 # smartcard
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 6ad8806..623e012 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -186,7 +186,7 @@  ETEXI
         .args_type  = "name:s,option:b",
         .params     = "name on|off",
         .help       = "changes status of a specific trace event",
-        .mhandler.cmd = do_change_trace_event_state,
+        .mhandler.cmd = do_trace_event_set_state,
     },
 
 STEXI
diff --git a/monitor.c b/monitor.c
index 67ceb46..f8954fa 100644
--- a/monitor.c
+++ b/monitor.c
@@ -58,7 +58,8 @@ 
 #include "osdep.h"
 #include "cpu.h"
 #ifdef CONFIG_SIMPLE_TRACE
-#include "trace.h"
+#include "trace/simple.h"
+#include "trace/control.h"
 #endif
 #include "ui/qemu-spice.h"
 
@@ -593,11 +594,11 @@  static void do_help_cmd(Monitor *mon, const QDict *qdict)
 }
 
 #ifdef CONFIG_SIMPLE_TRACE
-static void do_change_trace_event_state(Monitor *mon, const QDict *qdict)
+static void do_trace_event_set_state(Monitor *mon, const QDict *qdict)
 {
     const char *tp_name = qdict_get_str(qdict, "name");
     bool new_state = qdict_get_bool(qdict, "option");
-    int ret = st_change_trace_event_state(tp_name, new_state);
+    int ret = trace_event_set_state(tp_name, new_state);
 
     if (!ret) {
         monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
@@ -1002,9 +1003,9 @@  static void do_info_trace(Monitor *mon)
     st_print_trace((FILE *)mon, &monitor_fprintf);
 }
 
-static void do_info_trace_events(Monitor *mon)
+static void do_trace_print_events(Monitor *mon)
 {
-    st_print_trace_events((FILE *)mon, &monitor_fprintf);
+    trace_print_events((FILE *)mon, &monitor_fprintf);
 }
 #endif
 
@@ -3102,7 +3103,7 @@  static const mon_cmd_t info_cmds[] = {
         .args_type  = "",
         .params     = "",
         .help       = "show available trace-events & their state",
-        .mhandler.info = do_info_trace_events,
+        .mhandler.info = do_trace_print_events,
     },
 #endif
     {
diff --git a/trace/control.c b/trace/control.c
new file mode 100644
index 0000000..6138eab
--- /dev/null
+++ b/trace/control.c
@@ -0,0 +1,36 @@ 
+#include "trace/control.h"
+
+#include "trace.h"
+
+
+void trace_print_events(FILE *stream, fprintf_function stream_printf)
+{
+#if defined(CONFIG_SIMPLE_TRACE)
+    unsigned int i;
+
+    for (i = 0; i < NR_TRACE_EVENTS; i++) {
+        stream_printf(stream, "%s [Event ID %u] : state %u\n",
+                      trace_list[i].tp_name, i, trace_list[i].state);
+    }
+#else
+    fprintf(stderr, "qemu: warning: cannot print the trace events with the current backend\n");
+    stream_printf(stream, "error: operation not supported with the current backend\n");
+#endif
+}
+
+bool trace_event_set_state (const char *name, bool state)
+{
+#if defined(CONFIG_SIMPLE_TRACE)
+    unsigned int i;
+
+    for (i = 0; i < NR_TRACE_EVENTS; i++) {
+        if (!strcmp(trace_list[i].tp_name, name)) {
+            trace_list[i].state = state;
+            return true;
+        }
+    }
+#else
+    fprintf(stderr, "qemu: warning: cannot set the state of a trace event with the current backend\n");
+#endif
+    return false;
+}
diff --git a/trace/control.h b/trace/control.h
new file mode 100644
index 0000000..37d3aa0
--- /dev/null
+++ b/trace/control.h
@@ -0,0 +1,14 @@ 
+#ifndef TRACE_CONTROL_H
+#define TRACE_CONTROL_H
+
+#include <stdint.h>
+#include <stdbool.h>
+#include <stdio.h>
+
+#include "qemu-common.h"
+
+
+void trace_print_events (FILE *stream, fprintf_function stream_printf);
+bool trace_event_set_state (const char *name, bool state);
+
+#endif  /* TRACE_CONTROL_H */
diff --git a/trace/simple.c b/trace/simple.c
index f1dbb5e..8aa3376 100644
--- a/trace/simple.c
+++ b/trace/simple.c
@@ -302,29 +302,6 @@  void st_print_trace(FILE *stream, int (*stream_printf)(FILE *stream, const char
     }
 }
 
-void st_print_trace_events(FILE *stream, int (*stream_printf)(FILE *stream, const char *fmt, ...))
-{
-    unsigned int i;
-
-    for (i = 0; i < NR_TRACE_EVENTS; i++) {
-        stream_printf(stream, "%s [Event ID %u] : state %u\n",
-                      trace_list[i].tp_name, i, trace_list[i].state);
-    }
-}
-
-bool st_change_trace_event_state(const char *name, bool enabled)
-{
-    unsigned int i;
-
-    for (i = 0; i < NR_TRACE_EVENTS; i++) {
-        if (!strcmp(trace_list[i].tp_name, name)) {
-            trace_list[i].state = enabled;
-            return true;
-        }
-    }
-    return false;
-}
-
 void st_flush_trace_buffer(void)
 {
     flush_trace_file(true);
diff --git a/trace/simple.h b/trace/simple.h
index b19eef1..5a45250 100644
--- a/trace/simple.h
+++ b/trace/simple.h
@@ -31,8 +31,6 @@  void trace4(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t
 void trace5(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t x4, uint64_t x5);
 void trace6(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t x4, uint64_t x5, uint64_t x6);
 void st_print_trace(FILE *stream, fprintf_function stream_printf);
-void st_print_trace_events(FILE *stream, fprintf_function stream_printf);
-bool st_change_trace_event_state(const char *tname, bool tstate);
 void st_print_trace_file_status(FILE *stream, fprintf_function stream_printf);
 void st_set_trace_file_enabled(bool enable);
 bool st_set_trace_file(const char *file);