All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 00/20] Tracing patches
@ 2016-10-07 10:09 Stefan Hajnoczi
  2016-10-07 10:09 ` [Qemu-devel] [PULL 01/20] trace: move colo trace events to net/ sub-directory Stefan Hajnoczi
                   ` (20 more replies)
  0 siblings, 21 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2016-10-07 10:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Stefan Hajnoczi

The following changes since commit e902754e3d0890945ddcc1b33748ed73762ddb8d:

  Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-2.8-20161006' into staging (2016-10-06 13:34:00 +0100)

are available in the git repository at:

  git://github.com/stefanha/qemu.git tags/tracing-pull-request

for you to fetch changes up to bcdc1363dcb9adb5f7ea016a4d0b41f0357c8182:

  trace: Add missing execution mode of guest events (2016-10-07 09:17:49 +0100)

----------------------------------------------------------------

----------------------------------------------------------------

Daniel P. Berrange (19):
  trace: move colo trace events to net/ sub-directory
  trace: add trace event iterator APIs
  trace: convert code to use event iterators
  trace: remove some now unused functions
  trace: remove global 'uint16 dstate[]' array
  trace: remove duplicate control.h includes in generated-tracers.h
  trace: break circular dependency in event-internal.h
  trace: give each trace event a named TraceEvent struct
  trace: remove the TraceEventID and TraceEventVCPUID enums
  trace: emit name <-> ID mapping in simpletrace header
  trace: don't abort qemu if ftrace can't be initialized
  trace: provide mechanism for registering trace events
  trace: dynamically allocate trace_dstate in CPUState
  trace: dynamically allocate event IDs at runtime
  trace: get rid of generated-events.h/generated-events.c
  trace: rename _read_events to read_events
  trace: push reading of events up a level to tracetool main
  trace: pass trace-events to tracetool as a positional param
  trace: introduce a formal group name for trace events

Lluís Vilanova (1):
  trace: Add missing execution mode of guest events

 Makefile                                         |   3 -
 Makefile.target                                  |   6 +-
 include/qemu/module.h                            |   2 +
 include/qom/cpu.h                                |   9 +-
 include/trace-tcg.h                              |   1 -
 include/trace.h                                  |   1 -
 monitor.c                                        |  26 ++---
 net/trace-events                                 |  16 +++
 qemu-img.c                                       |   1 +
 qemu-io.c                                        |   1 +
 qemu-nbd.c                                       |   1 +
 qom/cpu.c                                        |   7 +-
 scripts/simpletrace.py                           |  56 +++++++----
 scripts/tracetool.py                             |  20 +++-
 scripts/tracetool/__init__.py                    |  28 ++++--
 scripts/tracetool/backend/__init__.py            |  12 +--
 scripts/tracetool/backend/dtrace.py              |   4 +-
 scripts/tracetool/backend/ftrace.py              |   5 +-
 scripts/tracetool/backend/log.py                 |   7 +-
 scripts/tracetool/backend/simple.py              |  12 +--
 scripts/tracetool/backend/syslog.py              |   5 +-
 scripts/tracetool/backend/ust.py                 |   4 +-
 scripts/tracetool/format/__init__.py             |   4 +-
 scripts/tracetool/format/c.py                    |  56 +++++++++--
 scripts/tracetool/format/d.py                    |   2 +-
 scripts/tracetool/format/events_c.py             |  44 --------
 scripts/tracetool/format/events_h.py             |  60 -----------
 scripts/tracetool/format/h.py                    |  37 +++++--
 scripts/tracetool/format/simpletrace_stap.py     |  26 ++++-
 scripts/tracetool/format/stap.py                 |   2 +-
 scripts/tracetool/format/tcg_h.py                |   8 +-
 scripts/tracetool/format/tcg_helper_c.py         |   2 +-
 scripts/tracetool/format/tcg_helper_h.py         |   2 +-
 scripts/tracetool/format/tcg_helper_wrapper_h.py |   2 +-
 scripts/tracetool/format/ust_events_c.py         |   2 +-
 scripts/tracetool/format/ust_events_h.py         |   9 +-
 stubs/trace-control.c                            |   9 +-
 trace-events                                     |  18 +---
 trace/Makefile.objs                              |  46 +++------
 trace/control-internal.h                         |  48 ++++-----
 trace/control-target.c                           |  35 +++----
 trace/control.c                                  | 123 +++++++++++++++--------
 trace/control.h                                  | 112 +++++++--------------
 trace/event-internal.h                           |  18 +++-
 trace/ftrace.c                                   |   6 ++
 trace/qmp.c                                      |  16 +--
 trace/simple.c                                   |  41 ++++++--
 trace/simple.h                                   |   6 +-
 vl.c                                             |   2 +
 49 files changed, 502 insertions(+), 461 deletions(-)
 delete mode 100644 scripts/tracetool/format/events_c.py
 delete mode 100644 scripts/tracetool/format/events_h.py

-- 
2.7.4

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PULL 01/20] trace: move colo trace events to net/ sub-directory
  2016-10-07 10:09 [Qemu-devel] [PULL 00/20] Tracing patches Stefan Hajnoczi
@ 2016-10-07 10:09 ` Stefan Hajnoczi
  2016-10-07 10:09 ` [Qemu-devel] [PULL 02/20] trace: add trace event iterator APIs Stefan Hajnoczi
                   ` (19 subsequent siblings)
  20 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2016-10-07 10:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange, Stefan Hajnoczi

From: "Daniel P. Berrange" <berrange@redhat.com>

The colo patch series added various trace events to the top
level trace-events file, despite the files using them being
in a sub-dir.

  commit 30656b097e9dd7978d3fe9416cb9f5a421a9e63e
  Author: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
  Date:   Tue Sep 27 10:22:34 2016 +0800

    filter-rewriter: rewrite tcp packet to keep secondary connection

  commit f4b618360e5a81b097e2e35d52011bec3c63af68
  Author: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
  Date:   Tue Sep 27 10:22:31 2016 +0800

    colo-compare: add TCP, UDP, ICMP packet comparison

    We add TCP,UDP,ICMP packet comparison to replace
    IP packet comparison. This can increase the
    accuracy of the package comparison.
    Less checkpoint more efficiency.

    Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
    Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
    Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
    Signed-off-by: Jason Wang <jasowang@redhat.com>

  commit 0682e15b19b2f41c0568142b42518b9471168597
  Author: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
  Date:   Tue Sep 27 10:22:30 2016 +0800

    colo-compare: introduce packet comparison thread

  commit 59509ec16b7ee92b3f8261c554023aa1d3169317
  Author: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
  Date:   Tue Sep 27 10:22:27 2016 +0800

    net/colo.c: add colo.c to define and handle packet

This moves all events into net/trace-events where they
were supposed to live.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 1475588159-30598-2-git-send-email-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 net/trace-events | 16 ++++++++++++++++
 trace-events     | 16 ----------------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/net/trace-events b/net/trace-events
index 65c46a4..d67f048 100644
--- a/net/trace-events
+++ b/net/trace-events
@@ -2,3 +2,19 @@
 
 # net/vhost-user.c
 vhost_user_event(const char *chr, int event) "chr: %s got event: %d"
+
+# net/colo.c
+colo_proxy_main(const char *chr) ": %s"
+
+# net/colo-compare.c
+colo_compare_main(const char *chr) ": %s"
+colo_compare_udp_miscompare(const char *sta, int size) ": %s = %d"
+colo_compare_icmp_miscompare(const char *sta, int size) ": %s = %d"
+colo_compare_ip_info(int psize, const char *sta, const char *stb, int ssize, const char *stc, const char *std) "ppkt size = %d, ip_src = %s, ip_dst = %s, spkt size = %d, ip_src = %s, ip_dst = %s"
+colo_old_packet_check_found(int64_t old_time) "%" PRId64
+colo_compare_miscompare(void) ""
+
+# net/filter-rewriter.c
+colo_filter_rewriter_debug(void) ""
+colo_filter_rewriter_pkt_info(const char *func, const char *src, const char *dst, uint32_t seq, uint32_t ack, uint32_t flag) "%s: src/dst: %s/%s p: seq/ack=%u/%u  flags=%x\n"
+colo_filter_rewriter_conn_offset(uint32_t offset) ": offset=%u\n"
diff --git a/trace-events b/trace-events
index 1a4e092..bd2199a 100644
--- a/trace-events
+++ b/trace-events
@@ -122,22 +122,6 @@ memory_region_subpage_write(int cpu_index, void *mr, uint64_t offset, uint64_t v
 memory_region_tb_read(int cpu_index, uint64_t addr, uint64_t value, unsigned size) "cpu %d addr %#"PRIx64" value %#"PRIx64" size %u"
 memory_region_tb_write(int cpu_index, uint64_t addr, uint64_t value, unsigned size) "cpu %d addr %#"PRIx64" value %#"PRIx64" size %u"
 
-# net/colo.c
-colo_proxy_main(const char *chr) ": %s"
-
-# net/colo-compare.c
-colo_compare_main(const char *chr) ": %s"
-colo_compare_udp_miscompare(const char *sta, int size) ": %s = %d"
-colo_compare_icmp_miscompare(const char *sta, int size) ": %s = %d"
-colo_compare_ip_info(int psize, const char *sta, const char *stb, int ssize, const char *stc, const char *std) "ppkt size = %d, ip_src = %s, ip_dst = %s, spkt size = %d, ip_src = %s, ip_dst = %s"
-colo_old_packet_check_found(int64_t old_time) "%" PRId64
-colo_compare_miscompare(void) ""
-
-# net/filter-rewriter.c
-colo_filter_rewriter_debug(void) ""
-colo_filter_rewriter_pkt_info(const char *func, const char *src, const char *dst, uint32_t seq, uint32_t ack, uint32_t flag) "%s: src/dst: %s/%s p: seq/ack=%u/%u  flags=%x\n"
-colo_filter_rewriter_conn_offset(uint32_t offset) ": offset=%u\n"
-
 ### Guest events, keep at bottom
 
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PULL 02/20] trace: add trace event iterator APIs
  2016-10-07 10:09 [Qemu-devel] [PULL 00/20] Tracing patches Stefan Hajnoczi
  2016-10-07 10:09 ` [Qemu-devel] [PULL 01/20] trace: move colo trace events to net/ sub-directory Stefan Hajnoczi
@ 2016-10-07 10:09 ` Stefan Hajnoczi
  2016-10-07 10:09 ` [Qemu-devel] [PULL 03/20] trace: convert code to use event iterators Stefan Hajnoczi
                   ` (18 subsequent siblings)
  20 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2016-10-07 10:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange, Stefan Hajnoczi

From: "Daniel P. Berrange" <berrange@redhat.com>

Currently methods which want to iterate over trace events,
do so using the trace_event_count() and trace_event_id()
methods. This leaks the concept of a single ID enum to
the callers. There is an alternative trace_event_pattern()
method which can be used in an iteration context, but its
design is stateless, so is not easy to expand it in the
future.

This defines a formal iterator API will provide a future-
proof way of iterating over events.

The iterator is also able to apply a pattern match filter
to events, further removing the need for the pattern

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Lluís Vilanova <vilanova@ac.upc.edu>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 1475588159-30598-4-git-send-email-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 trace/control.c | 21 +++++++++++++++++++++
 trace/control.h | 27 +++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/trace/control.c b/trace/control.c
index 10b3e9b..5a9bb5a 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -125,6 +125,27 @@ TraceEvent *trace_event_pattern(const char *pat, TraceEvent *ev)
     return NULL;
 }
 
+void trace_event_iter_init(TraceEventIter *iter, const char *pattern)
+{
+    iter->event = 0;
+    iter->pattern = pattern;
+}
+
+TraceEvent *trace_event_iter_next(TraceEventIter *iter)
+{
+    while (iter->event < TRACE_EVENT_COUNT) {
+        TraceEvent *ev = &(trace_events[iter->event]);
+        iter->event++;
+        if (!iter->pattern ||
+            pattern_glob(iter->pattern,
+                         trace_event_get_name(ev))) {
+            return ev;
+        }
+    }
+
+    return NULL;
+}
+
 void trace_list_events(void)
 {
     int i;
diff --git a/trace/control.h b/trace/control.h
index a22d112..36d7cd2 100644
--- a/trace/control.h
+++ b/trace/control.h
@@ -13,6 +13,10 @@
 #include "qemu-common.h"
 #include "trace/generated-events.h"
 
+typedef struct TraceEventIter {
+    size_t event;
+    const char *pattern;
+} TraceEventIter;
 
 /**
  * TraceEventID:
@@ -25,6 +29,29 @@
  */
 enum TraceEventID;
 
+
+/**
+ * trace_event_iter_init:
+ * @iter: the event iterator struct
+ * @pattern: optional pattern to filter events on name
+ *
+ * Initialize the event iterator struct @iter,
+ * optionally using @pattern to filter out events
+ * with non-matching names.
+ */
+void trace_event_iter_init(TraceEventIter *iter, const char *pattern);
+
+/**
+ * trace_event_iter_next:
+ * @iter: the event iterator struct
+ *
+ * Get the next event, if any. When this returns NULL,
+ * the iterator should no longer be used.
+ *
+ * Returns: the next event, or NULL if no more events exist
+ */
+TraceEvent *trace_event_iter_next(TraceEventIter *iter);
+
 /**
  * trace_event_id:
  * @id: Event identifier.
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PULL 03/20] trace: convert code to use event iterators
  2016-10-07 10:09 [Qemu-devel] [PULL 00/20] Tracing patches Stefan Hajnoczi
  2016-10-07 10:09 ` [Qemu-devel] [PULL 01/20] trace: move colo trace events to net/ sub-directory Stefan Hajnoczi
  2016-10-07 10:09 ` [Qemu-devel] [PULL 02/20] trace: add trace event iterator APIs Stefan Hajnoczi
@ 2016-10-07 10:09 ` Stefan Hajnoczi
  2016-10-07 10:09 ` [Qemu-devel] [PULL 04/20] trace: remove some now unused functions Stefan Hajnoczi
                   ` (17 subsequent siblings)
  20 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2016-10-07 10:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange, Stefan Hajnoczi

From: "Daniel P. Berrange" <berrange@redhat.com>

This converts the HMP/QMP monitor API implementations
and some internal trace control methods to use the new
trace event iterator APIs.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Lluís Vilanova <vilanova@ac.upc.edu>
Message-id: 1475588159-30598-5-git-send-email-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 monitor.c              | 26 +++++++++--------
 trace/control-target.c |  8 +++---
 trace/control.c        | 77 +++++++++++++++++++++++++++-----------------------
 trace/qmp.c            | 16 +++++++----
 4 files changed, 69 insertions(+), 58 deletions(-)

