[Qemu-devel] [PATCH v4 0/2] trace: Trace control commands

[Qemu-devel] [PATCH v4 0/2] trace: Trace control commands

[Lluís Vilanova]

Adds QAPI/QMP commands to control tracing events, and reimplements some of the
related HMP commands on top.

NOTE: The "trace-event-set-state" command uses a bool 'enable' argument instead
      of an enum 'state'. I'm still not sure if an enum is better than the two
      separate booleans.

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---

Changes in v4:

* Split QAPI/QMP and HMP changes into separate patches.
* Add copyright information in QAPI file (Eric Blake).
* Replace event state booleans with an enum (Eric Blake).
* Document pattern type for argument "name" (Eric Blake).
* Other documentation improvements (Markus Armbruster).
* Change "keepgoing" to "ignore-unavailable" (Markus Armbruster).
* Add examples in QMP command definition file (Eric Blake).
* Propagate QMP errors to HMP (Markus Armbruster, Stefan Hajnoczi).
* Remove trailing newlines when using error_setg (Stefan Hajnoczi).
* Avoid multiple invocations of error_setg (Stefan Hajnoczi).
* Various cosmetic changes (Markus Armbruster).

Lluís Vilanova (2):
      trace: [qmp] Add commands to query and control event tracing state
      trace: [hmp] Reimplement "trace-event" and "info trace-events" using QMP


 monitor.c           |   27 ++++++++++--------
 qapi-schema.json    |    3 ++
 qapi/trace.json     |   65 ++++++++++++++++++++++++++++++++++++++++++++
 qmp-commands.hx     |   35 ++++++++++++++++++++++++
 trace/Makefile.objs |    1 +
 trace/control.c     |   13 ---------
 trace/control.h     |    7 -----
 trace/qmp.c         |   75 +++++++++++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 193 insertions(+), 33 deletions(-)
 create mode 100644 qapi/trace.json
 create mode 100644 trace/qmp.c


To: qemu-devel@nongnu.org
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Luiz Capitulino <lcapitulino@redhat.com>

[Qemu-devel] [PATCH v4 1/2] trace: [qmp] Add commands to query and control event tracing state

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 qapi-schema.json    |    3 ++
 qapi/trace.json     |   65 ++++++++++++++++++++++++++++++++++++++++++++
 qmp-commands.hx     |   35 ++++++++++++++++++++++++
 trace/Makefile.objs |    1 +
 trace/qmp.c         |   75 +++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 179 insertions(+)
 create mode 100644 qapi/trace.json
 create mode 100644 trace/qmp.c

diff --git a/qapi-schema.json b/qapi-schema.json
index 341f417..42b90df 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -11,6 +11,9 @@
 # QAPI event definitions
 { 'include': 'qapi/event.json' }
 
+# Tracing commands
+{ 'include': 'qapi/trace.json' }
+
 ##
 # LostTickPolicy:
 #
