Re: [Qemu-devel] [PATCH v2 10/11] trace-state: [stderr] add support for dynamically enabling/disabling events
On 04/25/2011 08:10 PM, Paolo Bonzini wrote: On 04/25/2011 12:27 PM, Lluís wrote: But in any case, I'm still not sure if stderr should have programatic tracing state controls. Yes, please, stderr is even more useful than simple when you're using it under gdb. Agreed, trace control seems really useful with stderr, and we should be able to do this in a generic way (shared by simple and stderr backends). -- Fabien Chouteau
Re: [Qemu-devel] [PATCH v2 10/11] trace-state: [stderr] add support for dynamically enabling/disabling events
On Tue, Apr 26, 2011 at 1:30 PM, Fabien Chouteau chout...@adacore.com wrote: On 04/25/2011 08:10 PM, Paolo Bonzini wrote: On 04/25/2011 12:27 PM, Lluís wrote: But in any case, I'm still not sure if stderr should have programatic tracing state controls. Yes, please, stderr is even more useful than simple when you're using it under gdb. Agreed, trace control seems really useful with stderr, and we should be able to do this in a generic way (shared by simple and stderr backends). The commonality between stderr and simple is having a set of trace events with on/off states. Generating trace.h/trace.c is mostly common. Toggling trace event states from the monitor as well as -trace events=file are common. The simple backend additionally allows setting and flushing the output file. It also supports dumping the trace buffer. Stefan
Re: [Qemu-devel] [PATCH v2 10/11] trace-state: [stderr] add support for dynamically enabling/disabling events
On 04/26/2011 02:38 PM, Stefan Hajnoczi wrote: The simple backend additionally allows setting and flushing the output file. It also supports dumping the trace buffer. I agree that neither of these would be a particularly interesting addition to the stderr backend. Paolo
Re: [Qemu-devel] [PATCH v2 10/11] trace-state: [stderr] add support for dynamically enabling/disabling events
Stefan Hajnoczi writes: On Tue, Apr 26, 2011 at 1:30 PM, Fabien Chouteau chout...@adacore.com wrote: On 04/25/2011 08:10 PM, Paolo Bonzini wrote: On 04/25/2011 12:27 PM, Lluís wrote: But in any case, I'm still not sure if stderr should have programatic tracing state controls. Yes, please, stderr is even more useful than simple when you're using it under gdb. Agreed, trace control seems really useful with stderr, and we should be able to do this in a generic way (shared by simple and stderr backends). The commonality between stderr and simple is having a set of trace events with on/off states. Generating trace.h/trace.c is mostly common. Toggling trace event states from the monitor as well as -trace events=file are common. The simple backend additionally allows setting and flushing the output file. It also supports dumping the trace buffer. Ok, what I was thinking about long time ago is providing some generic trace_* interface for the common cmdline and monitor commands to use (basically list event names and query or change their dynamic state, which is the only common part on all backends). With this, every backend is responsible of providing a suitable implementation. If no implementation is provided, a default one will be used that simply results in qemu erroring out when any of these functionalities is invoked. I think this was already discussed, and the idea was rejected because it seemed too verbose for qemu to implement tracepoint controls when other external tools already exist for such backends (e.g., ust). Maybe providing the default implementation on the previous paragraph would solve the issues. Lluis -- And it's much the same thing with knowledge, for whenever you learn something new, the whole world becomes that much richer. -- The Princess of Pure Reason, as told by Norton Juster in The Phantom Tollbooth
Re: [Qemu-devel] [PATCH v2 10/11] trace-state: [stderr] add support for dynamically enabling/disabling events
Stefan Hajnoczi writes: On Sun, Apr 24, 2011 at 7:24 AM, Paolo Bonzini pbonz...@redhat.com wrote: On 04/23/2011 04:31 PM, Stefan Hajnoczi wrote: For tracing use cases that require performance or runtime enabling/disabling trace events, just use the simple, ust, or dtrace backends. Having -trace events for the stderr backend would still be nice. That should be doable without ifdefing and duplicating simpletrace. The tracer and monitor command parts of simpletrace need to be separated from common TraceEvent and tracetool generation, which can be reused by stderr. That's exactly what I thought, but I tried to preserve as much as possible the original patch that was sent to me. But in any case, I'm still not sure if stderr should have programatic tracing state controls. Lluis -- And it's much the same thing with knowledge, for whenever you learn something new, the whole world becomes that much richer. -- The Princess of Pure Reason, as told by Norton Juster in The Phantom Tollbooth
Re: [Qemu-devel] [PATCH v2 10/11] trace-state: [stderr] add support for dynamically enabling/disabling events
On 04/25/2011 12:27 PM, Lluís wrote: But in any case, I'm still not sure if stderr should have programatic tracing state controls. Yes, please, stderr is even more useful than simple when you're using it under gdb. Paolo
Re: [Qemu-devel] [PATCH v2 10/11] trace-state: [stderr] add support for dynamically enabling/disabling events
On 04/23/2011 04:31 PM, Stefan Hajnoczi wrote: For tracing use cases that require performance or runtime enabling/disabling trace events, just use the simple, ust, or dtrace backends. Having -trace events for the stderr backend would still be nice. Paolo
Re: [Qemu-devel] [PATCH v2 10/11] trace-state: [stderr] add support for dynamically enabling/disabling events
On Sun, Apr 24, 2011 at 7:24 AM, Paolo Bonzini pbonz...@redhat.com wrote: On 04/23/2011 04:31 PM, Stefan Hajnoczi wrote: For tracing use cases that require performance or runtime enabling/disabling trace events, just use the simple, ust, or dtrace backends. Having -trace events for the stderr backend would still be nice. That should be doable without ifdefing and duplicating simpletrace. The tracer and monitor command parts of simpletrace need to be separated from common TraceEvent and tracetool generation, which can be reused by stderr. Stefan
Re: [Qemu-devel] [PATCH v2 10/11] trace-state: [stderr] add support for dynamically enabling/disabling events
On Wed, Apr 6, 2011 at 7:35 PM, Lluís xscr...@gmx.net wrote: This includes all the control interfaces already provided by the simple backend (i.e., command line, programmatic and monitor). Signed-off-by: Fabien Chouteau chout...@adacore.com Signed-off-by: Lluís Vilanova vilan...@ac.upc.edu --- Makefile.objs | 8 +++- configure | 3 +++ docs/tracing.txt | 6 ++ hmp-commands.hx | 10 -- monitor.c | 8 +++- qemu-config.c | 7 --- qemu-options.hx | 8 +++- scripts/tracetool | 33 - stderrtrace.c | 24 stderrtrace.h | 14 ++ vl.c | 11 +-- 11 files changed, 113 insertions(+), 19 deletions(-) create mode 100644 stderrtrace.c create mode 100644 stderrtrace.h I feel that the monitor commands for the simple backend were a mistake. Simple trace is used during development, not production, so being able to toggle trace events at runtime is probably not worth the extra user interfaces we've added. But when the simple backend was written we thought it would be used in production and planned for libvirt interfaces and all ;). Let's not go down that road for the stderr backend which is very useful today at a tiny cost in code size. For tracing use cases that require performance or runtime enabling/disabling trace events, just use the simple, ust, or dtrace backends. Please drop this patch. Stefan
[Qemu-devel] [PATCH v2 10/11] trace-state: [stderr] add support for dynamically enabling/disabling events
This includes all the control interfaces already provided by the simple
backend (i.e., command line, programmatic and monitor).
Signed-off-by: Fabien Chouteau chout...@adacore.com
Signed-off-by: Lluís Vilanova vilan...@ac.upc.edu
---
Makefile.objs |8 +++-
configure |3 +++
docs/tracing.txt |6 ++
hmp-commands.hx | 10 --
monitor.c |8 +++-
qemu-config.c |7 ---
qemu-options.hx |8 +++-
scripts/tracetool | 33 -
stderrtrace.c | 24
stderrtrace.h | 14 ++
vl.c | 11 +--
11 files changed, 113 insertions(+), 19 deletions(-)
create mode 100644 stderrtrace.c
create mode 100644 stderrtrace.h
diff --git a/Makefile.objs b/Makefile.objs
index c05f5e5..f7cd67b 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -342,15 +342,22 @@ 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
else
trace-obj-y = trace.o
+
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
##
@@ -361,4 +368,3 @@ libcacard-y = cac.o event.o vcard.o vreader.o
vcard_emul_nss.o vcard_emul_type.o
vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS)
-
diff --git a/configure b/configure
index 8754060..7dc9bdc 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/docs/tracing.txt b/docs/tracing.txt
index 26b221f..51fe0ad 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -134,10 +134,8 @@ effectively turns trace events into debug printfs.
This is the simplest backend and can be used together with existing code that
uses DPRINTF().
-Note that with this backend trace events cannot be programmatically
-enabled/disabled. Thus, in order to trim down the amount of output and the
-performance impact of tracing, you might want to add the disable property in
-the trace-events file for those events you are not interested in.
+See the documentation in the simple backend for instruction on how to control
+trace event states from the command line, the monitor and/or programmatically.
=== Simpletrace ===
diff --git a/hmp-commands.hx b/hmp-commands.hx
index c3be311..7cca351 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -180,7 +180,7 @@ STEXI
Output logs to @var{filename}.
ETEXI
-#ifdef CONFIG_SIMPLE_TRACE
+#if defined(CONFIG_SIMPLE_TRACE) || defined(CONFIG_STDERR_TRACE)
{
.name = trace-event,
.args_type = name:s,option:b,
@@ -194,7 +194,9 @@ STEXI
@findex trace-event
changes status of a trace event
ETEXI
+#endif
+#if defined(CONFIG_SIMPLE_TRACE)
{
.name = trace-file,
.args_type = op:s?,arg:F?,
@@ -1355,10 +1357,14 @@ show roms
@end table
ETEXI
-#ifdef CONFIG_SIMPLE_TRACE
+#if defined(CONFIG_SIMPLE_TRACE)
STEXI
@item info trace
show contents of trace buffer
+ETEXI
+#endif
+#if defined(CONFIG_SIMPLE_TRACE) || defined(CONFIG_STDERR_TRACE)
+STEXI
@item info trace-events
show available trace events and their state
ETEXI
diff --git a/monitor.c b/monitor.c
index 377424e..7991c6c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -590,7 +590,7 @@ static void do_help_cmd(Monitor *mon, const QDict *qdict)
help_cmd(mon, qdict_get_try_str(qdict, name));
}
-#if defined(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);
@@ -601,7 +601,9 @@ static void do_change_trace_event_state(Monitor *mon, const
QDict *qdict)
monitor_printf(mon, unknown event name \%s\\n, tp_name);
}
}
+#endif
+#if defined(CONFIG_SIMPLE_TRACE)
static void do_trace_file(Monitor *mon, const QDict *qdict)
{
const char *op = qdict_get_try_str(qdict, op);
@@ -999,7 +1001,9 @@ static void do_info_trace(Monitor *mon)
{
st_print_trace((FILE *)mon, monitor_fprintf);
}
+#endif
+#if defined(CONFIG_SIMPLE_TRACE) || defined(CONFIG_STDERR_TRACE)
static void do_info_trace_events(Monitor *mon)
{
st_print_trace_events((FILE *)mon, monitor_fprintf);
@@ -3103,6 +3107,8 @@ static const mon_cmd_t info_cmds[] = {