diff --git a/monitor.c b/monitor.c
index 83c4edf..e3daffe 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3327,13 +3327,14 @@ void info_trace_events_completion(ReadLineState *rs, int nb_args, const char *st
     len = strlen(str);
     readline_set_completion_index(rs, len);
     if (nb_args == 2) {
-        TraceEventID id;
-        for (id = 0; id < trace_event_count(); id++) {
-            const char *event_name = trace_event_get_name(trace_event_id(id));
-            if (!strncmp(str, event_name, len)) {
-                readline_add_completion(rs, event_name);
-            }
+        TraceEventIter iter;
+        TraceEvent *ev;
+        char *pattern = g_strdup_printf("%s*", str);
+        trace_event_iter_init(&iter, pattern);
+        while ((ev = trace_event_iter_next(&iter)) != NULL) {
+            readline_add_completion(rs, trace_event_get_name(ev));
         }
+        g_free(pattern);
     }
 }
 
@@ -3344,13 +3345,14 @@ void trace_event_completion(ReadLineState *rs, int nb_args, const char *str)
     len = strlen(str);
     readline_set_completion_index(rs, len);
     if (nb_args == 2) {
-        TraceEventID id;
-        for (id = 0; id < trace_event_count(); id++) {
-            const char *event_name = trace_event_get_name(trace_event_id(id));
-            if (!strncmp(str, event_name, len)) {
-                readline_add_completion(rs, event_name);
-            }
+        TraceEventIter iter;
+        TraceEvent *ev;
+        char *pattern = g_strdup_printf("%s*", str);
+        trace_event_iter_init(&iter, pattern);
+        while ((ev = trace_event_iter_next(&iter)) != NULL) {
+            readline_add_completion(rs, trace_event_get_name(ev));
         }
+        g_free(pattern);
     } else if (nb_args == 3) {
         add_completion_option(rs, str, "on");
         add_completion_option(rs, str, "off");
diff --git a/trace/control-target.c b/trace/control-target.c
index 52fcce5..50bac4f 100644
--- a/trace/control-target.c
+++ b/trace/control-target.c
@@ -98,9 +98,10 @@ static bool adding_first_cpu(void)
 
 void trace_init_vcpu(CPUState *vcpu)
 {
-    TraceEvent *ev = NULL;
-
-    while ((ev = trace_event_pattern("*", ev)) != NULL) {
+    TraceEventIter iter;
+    TraceEvent *ev;
+    trace_event_iter_init(&iter, NULL);
+    while ((ev = trace_event_iter_next(&iter)) != NULL) {
         if (trace_event_is_vcpu(ev) &&
             trace_event_get_state_static(ev) &&
             trace_event_get_state_dynamic(ev)) {
@@ -118,6 +119,5 @@ void trace_init_vcpu(CPUState *vcpu)
             }
         }
     }
-
     trace_guest_cpu_enter(vcpu);
 }
diff --git a/trace/control.c b/trace/control.c
index 5a9bb5a..a107cc3 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -60,9 +60,10 @@ TraceEvent *trace_event_name(const char *name)
 {
     assert(name != NULL);
 
-    TraceEventID i;
-    for (i = 0; i < trace_event_count(); i++) {
-        TraceEvent *ev = trace_event_id(i);
+    TraceEventIter iter;
+    TraceEvent *ev;
+    trace_event_iter_init(&iter, NULL);
+    while ((ev = trace_event_iter_next(&iter)) != NULL) {
         if (strcmp(trace_event_get_name(ev), name) == 0) {
             return ev;
         }
@@ -105,21 +106,18 @@ TraceEvent *trace_event_pattern(const char *pat, TraceEvent *ev)
 {
     assert(pat != NULL);
 
-    TraceEventID i;
-
-    if (ev == NULL) {
-        i = -1;
-    } else {
-        i = trace_event_get_id(ev);
-    }
-    i++;
-
-    while (i < trace_event_count()) {
-        TraceEvent *res = trace_event_id(i);
-        if (pattern_glob(pat, trace_event_get_name(res))) {
-            return res;
+    bool matched = ev ? false : true;
+    TraceEventIter iter;
+    TraceEvent *thisev;
+    trace_event_iter_init(&iter, pat);
+    while ((thisev = trace_event_iter_next(&iter)) != NULL) {
+        if (matched) {
+            return thisev;
+        } else {
+            if (ev == thisev) {
+                matched = true;
+            }
         }
-        i++;
     }
 
     return NULL;
@@ -148,10 +146,11 @@ TraceEvent *trace_event_iter_next(TraceEventIter *iter)
 
 void trace_list_events(void)
 {
-    int i;
-    for (i = 0; i < trace_event_count(); i++) {
-        TraceEvent *res = trace_event_id(i);
-        fprintf(stderr, "%s\n", trace_event_get_name(res));
+    TraceEventIter iter;
+    TraceEvent *ev;
+    trace_event_iter_init(&iter, NULL);
+    while ((ev = trace_event_iter_next(&iter)) != NULL) {
+        fprintf(stderr, "%s\n", trace_event_get_name(ev));
     }
 }
 
@@ -159,26 +158,32 @@ static void do_trace_enable_events(const char *line_buf)
 {
     const bool enable = ('-' != line_buf[0]);
     const char *line_ptr = enable ? line_buf : line_buf + 1;
+    TraceEventIter iter;
+    TraceEvent *ev;
+    bool is_pattern = trace_event_is_pattern(line_ptr);
 
-    if (trace_event_is_pattern(line_ptr)) {
-        TraceEvent *ev = NULL;
-        while ((ev = trace_event_pattern(line_ptr, ev)) != NULL) {
-            if (trace_event_get_state_static(ev)) {
-                trace_event_set_state_dynamic_init(ev, enable);
+    trace_event_iter_init(&iter, line_ptr);
+    while ((ev = trace_event_iter_next(&iter)) != NULL) {
+        if (!trace_event_get_state_static(ev)) {
+            if (!is_pattern) {
+                error_report("WARNING: trace event '%s' is not traceable",
+                             line_ptr);
+                return;
             }
+            continue;
         }
-    } else {
-        TraceEvent *ev = trace_event_name(line_ptr);
-        if (ev == NULL) {
-            error_report("WARNING: trace event '%s' does not exist",
-                         line_ptr);
-        } else if (!trace_event_get_state_static(ev)) {
-            error_report("WARNING: trace event '%s' is not traceable",
-                         line_ptr);
-        } else {
-            trace_event_set_state_dynamic_init(ev, enable);
+
+        /* start tracing */
+        trace_event_set_state_dynamic(ev, enable);
+        if (!is_pattern) {
+            return;
         }
     }
+
+    if (!is_pattern) {
+        error_report("WARNING: trace event '%s' does not exist",
+                     line_ptr);
+    }
 }
 
 void trace_enable_events(const char *line_buf)
diff --git a/trace/qmp.c b/trace/qmp.c
index 11d2564..ac777d1 100644
--- a/trace/qmp.c
+++ b/trace/qmp.c
@@ -52,8 +52,10 @@ static bool check_events(bool has_vcpu, bool ignore_unavailable, bool is_pattern
         return true;
     } else {
         /* error for unavailable events */
-        TraceEvent *ev = NULL;
-        while ((ev = trace_event_pattern(name, ev)) != NULL) {
+        TraceEventIter iter;
+        TraceEvent *ev;
+        trace_event_iter_init(&iter, name);
+        while ((ev = trace_event_iter_next(&iter)) != NULL) {
             if (!ignore_unavailable && !trace_event_get_state_static(ev)) {
                 error_setg(errp, "event \"%s\" is disabled", trace_event_get_name(ev));
                 return false;
@@ -69,6 +71,7 @@ TraceEventInfoList *qmp_trace_event_get_state(const char *name,
 {
     Error *err = NULL;
     TraceEventInfoList *events = NULL;
+    TraceEventIter iter;
     TraceEvent *ev;
     bool is_pattern = trace_event_is_pattern(name);
     CPUState *cpu;
@@ -86,8 +89,8 @@ TraceEventInfoList *qmp_trace_event_get_state(const char *name,
     }
 
     /* Get states (all errors checked above) */
-    ev = NULL;
-    while ((ev = trace_event_pattern(name, ev)) != NULL) {
+    trace_event_iter_init(&iter, name);
+    while ((ev = trace_event_iter_next(&iter)) != NULL) {
         TraceEventInfoList *elem;
         bool is_vcpu = trace_event_is_vcpu(ev);
         if (has_vcpu && !is_vcpu) {
@@ -132,6 +135,7 @@ void qmp_trace_event_set_state(const char *name, bool enable,
                                Error **errp)
 {
     Error *err = NULL;
+    TraceEventIter iter;
     TraceEvent *ev;
     bool is_pattern = trace_event_is_pattern(name);
     CPUState *cpu;
@@ -150,8 +154,8 @@ void qmp_trace_event_set_state(const char *name, bool enable,
     }
 
     /* Apply changes (all errors checked above) */
-    ev = NULL;
-    while ((ev = trace_event_pattern(name, ev)) != NULL) {
+    trace_event_iter_init(&iter, name);
+    while ((ev = trace_event_iter_next(&iter)) != NULL) {
         if (!trace_event_get_state_static(ev) ||
             (has_vcpu && !trace_event_is_vcpu(ev))) {
             continue;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PULL 04/20] trace: remove some now unused functions
  2016-10-07 10:09 [Qemu-devel] [PULL 00/20] Tracing patches Stefan Hajnoczi
                   ` (2 preceding siblings ...)
  2016-10-07 10:09 ` [Qemu-devel] [PULL 03/20] trace: convert code to use event iterators Stefan Hajnoczi
@ 2016-10-07 10:09 ` Stefan Hajnoczi
  2016-10-07 10:09 ` [Qemu-devel] [PULL 05/20] trace: remove global 'uint16 dstate[]' array Stefan Hajnoczi
                   ` (16 subsequent siblings)
  20 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2016-10-07 10:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange, Stefan Hajnoczi

From: "Daniel P. Berrange" <berrange@redhat.com>

The trace_event_count, trace_event_id and
trace_event_pattern methods are no longer required
now that everything is using the iterator APIs

The trace_event_set_state and trace_event_set_vcpu_state
macros were also unused.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Lluís Vilanova <vilanova@ac.upc.edu>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 1475588159-30598-6-git-send-email-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 trace/control-internal.h | 11 ---------
 trace/control.c          | 20 ----------------
 trace/control.h          | 59 ------------------------------------------------
 3 files changed, 90 deletions(-)

diff --git a/trace/control-internal.h b/trace/control-internal.h
index a4e5f4a..7f31e39 100644
--- a/trace/control-internal.h
+++ b/trace/control-internal.h
@@ -20,17 +20,6 @@ extern uint16_t trace_events_dstate[];
 extern int trace_events_enabled_count;
 
 
-static inline TraceEventID trace_event_count(void)
-{
-    return TRACE_EVENT_COUNT;
-}
-
-static inline TraceEvent *trace_event_id(TraceEventID id)
-{
-    assert(id < trace_event_count());
-    return &trace_events[id];
-}
-
 static inline bool trace_event_is_pattern(const char *str)
 {
     assert(str != NULL);
diff --git a/trace/control.c b/trace/control.c
index a107cc3..a423f32 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -102,26 +102,6 @@ static bool pattern_glob(const char *pat, const char *ev)
     }
 }
 
-TraceEvent *trace_event_pattern(const char *pat, TraceEvent *ev)
-{
-    assert(pat != NULL);
-
-    bool matched = ev ? false : true;
-    TraceEventIter iter;
-    TraceEvent *thisev;
-    trace_event_iter_init(&iter, pat);
-    while ((thisev = trace_event_iter_next(&iter)) != NULL) {
-        if (matched) {
-            return thisev;
-        } else {
-            if (ev == thisev) {
-                matched = true;
-            }
-        }
-    }
-
-    return NULL;
-}
 
 void trace_event_iter_init(TraceEventIter *iter, const char *pattern)
 {
diff --git a/trace/control.h b/trace/control.h
index 36d7cd2..10d4aba 100644
--- a/trace/control.h
+++ b/trace/control.h
@@ -52,21 +52,6 @@ void trace_event_iter_init(TraceEventIter *iter, const char *pattern);
  */
 TraceEvent *trace_event_iter_next(TraceEventIter *iter);
 
-/**
- * trace_event_id:
- * @id: Event identifier.
- *
- * Get an event by its identifier.
- *
- * This routine has a constant cost, as opposed to trace_event_name and
- * trace_event_pattern.
- *
- * Pre-conditions: The identifier is valid.
- *
- * Returns: pointer to #TraceEvent.
- *
- */
-static TraceEvent *trace_event_id(TraceEventID id);
 
 /**
  * trace_event_name:
@@ -79,31 +64,12 @@ static TraceEvent *trace_event_id(TraceEventID id);
 TraceEvent *trace_event_name(const char *name);
 
 /**
- * trace_event_pattern:
- * @pat: Event name pattern.
- * @ev: Event to start searching from (not included).
- *
- * Get all events with a given name pattern.
- *
- * Returns: pointer to #TraceEvent or NULL if not found.
- */
-TraceEvent *trace_event_pattern(const char *pat, TraceEvent *ev);
-
-/**
  * trace_event_is_pattern:
  *
  * Whether the given string is an event name pattern.
  */
 static bool trace_event_is_pattern(const char *str);
 
-/**
- * trace_event_count:
- *
- * Return the number of events.
- */
-static TraceEventID trace_event_count(void);
-
-
 
 /**
  * trace_event_get_id:
@@ -194,31 +160,6 @@ static bool trace_event_get_state_dynamic(TraceEvent *ev);
  */
 static bool trace_event_get_vcpu_state_dynamic(CPUState *vcpu, TraceEvent *ev);
 
-/**
- * trace_event_set_state:
- *
- * Set the tracing state of an event (only if possible).
- */
-#define trace_event_set_state(id, state)                \
-    do {                                                \
-        if ((id ##_ENABLED)) {                          \
-            TraceEvent *_e = trace_event_id(id);        \
-            trace_event_set_state_dynamic(_e, state);   \
-        }                                               \
-    } while (0)
-
-/**
- * trace_event_set_vcpu_state:
- *
- * Set the tracing state of an event for the given vCPU (only if not disabled).
- */
-#define trace_event_set_vcpu_state(vcpu, id, state)                     \
-    do {                                                                \
-        if ((id ##_ENABLED)) {                                          \
-            TraceEvent *_e = trace_event_id(id);                        \
-            trace_event_set_vcpu_state_dynamic(vcpu, _e, state);        \
-        }                                                               \
-    } while (0)
 
 /**
  * trace_event_set_state_dynamic:
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PULL 05/20] trace: remove global 'uint16 dstate[]' array
  2016-10-07 10:09 [Qemu-devel] [PULL 00/20] Tracing patches Stefan Hajnoczi
                   ` (3 preceding siblings ...)
  2016-10-07 10:09 ` [Qemu-devel] [PULL 04/20] trace: remove some now unused functions Stefan Hajnoczi
@ 2016-10-07 10:09 ` Stefan Hajnoczi
  2016-10-07 10:09 ` [Qemu-devel] [PULL 06/20] trace: remove duplicate control.h includes in generated-tracers.h Stefan Hajnoczi
                   ` (15 subsequent siblings)
  20 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2016-10-07 10:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange, Stefan Hajnoczi

From: "Daniel P. Berrange" <berrange@redhat.com>

Instead of having a global dstate array, declare a single
'uint16 TRACE_${EVENT_NAME}_DSTATE' variable for each
trace event. Record a pointer to this variable in the
TraceEvent struct too.

By turning trace_event_get_state_dynamic_by_id into a
macro, this still hits the fast path, and cache affinity
is ensured by declaring all the uint16 vars adjacent to
each other.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Lluís Vilanova <vilanova@ac.upc.edu>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 1475588159-30598-7-git-send-email-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 scripts/tracetool/__init__.py        |  3 ++-
 scripts/tracetool/format/events_c.py |  9 +++++++--
 scripts/tracetool/format/events_h.py |  3 +++
 stubs/trace-control.c                |  9 ++++-----
 trace/control-internal.h             | 14 ++++----------
 trace/control-target.c               | 25 ++++++++++---------------
 trace/control.c                      |  6 ------
 trace/event-internal.h               |  7 +++++++
 8 files changed, 37 insertions(+), 39 deletions(-)

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index be24039..a11f36b 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -265,11 +265,12 @@ class Event(object):
 
     QEMU_TRACE               = "trace_%(name)s"
     QEMU_TRACE_TCG           = QEMU_TRACE + "_tcg"
+    QEMU_DSTATE              = "_TRACE_%(NAME)s_DSTATE"
 
     def api(self, fmt=None):
         if fmt is None:
             fmt = Event.QEMU_TRACE
-        return fmt % {"name": self.name}
+        return fmt % {"name": self.name, "NAME": self.name.upper()}
 
     def transform(self, *trans):
         """Return a new Event with transformed Arguments."""
diff --git a/scripts/tracetool/format/events_c.py b/scripts/tracetool/format/events_c.py
index 4012063..ef873fa 100644
--- a/scripts/tracetool/format/events_c.py
+++ b/scripts/tracetool/format/events_c.py
@@ -25,6 +25,9 @@ def generate(events, backend):
         '#include "trace/control.h"',
         '')
 
+    for e in events:
+        out('uint16_t %s;' % e.api(e.QEMU_DSTATE))
+
     out('TraceEvent trace_events[TRACE_EVENT_COUNT] = {')
 
     for e in events:
@@ -34,11 +37,13 @@ def generate(events, backend):
             vcpu_id = "TRACE_VCPU_EVENT_COUNT"
         out('    { .id = %(id)s, .vcpu_id = %(vcpu_id)s,'
             ' .name = \"%(name)s\",'
-            ' .sstate = %(sstate)s },',
+            ' .sstate = %(sstate)s,',
+            ' .dstate = &%(dstate)s, }, ',
             id = "TRACE_" + e.name.upper(),
             vcpu_id = vcpu_id,
             name = e.name,
-            sstate = "TRACE_%s_ENABLED" % e.name.upper())
+            sstate = "TRACE_%s_ENABLED" % e.name.upper(),
+            dstate = e.api(e.QEMU_DSTATE))
 
     out('};',
         '')
diff --git a/scripts/tracetool/format/events_h.py b/scripts/tracetool/format/events_h.py
index a9da60b..03417de 100644
--- a/scripts/tracetool/format/events_h.py
+++ b/scripts/tracetool/format/events_h.py
@@ -32,6 +32,9 @@ def generate(events, backend):
     out('    TRACE_EVENT_COUNT',
         '} TraceEventID;')
 
+    for e in events:
+        out('extern uint16_t %s;' % e.api(e.QEMU_DSTATE))
+
     # per-vCPU event identifiers
     out('typedef enum {')
 
diff --git a/stubs/trace-control.c b/stubs/trace-control.c
index f765a02..7f856e5 100644
--- a/stubs/trace-control.c
+++ b/stubs/trace-control.c
@@ -18,22 +18,21 @@ void trace_event_set_state_dynamic_init(TraceEvent *ev, bool state)
 
 void trace_event_set_state_dynamic(TraceEvent *ev, bool state)
 {
-    TraceEventID id;
     bool state_pre;
     assert(trace_event_get_state_static(ev));
-    id = trace_event_get_id(ev);
+
     /*
      * We ignore the "vcpu" property here, since there's no target code. Then
      * dstate can only be 1 or 0.
      */
-    state_pre = trace_events_dstate[id];
+    state_pre = *(ev->dstate);
     if (state_pre != state) {
         if (state) {
             trace_events_enabled_count++;
-            trace_events_dstate[id] = 1;
+            *(ev->dstate) = 1;
         } else {
             trace_events_enabled_count--;
-            trace_events_dstate[id] = 0;
+            *(ev->dstate) = 0;
         }
     }
 }
diff --git a/trace/control-internal.h b/trace/control-internal.h
index 7f31e39..a6d8d2e 100644
--- a/trace/control-internal.h
+++ b/trace/control-internal.h
@@ -16,7 +16,6 @@
 
 
 extern TraceEvent trace_events[];
-extern uint16_t trace_events_dstate[];
 extern int trace_events_enabled_count;
 
 
@@ -54,18 +53,13 @@ static inline bool trace_event_get_state_static(TraceEvent *ev)
     return ev->sstate;
 }
 
-static inline bool trace_event_get_state_dynamic_by_id(TraceEventID id)
-{
-    /* it's on fast path, avoid consistency checks (asserts) */
-    return unlikely(trace_events_enabled_count) && trace_events_dstate[id];
-}
+/* it's on fast path, avoid consistency checks (asserts) */
+#define trace_event_get_state_dynamic_by_id(id) \
+    (unlikely(trace_events_enabled_count) && _ ## id ## _DSTATE)
 
 static inline bool trace_event_get_state_dynamic(TraceEvent *ev)
 {
-    TraceEventID id;
-    assert(trace_event_get_state_static(ev));
-    id = trace_event_get_id(ev);
-    return trace_event_get_state_dynamic_by_id(id);
+    return unlikely(trace_events_enabled_count) && *ev->dstate;
 }
 
 static inline bool trace_event_get_vcpu_state_dynamic_by_vcpu_id(CPUState *vcpu,
diff --git a/trace/control-target.c b/trace/control-target.c
index 50bac4f..3b55941 100644
--- a/trace/control-target.c
+++ b/trace/control-target.c
@@ -16,21 +16,20 @@
 
 void trace_event_set_state_dynamic_init(TraceEvent *ev, bool state)
 {
-    TraceEventID id = trace_event_get_id(ev);
     bool state_pre;
     assert(trace_event_get_state_static(ev));
     /*
      * We ignore the "vcpu" property here, since no vCPUs have been created
      * yet. Then dstate can only be 1 or 0.
      */
-    state_pre = trace_events_dstate[id];
+    state_pre = *ev->dstate;
     if (state_pre != state) {
         if (state) {
             trace_events_enabled_count++;
-            trace_events_dstate[id] = 1;
+            *ev->dstate = 1;
         } else {
             trace_events_enabled_count--;
-            trace_events_dstate[id] = 0;
+            *ev->dstate = 0;
         }
     }
 }
@@ -45,15 +44,14 @@ void trace_event_set_state_dynamic(TraceEvent *ev, bool state)
         }
     } else {
         /* Without the "vcpu" property, dstate can only be 1 or 0 */
-        TraceEventID id = trace_event_get_id(ev);
-        bool state_pre = trace_events_dstate[id];
+        bool state_pre = *ev->dstate;
         if (state_pre != state) {
             if (state) {
                 trace_events_enabled_count++;
-                trace_events_dstate[id] = 1;
+                *ev->dstate = 1;
             } else {
                 trace_events_enabled_count--;
-                trace_events_dstate[id] = 0;
+                *ev->dstate = 0;
             }
         }
     }
@@ -62,23 +60,21 @@ void trace_event_set_state_dynamic(TraceEvent *ev, bool state)
 void trace_event_set_vcpu_state_dynamic(CPUState *vcpu,
                                         TraceEvent *ev, bool state)
 {
-    TraceEventID id;
     TraceEventVCPUID vcpu_id;
     bool state_pre;
     assert(trace_event_get_state_static(ev));
     assert(trace_event_is_vcpu(ev));
-    id = trace_event_get_id(ev);
     vcpu_id = trace_event_get_vcpu_id(ev);
     state_pre = test_bit(vcpu_id, vcpu->trace_dstate);
     if (state_pre != state) {
         if (state) {
             trace_events_enabled_count++;
             set_bit(vcpu_id, vcpu->trace_dstate);
-            trace_events_dstate[id]++;
+            (*ev->dstate)++;
         } else {
             trace_events_enabled_count--;
             clear_bit(vcpu_id, vcpu->trace_dstate);
-            trace_events_dstate[id]--;
+            (*ev->dstate)--;
         }
     }
 }
@@ -105,12 +101,11 @@ void trace_init_vcpu(CPUState *vcpu)
         if (trace_event_is_vcpu(ev) &&
             trace_event_get_state_static(ev) &&
             trace_event_get_state_dynamic(ev)) {
-            TraceEventID id = trace_event_get_id(ev);
             if (adding_first_cpu()) {
                 /* check preconditions */
-                assert(trace_events_dstate[id] == 1);
+                assert(*ev->dstate == 1);
                 /* disable early-init state ... */
-                trace_events_dstate[id] = 0;
+                *ev->dstate = 0;
                 trace_events_enabled_count--;
                 /* ... and properly re-enable */
                 trace_event_set_vcpu_state_dynamic(vcpu, ev, true);
diff --git a/trace/control.c b/trace/control.c
index a423f32..e57f2d3 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -28,12 +28,6 @@
 #include "monitor/monitor.h"
 
 int trace_events_enabled_count;
-/*
- * Interpretation depends on wether the event has the 'vcpu' property:
- * - false: Boolean value indicating whether the event is active.
- * - true : Integral counting the number of vCPUs that have this event enabled.
- */
-uint16_t trace_events_dstate[TRACE_EVENT_COUNT];
 
 QemuOptsList qemu_trace_opts = {
     .name = "trace",
diff --git a/trace/event-internal.h b/trace/event-internal.h
index 074faf6..4a98d09 100644
--- a/trace/event-internal.h
+++ b/trace/event-internal.h
@@ -19,6 +19,12 @@
  * @vcpu_id: Unique per-vCPU event identifier.
  * @name: Event name.
  * @sstate: Static tracing state.
+ * @dstate: Dynamic tracing state
+ *
+ * Interpretation of @dstate depends on whether the event has the 'vcpu'
+ *  property:
+ * - false: Boolean value indicating whether the event is active.
+ * - true : Integral counting the number of vCPUs that have this event enabled.
  *
  * Opaque generic description of a tracing event.
  */
@@ -27,6 +33,7 @@ typedef struct TraceEvent {
     TraceEventVCPUID vcpu_id;
     const char * name;
     const bool sstate;
+    uint16_t *dstate;
 } TraceEvent;
 
 void trace_event_set_state_dynamic_init(TraceEvent *ev, bool state);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PULL 06/20] trace: remove duplicate control.h includes in generated-tracers.h
  2016-10-07 10:09 [Qemu-devel] [PULL 00/20] Tracing patches Stefan Hajnoczi
                   ` (4 preceding siblings ...)
  2016-10-07 10:09 ` [Qemu-devel] [PULL 05/20] trace: remove global 'uint16 dstate[]' array Stefan Hajnoczi
@ 2016-10-07 10:09 ` Stefan Hajnoczi
  2016-10-07 10:09 ` [Qemu-devel] [PULL 07/20] trace: break circular dependency in event-internal.h Stefan Hajnoczi
                   ` (14 subsequent siblings)
  20 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2016-10-07 10:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange, Stefan Hajnoczi

From: "Daniel P. Berrange" <berrange@redhat.com>

The format/h.py file adds an include for control.h to
generated-tracers.h. ftrace, log and syslog, then
add more duplicate includes for control.h.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Lluís Vilanova <vilanova@ac.upc.edu>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 1475588159-30598-8-git-send-email-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 scripts/tracetool/backend/ftrace.py | 1 -
 scripts/tracetool/backend/log.py    | 3 +--
 scripts/tracetool/backend/syslog.py | 1 -
 3 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/scripts/tracetool/backend/ftrace.py b/scripts/tracetool/backend/ftrace.py
index 80dcf30..4b83b44 100644
--- a/scripts/tracetool/backend/ftrace.py
+++ b/scripts/tracetool/backend/ftrace.py
@@ -21,7 +21,6 @@ PUBLIC = True
 
 def generate_h_begin(events):
     out('#include "trace/ftrace.h"',
-        '#include "trace/control.h"',
         '')
 
 
diff --git a/scripts/tracetool/backend/log.py b/scripts/tracetool/backend/log.py
index b3ff064..426bb6e 100644
--- a/scripts/tracetool/backend/log.py
+++ b/scripts/tracetool/backend/log.py
@@ -20,8 +20,7 @@ PUBLIC = True
 
 
 def generate_h_begin(events):
-    out('#include "trace/control.h"',
-        '#include "qemu/log.h"',
+    out('#include "qemu/log.h"',
         '')
 
 
diff --git a/scripts/tracetool/backend/syslog.py b/scripts/tracetool/backend/syslog.py
index 89019bc..5f693bd 100644
--- a/scripts/tracetool/backend/syslog.py
+++ b/scripts/tracetool/backend/syslog.py
@@ -21,7 +21,6 @@ PUBLIC = True
 
 def generate_h_begin(events):
     out('#include <syslog.h>',
-        '#include "trace/control.h"',
         '')
 
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PULL 07/20] trace: break circular dependency in event-internal.h
  2016-10-07 10:09 [Qemu-devel] [PULL 00/20] Tracing patches Stefan Hajnoczi
                   ` (5 preceding siblings ...)
  2016-10-07 10:09 ` [Qemu-devel] [PULL 06/20] trace: remove duplicate control.h includes in generated-tracers.h Stefan Hajnoczi
@ 2016-10-07 10:09 ` Stefan Hajnoczi
  2016-10-07 10:09 ` [Qemu-devel] [PULL 08/20] trace: give each trace event a named TraceEvent struct Stefan Hajnoczi
                   ` (13 subsequent siblings)
  20 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2016-10-07 10:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange, Stefan Hajnoczi

From: "Daniel P. Berrange" <berrange@redhat.com>

Currently event-internal.h includes generated-events.h,
while generated-events.h includes event-internal.h
causing a circular dependency.

event-internal.h requires that the content of
generated-events.h comes first, so that it can see
the typedefs for TraceEventID and TraceEventVCPUID.

Switching the TraceEvent struct to use uint32_t
for the two ID fields removes the dependency on
the typedef, allowing events-internal.h to be a
self-contained header. This will then let the patch
following this move event-internal.h to the top of
generated-events.h, so we can expose TraceEvent
struct variables in generated-events.h

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Lluís Vilanova <vilanova@ac.upc.edu>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 1475588159-30598-9-git-send-email-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 trace/event-internal.h | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/trace/event-internal.h b/trace/event-internal.h
index 4a98d09..58f0551 100644
--- a/trace/event-internal.h
+++ b/trace/event-internal.h
@@ -10,9 +10,6 @@
 #ifndef TRACE__EVENT_INTERNAL_H
 #define TRACE__EVENT_INTERNAL_H
 
-#include "trace/generated-events.h"
-
-
 /**
  * TraceEvent:
  * @id: Unique event identifier.
@@ -29,8 +26,8 @@
  * Opaque generic description of a tracing event.
  */
 typedef struct TraceEvent {
-    TraceEventID id;
-    TraceEventVCPUID vcpu_id;
+    uint32_t id;
+    uint32_t vcpu_id;
     const char * name;
     const bool sstate;
     uint16_t *dstate;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PULL 08/20] trace: give each trace event a named TraceEvent struct
  2016-10-07 10:09 [Qemu-devel] [PULL 00/20] Tracing patches Stefan Hajnoczi
                   ` (6 preceding siblings ...)
  2016-10-07 10:09 ` [Qemu-devel] [PULL 07/20] trace: break circular dependency in event-internal.h Stefan Hajnoczi
@ 2016-10-07 10:09 ` Stefan Hajnoczi
  2016-10-07 10:09 ` [Qemu-devel] [PULL 09/20] trace: remove the TraceEventID and TraceEventVCPUID enums Stefan Hajnoczi
                   ` (12 subsequent siblings)
  20 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2016-10-07 10:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange, Stefan Hajnoczi

From: "Daniel P. Berrange" <berrange@redhat.com>

Currently we only expose a TraceEvent array, which must
be indexed via the TraceEventID enum constants. This
changes the generator to expose a named TraceEvent
instance for each event, with an _EVENT suffix.

Reviewed-by: Lluís Vilanova <vilanova@ac.upc.edu>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 1475588159-30598-10-git-send-email-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 scripts/tracetool/__init__.py        |  1 +
 scripts/tracetool/format/events_c.py | 19 +++++++++++++------
 scripts/tracetool/format/events_h.py | 11 ++++++++---
 trace/control-internal.h             |  2 +-
 trace/control.c                      |  2 +-
 5 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index a11f36b..dc93416 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -266,6 +266,7 @@ class Event(object):
     QEMU_TRACE               = "trace_%(name)s"
     QEMU_TRACE_TCG           = QEMU_TRACE + "_tcg"
     QEMU_DSTATE              = "_TRACE_%(NAME)s_DSTATE"
+    QEMU_EVENT               = "_TRACE_%(NAME)s_EVENT"
 
     def api(self, fmt=None):
         if fmt is None:
diff --git a/scripts/tracetool/format/events_c.py b/scripts/tracetool/format/events_c.py
index ef873fa..a97054f 100644
--- a/scripts/tracetool/format/events_c.py
+++ b/scripts/tracetool/format/events_c.py
@@ -28,22 +28,29 @@ def generate(events, backend):
     for e in events:
         out('uint16_t %s;' % e.api(e.QEMU_DSTATE))
 
-    out('TraceEvent trace_events[TRACE_EVENT_COUNT] = {')
-
     for e in events:
         if "vcpu" in e.properties:
             vcpu_id = "TRACE_VCPU_" + e.name.upper()
         else:
             vcpu_id = "TRACE_VCPU_EVENT_COUNT"
-        out('    { .id = %(id)s, .vcpu_id = %(vcpu_id)s,'
-            ' .name = \"%(name)s\",'
-            ' .sstate = %(sstate)s,',
-            ' .dstate = &%(dstate)s, }, ',
+        out('TraceEvent %(event)s = {',
+            '    .id = %(id)s,',
+            '    .vcpu_id = %(vcpu_id)s,',
+            '    .name = \"%(name)s\",',
+            '    .sstate = %(sstate)s,',
+            '    .dstate = &%(dstate)s ',
+            '};',
+            event = e.api(e.QEMU_EVENT),
             id = "TRACE_" + e.name.upper(),
             vcpu_id = vcpu_id,
             name = e.name,
             sstate = "TRACE_%s_ENABLED" % e.name.upper(),
             dstate = e.api(e.QEMU_DSTATE))
 
+    out('TraceEvent *trace_events[TRACE_EVENT_COUNT] = {')
+
+    for e in events:
+        out('    &%(event)s,', event = e.api(e.QEMU_EVENT))
+
     out('};',
         '')
diff --git a/scripts/tracetool/format/events_h.py b/scripts/tracetool/format/events_h.py
index 03417de..80a66c5 100644
--- a/scripts/tracetool/format/events_h.py
+++ b/scripts/tracetool/format/events_h.py
@@ -21,7 +21,13 @@ def generate(events, backend):
         '',
         '#ifndef TRACE__GENERATED_EVENTS_H',
         '#define TRACE__GENERATED_EVENTS_H',
-        '')
+        '',
+        '#include "trace/event-internal.h"',
+        )
+
+    for e in events:
+        out('extern TraceEvent %(event)s;',
+            event = e.api(e.QEMU_EVENT))
 
     # event identifiers
     out('typedef enum {')
@@ -58,6 +64,5 @@ def generate(events, backend):
                 enabled=enabled)
         out('#define TRACE_%s_ENABLED %d' % (e.name.upper(), enabled))
 
-    out('#include "trace/event-internal.h"',
-        '',
+    out('',
         '#endif  /* TRACE__GENERATED_EVENTS_H */')
diff --git a/trace/control-internal.h b/trace/control-internal.h
index a6d8d2e..808f85d 100644
--- a/trace/control-internal.h
+++ b/trace/control-internal.h
@@ -15,7 +15,7 @@
 #include "qom/cpu.h"
 
 
-extern TraceEvent trace_events[];
+extern TraceEvent *trace_events[];
 extern int trace_events_enabled_count;
 
 
diff --git a/trace/control.c b/trace/control.c
index e57f2d3..9035846 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -106,7 +106,7 @@ void trace_event_iter_init(TraceEventIter *iter, const char *pattern)
 TraceEvent *trace_event_iter_next(TraceEventIter *iter)
 {
     while (iter->event < TRACE_EVENT_COUNT) {
-        TraceEvent *ev = &(trace_events[iter->event]);
+        TraceEvent *ev = trace_events[iter->event];
         iter->event++;
         if (!iter->pattern ||
             pattern_glob(iter->pattern,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PULL 09/20] trace: remove the TraceEventID and TraceEventVCPUID enums
  2016-10-07 10:09 [Qemu-devel] [PULL 00/20] Tracing patches Stefan Hajnoczi
                   ` (7 preceding siblings ...)
  2016-10-07 10:09 ` [Qemu-devel] [PULL 08/20] trace: give each trace event a named TraceEvent struct Stefan Hajnoczi
@ 2016-10-07 10:09 ` Stefan Hajnoczi
  2016-10-07 10:09 ` [Qemu-devel] [PULL 10/20] trace: emit name <-> ID mapping in simpletrace header Stefan Hajnoczi
                   ` (11 subsequent siblings)
  20 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2016-10-07 10:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange, Stefan Hajnoczi

From: "Daniel P. Berrange" <berrange@redhat.com>

The TraceEventID and TraceEventVCPUID enums constants are
no longer actually used for anything critical.

The TRACE_EVENT_COUNT limit is used to determine the size
of the TraceEvents array, and can be removed if we just
NULL terminate the array instead.

The TRACE_VCPU_EVENT_COUNT limit is used as a magic value
for marking non-vCPU events, and also for declaring the
size of the trace dstate mask in the CPUState struct.
The former usage can be replaced by a dedicated constant
TRACE_EVENT_VCPU_NONE, defined as (uint32_t)-1. For the
latter usage, we can simply define a constant for the
number of VCPUs, avoiding the need for the full enum.

The only other usages of the enum values can be replaced
by accesing the id/vcpu_id fields via the named TraceEvent
structs.

Reviewed-by: Lluís Vilanova <vilanova@ac.upc.edu>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 1475588159-30598-11-git-send-email-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 scripts/tracetool/backend/simple.py  |  4 ++--
 scripts/tracetool/format/events_c.py | 16 +++++++++++-----
 scripts/tracetool/format/events_h.py | 19 ++-----------------
 scripts/tracetool/format/h.py        |  3 +--
 trace/control-internal.h             | 19 ++++++++++---------
 trace/control-target.c               |  2 +-
 trace/control.c                      |  2 +-
 trace/control.h                      | 32 +++++++++-----------------------
 trace/event-internal.h               |  6 ++++++
 trace/simple.c                       |  8 ++++----
 trace/simple.h                       |  2 +-
 11 files changed, 48 insertions(+), 65 deletions(-)

diff --git a/scripts/tracetool/backend/simple.py b/scripts/tracetool/backend/simple.py
index 1bccada..1114e35 100644
--- a/scripts/tracetool/backend/simple.py
+++ b/scripts/tracetool/backend/simple.py
@@ -80,11 +80,11 @@ def generate_c(event):
         '        return;',
         '    }',
         '',
-        '    if (trace_record_start(&rec, %(event_id)s, %(size_str)s)) {',
+        '    if (trace_record_start(&rec, %(event_obj)s.id, %(size_str)s)) {',
         '        return; /* Trace Buffer Full, Event Dropped ! */',
         '    }',
         cond=cond,
-        event_id=event_id,
+        event_obj=event.api(event.QEMU_EVENT),
         size_str=sizestr)
 
     if len(event.args) > 0:
diff --git a/scripts/tracetool/format/events_c.py b/scripts/tracetool/format/events_c.py
index a97054f..40ae395 100644
--- a/scripts/tracetool/format/events_c.py
+++ b/scripts/tracetool/format/events_c.py
@@ -28,11 +28,16 @@ def generate(events, backend):
     for e in events:
         out('uint16_t %s;' % e.api(e.QEMU_DSTATE))
 
+    next_id = 0
+    next_vcpu_id = 0
     for e in events:
+        id = next_id
+        next_id += 1
         if "vcpu" in e.properties:
-            vcpu_id = "TRACE_VCPU_" + e.name.upper()
+            vcpu_id = next_vcpu_id
+            next_vcpu_id += 1
         else:
-            vcpu_id = "TRACE_VCPU_EVENT_COUNT"
+            vcpu_id = "TRACE_VCPU_EVENT_NONE"
         out('TraceEvent %(event)s = {',
             '    .id = %(id)s,',
             '    .vcpu_id = %(vcpu_id)s,',
@@ -41,16 +46,17 @@ def generate(events, backend):
             '    .dstate = &%(dstate)s ',
             '};',
             event = e.api(e.QEMU_EVENT),
-            id = "TRACE_" + e.name.upper(),
+            id = id,
             vcpu_id = vcpu_id,
             name = e.name,
             sstate = "TRACE_%s_ENABLED" % e.name.upper(),
             dstate = e.api(e.QEMU_DSTATE))
 
-    out('TraceEvent *trace_events[TRACE_EVENT_COUNT] = {')
+    out('TraceEvent *trace_events[] = {')
 
     for e in events:
         out('    &%(event)s,', event = e.api(e.QEMU_EVENT))
 
-    out('};',
+    out('  NULL,',
+        '};',
         '')
diff --git a/scripts/tracetool/format/events_h.py b/scripts/tracetool/format/events_h.py
index 80a66c5..ca6d730 100644
--- a/scripts/tracetool/format/events_h.py
+++ b/scripts/tracetool/format/events_h.py
@@ -29,27 +29,12 @@ def generate(events, backend):
         out('extern TraceEvent %(event)s;',
             event = e.api(e.QEMU_EVENT))
 
-    # event identifiers
-    out('typedef enum {')
-
-    for e in events:
-        out('    TRACE_%s,' % e.name.upper())
-
-    out('    TRACE_EVENT_COUNT',
-        '} TraceEventID;')
-
     for e in events:
         out('extern uint16_t %s;' % e.api(e.QEMU_DSTATE))
 
-    # per-vCPU event identifiers
-    out('typedef enum {')
+    numvcpu = len([e for e in events if "vcpu" in e.properties])
 
-    for e in events:
-        if "vcpu" in e.properties:
-            out('    TRACE_VCPU_%s,' % e.name.upper())
-
-    out('    TRACE_VCPU_EVENT_COUNT',
-        '} TraceEventVCPUID;')
+    out("#define TRACE_VCPU_EVENT_COUNT %d" % numvcpu)
 
     # static state
     for e in events:
diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
index 3763e9a..64a6680 100644
--- a/scripts/tracetool/format/h.py
+++ b/scripts/tracetool/format/h.py
@@ -32,8 +32,7 @@ def generate(events, backend):
         if "vcpu" in e.properties:
             trace_cpu = next(iter(e.args))[1]
             cond = "trace_event_get_vcpu_state(%(cpu)s,"\
-                   " TRACE_%(id)s,"\
-                   " TRACE_VCPU_%(id)s)"\
+                   " TRACE_%(id)s)"\
                    % dict(
                        cpu=trace_cpu,
                        id=e.name.upper())
diff --git a/trace/control-internal.h b/trace/control-internal.h
index 808f85d..9abbc96 100644
--- a/trace/control-internal.h
+++ b/trace/control-internal.h
@@ -25,20 +25,20 @@ static inline bool trace_event_is_pattern(const char *str)
     return strchr(str, '*') != NULL;
 }
 
-static inline TraceEventID trace_event_get_id(TraceEvent *ev)
+static inline uint32_t trace_event_get_id(TraceEvent *ev)
 {
     assert(ev != NULL);
     return ev->id;
 }
 
-static inline TraceEventVCPUID trace_event_get_vcpu_id(TraceEvent *ev)
+static inline uint32_t trace_event_get_vcpu_id(TraceEvent *ev)
 {
     return ev->vcpu_id;
 }
 
 static inline bool trace_event_is_vcpu(TraceEvent *ev)
 {
-    return ev->vcpu_id != TRACE_VCPU_EVENT_COUNT;
+    return ev->vcpu_id != TRACE_VCPU_EVENT_NONE;
 }
 
 static inline const char * trace_event_get_name(TraceEvent *ev)
@@ -62,12 +62,13 @@ static inline bool trace_event_get_state_dynamic(TraceEvent *ev)
     return unlikely(trace_events_enabled_count) && *ev->dstate;
 }
 
-static inline bool trace_event_get_vcpu_state_dynamic_by_vcpu_id(CPUState *vcpu,
-                                                                 TraceEventVCPUID id)
+static inline bool
+trace_event_get_vcpu_state_dynamic_by_vcpu_id(CPUState *vcpu,
+                                              uint32_t vcpu_id)
 {
     /* it's on fast path, avoid consistency checks (asserts) */
     if (unlikely(trace_events_enabled_count)) {
-        return test_bit(id, vcpu->trace_dstate);
+        return test_bit(vcpu_id, vcpu->trace_dstate);
     } else {
         return false;
     }
@@ -76,10 +77,10 @@ static inline bool trace_event_get_vcpu_state_dynamic_by_vcpu_id(CPUState *vcpu,
 static inline bool trace_event_get_vcpu_state_dynamic(CPUState *vcpu,
                                                       TraceEvent *ev)
 {
-    TraceEventVCPUID id;
+    uint32_t vcpu_id;
     assert(trace_event_is_vcpu(ev));
-    id = trace_event_get_vcpu_id(ev);
-    return trace_event_get_vcpu_state_dynamic_by_vcpu_id(vcpu, id);
+    vcpu_id = trace_event_get_vcpu_id(ev);
+    return trace_event_get_vcpu_state_dynamic_by_vcpu_id(vcpu, vcpu_id);
 }
 
 #endif /* TRACE__CONTROL_INTERNAL_H */
diff --git a/trace/control-target.c b/trace/control-target.c
index 3b55941..7ebf6e0 100644
--- a/trace/control-target.c
+++ b/trace/control-target.c
@@ -60,7 +60,7 @@ void trace_event_set_state_dynamic(TraceEvent *ev, bool state)
 void trace_event_set_vcpu_state_dynamic(CPUState *vcpu,
                                         TraceEvent *ev, bool state)
 {
-    TraceEventVCPUID vcpu_id;
+    uint32_t vcpu_id;
     bool state_pre;
     assert(trace_event_get_state_static(ev));
     assert(trace_event_is_vcpu(ev));
diff --git a/trace/control.c b/trace/control.c
index 9035846..6b32511 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -105,7 +105,7 @@ void trace_event_iter_init(TraceEventIter *iter, const char *pattern)
 
 TraceEvent *trace_event_iter_next(TraceEventIter *iter)
 {
-    while (iter->event < TRACE_EVENT_COUNT) {
+    while (trace_events[iter->event] != NULL) {
         TraceEvent *ev = trace_events[iter->event];
         iter->event++;
         if (!iter->pattern ||
diff --git a/trace/control.h b/trace/control.h
index 10d4aba..cccd2a2 100644
--- a/trace/control.h
+++ b/trace/control.h
@@ -18,17 +18,6 @@ typedef struct TraceEventIter {
     const char *pattern;
 } TraceEventIter;
 
-/**
- * TraceEventID:
- *
- * Unique tracing event identifier.
- *
- * These are named as 'TRACE_${EVENT_NAME}'.
- *
- * See also: "trace/generated-events.h"
- */
-enum TraceEventID;
-
 
 /**
  * trace_event_iter_init:
@@ -76,17 +65,17 @@ static bool trace_event_is_pattern(const char *str);
  *
  * Get the identifier of an event.
  */
-static TraceEventID trace_event_get_id(TraceEvent *ev);
+static uint32_t trace_event_get_id(TraceEvent *ev);
 
 /**
  * trace_event_get_vcpu_id:
  *
  * Get the per-vCPU identifier of an event.
  *
- * Special value #TRACE_VCPU_EVENT_COUNT means the event is not vCPU-specific
+ * Special value #TRACE_VCPU_EVENT_NONE means the event is not vCPU-specific
  * (does not have the "vcpu" property).
  */
-static TraceEventVCPUID trace_event_get_vcpu_id(TraceEvent *ev);
+static uint32_t trace_event_get_vcpu_id(TraceEvent *ev);
 
 /**
  * trace_event_is_vcpu:
@@ -104,14 +93,12 @@ static const char * trace_event_get_name(TraceEvent *ev);
 
 /**
  * trace_event_get_state:
- * @id: Event identifier.
+ * @id: Event identifier name.
  *
  * Get the tracing state of an event (both static and dynamic).
  *
  * If the event has the disabled property, the check will have no performance
  * impact.
- *
- * As a down side, you must always use an immediate #TraceEventID value.
  */
 #define trace_event_get_state(id)                       \
     ((id ##_ENABLED) && trace_event_get_state_dynamic_by_id(id))
@@ -119,19 +106,18 @@ static const char * trace_event_get_name(TraceEvent *ev);
 /**
  * trace_event_get_vcpu_state:
  * @vcpu: Target vCPU.
- * @id: Event identifier (TraceEventID).
- * @vcpu_id: Per-vCPU event identifier (TraceEventVCPUID).
+ * @id: Event identifier name.
  *
  * Get the tracing state of an event (both static and dynamic) for the given
  * vCPU.
  *
  * If the event has the disabled property, the check will have no performance
  * impact.
- *
- * As a down side, you must always use an immediate #TraceEventID value.
  */
-#define trace_event_get_vcpu_state(vcpu, id, vcpu_id)                   \
-    ((id ##_ENABLED) && trace_event_get_vcpu_state_dynamic_by_vcpu_id(vcpu, vcpu_id))
+#define trace_event_get_vcpu_state(vcpu, id)                            \
+    ((id ##_ENABLED) &&                                                 \
+     trace_event_get_vcpu_state_dynamic_by_vcpu_id(                     \
+         vcpu, _ ## id ## _EVENT.vcpu_id))
 
 /**
  * trace_event_get_state_static:
diff --git a/trace/event-internal.h b/trace/event-internal.h
index 58f0551..f63500b 100644
--- a/trace/event-internal.h
+++ b/trace/event-internal.h
@@ -10,6 +10,12 @@
 #ifndef TRACE__EVENT_INTERNAL_H
 #define TRACE__EVENT_INTERNAL_H
 
+/*
+ * Special value for TraceEvent.vcpu_id field to indicate
+ * that the event is not VCPU specific
+ */
+#define TRACE_VCPU_EVENT_NONE ((uint32_t)-1)
+
 /**
  * TraceEvent:
  * @id: Unique event identifier.
diff --git a/trace/simple.c b/trace/simple.c
index 2f09daf..2ec32e1 100644
--- a/trace/simple.c
+++ b/trace/simple.c
@@ -17,8 +17,8 @@
 #include "trace/control.h"
 #include "trace/simple.h"
 
-/** Trace file header event ID */
-#define HEADER_EVENT_ID (~(uint64_t)0) /* avoids conflicting with TraceEventIDs */
+/** Trace file header event ID, picked to avoid conflict with real event IDs */
+#define HEADER_EVENT_ID (~(uint64_t)0)
 
 /** Trace file magic number */
 #define HEADER_MAGIC 0xf2b177cb0aa429b4ULL
@@ -58,7 +58,7 @@ static char *trace_file_name;
 
 /* * Trace buffer entry */
 typedef struct {
-    uint64_t event; /*   TraceEventID */
+    uint64_t event; /* event ID value */
     uint64_t timestamp_ns;
     uint32_t length;   /*    in bytes */
     uint32_t pid;
@@ -202,7 +202,7 @@ void trace_record_write_str(TraceBufferRecord *rec, const char *s, uint32_t slen
     rec->rec_off = write_to_buffer(rec->rec_off, (void*)s, slen);
 }
 
-int trace_record_start(TraceBufferRecord *rec, TraceEventID event, size_t datasize)
+int trace_record_start(TraceBufferRecord *rec, uint32_t event, size_t datasize)
 {
     unsigned int idx, rec_off, old_idx, new_idx;
     uint32_t rec_len = sizeof(TraceRecord) + datasize;
diff --git a/trace/simple.h b/trace/simple.h
index 1e7de45..17ce472 100644
--- a/trace/simple.h
+++ b/trace/simple.h
@@ -33,7 +33,7 @@ typedef struct {
  *
  * @arglen  number of bytes required for arguments
  */
-int trace_record_start(TraceBufferRecord *rec, TraceEventID id, size_t arglen);
+int trace_record_start(TraceBufferRecord *rec, uint32_t id, size_t arglen);
 
 /**
  * Append a 64-bit argument to a trace record
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PULL 10/20] trace: emit name <-> ID mapping in simpletrace header
  2016-10-07 10:09 [Qemu-devel] [PULL 00/20] Tracing patches Stefan Hajnoczi
                   ` (8 preceding siblings ...)
  2016-10-07 10:09 ` [Qemu-devel] [PULL 09/20] trace: remove the TraceEventID and TraceEventVCPUID enums Stefan Hajnoczi
@ 2016-10-07 10:09 ` Stefan Hajnoczi
  2016-10-07 10:09 ` [Qemu-devel] [PULL 11/20] trace: don't abort qemu if ftrace can't be initialized Stefan Hajnoczi
                   ` (10 subsequent siblings)
  20 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2016-10-07 10:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange, Stefan Hajnoczi

From: "Daniel P. Berrange" <berrange@redhat.com>

Currently simpletrace assumes that events are given IDs
starting from 0, based on the order in which they appear
in the trace-events file, with no gaps. When the
trace-events file is split up, this assumption becomes
problematic.

To deal with this, extend the simpletrace format so that
it outputs a table of event name <-> ID mappings. That
will allow QEMU to assign arbitrary IDs to events without
breaking simpletrace parsing.

The v3 simple trace format was

  FILE HEADER
  EVENT TRACE RECORD 0
  EVENT TRACE RECORD 1
  ...
  EVENT TRACE RECORD N

The v4 simple trace format is now

  FILE HEADER
  EVENT MAPPING RECORD 0
  EVENT MAPPING RECORD 1
  ...
  EVENT MAPPING RECORD M
  EVENT TRACE RECORD RECORD 0
  EVENT TRACE RECORD RECORD 1
  ...
  EVENT TRACE RECORD N

Although this shows all the mapping records being emitted
upfront, this is not required by the format. While the main
simpletrace backend will emit all mappings at startup,
the systemtap simpletrace.stp script will emit the mappings
at first use. eg

  FILE HEADER
  ...
  EVENT MAPPING RECORD 0
  EVENT TRACE RECORD RECORD 0
  EVENT TRACE RECORD RECORD 1
  EVENT MAPPING RECORD 1
  EVENT TRACE RECORD RECORD 2
  ...
  EVENT TRACE RECORD N

This is more space efficient given that most trace records
only include a subset of events.

In modifying the systemtap simpletrace code, a 'begin' probe
was added to emit the trace event header, so you no longer
need to add '--no-header' when running simpletrace.py for
systemtap generated trace files.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 1475588159-30598-12-git-send-email-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 scripts/simpletrace.py                       | 50 ++++++++++++++++++++--------
 scripts/tracetool/format/simpletrace_stap.py | 24 +++++++++++--
 trace/simple.c                               | 33 ++++++++++++++++--
 3 files changed, 89 insertions(+), 18 deletions(-)

diff --git a/scripts/simpletrace.py b/scripts/simpletrace.py
index 3916c6d..f40467a 100755
--- a/scripts/simpletrace.py
+++ b/scripts/simpletrace.py
@@ -19,6 +19,9 @@ header_event_id = 0xffffffffffffffff
 header_magic    = 0xf2b177cb0aa429b4
 dropped_event_id = 0xfffffffffffffffe
 
+record_type_mapping = 0
+record_type_event = 1
+
 log_header_fmt = '=QQQ'
 rec_header_fmt = '=QQII'
 
@@ -30,14 +33,16 @@ def read_header(fobj, hfmt):
         return None
     return struct.unpack(hfmt, hdr)
 
-def get_record(edict, rechdr, fobj):
-    """Deserialize a trace record from a file into a tuple (event_num, timestamp, pid, arg1, ..., arg6)."""
+def get_record(edict, idtoname, rechdr, fobj):
+    """Deserialize a trace record from a file into a tuple
+       (name, timestamp, pid, arg1, ..., arg6)."""
     if rechdr is None:
         return None
-    rec = (rechdr[0], rechdr[1], rechdr[3])
     if rechdr[0] != dropped_event_id:
         event_id = rechdr[0]
-        event = edict[event_id]
+        name = idtoname[event_id]
+        rec = (name, rechdr[1], rechdr[3])
+        event = edict[name]
         for type, name in event.args:
             if is_string(type):
                 l = fobj.read(4)
@@ -48,15 +53,22 @@ def get_record(edict, rechdr, fobj):
                 (value,) = struct.unpack('=Q', fobj.read(8))
                 rec = rec + (value,)
     else:
+        rec = ("dropped", rechdr[1], rechdr[3])
         (value,) = struct.unpack('=Q', fobj.read(8))
         rec = rec + (value,)
     return rec
 
+def get_mapping(fobj):
+    (event_id, ) = struct.unpack('=Q', fobj.read(8))
+    (len, ) = struct.unpack('=L', fobj.read(4))
+    name = fobj.read(len)
 
-def read_record(edict, fobj):
+    return (event_id, name)
+
+def read_record(edict, idtoname, fobj):
     """Deserialize a trace record from a file into a tuple (event_num, timestamp, pid, arg1, ..., arg6)."""
     rechdr = read_header(fobj, rec_header_fmt)
-    return get_record(edict, rechdr, fobj) # return tuple of record elements
+    return get_record(edict, idtoname, rechdr, fobj)
 
 def read_trace_header(fobj):
     """Read and verify trace file header"""
@@ -67,20 +79,30 @@ def read_trace_header(fobj):
         raise ValueError('Not a valid trace file!')
 
     log_version = header[2]
-    if log_version not in [0, 2, 3]:
+    if log_version not in [0, 2, 3, 4]:
         raise ValueError('Unknown version of tracelog format!')
-    if log_version != 3:
+    if log_version != 4:
         raise ValueError('Log format %d not supported with this QEMU release!'
                          % log_version)
 
 def read_trace_records(edict, fobj):
     """Deserialize trace records from a file, yielding record tuples (event_num, timestamp, pid, arg1, ..., arg6)."""
+    idtoname = {
+        dropped_event_id: "dropped"
+    }
     while True:
-        rec = read_record(edict, fobj)
-        if rec is None:
+        t = fobj.read(8)
+        if len(t) == 0:
             break
 
-        yield rec
+        (rectype, ) = struct.unpack('=Q', t)
+        if rectype == record_type_mapping:
+            event_id, name = get_mapping(fobj)
+            idtoname[event_id] = name
+        else:
+            rec = read_record(edict, idtoname, fobj)
+
+            yield rec
 
 class Analyzer(object):
     """A trace file analyzer which processes trace records.
@@ -115,10 +137,10 @@ def process(events, log, analyzer, read_header=True):
         read_trace_header(log)
 
     dropped_event = Event.build("Dropped_Event(uint64_t num_events_dropped)")
-    edict = {dropped_event_id: dropped_event}
+    edict = {"dropped": dropped_event}
 
-    for num, event in enumerate(events):
-        edict[num] = event
+    for event in events:
+        edict[event.name] = event
 
     def build_fn(analyzer, event):
         if isinstance(event, str):
diff --git a/scripts/tracetool/format/simpletrace_stap.py b/scripts/tracetool/format/simpletrace_stap.py
index 7e44bc1..ac3580f 100644
--- a/scripts/tracetool/format/simpletrace_stap.py
+++ b/scripts/tracetool/format/simpletrace_stap.py
@@ -21,6 +21,25 @@ from tracetool.format.stap import stap_escape
 
 def generate(events, backend):
     out('/* This file is autogenerated by tracetool, do not edit. */',
+        '',
+        'global event_name_to_id_map',
+        'global event_next_id',
+        'function simple_trace_map_event(name)',
+        '',
+        '{',
+        '    if (!([name] in event_name_to_id_map)) {',
+        '        event_name_to_id_map[name] = event_next_id',
+        '        name_len = strlen(name)',
+        '        printf("%%8b%%8b%%4b%%.*s", 0, ',
+        '               event_next_id, name_len, name_len, name)',
+        '        event_next_id = event_next_id + 1',
+        '    }',
+        '    return event_name_to_id_map[name]',
+        '}',
+        'probe begin',
+        '{',
+        '    printf("%%8b%%8b%%8b", 0xffffffffffffffff, 0xf2b177cb0aa429b4, 4)',
+        '}',
         '')
 
     for event_id, e in enumerate(events):
@@ -29,6 +48,7 @@ def generate(events, backend):
 
         out('probe %(probeprefix)s.simpletrace.%(name)s = %(probeprefix)s.%(name)s ?',
             '{',
+            '    id = simple_trace_map_event("%(name)s")',
             probeprefix=probeprefix(),
             name=e.name)
 
@@ -48,7 +68,7 @@ def generate(events, backend):
         sizestr = ' + '.join(sizes)
 
         # Generate format string and value pairs for record header and arguments
-        fields = [('8b', str(event_id)),
+        fields = [('8b', 'id'),
                   ('8b', 'gettimeofday_ns()'),
                   ('4b', sizestr),
                   ('4b', 'pid()')]
@@ -63,7 +83,7 @@ def generate(events, backend):
         # Emit the entire record in a single SystemTap printf()
         fmt_str = '%'.join(fmt for fmt, _ in fields)
         arg_str = ', '.join(arg for _, arg in fields)
-        out('    printf("%%%(fmt_str)s", %(arg_str)s)',
+        out('    printf("%%8b%%%(fmt_str)s", 1, %(arg_str)s)',
             fmt_str=fmt_str, arg_str=arg_str)
 
         out('}')
diff --git a/trace/simple.c b/trace/simple.c
index 2ec32e1..b263622 100644
--- a/trace/simple.c
+++ b/trace/simple.c
@@ -24,7 +24,7 @@
 #define HEADER_MAGIC 0xf2b177cb0aa429b4ULL
 
 /** Trace file version number, bump if format changes */
-#define HEADER_VERSION 3
+#define HEADER_VERSION 4
 
 /** Records were dropped event ID */
 #define DROPPED_EVENT_ID (~(uint64_t)0 - 1)
@@ -56,6 +56,9 @@ static uint32_t trace_pid;
 static FILE *trace_fp;
 static char *trace_file_name;
 
+#define TRACE_RECORD_TYPE_MAPPING 0
+#define TRACE_RECORD_TYPE_EVENT   1
+
 /* * Trace buffer entry */
 typedef struct {
     uint64_t event; /* event ID value */
@@ -160,6 +163,7 @@ static gpointer writeout_thread(gpointer opaque)
     unsigned int idx = 0;
     int dropped_count;
     size_t unused __attribute__ ((unused));
+    uint64_t type = TRACE_RECORD_TYPE_EVENT;
 
     for (;;) {
         wait_for_trace_records_available();
@@ -174,10 +178,12 @@ static gpointer writeout_thread(gpointer opaque)
             } while (!g_atomic_int_compare_and_exchange(&dropped_events,
                                                         dropped_count, 0));
             dropped.rec.arguments[0] = dropped_count;
+            unused = fwrite(&type, sizeof(type), 1, trace_fp);
             unused = fwrite(&dropped.rec, dropped.rec.length, 1, trace_fp);
         }
 
         while (get_trace_record(idx, &recordptr)) {
+            unused = fwrite(&type, sizeof(type), 1, trace_fp);
             unused = fwrite(recordptr, recordptr->length, 1, trace_fp);
             writeout_idx += recordptr->length;
             free(recordptr); /* don't use g_free, can deadlock when traced */
@@ -273,6 +279,28 @@ void trace_record_finish(TraceBufferRecord *rec)
     }
 }
 
+static int st_write_event_mapping(void)
+{
+    uint64_t type = TRACE_RECORD_TYPE_MAPPING;
+    TraceEventIter iter;
+    TraceEvent *ev;
+
+    trace_event_iter_init(&iter, NULL);
+    while ((ev = trace_event_iter_next(&iter)) != NULL) {
+        uint64_t id = trace_event_get_id(ev);
+        const char *name = trace_event_get_name(ev);
+        uint32_t len = strlen(name);
+        if (fwrite(&type, sizeof(type), 1, trace_fp) != 1 ||
+            fwrite(&id, sizeof(id), 1, trace_fp) != 1 ||
+            fwrite(&len, sizeof(len), 1, trace_fp) != 1 ||
+            fwrite(name, len, 1, trace_fp) != 1) {
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
 void st_set_trace_file_enabled(bool enable)
 {
     if (enable == !!trace_fp) {
@@ -297,7 +325,8 @@ void st_set_trace_file_enabled(bool enable)
             return;
         }
 
-        if (fwrite(&header, sizeof header, 1, trace_fp) != 1) {
+        if (fwrite(&header, sizeof header, 1, trace_fp) != 1 ||
+            st_write_event_mapping() < 0) {
             fclose(trace_fp);
             trace_fp = NULL;
             return;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PULL 11/20] trace: don't abort qemu if ftrace can't be initialized
  2016-10-07 10:09 [Qemu-devel] [PULL 00/20] Tracing patches Stefan Hajnoczi
                   ` (9 preceding siblings ...)
  2016-10-07 10:09 ` [Qemu-devel] [PULL 10/20] trace: emit name <-> ID mapping in simpletrace header Stefan Hajnoczi
@ 2016-10-07 10:09 ` Stefan Hajnoczi
  2016-10-07 10:09 ` [Qemu-devel] [PULL 12/20] trace: provide mechanism for registering trace events Stefan Hajnoczi
                   ` (9 subsequent siblings)
  20 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2016-10-07 10:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange, Stefan Hajnoczi

From: "Daniel P. Berrange" <berrange@redhat.com>

If the ftrace backend is compiled into QEMU, any attempt
to start QEMU while non-root will fail due to the
inability to open /sys/kernel/debug/tracing/tracing_on.

Add a fallback into the code so that it connects up the
trace_marker_fd variable to /dev/null when getting
EACCES on the 'trace_on' file. This allows QEMU to
run, with ftrace turned into a no-op.

[Fixed s/setting/getting/ and s/EACCESS/EACCES/ errors pointed out by
Eric Blake <eblake@redhat.com>.
--Stefan]

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 1475588159-30598-13-git-send-email-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 trace/ftrace.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/trace/ftrace.c b/trace/ftrace.c
index e953922..3588bb0 100644
--- a/trace/ftrace.c
+++ b/trace/ftrace.c
@@ -51,6 +51,12 @@ bool ftrace_init(void)
         snprintf(path, PATH_MAX, "%s/tracing/tracing_on", debugfs);
         trace_fd = open(path, O_WRONLY);
         if (trace_fd < 0) {
+            if (errno == EACCES) {
+                trace_marker_fd = open("/dev/null", O_WRONLY);
+                if (trace_marker_fd != -1) {
+                    return true;
+                }
+            }
             perror("Could not open ftrace 'tracing_on' file");
             return false;
         } else {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PULL 12/20] trace: provide mechanism for registering trace events
  2016-10-07 10:09 [Qemu-devel] [PULL 00/20] Tracing patches Stefan Hajnoczi
                   ` (10 preceding siblings ...)
  2016-10-07 10:09 ` [Qemu-devel] [PULL 11/20] trace: don't abort qemu if ftrace can't be initialized Stefan Hajnoczi
@ 2016-10-07 10:09 ` Stefan Hajnoczi
  2016-10-07 10:09 ` [Qemu-devel] [PULL 13/20] trace: dynamically allocate trace_dstate in CPUState Stefan Hajnoczi
                   ` (8 subsequent siblings)
  20 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2016-10-07 10:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange, Stefan Hajnoczi

From: "Daniel P. Berrange" <berrange@redhat.com>

Remove the notion of there being a single global array
of trace events, by introducing a method for registering
groups of events.

The module_call_init() needs to be invoked at the start
of any program that wants to make use of the trace
support. Currently this covers system emulators qemu-nbd,
qemu-img and qemu-io.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Lluís Vilanova <vilanova@ac.upc.edu>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 1475588159-30598-14-git-send-email-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/qemu/module.h                |  2 ++
 qemu-img.c                           |  1 +
 qemu-io.c                            |  1 +
 qemu-nbd.c                           |  1 +
 scripts/tracetool/format/events_c.py |  6 ++++++
 trace/control-internal.h             |  4 +++-
 trace/control.c                      | 25 +++++++++++++++++++++++--
 trace/control.h                      |  1 +
 vl.c                                 |  2 ++
 9 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/include/qemu/module.h b/include/qemu/module.h
index dc2c9d4..877cca7 100644
--- a/include/qemu/module.h
+++ b/include/qemu/module.h
@@ -44,6 +44,7 @@ typedef enum {
     MODULE_INIT_OPTS,
     MODULE_INIT_QAPI,
     MODULE_INIT_QOM,
+    MODULE_INIT_TRACE,
     MODULE_INIT_MAX
 } module_init_type;
 
@@ -51,6 +52,7 @@ typedef enum {
 #define opts_init(function) module_init(function, MODULE_INIT_OPTS)
 #define qapi_init(function) module_init(function, MODULE_INIT_QAPI)
 #define type_init(function) module_init(function, MODULE_INIT_QOM)
+#define trace_init(function) module_init(function, MODULE_INIT_TRACE)
 
 #define block_module_load_one(lib) module_load_one("block-", lib)
 
diff --git a/qemu-img.c b/qemu-img.c
index ceffefe..02c07b9 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -4165,6 +4165,7 @@ int main(int argc, char **argv)
     signal(SIGPIPE, SIG_IGN);
 #endif
 
+    module_call_init(MODULE_INIT_TRACE);
     error_set_progname(argv[0]);
     qemu_init_exec_dir(argv[0]);
 
diff --git a/qemu-io.c b/qemu-io.c
index db129ea..23a229f 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -467,6 +467,7 @@ int main(int argc, char **argv)
     signal(SIGPIPE, SIG_IGN);
 #endif
 
+    module_call_init(MODULE_INIT_TRACE);
     progname = basename(argv[0]);
     qemu_init_exec_dir(argv[0]);
 
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 99297a5..361880b 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -533,6 +533,7 @@ int main(int argc, char **argv)
     sa_sigterm.sa_handler = termsig_handler;
     sigaction(SIGTERM, &sa_sigterm, NULL);
 
+    module_call_init(MODULE_INIT_TRACE);
     qcrypto_init(&error_fatal);
 
     module_call_init(MODULE_INIT_QOM);
diff --git a/scripts/tracetool/format/events_c.py b/scripts/tracetool/format/events_c.py
index 40ae395..8817555 100644
--- a/scripts/tracetool/format/events_c.py
+++ b/scripts/tracetool/format/events_c.py
@@ -60,3 +60,9 @@ def generate(events, backend):
     out('  NULL,',
         '};',
         '')
+
+    out('static void trace_register_events(void)',
+        '{',
+        '    trace_event_register_group(trace_events);',
+        '}',
+        'trace_init(trace_register_events)')
diff --git a/trace/control-internal.h b/trace/control-internal.h
index 9abbc96..a9d395a 100644
--- a/trace/control-internal.h
+++ b/trace/control-internal.h
@@ -15,7 +15,6 @@
 #include "qom/cpu.h"
 
 
-extern TraceEvent *trace_events[];
 extern int trace_events_enabled_count;
 
 
@@ -83,4 +82,7 @@ static inline bool trace_event_get_vcpu_state_dynamic(CPUState *vcpu,
     return trace_event_get_vcpu_state_dynamic_by_vcpu_id(vcpu, vcpu_id);
 }
 
+
+void trace_event_register_group(TraceEvent **events);
+
 #endif /* TRACE__CONTROL_INTERNAL_H */
diff --git a/trace/control.c b/trace/control.c
index 6b32511..a231327 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -29,6 +29,13 @@
 
 int trace_events_enabled_count;
 
+typedef struct TraceEventGroup {
+    TraceEvent **events;
+} TraceEventGroup;
+
+static TraceEventGroup *event_groups;
+static size_t nevent_groups;
+
 QemuOptsList qemu_trace_opts = {
     .name = "trace",
     .implied_opt_name = "enable",
@@ -50,6 +57,14 @@ QemuOptsList qemu_trace_opts = {
 };
 
 
+void trace_event_register_group(TraceEvent **events)
+{
+    event_groups = g_renew(TraceEventGroup, event_groups, nevent_groups + 1);
+    event_groups[nevent_groups].events = events;
+    nevent_groups++;
+}
+
+
 TraceEvent *trace_event_name(const char *name)
 {
     assert(name != NULL);
@@ -100,14 +115,20 @@ static bool pattern_glob(const char *pat, const char *ev)
 void trace_event_iter_init(TraceEventIter *iter, const char *pattern)
 {
     iter->event = 0;
+    iter->group = 0;
     iter->pattern = pattern;
 }
 
 TraceEvent *trace_event_iter_next(TraceEventIter *iter)
 {
-    while (trace_events[iter->event] != NULL) {
-        TraceEvent *ev = trace_events[iter->event];
+    while (iter->group < nevent_groups &&
+           event_groups[iter->group].events[iter->event] != NULL) {
+        TraceEvent *ev = event_groups[iter->group].events[iter->event];
         iter->event++;
+        if (event_groups[iter->group].events[iter->event] == NULL) {
+            iter->event = 0;
+            iter->group++;
+        }
         if (!iter->pattern ||
             pattern_glob(iter->pattern,
                          trace_event_get_name(ev))) {
diff --git a/trace/control.h b/trace/control.h
index cccd2a2..3f30a0c 100644
--- a/trace/control.h
+++ b/trace/control.h
@@ -15,6 +15,7 @@
 
 typedef struct TraceEventIter {
     size_t event;
+    size_t group;
     const char *pattern;
 } TraceEventIter;
 
diff --git a/vl.c b/vl.c
index f3abd99..610539a 100644
--- a/vl.c
+++ b/vl.c
@@ -3020,6 +3020,8 @@ int main(int argc, char **argv, char **envp)
     Error *err = NULL;
     bool list_data_dirs = false;
 
+    module_call_init(MODULE_INIT_TRACE);
+
     qemu_init_cpu_list();
     qemu_init_cpu_loop();
     qemu_mutex_lock_iothread();
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PULL 13/20] trace: dynamically allocate trace_dstate in CPUState
  2016-10-07 10:09 [Qemu-devel] [PULL 00/20] Tracing patches Stefan Hajnoczi
                   ` (11 preceding siblings ...)
  2016-10-07 10:09 ` [Qemu-devel] [PULL 12/20] trace: provide mechanism for registering trace events Stefan Hajnoczi
@ 2016-10-07 10:09 ` Stefan Hajnoczi
  2016-10-07 10:09 ` [Qemu-devel] [PULL 14/20] trace: dynamically allocate event IDs at runtime Stefan Hajnoczi
                   ` (7 subsequent siblings)
  20 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2016-10-07 10:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange, Stefan Hajnoczi

From: "Daniel P. Berrange" <berrange@redhat.com>

The CPUState struct has a bitmap tracking which VCPU
events are currently active. This is indexed based on
the event ID values, and sized according the maximum
TraceEventVCPUID enum value.

When we start dynamically assigning IDs at runtime,
we can't statically declare a bitmap without making
an assumption about the max event count. This problem
can be solved by dynamically allocating the per-CPU
dstate bitmap.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Lluís Vilanova <vilanova@ac.upc.edu>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 1475588159-30598-15-git-send-email-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/qom/cpu.h | 9 ++++++---
 qom/cpu.c         | 7 +++++--
 trace/control.c   | 5 +++++
 trace/control.h   | 7 +++++++
 4 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 22b54d6..6d481a1 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -27,7 +27,6 @@
 #include "qemu/bitmap.h"
 #include "qemu/queue.h"
 #include "qemu/thread.h"
-#include "trace/generated-events.h"
 
 typedef int (*WriteCoreDumpFunction)(const void *buf, size_t size,
                                      void *opaque);
@@ -345,8 +344,12 @@ struct CPUState {
     struct KVMState *kvm_state;
     struct kvm_run *kvm_run;
 
-    /* Used for events with 'vcpu' and *without* the 'disabled' properties */
-    DECLARE_BITMAP(trace_dstate, TRACE_VCPU_EVENT_COUNT);
+    /*
+     * Used for events with 'vcpu' and *without* the 'disabled' properties.
+     * Dynamically allocated based on bitmap requried to hold up to
+     * trace_get_vcpu_event_count() entries.
+     */
+    unsigned long *trace_dstate;
 
     /* TODO Move common fields from CPUArchState here. */
     int cpu_index; /* used by alpha TCG */
diff --git a/qom/cpu.c b/qom/cpu.c
index 484c493..40f2eb1 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -356,12 +356,15 @@ static void cpu_common_initfn(Object *obj)
     qemu_mutex_init(&cpu->work_mutex);
     QTAILQ_INIT(&cpu->breakpoints);
     QTAILQ_INIT(&cpu->watchpoints);
-    bitmap_zero(cpu->trace_dstate, TRACE_VCPU_EVENT_COUNT);
+
+    cpu->trace_dstate = bitmap_new(trace_get_vcpu_event_count());
 }
 
 static void cpu_common_finalize(Object *obj)
 {
-    cpu_exec_exit(CPU(obj));
+    CPUState *cpu = CPU(obj);
+    cpu_exec_exit(cpu);
+    g_free(cpu->trace_dstate);
 }
 
 static int64_t cpu_common_get_arch_id(CPUState *cpu)
diff --git a/trace/control.c b/trace/control.c
index a231327..5f10e2d 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -290,3 +290,8 @@ char *trace_opt_parse(const char *optarg)
 
     return trace_file;
 }
+
+uint32_t trace_get_vcpu_event_count(void)
+{
+    return TRACE_VCPU_EVENT_COUNT;
+}
diff --git a/trace/control.h b/trace/control.h
index 3f30a0c..69635bf 100644
--- a/trace/control.h
+++ b/trace/control.h
@@ -232,6 +232,13 @@ extern QemuOptsList qemu_trace_opts;
  */
 char *trace_opt_parse(const char *optarg);
 
+/**
+ * trace_get_vcpu_event_count:
+ *
+ * Return the number of known vcpu-specific events
+ */
+uint32_t trace_get_vcpu_event_count(void);
+
 
 #include "trace/control-internal.h"
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PULL 14/20] trace: dynamically allocate event IDs at runtime
  2016-10-07 10:09 [Qemu-devel] [PULL 00/20] Tracing patches Stefan Hajnoczi
                   ` (12 preceding siblings ...)
  2016-10-07 10:09 ` [Qemu-devel] [PULL 13/20] trace: dynamically allocate trace_dstate in CPUState Stefan Hajnoczi
@ 2016-10-07 10:09 ` Stefan Hajnoczi
  2016-10-07 10:09 ` [Qemu-devel] [PULL 15/20] trace: get rid of generated-events.h/generated-events.c Stefan Hajnoczi
                   ` (6 subsequent siblings)
  20 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2016-10-07 10:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange, Stefan Hajnoczi

From: "Daniel P. Berrange" <berrange@redhat.com>

Instead of having the code generator assign event IDs and
event VCPU IDs, assign them when the events are registered
at runtime. This will allow code to be generated from
individual trace-events without having to figure out
globally unique numbering at build time.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Lluís Vilanova <vilanova@ac.upc.edu>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 1475588159-30598-16-git-send-email-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 scripts/tracetool/format/events_c.py | 10 ++--------
 scripts/tracetool/format/events_h.py |  4 ----
 trace/control.c                      | 11 ++++++++++-
 3 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/scripts/tracetool/format/events_c.py b/scripts/tracetool/format/events_c.py
index 8817555..a976c22 100644
--- a/scripts/tracetool/format/events_c.py
+++ b/scripts/tracetool/format/events_c.py
@@ -28,25 +28,19 @@ def generate(events, backend):
     for e in events:
         out('uint16_t %s;' % e.api(e.QEMU_DSTATE))
 
-    next_id = 0
-    next_vcpu_id = 0
     for e in events:
-        id = next_id
-        next_id += 1
         if "vcpu" in e.properties:
-            vcpu_id = next_vcpu_id
-            next_vcpu_id += 1
+            vcpu_id = 0
         else:
             vcpu_id = "TRACE_VCPU_EVENT_NONE"
         out('TraceEvent %(event)s = {',
-            '    .id = %(id)s,',
+            '    .id = 0,',
             '    .vcpu_id = %(vcpu_id)s,',
             '    .name = \"%(name)s\",',
             '    .sstate = %(sstate)s,',
             '    .dstate = &%(dstate)s ',
             '};',
             event = e.api(e.QEMU_EVENT),
-            id = id,
             vcpu_id = vcpu_id,
             name = e.name,
             sstate = "TRACE_%s_ENABLED" % e.name.upper(),
diff --git a/scripts/tracetool/format/events_h.py b/scripts/tracetool/format/events_h.py
index ca6d730..1cb332b 100644
--- a/scripts/tracetool/format/events_h.py
+++ b/scripts/tracetool/format/events_h.py
@@ -32,10 +32,6 @@ def generate(events, backend):
     for e in events:
         out('extern uint16_t %s;' % e.api(e.QEMU_DSTATE))
 
-    numvcpu = len([e for e in events if "vcpu" in e.properties])
-
-    out("#define TRACE_VCPU_EVENT_COUNT %d" % numvcpu)
-
     # static state
     for e in events:
         if 'disable' in e.properties:
diff --git a/trace/control.c b/trace/control.c
index 5f10e2d..1a7bee6 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -35,6 +35,8 @@ typedef struct TraceEventGroup {
 
 static TraceEventGroup *event_groups;
 static size_t nevent_groups;
+static uint32_t next_id;
+static uint32_t next_vcpu_id;
 
 QemuOptsList qemu_trace_opts = {
     .name = "trace",
@@ -59,6 +61,13 @@ QemuOptsList qemu_trace_opts = {
 
 void trace_event_register_group(TraceEvent **events)
 {
+    size_t i;
+    for (i = 0; events[i] != NULL; i++) {
+        events[i]->id = next_id++;
+        if (events[i]->vcpu_id != TRACE_VCPU_EVENT_NONE) {
+            events[i]->vcpu_id = next_vcpu_id++;
+        }
+    }
     event_groups = g_renew(TraceEventGroup, event_groups, nevent_groups + 1);
     event_groups[nevent_groups].events = events;
     nevent_groups++;
@@ -293,5 +302,5 @@ char *trace_opt_parse(const char *optarg)
 
 uint32_t trace_get_vcpu_event_count(void)
 {
-    return TRACE_VCPU_EVENT_COUNT;
+    return next_vcpu_id;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PULL 15/20] trace: get rid of generated-events.h/generated-events.c
  2016-10-07 10:09 [Qemu-devel] [PULL 00/20] Tracing patches Stefan Hajnoczi
                   ` (13 preceding siblings ...)
  2016-10-07 10:09 ` [Qemu-devel] [PULL 14/20] trace: dynamically allocate event IDs at runtime Stefan Hajnoczi
@ 2016-10-07 10:09 ` Stefan Hajnoczi
  2016-10-07 10:09 ` [Qemu-devel] [PULL 16/20] trace: rename _read_events to read_events Stefan Hajnoczi
                   ` (5 subsequent siblings)
  20 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2016-10-07 10:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange, Stefan Hajnoczi

From: "Daniel P. Berrange" <berrange@redhat.com>

Currently the generated-events.[ch] files contain the
event dstates, constants and TraceEvent structs, while the
generated-tracers.[ch] files contain the actual trace
probe logic. With the removal of usage of the event enums
from the API there is no longer any compelling reason for
the separation between these files. The generated-events.h
content is only ever needed from the generated-tracers.[ch]
files.

The enums/constants/structs from generated-events.[ch] are
thus moved into the generated-tracers.[ch], so that there
is one less file to be generated.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Lluís Vilanova <vilanova@ac.upc.edu>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 1475588159-30598-17-git-send-email-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 Makefile                             |  3 --
 include/trace-tcg.h                  |  1 -
 include/trace.h                      |  1 -
 scripts/tracetool/format/c.py        | 50 ++++++++++++++++++++++++++---
 scripts/tracetool/format/events_c.py | 62 ------------------------------------
 scripts/tracetool/format/events_h.py | 49 ----------------------------
 scripts/tracetool/format/h.py        | 20 ++++++++++++
 trace/Makefile.objs                  | 28 +++-------------
 trace/control.h                      |  2 +-
 trace/simple.h                       |  4 ---
 10 files changed, 70 insertions(+), 150 deletions(-)
 delete mode 100644 scripts/tracetool/format/events_c.py
 delete mode 100644 scripts/tracetool/format/events_h.py

diff --git a/Makefile b/Makefile
index b103a98..699ca5f 100644
--- a/Makefile
+++ b/Makefile
@@ -56,9 +56,6 @@ GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c
 GENERATED_HEADERS += qmp-introspect.h
 GENERATED_SOURCES += qmp-introspect.c
 
-GENERATED_HEADERS += trace/generated-events.h
-GENERATED_SOURCES += trace/generated-events.c
-
 GENERATED_HEADERS += trace/generated-tracers.h
 ifeq ($(findstring dtrace,$(TRACE_BACKENDS)),dtrace)
 GENERATED_HEADERS += trace/generated-tracers-dtrace.h
diff --git a/include/trace-tcg.h b/include/trace-tcg.h
index edab4b1..da68608 100644
--- a/include/trace-tcg.h
+++ b/include/trace-tcg.h
@@ -2,6 +2,5 @@
 #define TRACE_TCG_H
 
 #include "trace/generated-tcg-tracers.h"
-#include "trace/generated-events.h"
 
 #endif /* TRACE_TCG_H */
diff --git a/include/trace.h b/include/trace.h
index 9a01e44..ac9ff3d 100644
--- a/include/trace.h
+++ b/include/trace.h
@@ -2,6 +2,5 @@
 #define TRACE_H
 
 #include "trace/generated-tracers.h"
-#include "trace/generated-events.h"
 
 #endif /* TRACE_H */
diff --git a/scripts/tracetool/format/c.py b/scripts/tracetool/format/c.py
index 699598f..7ac6d4c 100644
--- a/scripts/tracetool/format/c.py
+++ b/scripts/tracetool/format/c.py
@@ -17,12 +17,52 @@ from tracetool import out
 
 
 def generate(events, backend):
-    events = [e for e in events
-              if "disable" not in e.properties]
+    active_events = [e for e in events
+                     if "disable" not in e.properties]
 
     out('/* This file is autogenerated by tracetool, do not edit. */',
+        '',
+        '#include "qemu/osdep.h"',
+        '#include "trace.h"',
         '')
-    backend.generate_begin(events)
-    for event in events:
+
+    for e in events:
+        out('uint16_t %s;' % e.api(e.QEMU_DSTATE))
+
+    for e in events:
+        if "vcpu" in e.properties:
+            vcpu_id = 0
+        else:
+            vcpu_id = "TRACE_VCPU_EVENT_NONE"
+        out('TraceEvent %(event)s = {',
+            '    .id = 0,',
+            '    .vcpu_id = %(vcpu_id)s,',
+            '    .name = \"%(name)s\",',
+            '    .sstate = %(sstate)s,',
+            '    .dstate = &%(dstate)s ',
+            '};',
+            event = e.api(e.QEMU_EVENT),
+            vcpu_id = vcpu_id,
+            name = e.name,
+            sstate = "TRACE_%s_ENABLED" % e.name.upper(),
+            dstate = e.api(e.QEMU_DSTATE))
+
+    out('TraceEvent *trace_events[] = {')
+
+    for e in events:
+        out('    &%(event)s,', event = e.api(e.QEMU_EVENT))
+
+    out('  NULL,',
+        '};',
+        '')
+
+    out('static void trace_register_events(void)',
+        '{',
+        '    trace_event_register_group(trace_events);',
+        '}',
+        'trace_init(trace_register_events)')
+
+    backend.generate_begin(active_events)
+    for event in active_events:
         backend.generate(event)
-    backend.generate_end(events)
+    backend.generate_end(active_events)
diff --git a/scripts/tracetool/format/events_c.py b/scripts/tracetool/format/events_c.py
deleted file mode 100644
index a976c22..0000000
--- a/scripts/tracetool/format/events_c.py
+++ /dev/null
@@ -1,62 +0,0 @@
-#!/usr/bin/env python
-# -*- coding: utf-8 -*-
-
-"""
-trace/generated-events.c
-"""
-
-__author__     = "Lluís Vilanova <vilanova@ac.upc.edu>"
-__copyright__  = "Copyright 2012-2016, Lluís Vilanova <vilanova@ac.upc.edu>"
-__license__    = "GPL version 2 or (at your option) any later version"
-
-__maintainer__ = "Stefan Hajnoczi"
-__email__      = "stefanha@linux.vnet.ibm.com"
-
-
-from tracetool import out
-
-
-def generate(events, backend):
-    out('/* This file is autogenerated by tracetool, do not edit. */',
-        '',
-        '#include "qemu/osdep.h"',
-        '#include "trace.h"',
-        '#include "trace/generated-events.h"',
-        '#include "trace/control.h"',
-        '')
-
-    for e in events:
-        out('uint16_t %s;' % e.api(e.QEMU_DSTATE))
-
-    for e in events:
-        if "vcpu" in e.properties:
-            vcpu_id = 0
-        else:
-            vcpu_id = "TRACE_VCPU_EVENT_NONE"
-        out('TraceEvent %(event)s = {',
-            '    .id = 0,',
-            '    .vcpu_id = %(vcpu_id)s,',
-            '    .name = \"%(name)s\",',
-            '    .sstate = %(sstate)s,',
-            '    .dstate = &%(dstate)s ',
-            '};',
-            event = e.api(e.QEMU_EVENT),
-            vcpu_id = vcpu_id,
-            name = e.name,
-            sstate = "TRACE_%s_ENABLED" % e.name.upper(),
-            dstate = e.api(e.QEMU_DSTATE))
-
-    out('TraceEvent *trace_events[] = {')
-
-    for e in events:
-        out('    &%(event)s,', event = e.api(e.QEMU_EVENT))
-
-    out('  NULL,',
-        '};',
-        '')
-
-    out('static void trace_register_events(void)',
-        '{',
-        '    trace_event_register_group(trace_events);',
-        '}',
-        'trace_init(trace_register_events)')
diff --git a/scripts/tracetool/format/events_h.py b/scripts/tracetool/format/events_h.py
deleted file mode 100644
index 1cb332b..0000000
--- a/scripts/tracetool/format/events_h.py
+++ /dev/null
@@ -1,49 +0,0 @@
-#!/usr/bin/env python
-# -*- coding: utf-8 -*-
-
-"""
-trace/generated-events.h
-"""
-
-__author__     = "Lluís Vilanova <vilanova@ac.upc.edu>"
-__copyright__  = "Copyright 2012-2016, Lluís Vilanova <vilanova@ac.upc.edu>"
-__license__    = "GPL version 2 or (at your option) any later version"
-
-__maintainer__ = "Stefan Hajnoczi"
-__email__      = "stefanha@linux.vnet.ibm.com"
-
-
-from tracetool import out
-
-
-def generate(events, backend):
-    out('/* This file is autogenerated by tracetool, do not edit. */',
-        '',
-        '#ifndef TRACE__GENERATED_EVENTS_H',
-        '#define TRACE__GENERATED_EVENTS_H',
-        '',
-        '#include "trace/event-internal.h"',
-        )
-
-    for e in events:
-        out('extern TraceEvent %(event)s;',
-            event = e.api(e.QEMU_EVENT))
-
-    for e in events:
-        out('extern uint16_t %s;' % e.api(e.QEMU_DSTATE))
-
-    # static state
-    for e in events:
-        if 'disable' in e.properties:
-            enabled = 0
-        else:
-            enabled = 1
-        if "tcg-exec" in e.properties:
-            # a single define for the two "sub-events"
-            out('#define TRACE_%(name)s_ENABLED %(enabled)d',
-                name=e.original.name.upper(),
-                enabled=enabled)
-        out('#define TRACE_%s_ENABLED %d' % (e.name.upper(), enabled))
-
-    out('',
-        '#endif  /* TRACE__GENERATED_EVENTS_H */')
diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
index 64a6680..f4748c3 100644
--- a/scripts/tracetool/format/h.py
+++ b/scripts/tracetool/format/h.py
@@ -26,6 +26,26 @@ def generate(events, backend):
         '#include "trace/control.h"',
         '')
 
+    for e in events:
+        out('extern TraceEvent %(event)s;',
+            event = e.api(e.QEMU_EVENT))
+
+    for e in events:
+        out('extern uint16_t %s;' % e.api(e.QEMU_DSTATE))
+
+    # static state
+    for e in events:
+        if 'disable' in e.properties:
+            enabled = 0
+        else:
+            enabled = 1
+        if "tcg-exec" in e.properties:
+            # a single define for the two "sub-events"
+            out('#define TRACE_%(name)s_ENABLED %(enabled)d',
+                name=e.original.name.upper(),
+                enabled=enabled)
+        out('#define TRACE_%s_ENABLED %d' % (e.name.upper(), enabled))
+
     backend.generate_begin(events)
 
     for e in events:
diff --git a/trace/Makefile.objs b/trace/Makefile.objs
index 24d3b37..abac333 100644
--- a/trace/Makefile.objs
+++ b/trace/Makefile.objs
@@ -32,32 +32,11 @@ $(obj)/generated-ust.c-timestamp: $(BUILD_DIR)/trace-events-all $(tracetool-y)
 		--backends=$(TRACE_BACKENDS) \
 		< $< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
 
-$(obj)/generated-events.h: $(obj)/generated-ust-provider.h
-$(obj)/generated-events.c: $(obj)/generated-ust.c
+$(obj)/generated-tracers.h: $(obj)/generated-ust-provider.h
+$(obj)/generated-tracers.c: $(obj)/generated-ust.c
 
 endif
 
-######################################################################
-# Auto-generated event descriptions
-
-$(obj)/generated-events.h: $(obj)/generated-events.h-timestamp
-	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-events.h-timestamp: $(BUILD_DIR)/trace-events-all $(tracetool-y)
-	$(call quiet-command,$(TRACETOOL) \
-		--format=events-h \
-		--backends=$(TRACE_BACKENDS) \
-		< $< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
-
-$(obj)/generated-events.c: $(obj)/generated-events.c-timestamp $(BUILD_DIR)/config-host.mak
-	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-events.c-timestamp: $(BUILD_DIR)/trace-events-all $(tracetool-y)
-	$(call quiet-command,$(TRACETOOL) \
-		--format=events-c \
-		--backends=$(TRACE_BACKENDS) \
-		< $< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
-
-util-obj-y += generated-events.o
-
 
 ######################################################################
 # Auto-generated tracing routines
@@ -154,7 +133,8 @@ $(obj)/generated-tcg-tracers.h-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_
 ######################################################################
 # Backend code
 
-util-obj-$(CONFIG_TRACE_SIMPLE) += simple.o generated-tracers.o
+util-obj-y += generated-tracers.o
+util-obj-$(CONFIG_TRACE_SIMPLE) += simple.o
 util-obj-$(CONFIG_TRACE_FTRACE) += ftrace.o
 util-obj-$(CONFIG_TRACE_UST) += generated-ust.o
 util-obj-y += control.o
diff --git a/trace/control.h b/trace/control.h
index 69635bf..ccaeac8 100644
--- a/trace/control.h
+++ b/trace/control.h
@@ -11,7 +11,7 @@
 #define TRACE__CONTROL_H
 
 #include "qemu-common.h"
-#include "trace/generated-events.h"
+#include "event-internal.h"
 
 typedef struct TraceEventIter {
     size_t event;
diff --git a/trace/simple.h b/trace/simple.h
index 17ce472..9931808 100644
--- a/trace/simple.h
+++ b/trace/simple.h
@@ -11,10 +11,6 @@
 #ifndef TRACE_SIMPLE_H
 #define TRACE_SIMPLE_H
 
-
-#include "trace/generated-events.h"
-
-
 void st_print_trace_file_status(FILE *stream, fprintf_function stream_printf);
 void st_set_trace_file_enabled(bool enable);
 void st_set_trace_file(const char *file);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PULL 16/20] trace: rename _read_events to read_events
  2016-10-07 10:09 [Qemu-devel] [PULL 00/20] Tracing patches Stefan Hajnoczi
                   ` (14 preceding siblings ...)
  2016-10-07 10:09 ` [Qemu-devel] [PULL 15/20] trace: get rid of generated-events.h/generated-events.c Stefan Hajnoczi
@ 2016-10-07 10:09 ` Stefan Hajnoczi
  2016-10-07 10:09 ` [Qemu-devel] [PULL 17/20] trace: push reading of events up a level to tracetool main Stefan Hajnoczi
                   ` (4 subsequent siblings)
  20 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2016-10-07 10:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange, Stefan Hajnoczi

From: "Daniel P. Berrange" <berrange@redhat.com>

The _read_events method is used by callers outside of
its module, so should be a public method, not private.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Lluís Vilanova <vilanova@ac.upc.edu>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 1475588159-30598-18-git-send-email-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 scripts/simpletrace.py        |  6 +++---
 scripts/tracetool/__init__.py | 14 ++++++++++++--
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/scripts/simpletrace.py b/scripts/simpletrace.py
index f40467a..4ca903d 100755
--- a/scripts/simpletrace.py
+++ b/scripts/simpletrace.py
@@ -12,7 +12,7 @@
 import struct
 import re
 import inspect
-from tracetool import _read_events, Event
+from tracetool import read_events, Event
 from tracetool.backend.simple import is_string
 
 header_event_id = 0xffffffffffffffff
@@ -129,7 +129,7 @@ class Analyzer(object):
 def process(events, log, analyzer, read_header=True):
     """Invoke an analyzer on each event in a log."""
     if isinstance(events, str):
-        events = _read_events(open(events, 'r'))
+        events = read_events(open(events, 'r'))
     if isinstance(log, str):
         log = open(log, 'rb')
 
@@ -188,7 +188,7 @@ def run(analyzer):
                          '<trace-file>\n' % sys.argv[0])
         sys.exit(1)
 
-    events = _read_events(open(sys.argv[1], 'r'))
+    events = read_events(open(sys.argv[1], 'r'))
     process(events, sys.argv[2], analyzer, read_header=read_header)
 
 if __name__ == '__main__':
diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index dc93416..bf01e93 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -282,7 +282,17 @@ class Event(object):
                      self)
 
 
-def _read_events(fobj):
+def read_events(fobj):
+    """Generate the output for the given (format, backends) pair.
+
+    Parameters
+    ----------
+    fobj : file
+        Event description file.
+
+    Returns a list of Event objects
+    """
+
     events = []
     for line in fobj:
         if not line.strip():
@@ -391,6 +401,6 @@ def generate(fevents, format, backends,
     tracetool.backend.dtrace.BINARY = binary
     tracetool.backend.dtrace.PROBEPREFIX = probe_prefix
 
-    events = _read_events(fevents)
+    events = read_events(fevents)
 
     tracetool.format.generate(events, format, backend)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PULL 17/20] trace: push reading of events up a level to tracetool main
  2016-10-07 10:09 [Qemu-devel] [PULL 00/20] Tracing patches Stefan Hajnoczi
                   ` (15 preceding siblings ...)
  2016-10-07 10:09 ` [Qemu-devel] [PULL 16/20] trace: rename _read_events to read_events Stefan Hajnoczi
@ 2016-10-07 10:09 ` Stefan Hajnoczi
  2016-10-07 10:09 ` [Qemu-devel] [PULL 18/20] trace: pass trace-events to tracetool as a positional param Stefan Hajnoczi
                   ` (3 subsequent siblings)
  20 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2016-10-07 10:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange, Stefan Hajnoczi

From: "Daniel P. Berrange" <berrange@redhat.com>

Move the reading of events out of the 'tracetool.generate'
method and into tracetool.main, so that the latter is not
tied to generating from a single source of events.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Lluís Vilanova <vilanova@ac.upc.edu>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 1475588159-30598-19-git-send-email-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 scripts/tracetool.py          | 4 +++-
 scripts/tracetool/__init__.py | 8 +++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index 7b82959..6accbbf 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -129,8 +129,10 @@ def main(args):
         if probe_prefix is None:
             probe_prefix = ".".join(["qemu", target_type, target_name])
 
+    events = tracetool.read_events(sys.stdin)
+
     try:
-        tracetool.generate(sys.stdin, arg_format, arg_backends,
+        tracetool.generate(events, arg_format, arg_backends,
                            binary=binary, probe_prefix=probe_prefix)
     except tracetool.TracetoolError as e:
         error_opt(str(e))
diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index bf01e93..ac2cb8f 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -364,14 +364,14 @@ def try_import(mod_name, attr_name=None, attr_default=None):
         return False, None
 
 
-def generate(fevents, format, backends,
+def generate(events, format, backends,
              binary=None, probe_prefix=None):
     """Generate the output for the given (format, backends) pair.
 
     Parameters
     ----------
-    fevents : file
-        Event description file.
+    events : list
+        list of Event objects to generate for
     format : str
         Output format name.
     backends : list
@@ -401,6 +401,4 @@ def generate(fevents, format, backends,
     tracetool.backend.dtrace.BINARY = binary
     tracetool.backend.dtrace.PROBEPREFIX = probe_prefix
 
-    events = read_events(fevents)
-
     tracetool.format.generate(events, format, backend)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PULL 18/20] trace: pass trace-events to tracetool as a positional param
  2016-10-07 10:09 [Qemu-devel] [PULL 00/20] Tracing patches Stefan Hajnoczi
                   ` (16 preceding siblings ...)
  2016-10-07 10:09 ` [Qemu-devel] [PULL 17/20] trace: push reading of events up a level to tracetool main Stefan Hajnoczi
@ 2016-10-07 10:09 ` Stefan Hajnoczi
  2016-10-07 10:09 ` [Qemu-devel] [PULL 19/20] trace: introduce a formal group name for trace events Stefan Hajnoczi
                   ` (2 subsequent siblings)
  20 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2016-10-07 10:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange, Stefan Hajnoczi

From: "Daniel P. Berrange" <berrange@redhat.com>

Instead of reading the contents of 'trace-events' from stdin,
accept the filename as a positional parameter. This also
allows for reading from multiple files, though this facility
is not used at this time.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Lluís Vilanova <vilanova@ac.upc.edu>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 1475588159-30598-20-git-send-email-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 Makefile.target      |  6 +++---
 scripts/tracetool.py |  5 ++++-
 trace/Makefile.objs  | 18 +++++++++---------
 3 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index 9968871..2c46091 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -55,7 +55,7 @@ $(QEMU_PROG).stp-installed: $(BUILD_DIR)/trace-events-all
 		--binary=$(bindir)/$(QEMU_PROG) \
 		--target-name=$(TARGET_NAME) \
 		--target-type=$(TARGET_TYPE) \
-		< $< > $@,"GEN","$(TARGET_DIR)$(QEMU_PROG).stp-installed")
+		$< > $@,"GEN","$(TARGET_DIR)$(QEMU_PROG).stp-installed")
 
 $(QEMU_PROG).stp: $(BUILD_DIR)/trace-events-all
 	$(call quiet-command,$(TRACETOOL) \
@@ -64,14 +64,14 @@ $(QEMU_PROG).stp: $(BUILD_DIR)/trace-events-all
 		--binary=$(realpath .)/$(QEMU_PROG) \
 		--target-name=$(TARGET_NAME) \
 		--target-type=$(TARGET_TYPE) \
-		< $< > $@,"GEN","$(TARGET_DIR)$(QEMU_PROG).stp")
+		$< > $@,"GEN","$(TARGET_DIR)$(QEMU_PROG).stp")
 
 $(QEMU_PROG)-simpletrace.stp: $(BUILD_DIR)/trace-events-all
 	$(call quiet-command,$(TRACETOOL) \
 		--format=simpletrace-stap \
 		--backends=$(TRACE_BACKENDS) \
 		--probe-prefix=qemu.$(TARGET_TYPE).$(TARGET_NAME) \
-		< $< > $@,"GEN","$(TARGET_DIR)$(QEMU_PROG)-simpletrace.stp")
+		$< > $@,"GEN","$(TARGET_DIR)$(QEMU_PROG)-simpletrace.stp")
 
 else
 stap:
diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index 6accbbf..f66e767 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -129,7 +129,10 @@ def main(args):
         if probe_prefix is None:
             probe_prefix = ".".join(["qemu", target_type, target_name])
 
-    events = tracetool.read_events(sys.stdin)
+    if len(args) != 1:
+        error_opt("missing trace-events filepath")
+    with open(args[0], "r") as fh:
+        events = tracetool.read_events(fh)
 
     try:
         tracetool.generate(events, arg_format, arg_backends,
diff --git a/trace/Makefile.objs b/trace/Makefile.objs
index abac333..1e1ce74 100644
--- a/trace/Makefile.objs
+++ b/trace/Makefile.objs
@@ -22,7 +22,7 @@ $(obj)/generated-ust-provider.h-timestamp: $(BUILD_DIR)/trace-events-all $(trace
 	$(call quiet-command,$(TRACETOOL) \
 		--format=ust-events-h \
 		--backends=$(TRACE_BACKENDS) \
-		< $< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
+		$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
 
 $(obj)/generated-ust.c: $(obj)/generated-ust.c-timestamp $(BUILD_DIR)/config-host.mak
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
@@ -30,7 +30,7 @@ $(obj)/generated-ust.c-timestamp: $(BUILD_DIR)/trace-events-all $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
 		--format=ust-events-c \
 		--backends=$(TRACE_BACKENDS) \
-		< $< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
+		$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
 
 $(obj)/generated-tracers.h: $(obj)/generated-ust-provider.h
 $(obj)/generated-tracers.c: $(obj)/generated-ust.c
@@ -50,7 +50,7 @@ $(obj)/generated-tracers.h-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)
 	$(call quiet-command,$(TRACETOOL) \
 		--format=h \
 		--backends=$(TRACE_BACKENDS) \
-		< $< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
+		$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
 
 ##############################
 # non-DTrace
@@ -61,7 +61,7 @@ $(obj)/generated-tracers.c-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)
 	$(call quiet-command,$(TRACETOOL) \
 		--format=c \
 		--backends=$(TRACE_BACKENDS) \
-		< $< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
+		$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
 
 $(obj)/generated-tracers.o: $(obj)/generated-tracers.c $(obj)/generated-tracers.h
 
@@ -79,7 +79,7 @@ $(obj)/generated-tracers-dtrace.dtrace-timestamp: $(BUILD_DIR)/trace-events-all
 	$(call quiet-command,$(TRACETOOL) \
 		--format=d \
 		--backends=$(TRACE_BACKENDS) \
-		< $< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
+		$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
 
 $(obj)/generated-tracers-dtrace.h: $(obj)/generated-tracers-dtrace.dtrace
 	$(call quiet-command,dtrace -o $@ -h -s $<,"GEN","$@")
@@ -98,7 +98,7 @@ $(obj)/generated-helpers-wrappers.h-timestamp: $(BUILD_DIR)/trace-events-all $(B
 	$(call quiet-command,$(TRACETOOL) \
 		--format=tcg-helper-wrapper-h \
 		--backend=$(TRACE_BACKENDS) \
-		< $< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
+		$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
 
 $(obj)/generated-helpers.h: $(obj)/generated-helpers.h-timestamp
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
@@ -106,7 +106,7 @@ $(obj)/generated-helpers.h-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)
 	$(call quiet-command,$(TRACETOOL) \
 		--format=tcg-helper-h \
 		--backend=$(TRACE_BACKENDS) \
-		< $< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
+		$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
 
 $(obj)/generated-helpers.c: $(obj)/generated-helpers.c-timestamp
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
@@ -114,7 +114,7 @@ $(obj)/generated-helpers.c-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)
 	$(call quiet-command,$(TRACETOOL) \
 		--format=tcg-helper-c \
 		--backend=$(TRACE_BACKENDS) \
-		< $< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
+		$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
 
 $(obj)/generated-helpers.o: $(obj)/generated-helpers.c
 
@@ -127,7 +127,7 @@ $(obj)/generated-tcg-tracers.h-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_
 	$(call quiet-command,$(TRACETOOL) \
 		--format=tcg-h \
 		--backend=$(TRACE_BACKENDS) \
-		< $< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
+		$< > $@,"GEN","$(patsubst %-timestamp,%,$@)")
 
 
 ######################################################################
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PULL 19/20] trace: introduce a formal group name for trace events
  2016-10-07 10:09 [Qemu-devel] [PULL 00/20] Tracing patches Stefan Hajnoczi
                   ` (17 preceding siblings ...)
  2016-10-07 10:09 ` [Qemu-devel] [PULL 18/20] trace: pass trace-events to tracetool as a positional param Stefan Hajnoczi
@ 2016-10-07 10:09 ` Stefan Hajnoczi
  2016-10-07 10:09 ` [Qemu-devel] [PULL 20/20] trace: Add missing execution mode of guest events Stefan Hajnoczi
  2016-10-07 13:22 ` [Qemu-devel] [PULL 00/20] Tracing patches Peter Maydell
  20 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2016-10-07 10:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange, Stefan Hajnoczi

From: "Daniel P. Berrange" <berrange@redhat.com>

The declarations in the generated-tracers.h file are
assuming there's only ever going to be one instance
of this header, as they are not namespaced. When we
have one header per event group, if a single source
file needs to include multiple sets of trace events,
the symbols will all clash.

This change thus introduces a '--group NAME' arg to the
'tracetool' program. This will cause all the symbols in
the generated header files to be given a unique namespace.

If no group is given, the group name 'common' is used,
which is suitable for the current usage where there is
only one global trace-events file used for code generation.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Lluís Vilanova <vilanova@ac.upc.edu>
Message-id: 1475588159-30598-21-git-send-email-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 scripts/tracetool.py                             | 15 ++++++++++++++-
 scripts/tracetool/__init__.py                    |  6 ++++--
 scripts/tracetool/backend/__init__.py            | 12 ++++++------
 scripts/tracetool/backend/dtrace.py              |  4 ++--
 scripts/tracetool/backend/ftrace.py              |  4 ++--
 scripts/tracetool/backend/log.py                 |  4 ++--
 scripts/tracetool/backend/simple.py              |  8 ++++----
 scripts/tracetool/backend/syslog.py              |  4 ++--
 scripts/tracetool/backend/ust.py                 |  4 ++--
 scripts/tracetool/format/__init__.py             |  4 ++--
 scripts/tracetool/format/c.py                    | 18 ++++++++++--------
 scripts/tracetool/format/d.py                    |  2 +-
 scripts/tracetool/format/h.py                    | 14 +++++++-------
 scripts/tracetool/format/simpletrace_stap.py     |  2 +-
 scripts/tracetool/format/stap.py                 |  2 +-
 scripts/tracetool/format/tcg_h.py                |  8 ++++----
 scripts/tracetool/format/tcg_helper_c.py         |  2 +-
 scripts/tracetool/format/tcg_helper_h.py         |  2 +-
 scripts/tracetool/format/tcg_helper_wrapper_h.py |  2 +-
 scripts/tracetool/format/ust_events_c.py         |  2 +-
 scripts/tracetool/format/ust_events_h.py         |  9 +++++----
 21 files changed, 73 insertions(+), 55 deletions(-)

diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index f66e767..629b259 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -15,6 +15,8 @@ __email__      = "stefanha@linux.vnet.ibm.com"
 
 import sys
 import getopt
+import os.path
+import re
 
 from tracetool import error_write, out
 import tracetool.backend
@@ -60,6 +62,15 @@ Options:
     else:
         sys.exit(1)
 
+def make_group_name(filename):
+    dirname = os.path.realpath(os.path.dirname(filename))
+    basedir = os.path.join(os.path.dirname(__file__), os.pardir)
+    basedir = os.path.realpath(os.path.abspath(basedir))
+    dirname = dirname[len(basedir) + 1:]
+
+    if dirname == "":
+        return "common"
+    return re.sub(r"/|-", "_", dirname)
 
 def main(args):
     global _SCRIPT
@@ -134,8 +145,10 @@ def main(args):
     with open(args[0], "r") as fh:
         events = tracetool.read_events(fh)
 
+    group = make_group_name(args[0])
+
     try:
-        tracetool.generate(events, arg_format, arg_backends,
+        tracetool.generate(events, group, arg_format, arg_backends,
                            binary=binary, probe_prefix=probe_prefix)
     except tracetool.TracetoolError as e:
         error_opt(str(e))
diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index ac2cb8f..365446f 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -364,7 +364,7 @@ def try_import(mod_name, attr_name=None, attr_default=None):
         return False, None
 
 
-def generate(events, format, backends,
+def generate(events, group, format, backends,
              binary=None, probe_prefix=None):
     """Generate the output for the given (format, backends) pair.
 
@@ -372,6 +372,8 @@ def generate(events, format, backends,
     ----------
     events : list
         list of Event objects to generate for
+    group: str
+        Name of the tracing group
     format : str
         Output format name.
     backends : list
@@ -401,4 +403,4 @@ def generate(events, format, backends,
     tracetool.backend.dtrace.BINARY = binary
     tracetool.backend.dtrace.PROBEPREFIX = probe_prefix
 
-    tracetool.format.generate(events, format, backend)
+    tracetool.format.generate(events, format, backend, group)
diff --git a/scripts/tracetool/backend/__init__.py b/scripts/tracetool/backend/__init__.py
index d4b6dab..f735a25 100644
--- a/scripts/tracetool/backend/__init__.py
+++ b/scripts/tracetool/backend/__init__.py
@@ -113,11 +113,11 @@ class Wrapper:
             if func is not None:
                 func(*args, **kwargs)
 
-    def generate_begin(self, events):
-        self._run_function("generate_%s_begin", events)
+    def generate_begin(self, events, group):
+        self._run_function("generate_%s_begin", events, group)
 
-    def generate(self, event):
-        self._run_function("generate_%s", event)
+    def generate(self, event, group):
+        self._run_function("generate_%s", event, group)
 
-    def generate_end(self, events):
-        self._run_function("generate_%s_end", events)
+    def generate_end(self, events, group):
+        self._run_function("generate_%s_end", events, group)
diff --git a/scripts/tracetool/backend/dtrace.py b/scripts/tracetool/backend/dtrace.py
index ab9ecfa..79505c6 100644
--- a/scripts/tracetool/backend/dtrace.py
+++ b/scripts/tracetool/backend/dtrace.py
@@ -35,12 +35,12 @@ def binary():
     return BINARY
 
 
-def generate_h_begin(events):
+def generate_h_begin(events, group):
     out('#include "trace/generated-tracers-dtrace.h"',
         '')
 
 
-def generate_h(event):
+def generate_h(event, group):
     out('        QEMU_%(uppername)s(%(argnames)s);',
         uppername=event.name.upper(),
         argnames=", ".join(event.args.names()))
diff --git a/scripts/tracetool/backend/ftrace.py b/scripts/tracetool/backend/ftrace.py
index 4b83b44..db9fe7a 100644
--- a/scripts/tracetool/backend/ftrace.py
+++ b/scripts/tracetool/backend/ftrace.py
@@ -19,12 +19,12 @@ from tracetool import out
 PUBLIC = True
 
 
-def generate_h_begin(events):
+def generate_h_begin(events, group):
     out('#include "trace/ftrace.h"',
         '')
 
 
-def generate_h(event):
+def generate_h(event, group):
     argnames = ", ".join(event.args.names())
     if len(event.args) > 0:
         argnames = ", " + argnames
diff --git a/scripts/tracetool/backend/log.py b/scripts/tracetool/backend/log.py
index 426bb6e..4f4a4d3 100644
--- a/scripts/tracetool/backend/log.py
+++ b/scripts/tracetool/backend/log.py
@@ -19,12 +19,12 @@ from tracetool import out
 PUBLIC = True
 
 
-def generate_h_begin(events):
+def generate_h_begin(events, group):
     out('#include "qemu/log.h"',
         '')
 
 
-def generate_h(event):
+def generate_h(event, group):
     argnames = ", ".join(event.args.names())
     if len(event.args) > 0:
         argnames = ", " + argnames
diff --git a/scripts/tracetool/backend/simple.py b/scripts/tracetool/backend/simple.py
index 1114e35..9885e83 100644
--- a/scripts/tracetool/backend/simple.py
+++ b/scripts/tracetool/backend/simple.py
@@ -27,7 +27,7 @@ def is_string(arg):
         return False
 
 
-def generate_h_begin(events):
+def generate_h_begin(events, group):
     for event in events:
         out('void _simple_%(api)s(%(args)s);',
             api=event.api(),
@@ -35,13 +35,13 @@ def generate_h_begin(events):
     out('')
 
 
-def generate_h(event):
+def generate_h(event, group):
     out('        _simple_%(api)s(%(args)s);',
         api=event.api(),
         args=", ".join(event.args.names()))
 
 
-def generate_c_begin(events):
+def generate_c_begin(events, group):
     out('#include "qemu/osdep.h"',
         '#include "trace.h"',
         '#include "trace/control.h"',
@@ -49,7 +49,7 @@ def generate_c_begin(events):
         '')
 
 
-def generate_c(event):
+def generate_c(event, group):
     out('void _simple_%(api)s(%(args)s)',
         '{',
         '    TraceBufferRecord rec;',
diff --git a/scripts/tracetool/backend/syslog.py b/scripts/tracetool/backend/syslog.py
index 5f693bd..b8ff279 100644
--- a/scripts/tracetool/backend/syslog.py
+++ b/scripts/tracetool/backend/syslog.py
@@ -19,12 +19,12 @@ from tracetool import out
 PUBLIC = True
 
 
-def generate_h_begin(events):
+def generate_h_begin(events, group):
     out('#include <syslog.h>',
         '')
 
 
-def generate_h(event):
+def generate_h(event, group):
     argnames = ", ".join(event.args.names())
     if len(event.args) > 0:
         argnames = ", " + argnames
diff --git a/scripts/tracetool/backend/ust.py b/scripts/tracetool/backend/ust.py
index ed4c227..4594db6 100644
--- a/scripts/tracetool/backend/ust.py
+++ b/scripts/tracetool/backend/ust.py
@@ -19,13 +19,13 @@ from tracetool import out
 PUBLIC = True
 
 
-def generate_h_begin(events):
+def generate_h_begin(events, group):
     out('#include <lttng/tracepoint.h>',
         '#include "trace/generated-ust-provider.h"',
         '')
 
 
-def generate_h(event):
+def generate_h(event, group):
     argnames = ", ".join(event.args.names())
     if len(event.args) > 0:
         argnames = ", " + argnames
diff --git a/scripts/tracetool/format/__init__.py b/scripts/tracetool/format/__init__.py
index 812570f..cf6e0e2 100644
--- a/scripts/tracetool/format/__init__.py
+++ b/scripts/tracetool/format/__init__.py
@@ -74,7 +74,7 @@ def exists(name):
     return tracetool.try_import("tracetool.format." + name)[1]
 
 
-def generate(events, format, backend):
+def generate(events, format, backend, group):
     if not exists(format):
         raise ValueError("unknown format: %s" % format)
     format = format.replace("-", "_")
@@ -82,4 +82,4 @@ def generate(events, format, backend):
                                 "generate")[1]
     if func is None:
         raise AttributeError("format has no 'generate': %s" % format)
-    func(events, backend)
+    func(events, backend, group)
diff --git a/scripts/tracetool/format/c.py b/scripts/tracetool/format/c.py
index 7ac6d4c..47115ed 100644
--- a/scripts/tracetool/format/c.py
+++ b/scripts/tracetool/format/c.py
@@ -16,7 +16,7 @@ __email__      = "stefanha@linux.vnet.ibm.com"
 from tracetool import out
 
 
-def generate(events, backend):
+def generate(events, backend, group):
     active_events = [e for e in events
                      if "disable" not in e.properties]
 
@@ -47,7 +47,8 @@ def generate(events, backend):
             sstate = "TRACE_%s_ENABLED" % e.name.upper(),
             dstate = e.api(e.QEMU_DSTATE))
 
-    out('TraceEvent *trace_events[] = {')
+    out('TraceEvent *%(group)s_trace_events[] = {',
+        group = group.lower())
 
     for e in events:
         out('    &%(event)s,', event = e.api(e.QEMU_EVENT))
@@ -56,13 +57,14 @@ def generate(events, backend):
         '};',
         '')
 
-    out('static void trace_register_events(void)',
+    out('static void trace_%(group)s_register_events(void)',
         '{',
-        '    trace_event_register_group(trace_events);',
+        '    trace_event_register_group(%(group)s_trace_events);',
         '}',
-        'trace_init(trace_register_events)')
+        'trace_init(trace_%(group)s_register_events)',
+        group = group.lower())
 
-    backend.generate_begin(active_events)
+    backend.generate_begin(active_events, group)
     for event in active_events:
-        backend.generate(event)
-    backend.generate_end(active_events)
+        backend.generate(event, group)
+    backend.generate_end(active_events, group)
diff --git a/scripts/tracetool/format/d.py b/scripts/tracetool/format/d.py
index c77d5b7..78397c2 100644
--- a/scripts/tracetool/format/d.py
+++ b/scripts/tracetool/format/d.py
@@ -29,7 +29,7 @@ RESERVED_WORDS = (
 )
 
 
-def generate(events, backend):
+def generate(events, backend, group):
     events = [e for e in events
               if "disable" not in e.properties]
 
diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
index f4748c3..3682f4e 100644
--- a/scripts/tracetool/format/h.py
+++ b/scripts/tracetool/format/h.py
@@ -16,11 +16,11 @@ __email__      = "stefanha@linux.vnet.ibm.com"
 from tracetool import out
 
 
-def generate(events, backend):
+def generate(events, backend, group):
     out('/* This file is autogenerated by tracetool, do not edit. */',
         '',
-        '#ifndef TRACE__GENERATED_TRACERS_H',
-        '#define TRACE__GENERATED_TRACERS_H',
+        '#ifndef TRACE_%s_GENERATED_TRACERS_H' % group.upper(),
+        '#define TRACE_%s_GENERATED_TRACERS_H' % group.upper(),
         '',
         '#include "qemu-common.h"',
         '#include "trace/control.h"',
@@ -46,7 +46,7 @@ def generate(events, backend):
                 enabled=enabled)
         out('#define TRACE_%s_ENABLED %d' % (e.name.upper(), enabled))
 
-    backend.generate_begin(events)
+    backend.generate_begin(events, group)
 
     for e in events:
         if "vcpu" in e.properties:
@@ -68,11 +68,11 @@ def generate(events, backend):
             cond=cond)
 
         if "disable" not in e.properties:
-            backend.generate(e)
+            backend.generate(e, group)
 
         out('    }',
             '}')
 
-    backend.generate_end(events)
+    backend.generate_end(events, group)
 
-    out('#endif /* TRACE__GENERATED_TRACERS_H */')
+    out('#endif /* TRACE_%s_GENERATED_TRACERS_H */' % group.upper())
diff --git a/scripts/tracetool/format/simpletrace_stap.py b/scripts/tracetool/format/simpletrace_stap.py
index ac3580f..c35e662 100644
--- a/scripts/tracetool/format/simpletrace_stap.py
+++ b/scripts/tracetool/format/simpletrace_stap.py
@@ -19,7 +19,7 @@ from tracetool.backend.simple import is_string
 from tracetool.format.stap import stap_escape
 
 
-def generate(events, backend):
+def generate(events, backend, group):
     out('/* This file is autogenerated by tracetool, do not edit. */',
         '',
         'global event_name_to_id_map',
diff --git a/scripts/tracetool/format/stap.py b/scripts/tracetool/format/stap.py
index 9e780f1..e8ef3e7 100644
--- a/scripts/tracetool/format/stap.py
+++ b/scripts/tracetool/format/stap.py
@@ -34,7 +34,7 @@ def stap_escape(identifier):
     return identifier
 
 
-def generate(events, backend):
+def generate(events, backend, group):
     events = [e for e in events
               if "disable" not in e.properties]
 
diff --git a/scripts/tracetool/format/tcg_h.py b/scripts/tracetool/format/tcg_h.py
index e2331f2..5f213f6 100644
--- a/scripts/tracetool/format/tcg_h.py
+++ b/scripts/tracetool/format/tcg_h.py
@@ -27,12 +27,12 @@ def vcpu_transform_args(args):
     ])
 
 
-def generate(events, backend):
+def generate(events, backend, group):
     out('/* This file is autogenerated by tracetool, do not edit. */',
         '/* You must include this file after the inclusion of helper.h */',
         '',
-        '#ifndef TRACE__GENERATED_TCG_TRACERS_H',
-        '#define TRACE__GENERATED_TCG_TRACERS_H',
+        '#ifndef TRACE_%s_GENERATED_TCG_TRACERS_H' % group.upper(),
+        '#define TRACE_%s_GENERATED_TCG_TRACERS_H' % group.upper(),
         '',
         '#include "trace.h"',
         '#include "exec/helper-proto.h"',
@@ -63,4 +63,4 @@ def generate(events, backend):
         out('}')
 
     out('',
-        '#endif /* TRACE__GENERATED_TCG_TRACERS_H */')
+        '#endif /* TRACE_%s_GENERATED_TCG_TRACERS_H */' % group.upper())
diff --git a/scripts/tracetool/format/tcg_helper_c.py b/scripts/tracetool/format/tcg_helper_c.py
index e3485b7..cc26e03 100644
--- a/scripts/tracetool/format/tcg_helper_c.py
+++ b/scripts/tracetool/format/tcg_helper_c.py
@@ -40,7 +40,7 @@ def vcpu_transform_args(args, mode):
             assert False
 
 
-def generate(events, backend):
+def generate(events, backend, group):
     events = [e for e in events
               if "disable" not in e.properties]
 
diff --git a/scripts/tracetool/format/tcg_helper_h.py b/scripts/tracetool/format/tcg_helper_h.py
index dc76c15..6b184b6 100644
--- a/scripts/tracetool/format/tcg_helper_h.py
+++ b/scripts/tracetool/format/tcg_helper_h.py
@@ -18,7 +18,7 @@ from tracetool.transform import *
 import tracetool.vcpu
 
 
-def generate(events, backend):
+def generate(events, backend, group):
     events = [e for e in events
               if "disable" not in e.properties]
 
diff --git a/scripts/tracetool/format/tcg_helper_wrapper_h.py b/scripts/tracetool/format/tcg_helper_wrapper_h.py
index 020f442..ff53447 100644
--- a/scripts/tracetool/format/tcg_helper_wrapper_h.py
+++ b/scripts/tracetool/format/tcg_helper_wrapper_h.py
@@ -18,7 +18,7 @@ from tracetool.transform import *
 import tracetool.vcpu
 
 
-def generate(events, backend):
+def generate(events, backend, group):
     events = [e for e in events
               if "disable" not in e.properties]
 
diff --git a/scripts/tracetool/format/ust_events_c.py b/scripts/tracetool/format/ust_events_c.py
index 9967c7a..cd87d8a 100644
--- a/scripts/tracetool/format/ust_events_c.py
+++ b/scripts/tracetool/format/ust_events_c.py
@@ -16,7 +16,7 @@ __email__      = "stefanha@redhat.com"
 from tracetool import out
 
 
-def generate(events, backend):
+def generate(events, backend, group):
     events = [e for e in events
               if "disabled" not in e.properties]
 
diff --git a/scripts/tracetool/format/ust_events_h.py b/scripts/tracetool/format/ust_events_h.py
index 3e8a7cd..d853155 100644
--- a/scripts/tracetool/format/ust_events_h.py
+++ b/scripts/tracetool/format/ust_events_h.py
@@ -16,7 +16,7 @@ __email__      = "stefanha@redhat.com"
 from tracetool import out
 
 
-def generate(events, backend):
+def generate(events, backend, group):
     events = [e for e in events
               if "disabled" not in e.properties]
 
@@ -28,8 +28,9 @@ def generate(events, backend):
         '#undef TRACEPOINT_INCLUDE_FILE',
         '#define TRACEPOINT_INCLUDE_FILE ./generated-ust-provider.h',
         '',
-        '#if !defined (TRACE__GENERATED_UST_H) || defined(TRACEPOINT_HEADER_MULTI_READ)',
-        '#define TRACE__GENERATED_UST_H',
+        '#if !defined (TRACE_%s_GENERATED_UST_H) || \\'  % group.upper(),
+        '     defined(TRACEPOINT_HEADER_MULTI_READ)',
+        '#define TRACE_%s_GENERATED_UST_H' % group.upper(),
         '',
         '#include "qemu-common.h"',
         '#include <lttng/tracepoint.h>',
@@ -94,7 +95,7 @@ def generate(events, backend):
                 '',
                 name=e.name)
 
-    out('#endif /* TRACE__GENERATED_UST_H */',
+    out('#endif /* TRACE_%s_GENERATED_UST_H */' % group.upper(),
         '',
         '/* This part must be outside ifdef protection */',
         '#include <lttng/tracepoint-event.h>')
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PULL 20/20] trace: Add missing execution mode of guest events
  2016-10-07 10:09 [Qemu-devel] [PULL 00/20] Tracing patches Stefan Hajnoczi
                   ` (18 preceding siblings ...)
  2016-10-07 10:09 ` [Qemu-devel] [PULL 19/20] trace: introduce a formal group name for trace events Stefan Hajnoczi
@ 2016-10-07 10:09 ` Stefan Hajnoczi
  2016-10-07 13:22 ` [Qemu-devel] [PULL 00/20] Tracing patches Peter Maydell
  20 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2016-10-07 10:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Lluís Vilanova, Stefan Hajnoczi

From: Lluís Vilanova <vilanova@ac.upc.edu>

Add missing execution mode documentation for the 'guest_cpu_enter' and
'guest_cpu_reset' events.

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
Message-id: 147566900921.7708.656450813307396468.stgit@fimbulvetr.bsc.es
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 trace-events | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/trace-events b/trace-events
index bd2199a..8ecded5 100644
--- a/trace-events
+++ b/trace-events
@@ -129,11 +129,13 @@ memory_region_tb_write(int cpu_index, uint64_t addr, uint64_t value, unsigned si
 
 # Hot-plug a new virtual (guest) CPU
 #
+# Mode: user, softmmu
 # Targets: all
 vcpu guest_cpu_enter(void)
 
 # Reset the state of a virtual (guest) CPU
 #
+# Mode: user, softmmu
 # Targets: all
 vcpu guest_cpu_reset(void)
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [Qemu-devel] [PULL 00/20] Tracing patches
  2016-10-07 10:09 [Qemu-devel] [PULL 00/20] Tracing patches Stefan Hajnoczi
                   ` (19 preceding siblings ...)
  2016-10-07 10:09 ` [Qemu-devel] [PULL 20/20] trace: Add missing execution mode of guest events Stefan Hajnoczi
@ 2016-10-07 13:22 ` Peter Maydell
  2016-10-07 13:35   ` Daniel P. Berrange
  2016-10-07 13:46   ` Lluís Vilanova
  20 siblings, 2 replies; 27+ messages in thread
From: Peter Maydell @ 2016-10-07 13:22 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: QEMU Developers

On 7 October 2016 at 11:09, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> The following changes since commit e902754e3d0890945ddcc1b33748ed73762ddb8d:
>
>   Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-2.8-20161006' into staging (2016-10-06 13:34:00 +0100)
>
> are available in the git repository at:
>
>   git://github.com/stefanha/qemu.git tags/tracing-pull-request
>
> for you to fetch changes up to bcdc1363dcb9adb5f7ea016a4d0b41f0357c8182:
>
>   trace: Add missing execution mode of guest events (2016-10-07 09:17:49 +0100)
>
> ----------------------------------------------------------------
>
> ----------------------------------------------------------------

This makes all the linux-user binaries dump core on startup:

$ gdb --args /home/petmay01/linaro/qemu-for-merges/build/all-linux-static/x86_64-linux-user/qemu-x86_64
/bin/ls
(gdb) r
Starting program:
/home/petmay01/linaro/qemu-for-merges/build/all-linux-static/x86_64-linux-user/qemu-x86_64
/bin/ls
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
[New Thread 0x7ffff7ffa700 (LWP 14965)]

Thread 1 "qemu-x86_64" received signal SIGABRT, Aborted.
0x000000006019fbe8 in raise ()
(gdb) bt
#0  0x000000006019fbe8 in raise ()
#1  0x00000000601a030a in abort ()
#2  0x00000000600bdd02 in bitmap_new (nbits=0)
    at /home/petmay01/linaro/qemu-for-merges/include/qemu/bitmap.h:99
#3  0x00000000600be99f in cpu_common_initfn (obj=0x625f6340)
    at /home/petmay01/linaro/qemu-for-merges/qom/cpu.c:360
#4  0x00000000600bfe97 in object_init_with_type (obj=0x625f6340, ti=0x625eb9a0)
    at /home/petmay01/linaro/qemu-for-merges/qom/object.c:339
#5  0x00000000600bfe79 in object_init_with_type (obj=0x625f6340, ti=0x625e2d80)
    at /home/petmay01/linaro/qemu-for-merges/qom/object.c:335
#6  0x00000000600bfe79 in object_init_with_type (obj=0x625f6340, ti=0x625e78d0)
    at /home/petmay01/linaro/qemu-for-merges/qom/object.c:335
#7  0x00000000600c00aa in object_initialize_with_type
(data=0x625f6340, size=38416, type=0x625e78d0)
    at /home/petmay01/linaro/qemu-for-merges/qom/object.c:370
#8  0x00000000600c0508 in object_new_with_type (type=0x625e78d0)
    at /home/petmay01/linaro/qemu-for-merges/qom/object.c:478
#9  0x00000000600c0542 in object_new (typename=0x625e2fe0 "qemu64-x86_64-cpu")
    at /home/petmay01/linaro/qemu-for-merges/qom/object.c:488
#10 0x00000000600be03f in cpu_generic_init (typename=0x6025ad54
"x86_64-cpu", cpu_model=0x60247ca5 "qemu64")
    at /home/petmay01/linaro/qemu-for-merges/qom/cpu.c:76
#11 0x0000000060089438 in cpu_x86_init (cpu_model=0x60247ca5 "qemu64")
    at /home/petmay01/linaro/qemu-for-merges/target-i386/cpu.c:2266
#12 0x0000000060031031 in main (argc=2, argv=0x7fffffffe4b8,
envp=0x7fffffffe4d0)
    at /home/petmay01/linaro/qemu-for-merges/linux-user/main.c:4253

thanks
-- PMM

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [Qemu-devel] [PULL 00/20] Tracing patches
  2016-10-07 13:22 ` [Qemu-devel] [PULL 00/20] Tracing patches Peter Maydell
@ 2016-10-07 13:35   ` Daniel P. Berrange
  2016-10-07 13:46   ` Lluís Vilanova
  1 sibling, 0 replies; 27+ messages in thread
From: Daniel P. Berrange @ 2016-10-07 13:35 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Stefan Hajnoczi, QEMU Developers

On Fri, Oct 07, 2016 at 02:22:38PM +0100, Peter Maydell wrote:
> On 7 October 2016 at 11:09, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > The following changes since commit e902754e3d0890945ddcc1b33748ed73762ddb8d:
> >
> >   Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-2.8-20161006' into staging (2016-10-06 13:34:00 +0100)
> >
> > are available in the git repository at:
> >
> >   git://github.com/stefanha/qemu.git tags/tracing-pull-request
> >
> > for you to fetch changes up to bcdc1363dcb9adb5f7ea016a4d0b41f0357c8182:
> >
> >   trace: Add missing execution mode of guest events (2016-10-07 09:17:49 +0100)
> >
> > ----------------------------------------------------------------
> >
> > ----------------------------------------------------------------
> 
> This makes all the linux-user binaries dump core on startup:

[snip]

This will be due to my patches - I'll investigate and send a fix.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [Qemu-devel] [PULL 00/20] Tracing patches
  2016-10-07 13:22 ` [Qemu-devel] [PULL 00/20] Tracing patches Peter Maydell
  2016-10-07 13:35   ` Daniel P. Berrange
@ 2016-10-07 13:46   ` Lluís Vilanova
  2016-10-07 14:16     ` Peter Maydell
  2016-10-07 14:19     ` Daniel P. Berrange
  1 sibling, 2 replies; 27+ messages in thread
From: Lluís Vilanova @ 2016-10-07 13:46 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Stefan Hajnoczi, QEMU Developers

Peter Maydell writes:

> On 7 October 2016 at 11:09, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>> The following changes since commit e902754e3d0890945ddcc1b33748ed73762ddb8d:
>> 
>> Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-2.8-20161006' into staging (2016-10-06 13:34:00 +0100)
>> 
>> are available in the git repository at:
>> 
>> git://github.com/stefanha/qemu.git tags/tracing-pull-request
>> 
>> for you to fetch changes up to bcdc1363dcb9adb5f7ea016a4d0b41f0357c8182:
>> 
>> trace: Add missing execution mode of guest events (2016-10-07 09:17:49 +0100)
>> 
>> ----------------------------------------------------------------
>> 
>> ----------------------------------------------------------------

> This makes all the linux-user binaries dump core on startup:

> $ gdb --args /home/petmay01/linaro/qemu-for-merges/build/all-linux-static/x86_64-linux-user/qemu-x86_64
> /bin/ls
> (gdb) r
> Starting program:
> /home/petmay01/linaro/qemu-for-merges/build/all-linux-static/x86_64-linux-user/qemu-x86_64
> /bin/ls
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
> [New Thread 0x7ffff7ffa700 (LWP 14965)]

> Thread 1 "qemu-x86_64" received signal SIGABRT, Aborted.
> 0x000000006019fbe8 in raise ()
> (gdb) bt
> #0  0x000000006019fbe8 in raise ()
> #1  0x00000000601a030a in abort ()
> #2  0x00000000600bdd02 in bitmap_new (nbits=0)
>     at /home/petmay01/linaro/qemu-for-merges/include/qemu/bitmap.h:99
> #3  0x00000000600be99f in cpu_common_initfn (obj=0x625f6340)
>     at /home/petmay01/linaro/qemu-for-merges/qom/cpu.c:360
> #4  0x00000000600bfe97 in object_init_with_type (obj=0x625f6340, ti=0x625eb9a0)
>     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:339
> #5  0x00000000600bfe79 in object_init_with_type (obj=0x625f6340, ti=0x625e2d80)
>     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:335
> #6  0x00000000600bfe79 in object_init_with_type (obj=0x625f6340, ti=0x625e78d0)
>     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:335
> #7  0x00000000600c00aa in object_initialize_with_type
> (data=0x625f6340, size=38416, type=0x625e78d0)
>     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:370
> #8  0x00000000600c0508 in object_new_with_type (type=0x625e78d0)
>     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:478
> #9  0x00000000600c0542 in object_new (typename=0x625e2fe0 "qemu64-x86_64-cpu")
>     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:488
> #10 0x00000000600be03f in cpu_generic_init (typename=0x6025ad54
> "x86_64-cpu", cpu_model=0x60247ca5 "qemu64")
>     at /home/petmay01/linaro/qemu-for-merges/qom/cpu.c:76
> #11 0x0000000060089438 in cpu_x86_init (cpu_model=0x60247ca5 "qemu64")
>     at /home/petmay01/linaro/qemu-for-merges/target-i386/cpu.c:2266
> #12 0x0000000060031031 in main (argc=2, argv=0x7fffffffe4b8,
> envp=0x7fffffffe4d0)
>     at /home/petmay01/linaro/qemu-for-merges/linux-user/main.c:4253

Aha! I didn't take that abort into consideration during the review. The fix
should be on patch 13 of the series (trace: dynamically allocate trace_dstate in
CPUState):

diff --git a/qom/cpu.c b/qom/cpu.c
index 484c493..40f2eb1 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -356,12 +356,15 @@ static void cpu_common_initfn(Object *obj)
     qemu_mutex_init(&cpu->work_mutex);
     QTAILQ_INIT(&cpu->breakpoints);
     QTAILQ_INIT(&cpu->watchpoints);
-    bitmap_zero(cpu->trace_dstate, TRACE_VCPU_EVENT_COUNT);
+
+    cpu->trace_dstate = bitmap_new(min(trace_get_vcpu_event_count(), 1);
 }
 
 static void cpu_common_finalize(Object *obj)
 {
-    cpu_exec_exit(CPU(obj));
+    CPUState *cpu = CPU(obj);
+    cpu_exec_exit(cpu);
+    g_free(cpu->trace_dstate);
 }
 
 static int64_t cpu_common_get_arch_id(CPUState *cpu)

Beware that min() needs to be defined somewhere on that file first (should it be
available on some generic header?).

Cheers,
  Lluis

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [Qemu-devel] [PULL 00/20] Tracing patches
  2016-10-07 13:46   ` Lluís Vilanova
@ 2016-10-07 14:16     ` Peter Maydell
  2016-10-07 14:19     ` Daniel P. Berrange
  1 sibling, 0 replies; 27+ messages in thread
From: Peter Maydell @ 2016-10-07 14:16 UTC (permalink / raw)
  To: Peter Maydell, Stefan Hajnoczi, QEMU Developers

On 7 October 2016 at 14:46, Lluís Vilanova <vilanova@ac.upc.edu> wrote:
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 484c493..40f2eb1 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -356,12 +356,15 @@ static void cpu_common_initfn(Object *obj)
>      qemu_mutex_init(&cpu->work_mutex);
>      QTAILQ_INIT(&cpu->breakpoints);
>      QTAILQ_INIT(&cpu->watchpoints);
> -    bitmap_zero(cpu->trace_dstate, TRACE_VCPU_EVENT_COUNT);
> +
> +    cpu->trace_dstate = bitmap_new(min(trace_get_vcpu_event_count(), 1);
>  }
>
>  static void cpu_common_finalize(Object *obj)
>  {
> -    cpu_exec_exit(CPU(obj));
> +    CPUState *cpu = CPU(obj);
> +    cpu_exec_exit(cpu);
> +    g_free(cpu->trace_dstate);
>  }
>
>  static int64_t cpu_common_get_arch_id(CPUState *cpu)
>
> Beware that min() needs to be defined somewhere on that file first (should it be
> available on some generic header?).

osdep.h defines a MIN() macro.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [Qemu-devel] [PULL 00/20] Tracing patches
  2016-10-07 13:46   ` Lluís Vilanova
  2016-10-07 14:16     ` Peter Maydell
@ 2016-10-07 14:19     ` Daniel P. Berrange
  2016-10-07 17:51       ` Lluís Vilanova
  1 sibling, 1 reply; 27+ messages in thread
From: Daniel P. Berrange @ 2016-10-07 14:19 UTC (permalink / raw)
  To: Peter Maydell, Stefan Hajnoczi, QEMU Developers

On Fri, Oct 07, 2016 at 03:46:27PM +0200, Lluís Vilanova wrote:
> Peter Maydell writes:
> 
> > On 7 October 2016 at 11:09, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >> The following changes since commit e902754e3d0890945ddcc1b33748ed73762ddb8d:
> >> 
> >> Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-2.8-20161006' into staging (2016-10-06 13:34:00 +0100)
> >> 
> >> are available in the git repository at:
> >> 
> >> git://github.com/stefanha/qemu.git tags/tracing-pull-request
> >> 
> >> for you to fetch changes up to bcdc1363dcb9adb5f7ea016a4d0b41f0357c8182:
> >> 
> >> trace: Add missing execution mode of guest events (2016-10-07 09:17:49 +0100)
> >> 
> >> ----------------------------------------------------------------
> >> 
> >> ----------------------------------------------------------------
> 
> > This makes all the linux-user binaries dump core on startup:
> 
> > $ gdb --args /home/petmay01/linaro/qemu-for-merges/build/all-linux-static/x86_64-linux-user/qemu-x86_64
> > /bin/ls
> > (gdb) r
> > Starting program:
> > /home/petmay01/linaro/qemu-for-merges/build/all-linux-static/x86_64-linux-user/qemu-x86_64
> > /bin/ls
> > [Thread debugging using libthread_db enabled]
> > Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
> > [New Thread 0x7ffff7ffa700 (LWP 14965)]
> 
> > Thread 1 "qemu-x86_64" received signal SIGABRT, Aborted.
> > 0x000000006019fbe8 in raise ()
> > (gdb) bt
> > #0  0x000000006019fbe8 in raise ()
> > #1  0x00000000601a030a in abort ()
> > #2  0x00000000600bdd02 in bitmap_new (nbits=0)
> >     at /home/petmay01/linaro/qemu-for-merges/include/qemu/bitmap.h:99
> > #3  0x00000000600be99f in cpu_common_initfn (obj=0x625f6340)
> >     at /home/petmay01/linaro/qemu-for-merges/qom/cpu.c:360
> > #4  0x00000000600bfe97 in object_init_with_type (obj=0x625f6340, ti=0x625eb9a0)
> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:339
> > #5  0x00000000600bfe79 in object_init_with_type (obj=0x625f6340, ti=0x625e2d80)
> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:335
> > #6  0x00000000600bfe79 in object_init_with_type (obj=0x625f6340, ti=0x625e78d0)
> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:335
> > #7  0x00000000600c00aa in object_initialize_with_type
> > (data=0x625f6340, size=38416, type=0x625e78d0)
> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:370
> > #8  0x00000000600c0508 in object_new_with_type (type=0x625e78d0)
> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:478
> > #9  0x00000000600c0542 in object_new (typename=0x625e2fe0 "qemu64-x86_64-cpu")
> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:488
> > #10 0x00000000600be03f in cpu_generic_init (typename=0x6025ad54
> > "x86_64-cpu", cpu_model=0x60247ca5 "qemu64")
> >     at /home/petmay01/linaro/qemu-for-merges/qom/cpu.c:76
> > #11 0x0000000060089438 in cpu_x86_init (cpu_model=0x60247ca5 "qemu64")
> >     at /home/petmay01/linaro/qemu-for-merges/target-i386/cpu.c:2266
> > #12 0x0000000060031031 in main (argc=2, argv=0x7fffffffe4b8,
> > envp=0x7fffffffe4d0)
> >     at /home/petmay01/linaro/qemu-for-merges/linux-user/main.c:4253
> 
> Aha! I didn't take that abort into consideration during the review. The fix
> should be on patch 13 of the series (trace: dynamically allocate trace_dstate in
> CPUState):
> 
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 484c493..40f2eb1 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -356,12 +356,15 @@ static void cpu_common_initfn(Object *obj)
>      qemu_mutex_init(&cpu->work_mutex);
>      QTAILQ_INIT(&cpu->breakpoints);
>      QTAILQ_INIT(&cpu->watchpoints);
> -    bitmap_zero(cpu->trace_dstate, TRACE_VCPU_EVENT_COUNT);
> +
> +    cpu->trace_dstate = bitmap_new(min(trace_get_vcpu_event_count(), 1);
>  }
>  
>  static void cpu_common_finalize(Object *obj)
>  {
> -    cpu_exec_exit(CPU(obj));
> +    CPUState *cpu = CPU(obj);
> +    cpu_exec_exit(cpu);
> +    g_free(cpu->trace_dstate);
>  }
>  
>  static int64_t cpu_common_get_arch_id(CPUState *cpu)

No, this isn't what we want todo. The real problem is that we should
have been registering the trace events in the linux-user/bsd-user
programs. I sent a fix for that.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [Qemu-devel] [PULL 00/20] Tracing patches
  2016-10-07 14:19     ` Daniel P. Berrange
@ 2016-10-07 17:51       ` Lluís Vilanova
  0 siblings, 0 replies; 27+ messages in thread
From: Lluís Vilanova @ 2016-10-07 17:51 UTC (permalink / raw)
  To: Daniel P. Berrange; +Cc: Peter Maydell, Stefan Hajnoczi, QEMU Developers

Daniel P Berrange writes:

> On Fri, Oct 07, 2016 at 03:46:27PM +0200, Lluís Vilanova wrote:
>> Peter Maydell writes:
>> 
>> > On 7 October 2016 at 11:09, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>> >> The following changes since commit e902754e3d0890945ddcc1b33748ed73762ddb8d:
>> >> 
>> >> Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-2.8-20161006' into staging (2016-10-06 13:34:00 +0100)
>> >> 
>> >> are available in the git repository at:
>> >> 
>> >> git://github.com/stefanha/qemu.git tags/tracing-pull-request
>> >> 
>> >> for you to fetch changes up to bcdc1363dcb9adb5f7ea016a4d0b41f0357c8182:
>> >> 
>> >> trace: Add missing execution mode of guest events (2016-10-07 09:17:49 +0100)
>> >> 
>> >> ----------------------------------------------------------------
>> >> 
>> >> ----------------------------------------------------------------
>> 
>> > This makes all the linux-user binaries dump core on startup:
>> 
>> > $ gdb --args /home/petmay01/linaro/qemu-for-merges/build/all-linux-static/x86_64-linux-user/qemu-x86_64
>> > /bin/ls
>> > (gdb) r
>> > Starting program:
>> > /home/petmay01/linaro/qemu-for-merges/build/all-linux-static/x86_64-linux-user/qemu-x86_64
>> > /bin/ls
>> > [Thread debugging using libthread_db enabled]
>> > Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
>> > [New Thread 0x7ffff7ffa700 (LWP 14965)]
>> 
>> > Thread 1 "qemu-x86_64" received signal SIGABRT, Aborted.
>> > 0x000000006019fbe8 in raise ()
>> > (gdb) bt
>> > #0  0x000000006019fbe8 in raise ()
>> > #1  0x00000000601a030a in abort ()
>> > #2  0x00000000600bdd02 in bitmap_new (nbits=0)
>> >     at /home/petmay01/linaro/qemu-for-merges/include/qemu/bitmap.h:99
>> > #3  0x00000000600be99f in cpu_common_initfn (obj=0x625f6340)
>> >     at /home/petmay01/linaro/qemu-for-merges/qom/cpu.c:360
>> > #4  0x00000000600bfe97 in object_init_with_type (obj=0x625f6340, ti=0x625eb9a0)
>> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:339
>> > #5  0x00000000600bfe79 in object_init_with_type (obj=0x625f6340, ti=0x625e2d80)
>> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:335
>> > #6  0x00000000600bfe79 in object_init_with_type (obj=0x625f6340, ti=0x625e78d0)
>> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:335
>> > #7  0x00000000600c00aa in object_initialize_with_type
>> > (data=0x625f6340, size=38416, type=0x625e78d0)
>> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:370
>> > #8  0x00000000600c0508 in object_new_with_type (type=0x625e78d0)
>> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:478
>> > #9  0x00000000600c0542 in object_new (typename=0x625e2fe0 "qemu64-x86_64-cpu")
>> >     at /home/petmay01/linaro/qemu-for-merges/qom/object.c:488
>> > #10 0x00000000600be03f in cpu_generic_init (typename=0x6025ad54
>> > "x86_64-cpu", cpu_model=0x60247ca5 "qemu64")
>> >     at /home/petmay01/linaro/qemu-for-merges/qom/cpu.c:76
>> > #11 0x0000000060089438 in cpu_x86_init (cpu_model=0x60247ca5 "qemu64")
>> >     at /home/petmay01/linaro/qemu-for-merges/target-i386/cpu.c:2266
>> > #12 0x0000000060031031 in main (argc=2, argv=0x7fffffffe4b8,
>> > envp=0x7fffffffe4d0)
>> >     at /home/petmay01/linaro/qemu-for-merges/linux-user/main.c:4253
>> 
>> Aha! I didn't take that abort into consideration during the review. The fix
>> should be on patch 13 of the series (trace: dynamically allocate trace_dstate in
>> CPUState):
>> 
>> diff --git a/qom/cpu.c b/qom/cpu.c
>> index 484c493..40f2eb1 100644
>> --- a/qom/cpu.c
>> +++ b/qom/cpu.c
>> @@ -356,12 +356,15 @@ static void cpu_common_initfn(Object *obj)
>> qemu_mutex_init(&cpu->work_mutex);
>> QTAILQ_INIT(&cpu->breakpoints);
>> QTAILQ_INIT(&cpu->watchpoints);
>> -    bitmap_zero(cpu->trace_dstate, TRACE_VCPU_EVENT_COUNT);
>> +
>> +    cpu->trace_dstate = bitmap_new(min(trace_get_vcpu_event_count(), 1);
>> }
>> 
>> static void cpu_common_finalize(Object *obj)
>> {
>> -    cpu_exec_exit(CPU(obj));
>> +    CPUState *cpu = CPU(obj);
>> +    cpu_exec_exit(cpu);
>> +    g_free(cpu->trace_dstate);
>> }
>> 
>> static int64_t cpu_common_get_arch_id(CPUState *cpu)

> No, this isn't what we want todo. The real problem is that we should
> have been registering the trace events in the linux-user/bsd-user
> programs. I sent a fix for that.

Well, that'll work as long as we keep at least one vcpu event :)

Cheers,
  Lluis

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2016-10-07 17:51 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-07 10:09 [Qemu-devel] [PULL 00/20] Tracing patches Stefan Hajnoczi
2016-10-07 10:09 ` [Qemu-devel] [PULL 01/20] trace: move colo trace events to net/ sub-directory Stefan Hajnoczi
2016-10-07 10:09 ` [Qemu-devel] [PULL 02/20] trace: add trace event iterator APIs Stefan Hajnoczi
2016-10-07 10:09 ` [Qemu-devel] [PULL 03/20] trace: convert code to use event iterators Stefan Hajnoczi
2016-10-07 10:09 ` [Qemu-devel] [PULL 04/20] trace: remove some now unused functions Stefan Hajnoczi
2016-10-07 10:09 ` [Qemu-devel] [PULL 05/20] trace: remove global 'uint16 dstate[]' array Stefan Hajnoczi
2016-10-07 10:09 ` [Qemu-devel] [PULL 06/20] trace: remove duplicate control.h includes in generated-tracers.h Stefan Hajnoczi
2016-10-07 10:09 ` [Qemu-devel] [PULL 07/20] trace: break circular dependency in event-internal.h Stefan Hajnoczi
2016-10-07 10:09 ` [Qemu-devel] [PULL 08/20] trace: give each trace event a named TraceEvent struct Stefan Hajnoczi
2016-10-07 10:09 ` [Qemu-devel] [PULL 09/20] trace: remove the TraceEventID and TraceEventVCPUID enums Stefan Hajnoczi
2016-10-07 10:09 ` [Qemu-devel] [PULL 10/20] trace: emit name <-> ID mapping in simpletrace header Stefan Hajnoczi
2016-10-07 10:09 ` [Qemu-devel] [PULL 11/20] trace: don't abort qemu if ftrace can't be initialized Stefan Hajnoczi
2016-10-07 10:09 ` [Qemu-devel] [PULL 12/20] trace: provide mechanism for registering trace events Stefan Hajnoczi
2016-10-07 10:09 ` [Qemu-devel] [PULL 13/20] trace: dynamically allocate trace_dstate in CPUState Stefan Hajnoczi
2016-10-07 10:09 ` [Qemu-devel] [PULL 14/20] trace: dynamically allocate event IDs at runtime Stefan Hajnoczi
2016-10-07 10:09 ` [Qemu-devel] [PULL 15/20] trace: get rid of generated-events.h/generated-events.c Stefan Hajnoczi
2016-10-07 10:09 ` [Qemu-devel] [PULL 16/20] trace: rename _read_events to read_events Stefan Hajnoczi
2016-10-07 10:09 ` [Qemu-devel] [PULL 17/20] trace: push reading of events up a level to tracetool main Stefan Hajnoczi
2016-10-07 10:09 ` [Qemu-devel] [PULL 18/20] trace: pass trace-events to tracetool as a positional param Stefan Hajnoczi
2016-10-07 10:09 ` [Qemu-devel] [PULL 19/20] trace: introduce a formal group name for trace events Stefan Hajnoczi
2016-10-07 10:09 ` [Qemu-devel] [PULL 20/20] trace: Add missing execution mode of guest events Stefan Hajnoczi
2016-10-07 13:22 ` [Qemu-devel] [PULL 00/20] Tracing patches Peter Maydell
2016-10-07 13:35   ` Daniel P. Berrange
2016-10-07 13:46   ` Lluís Vilanova
2016-10-07 14:16     ` Peter Maydell
2016-10-07 14:19     ` Daniel P. Berrange
2016-10-07 17:51       ` Lluís Vilanova

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.