diff --git a/qapi/trace.json b/qapi/trace.json
new file mode 100644
index 0000000..06c613c
--- /dev/null
+++ b/qapi/trace.json
@@ -0,0 +1,65 @@
+# -*- mode: python -*-
+#
+# Copyright (C) 2011-2014 Lluís Vilanova <vilanova@ac.upc.edu>
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or later.
+# See the COPYING file in the top-level directory.
+
+
+##
+# @TraceEventState:
+#
+# State of a tracing event.
+#
+# @unavailable: The event is statically disabled.
+#
+# @disabled: The event is dynamically disabled.
+#
+# @enabled: The event is dynamically enabled.
+#
+# Since 2.2
+##
+{ 'enum': 'TraceEventState',
+  'data': ['unavailable', 'disabled', 'enabled'] }
+
+##
+# @TraceEventInfo:
+#
+# Information of a tracing event.
+#
+# @name: Event name.
+# @state: Tracing state.
+#
+# Since 2.2
+##
+{ 'type': 'TraceEventInfo',
+  'data': {'name': 'str', 'state': 'TraceEventState'} }
+
+##
+# @trace-event-get-state:
+#
+# Query the state of events.
+#
+# @name: Event name pattern (case-sensitive glob).
+#
+# Returns: a list of @TraceEventInfo for the matching events
+#
+# Since 2.2
+##
+{ 'command': 'trace-event-get-state',
+  'data': {'name': 'str'},
+  'returns': ['TraceEventInfo'] }
+
+##
+# @trace-event-set-state:
+#
+# Set the dynamic tracing state of events.
+#
+# @name: Event name pattern (case-sensitive glob).
+# @enable: Whether to enable tracing.
+# @ignore-unavailable: #optional Do not match unavailable events with @name.
+#
+# Since 2.2
+##
+{ 'command': 'trace-event-set-state',
+  'data': {'name': 'str', 'enable': 'bool', '*ignore-unavailable': 'bool'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 4be4765..b605517 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3753,5 +3753,40 @@ Example:
 
 -> { "execute": "rtc-reset-reinjection" }
 <- { "return": {} }
+EQMP
+
+    {
+        .name       = "trace-event-get-state",
+        .args_type  = "name:s",
+        .mhandler.cmd_new = qmp_marshal_input_trace_event_get_state,
+    },
+
+SQMP
+trace-event-get-state
+---------------------
+
+Query the state of events.
+
+Example:
+
+-> { "execute": "trace-event-get-state", "arguments": { "name": "qemu_memalign" } }
+<- { "return": [ { "name": "qemu_memalign", "state": "disabled" } ] }
+EQMP
+
+    {
+        .name       = "trace-event-set-state",
+        .args_type  = "name:s,enable:b,ignore-unavailable:b?",
+        .mhandler.cmd_new = qmp_marshal_input_trace_event_set_state,
+    },
 
+SQMP
+trace-event-set-state
+---------------------
+
+Set the state of events.
+
+Example:
+
+-> { "execute": "trace-event-set-state", "arguments": { "name": "qemu_memalign", "enable": "true" } }
+<- { "return": {} }
 EQMP
diff --git a/trace/Makefile.objs b/trace/Makefile.objs
index 387f191..01b3718 100644
--- a/trace/Makefile.objs
+++ b/trace/Makefile.objs
@@ -145,3 +145,4 @@ util-obj-$(CONFIG_TRACE_FTRACE) += ftrace.o
 util-obj-$(CONFIG_TRACE_UST) += generated-ust.o
 util-obj-y += control.o
 util-obj-y += generated-tracers.o
+util-obj-y += qmp.o
diff --git a/trace/qmp.c b/trace/qmp.c
new file mode 100644
index 0000000..0b19489
--- /dev/null
+++ b/trace/qmp.c
@@ -0,0 +1,75 @@
+/*
+ * QMP commands for tracing events.
+ *
+ * Copyright (C) 2014 Lluís Vilanova <vilanova@ac.upc.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/typedefs.h"
+#include "qmp-commands.h"
+#include "trace/control.h"
+
+
+TraceEventInfoList *qmp_trace_event_get_state(const char *name, Error **errp)
+{
+    TraceEventInfoList *events = NULL;
+    bool found = false;
+    TraceEvent *ev;
+
+    ev = NULL;
+    while ((ev = trace_event_pattern(name, ev)) != NULL) {
+        TraceEventInfoList *elem = g_new(TraceEventInfoList, 1);
+        elem->value = g_new(TraceEventInfo, 1);
+        elem->value->name = g_strdup(trace_event_get_name(ev));
+        if (!trace_event_get_state_static(ev)) {
+            elem->value->state = TRACE_EVENT_STATE_UNAVAILABLE;
+        } else if (!trace_event_get_state_dynamic(ev)) {
+            elem->value->state = TRACE_EVENT_STATE_DISABLED;
+        } else {
+            elem->value->state = TRACE_EVENT_STATE_ENABLED;
+        }
+        elem->next = events;
+        events = elem;
+        found = true;
+    }
+
+    if (!found && !trace_event_is_pattern(name)) {
+        error_setg(errp, "unknown event \"%s\"", name);
+    }
+
+    return events;
+}
+
+void qmp_trace_event_set_state(const char *name, bool enable,
+                               bool has_ignore_unavailable,
+                               bool ignore_unavailable, Error **errp)
+{
+    bool found = false;
+    TraceEvent *ev;
+
+    /* Check all selected events are dynamic */
+    ev = NULL;
+    while ((ev = trace_event_pattern(name, ev)) != NULL) {
+        found = true;
+        if (!(has_ignore_unavailable && ignore_unavailable) &&
+            !trace_event_get_state_static(ev)) {
+            error_setg(errp, "cannot set dynamic tracing state for \"%s\"",
+                       trace_event_get_name(ev));
+            return;
+        }
+    }
+    if (!found && !trace_event_is_pattern(name)) {
+        error_setg(errp, "unknown event \"%s\"", name);
+        return;
+    }
+
+    /* Apply changes */
+    ev = NULL;
+    while ((ev = trace_event_pattern(name, ev)) != NULL) {
+        if (trace_event_get_state_static(ev)) {
+            trace_event_set_state_dynamic(ev, enable);
+        }
+    }
+}

Re: [Qemu-devel] [PATCH v4 1/2] trace: [qmp] Add commands to query and control event tracing state

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1418 bytes --]

