On 04/06/2011 01:42 PM, Stefan Hajnoczi wrote:
> On Tue, Apr 5, 2011 at 2:30 PM, Stefan Hajnoczi <stefa...@gmail.com> wrote:
>> On Mon, Apr 4, 2011 at 10:49 PM, Lluís <xscr...@gmx.net> wrote:
>>> This patch defines the "disable" trace event state to always use the "nop"
>>> backend.
>>>
>>> As a side-effect, all events are now enabled (without "disable") by
>>> default, as
>>> all backends (except "stderr") have programmatic support for dynamically
>>> (de)activating each trace event.
>>>
>>> In order to make this true, the "simple" backend now has a "-trace
>>> events=<file>" argument to let the user select which events must be enabled
>>> from
>>> the very beginning.
>>>
>>> Signed-off-by: Lluís Vilanova <vilan...@ac.upc.edu>
>>> ---
>>>
>>> Lluís Vilanova (6):
>>> trace: [ust] fix generation of 'trace.c' on events without args
>>> trace: generalize the "property" concept in the trace-events file
>>> trace-state: always use the "nop" backend on events with the "disable"
>>> keyword
>>> trace-state: [simple] disable all trace points by default
>>> trace-state: [simple] add "-trace events" argument to control initial
>>> state
>>> trace: enable all events
>>>
>>>
>>> docs/tracing.txt | 12 +-
>>> qemu-config.c | 5 +
>>> qemu-options.hx | 18 ++
>>> scripts/tracetool | 88 +++++-------
>>> trace-events | 385
>>> ++++++++++++++++++++++++++---------------------------
>>> vl.c | 94 ++++++++-----
>>> 6 files changed, 313 insertions(+), 289 deletions(-)
>>
>> Excellent, thanks for implementing this. I'll review the patches in
>> detail shortly.
>
> I've left feedback on the individual patches. This is a nice cleanup,
> thanks for doing this work!
>
> The stderr backend is impacted - but not severely. You now need to
> disable all trace events that should not generate output. Previously
> it was the opposite; you needed to enable all trace events that should
> generate output.
>
> Adding Prerna (simpletrace) and Fabien (stderr) on CC so they can take a look.
Patches look good to me, it will be very useful to change event selections
without recompiling (I didn't know that trace-events could be enabled/disabled
in the monitor...).
I attach a patch that adds event selection to stderr based on what is done for
the simple backend.
Lluís can you please add it in your patch set?
>From 71fd4ae792aea78691415241be5cec5f2e9afbca Mon Sep 17 00:00:00 2001
From: Fabien Chouteau <chout...@adacore.com>
Date: Wed, 6 Apr 2011 16:15:53 +0200
Subject: [PATCH 1/1] Add trace-event selection from monitor in the stderr
backend
Signed-off-by: Fabien Chouteau <chout...@adacore.com>
---
Makefile.objs | 5 +++++
configure | 3 +++
monitor.c | 6 ++++--
qemu-config.c | 2 +-
qemu-options.hx | 2 +-
scripts/tracetool | 33 ++++++++++++++++++++++++++++-----
stderrtrace.c | 14 ++++++++++++++
stderrtrace.h | 13 +++++++++++++
vl.c | 10 ++++++++--
9 files changed, 77 insertions(+), 11 deletions(-)
create mode 100644 stderrtrace.c
create mode 100644 stderrtrace.h
diff --git a/Makefile.objs b/Makefile.objs
index c05f5e5..d58565a 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -342,6 +342,7 @@ trace-dtrace.o: trace-dtrace.dtrace $(GENERATED_HEADERS)
$(call quiet-command,dtrace -o $@ -G -s $<, " GEN trace-dtrace.o")
simpletrace.o: simpletrace.c $(GENERATED_HEADERS)
+stderrtrace.o: stderrtrace.c $(GENERATED_HEADERS)
ifeq ($(TRACE_BACKEND),dtrace)
trace-obj-y = trace-dtrace.o
@@ -351,6 +352,10 @@ ifeq ($(TRACE_BACKEND),simple)
trace-obj-y += simpletrace.o
user-obj-y += qemu-timer-common.o
endif
+
+ifeq ($(TRACE_BACKEND),stderr)
+trace-obj-y += stderrtrace.o
+endif
endif
######################################################################
diff --git a/configure b/configure
index faaed60..d80bb38 100755
--- a/configure
+++ b/configure
@@ -2933,6 +2933,9 @@ echo "TRACE_BACKEND=$trace_backend" >> $config_host_mak
if test "$trace_backend" = "simple"; then
echo "CONFIG_SIMPLE_TRACE=y" >> $config_host_mak
fi
+if test "$trace_backend" = "stderr"; then
+ echo "CONFIG_STDERR_TRACE=y" >> $config_host_mak
+fi
# Set the appropriate trace file.
if test "$trace_backend" = "simple"; then
trace_file="\"$trace_file-%u\""
diff --git a/monitor.c b/monitor.c
index f1a08dc..cb695c6 100644
--- a/monitor.c
+++ b/monitor.c
@@ -57,7 +57,7 @@
#include "json-parser.h"
#include "osdep.h"
#include "exec-all.h"
-#ifdef CONFIG_SIMPLE_TRACE
+#if defined(CONFIG_SIMPLE_TRACE) || defined(CONFIG_STDERR_TRACE)
#include "trace.h"
#endif
#include "ui/qemu-spice.h"
@@ -592,7 +592,7 @@ static void do_help_cmd(Monitor *mon, const QDict *qdict)
help_cmd(mon, qdict_get_try_str(qdict, "name"));
}
-#ifdef CONFIG_SIMPLE_TRACE
+#if defined(CONFIG_SIMPLE_TRACE) || defined(CONFIG_STDERR_TRACE)
static void do_change_trace_event_state(Monitor *mon, const QDict *qdict)
{
const char *tp_name = qdict_get_str(qdict, "name");
@@ -603,7 +603,9 @@ static void do_change_trace_event_state(Monitor *mon, const
QDict *qdict)
monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
}
}
+#endif
+#ifdef CONFIG_SIMPLE_TRACE
static void do_trace_file(Monitor *mon, const QDict *qdict)
{
const char *op = qdict_get_try_str(qdict, "op");
diff --git a/qemu-config.c b/qemu-config.c
index 0a00081..a524680 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -297,7 +297,7 @@ static QemuOptsList qemu_mon_opts = {
},
};
-#ifdef CONFIG_SIMPLE_TRACE
+#if defined(CONFIG_SIMPLE_TRACE) || defined(CONFIG_STDERR_TRACE)
static QemuOptsList qemu_trace_opts = {
.name = "trace",
.implied_opt_name = "file",
diff --git a/qemu-options.hx b/qemu-options.hx
index e7b93b5..13e3d71 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2356,7 +2356,7 @@ Normally QEMU loads a configuration file from
@var{sysconfdir}/qemu.conf and
@var{sysconfdir}/target-@var{ARCH}.conf on startup. The @code{-nodefconfig}
option will prevent QEMU from loading these configuration files at startup.
ETEXI
-#ifdef CONFIG_SIMPLE_TRACE
+#if defined(CONFIG_SIMPLE_TRACE) || defined(CONFIG_STDERR_TRACE)
DEF("trace", HAS_ARG, QEMU_OPTION_trace,
"-trace [file=]<file>[,events=<file>]\n"
" specify tracing options\n",
diff --git a/scripts/tracetool b/scripts/tracetool
index e3aec89..d43fbf0 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -241,7 +241,12 @@ linetoh_begin_stderr()
{
cat <<EOF
#include <stdio.h>
+#include "stderrtrace.h"
+
+extern TraceEvent trace_list[];
EOF
+
+ simple_event_num=0
}
linetoh_stderr()
@@ -260,29 +265,47 @@ linetoh_stderr()
cat <<EOF
static inline void trace_$name($args)
{
- fprintf(stderr, "$name $fmt\n" $argnames);
+ if (trace_list[$simple_event_num].state != 0) {
+ fprintf(stderr, "$name $fmt\n" $argnames);
+ }
}
EOF
+ simple_event_num=$((simple_event_num + 1))
+
}
linetoh_end_stderr()
{
-return
+ cat <<EOF
+#define NR_TRACE_EVENTS $simple_event_num
+EOF
}
linetoc_begin_stderr()
{
-return
+ cat <<EOF
+#include "trace.h"
+
+TraceEvent trace_list[] = {
+EOF
+ simple_event_num=0
}
linetoc_stderr()
{
-return
+ local name
+ name=$(get_name "$1")
+ cat <<EOF
+{.tp_name = "$name", .state=0},
+EOF
+ simple_event_num=$(($simple_event_num + 1))
}
linetoc_end_stderr()
{
-return
+ cat <<EOF
+};
+EOF
}
#END OF STDERR
diff --git a/stderrtrace.c b/stderrtrace.c
new file mode 100644
index 0000000..7c27203
--- /dev/null
+++ b/stderrtrace.c
@@ -0,0 +1,14 @@
+#include "trace.h"
+
+bool st_change_trace_event_state(const char *name, bool enabled)
+{
+ 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;
+}
diff --git a/stderrtrace.h b/stderrtrace.h
new file mode 100644
index 0000000..c2b148c
--- /dev/null
+++ b/stderrtrace.h
@@ -0,0 +1,13 @@
+#ifndef _STDERRTRACE_H_
+#define _STDERRTRACE_H_
+
+typedef uint64_t TraceEventID;
+
+typedef struct {
+ const char *tp_name;
+ bool state;
+} TraceEvent;
+
+bool st_change_trace_event_state(const char *name, bool enabled);
+
+#endif /* ! _STDERRTRACE_H_ */
diff --git a/vl.c b/vl.c
index 9fb6a41..d9160bf 100644
--- a/vl.c
+++ b/vl.c
@@ -155,8 +155,9 @@ int main(int argc, char **argv)
#include "slirp/libslirp.h"
+#if defined(CONFIG_SIMPLE_TRACE) || defined(CONFIG_STDERR_TRACE)
#include "trace.h"
-#include "simpletrace.h"
+#endif
#include "qemu-queue.h"
#include "cpus.h"
#include "arch_init.h"
@@ -2761,7 +2762,7 @@ int main(int argc, char **argv, char **envp)
}
xen_mode = XEN_ATTACH;
break;
-#ifdef CONFIG_SIMPLE_TRACE
+#if defined(CONFIG_SIMPLE_TRACE) || defined(CONFIG_STDERR_TRACE)
case QEMU_OPTION_trace:
opts = qemu_opts_parse(qemu_find_opts("trace"), optarg, 1);
if (opts) {
@@ -2815,9 +2816,13 @@ int main(int argc, char **argv, char **envp)
}
loc_set_none();
+#if defined(CONFIG_SIMPLE_TRACE) || defined(CONFIG_STDERR_TRACE)
+
+# if defined(CONFIG_SIMPLE_TRACE)
if (!st_init(trace_file)) {
fprintf(stderr, "warning: unable to initialize simple trace
backend\n");
}
+# endif
if (trace_events_file) {
FILE *trace_events_fp = fopen(trace_events_file, "r");
if (!trace_events_fp) {
@@ -2844,6 +2849,7 @@ int main(int argc, char **argv, char **envp)
exit(1);
}
}
+#endif
/* If no data_dir is specified then try to find it relative to the
executable path. */
--
1.7.1
--
Fabien Chouteau