On 08/25/2014 05:19 AM, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
>  qapi-schema.json    |    3 ++
>  qapi/trace.json     |   65 ++++++++++++++++++++++++++++++++++++++++++++
>  qmp-commands.hx     |   35 ++++++++++++++++++++++++
>  trace/Makefile.objs |    1 +
>  trace/qmp.c         |   75 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 179 insertions(+)
>  create mode 100644 qapi/trace.json
>  create mode 100644 trace/qmp.c

> +##
> +# @trace-event-set-state:
> +#
> +# Set the dynamic tracing state of events.
> +#
> +# @name: Event name pattern (case-sensitive glob).
> +# @enable: Whether to enable tracing.
> +# @ignore-unavailable: #optional Do not match unavailable events with @name.

Would be nice to state that this bool defaults to false (at least,
that's my assumption).


> +
> +Query the state of events.
> +
> +Example:
> +
> +-> { "execute": "trace-event-get-state", "arguments": { "name": "qemu_memalign" } }
> +<- { "return": [ { "name": "qemu_memalign", "state": "disabled" } ] }

It would be nice to use a glob in the "name" of the example, and show a
multi-element array as the result.

Those changes can be done in a followup patch, since Stefan has already
queued this patch.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

[Qemu-devel] [PATCH v4 2/2] trace: [hmp] Reimplement "trace-event" and "info trace-events" using QMP

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 monitor.c       |   27 ++++++++++++++-------------
 trace/control.c |   13 -------------
 trace/control.h |    7 -------
 3 files changed, 14 insertions(+), 33 deletions(-)

diff --git a/monitor.c b/monitor.c
index cdbaa60..551dd0b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -886,19 +886,12 @@ 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");
+    Error *local_err = NULL;
 
-    bool found = false;
-    TraceEvent *ev = NULL;
-    while ((ev = trace_event_pattern(tp_name, ev)) != NULL) {
-        found = true;
-        if (!trace_event_get_state_static(ev)) {
-            monitor_printf(mon, "event \"%s\" is not traceable\n", tp_name);
-        } else {
-            trace_event_set_state_dynamic(ev, new_state);
-        }
-    }
-    if (!trace_event_is_pattern(tp_name) && !found) {
-        monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
+    qmp_trace_event_set_state(tp_name, new_state, true, true, &local_err);
+    if (local_err) {
+        qerror_report_err(local_err);
+        error_free(local_err);
     }
 }
 
@@ -1079,7 +1072,15 @@ static void do_info_cpu_stats(Monitor *mon, const QDict *qdict)
 
 static void do_trace_print_events(Monitor *mon, const QDict *qdict)
 {
-    trace_print_events((FILE *)mon, &monitor_fprintf);
+    TraceEventInfoList *events = qmp_trace_event_get_state("*", NULL);
+    TraceEventInfoList *elem;
+
+    for (elem = events; elem != NULL; elem = elem->next) {
+        monitor_printf(mon, "%s : state %u\n",
+                       elem->value->name,
+                       elem->value->state == TRACE_EVENT_STATE_ENABLED ? 1 : 0);
+    }
+    qapi_free_TraceEventInfoList(events);
 }
 
 static int client_migrate_info(Monitor *mon, const QDict *qdict,
diff --git a/trace/control.c b/trace/control.c
index 9631a40..0d30801 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -85,19 +85,6 @@ TraceEvent *trace_event_pattern(const char *pat, TraceEvent *ev)
     return NULL;
 }
 
-void trace_print_events(FILE *stream, fprintf_function stream_printf)
-{
-    TraceEventID i;
-
-    for (i = 0; i < trace_event_count(); i++) {
-        TraceEvent *ev = trace_event_id(i);
-        stream_printf(stream, "%s [Event ID %u] : state %u\n",
-                      trace_event_get_name(ev), i,
-                      trace_event_get_state_static(ev) &&
-                      trace_event_get_state_dynamic(ev));
-    }
-}
-
 static void trace_init_events(const char *fname)
 {
     Location loc;
diff --git a/trace/control.h b/trace/control.h
index e1ec033..da9bb6b 100644
--- a/trace/control.h
+++ b/trace/control.h
@@ -149,13 +149,6 @@ static void trace_event_set_state_dynamic(TraceEvent *ev, bool state);
 
 
 /**
- * trace_print_events:
- *
- * Print the state of all events.
- */
-void trace_print_events(FILE *stream, fprintf_function stream_printf);
-
-/**
  * trace_init_backends:
  * @events: Name of file with events to be enabled at startup; may be NULL.
  *          Corresponds to commandline option "-trace events=...".

Re: [Qemu-devel] [PATCH v4 0/2] trace: Trace control commands

[Lluís Vilanova]

Lluís Vilanova writes:

> Adds QAPI/QMP commands to control tracing events, and reimplements some of the
> related HMP commands on top.

Ping.


Thanks,
  Lluis


> NOTE: The "trace-event-set-state" command uses a bool 'enable' argument instead
>       of an enum 'state'. I'm still not sure if an enum is better than the two
>       separate booleans.

> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---

> Changes in v4:

> * Split QAPI/QMP and HMP changes into separate patches.
> * Add copyright information in QAPI file (Eric Blake).
> * Replace event state booleans with an enum (Eric Blake).
> * Document pattern type for argument "name" (Eric Blake).
> * Other documentation improvements (Markus Armbruster).
> * Change "keepgoing" to "ignore-unavailable" (Markus Armbruster).
> * Add examples in QMP command definition file (Eric Blake).
> * Propagate QMP errors to HMP (Markus Armbruster, Stefan Hajnoczi).
> * Remove trailing newlines when using error_setg (Stefan Hajnoczi).
> * Avoid multiple invocations of error_setg (Stefan Hajnoczi).
> * Various cosmetic changes (Markus Armbruster).

> Lluís Vilanova (2):
>       trace: [qmp] Add commands to query and control event tracing state
>       trace: [hmp] Reimplement "trace-event" and "info trace-events" using QMP


>  monitor.c           |   27 ++++++++++--------
>  qapi-schema.json    |    3 ++
>  qapi/trace.json     |   65 ++++++++++++++++++++++++++++++++++++++++++++
>  qmp-commands.hx     |   35 ++++++++++++++++++++++++
>  trace/Makefile.objs |    1 +
>  trace/control.c     |   13 ---------
>  trace/control.h     |    7 -----
>  trace/qmp.c         |   75 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  8 files changed, 193 insertions(+), 33 deletions(-)
>  create mode 100644 qapi/trace.json
>  create mode 100644 trace/qmp.c


> To: qemu-devel@nongnu.org
> Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: Luiz Capitulino <lcapitulino@redhat.com>


-- 
 "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 v4 0/2] trace: Trace control commands

[Luiz Capitulino]

On Mon, 08 Sep 2014 15:03:41 -0500
Lluís Vilanova <vilanova@ac.upc.edu> wrote:

> Lluís Vilanova writes:
> 
> > Adds QAPI/QMP commands to control tracing events, and reimplements some of the
> > related HMP commands on top.
> 
> Ping.

I need reviewers. Stefan and Eric are obvious candidates.

Re: [Qemu-devel] [PATCH v4 0/2] trace: Trace control commands

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1669 bytes --]

On Mon, Aug 25, 2014 at 01:19:51PM +0200, Lluís Vilanova wrote:
> Adds QAPI/QMP commands to control tracing events, and reimplements some of the
> related HMP commands on top.
> 
> NOTE: The "trace-event-set-state" command uses a bool 'enable' argument instead
>       of an enum 'state'. I'm still not sure if an enum is better than the two
>       separate booleans.
> 
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
> 
> Changes in v4:
> 
> * Split QAPI/QMP and HMP changes into separate patches.
> * Add copyright information in QAPI file (Eric Blake).
> * Replace event state booleans with an enum (Eric Blake).
> * Document pattern type for argument "name" (Eric Blake).
> * Other documentation improvements (Markus Armbruster).
> * Change "keepgoing" to "ignore-unavailable" (Markus Armbruster).
> * Add examples in QMP command definition file (Eric Blake).
> * Propagate QMP errors to HMP (Markus Armbruster, Stefan Hajnoczi).
> * Remove trailing newlines when using error_setg (Stefan Hajnoczi).
> * Avoid multiple invocations of error_setg (Stefan Hajnoczi).
> * Various cosmetic changes (Markus Armbruster).

Sorry for the long wait.  I am merging it now.

The only thing I wonder about is adding trace/qmp.o to util-obj-y.  But
I've built QEMU from scratch also for Windows and the build completes
successfully.  I was worried that a utility like qemu-img might not have
the necessary headers or objects for QMP code but it seems to work.

If there are issues we can still fix it later in the 2.2 release cycle.

Thanks, applied to my tracing tree:
https://github.com/stefanha/qemu/commits/tracing

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]