[Qemu-devel] [PATCH v4 0/7] trace: Generic event state description

[Qemu-devel] [PATCH v4 0/7] trace: Generic event state description

[Lluís Vilanova]

Provides a generic event state description and a more detailed event control and
query interface.

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

Changes in v4:

* Documentation fixes and (re)formatting.

Changes in v3:

* Add some assertions.

* Remove debugging printf's.

* Improve documentation.

* Make 'trace_event_get_state_static' use run-time information, and leave
  TRACE_*_ENABLED for compile-time checks.


Changes in v2:

* Minor compilation fixes.


Lluís Vilanova (7):
      tracetool: Explicitly identify public backends
      trace: Provide a generic tracing event descriptor
      trace: Provide a detailed event control interface
      trace: [monitor] Use new event control interface
      trace: [default] Use new event control interface
      trace: [simple] Port to generic event information and new control interface
      trace: [stderr] Port to generic event information and new control interface


 Makefile                              |    5 +
 Makefile.objs                         |   21 ++++
 docs/tracing.txt                      |   42 +++----
 monitor.c                             |   15 ++-
 scripts/tracetool.py                  |    4 -
 scripts/tracetool/backend/__init__.py |   16 +++
 scripts/tracetool/backend/dtrace.py   |    3 +
 scripts/tracetool/backend/events.py   |   23 ++++
 scripts/tracetool/backend/simple.py   |   15 +--
 scripts/tracetool/backend/stderr.py   |   28 ++---
 scripts/tracetool/backend/ust.py      |    3 +
 scripts/tracetool/format/events_c.py  |   39 +++++++
 scripts/tracetool/format/events_h.py  |   50 +++++++++
 scripts/tracetool/format/h.py         |    9 --
 trace/control-internal.h              |   60 +++++++++++
 trace/control.c                       |   92 +++++++++++++++-
 trace/control.h                       |  185 ++++++++++++++++++++++++++++++---
 trace/default.c                       |    6 +
 trace/event-internal.h                |   33 ++++++
 trace/simple.c                        |   37 ++-----
 trace/simple.h                        |    6 -
 trace/stderr.c                        |   35 +-----
 trace/stderr.h                        |   11 --
 23 files changed, 568 insertions(+), 170 deletions(-)
 create mode 100644 scripts/tracetool/backend/events.py
 create mode 100644 scripts/tracetool/format/events_c.py
 create mode 100644 scripts/tracetool/format/events_h.py
 create mode 100644 trace/control-internal.h
 create mode 100644 trace/event-internal.h
 delete mode 100644 trace/stderr.h


To: qemu-devel@nongnu.org
Cc: Stefan Hajnoczi <stefanha@gmail.com>
Cc: Blue Swirl <blauwirbel@gmail.com>

[Qemu-devel] [PATCH v4 1/7] tracetool: Explicitly identify public backends

[Lluís Vilanova]

Public backends are those printed by "--list-backends" and thus considered valid
by the configure script.

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 scripts/tracetool.py                  |    4 ++--
 scripts/tracetool/backend/__init__.py |   16 +++++++++++++++-
 scripts/tracetool/backend/dtrace.py   |    3 +++
 scripts/tracetool/backend/simple.py   |    3 +++
 scripts/tracetool/backend/stderr.py   |    3 +++
 scripts/tracetool/backend/ust.py      |    3 +++
 6 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index c003cf6..a79ec0f 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -90,8 +90,8 @@ def main(args):
             arg_format = arg
 
         elif opt == "--list-backends":
-            backends = tracetool.backend.get_list()
-            out(", ".join([ b for b,_ in backends ]))
+            public_backends = tracetool.backend.get_list(only_public = True)
+            out(", ".join([ b for b,_ in public_backends ]))
             sys.exit(0)
         elif opt == "--check-backend":
             check_backend = True
diff --git a/scripts/tracetool/backend/__init__.py b/scripts/tracetool/backend/__init__.py
index be43472..f0314ee 100644
--- a/scripts/tracetool/backend/__init__.py
+++ b/scripts/tracetool/backend/__init__.py
@@ -17,6 +17,16 @@ considered its short description.
 All backends must generate their contents through the 'tracetool.out' routine.
 
 
+Backend attributes
+------------------
+
+========= ====================================================================
+Attribute Description
+========= ====================================================================
+PUBLIC    If exists and is set to 'True', the backend is considered "public".
+========= ====================================================================
+
+
 Backend functions
 -----------------
 
@@ -42,7 +52,7 @@ import os
 import tracetool
 
 
-def get_list():
+def get_list(only_public = False):
     """Get a list of (name, description) pairs."""
     res = [("nop", "Tracing disabled.")]
     modnames = []
@@ -57,6 +67,10 @@ def get_list():
             continue
         module = module[1]
 
+        public = getattr(module, "PUBLIC", False)
+        if only_public and not public:
+            continue
+
         doc = module.__doc__
         if doc is None:
             doc = ""
diff --git a/scripts/tracetool/backend/dtrace.py b/scripts/tracetool/backend/dtrace.py
index 9cab75c..5bcf513 100644
--- a/scripts/tracetool/backend/dtrace.py
+++ b/scripts/tracetool/backend/dtrace.py
@@ -16,6 +16,9 @@ __email__      = "stefanha@linux.vnet.ibm.com"
 from tracetool import out
 
 
+PUBLIC = True
+
+
 PROBEPREFIX = None
 
 def _probeprefix():
diff --git a/scripts/tracetool/backend/simple.py b/scripts/tracetool/backend/simple.py
index fbb5717..9a4f512 100644
--- a/scripts/tracetool/backend/simple.py
+++ b/scripts/tracetool/backend/simple.py
@@ -16,6 +16,9 @@ __email__      = "stefanha@linux.vnet.ibm.com"
 from tracetool import out
 
 
+PUBLIC = True
+
+
 def c(events):
     out('#include "trace.h"',
         '',
diff --git a/scripts/tracetool/backend/stderr.py b/scripts/tracetool/backend/stderr.py
index 917fde7..a10fbb8 100644
--- a/scripts/tracetool/backend/stderr.py
+++ b/scripts/tracetool/backend/stderr.py
@@ -16,6 +16,9 @@ __email__      = "stefanha@linux.vnet.ibm.com"
 from tracetool import out
 
 
+PUBLIC = True
+
+
 def c(events):
     out('#include "trace.h"',
         '',
diff --git a/scripts/tracetool/backend/ust.py b/scripts/tracetool/backend/ust.py
index 31a2ff0..ea36995 100644
--- a/scripts/tracetool/backend/ust.py
+++ b/scripts/tracetool/backend/ust.py
@@ -16,6 +16,9 @@ __email__      = "stefanha@linux.vnet.ibm.com"
 from tracetool import out
 
 
+PUBLIC = True
+
+
 def c(events):
     out('#include <ust/marker.h>',
         '#undef mutex_lock',

[Qemu-devel] [PATCH v4 2/7] trace: Provide a generic tracing event descriptor

[Lluís Vilanova]

Uses tracetool to generate a backend-independent tracing event description.

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 Makefile                             |    5 +++
 Makefile.objs                        |   21 ++++++++++++++
 scripts/tracetool/backend/events.py  |   23 ++++++++++++++++
 scripts/tracetool/format/events_c.py |   39 +++++++++++++++++++++++++++
 scripts/tracetool/format/events_h.py |   50 ++++++++++++++++++++++++++++++++++
 scripts/tracetool/format/h.py        |    9 +-----
 trace/event-internal.h               |   33 ++++++++++++++++++++++
 7 files changed, 171 insertions(+), 9 deletions(-)
 create mode 100644 scripts/tracetool/backend/events.py
 create mode 100644 scripts/tracetool/format/events_c.py
 create mode 100644 scripts/tracetool/format/events_h.py
 create mode 100644 trace/event-internal.h

diff --git a/Makefile b/Makefile
index 5829611..36b97d5 100644
--- a/Makefile
+++ b/Makefile
@@ -19,9 +19,14 @@ config-host.mak:
 endif
 
 GENERATED_HEADERS = config-host.h trace.h qemu-options.def
+
+GENERATED_HEADERS += trace-events.h
+GENERATED_SOURCES += trace-events.c
+
 ifeq ($(TRACE_BACKEND),dtrace)
 GENERATED_HEADERS += trace-dtrace.h
 endif
+
 GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
 GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c trace.c
 
diff --git a/Makefile.objs b/Makefile.objs
index 70c5c79..c17c231 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -372,6 +372,25 @@ libdis-$(CONFIG_LM32_DIS) += lm32-dis.o
 ######################################################################
 # trace
 
+trace-events.h: trace-events.h-timestamp
+trace-events.h-timestamp: $(SRC_PATH)/trace-events
+	$(call quiet-command,$(TRACETOOL) \
+		--format=events-h \
+		--backend=events \
+		< $< > $@,"  GEN   trace-events.h")
+	@cmp -s $@ trace-events.h || cp $@ trace-events.h
+
+trace-events.c: trace-events.c-timestamp
+trace-events.c-timestamp: $(SRC_PATH)/trace-events
+	$(call quiet-command,$(TRACETOOL) \
+		--format=events-c \
+		--backend=events \
+		< $< > $@,"  GEN   trace-events.c")
+	@cmp -s $@ trace-events.c || cp $@ trace-events.c
+
+trace-obj-y += trace-events.o
+
+
 ifeq ($(TRACE_BACKEND),dtrace)
 TRACE_H_EXTRA_DEPS=trace-dtrace.h
 endif
@@ -422,7 +441,7 @@ trace/simple.o: trace/simple.c $(GENERATED_HEADERS)
 
 trace-obj-$(CONFIG_TRACE_DTRACE) += trace-dtrace.o
 ifneq ($(TRACE_BACKEND),dtrace)
-trace-obj-y = trace.o
+trace-obj-y += trace.o
 endif
 
 trace-nested-$(CONFIG_TRACE_DEFAULT) += default.o
diff --git a/scripts/tracetool/backend/events.py b/scripts/tracetool/backend/events.py
new file mode 100644
index 0000000..5afce3e
--- /dev/null
+++ b/scripts/tracetool/backend/events.py
@@ -0,0 +1,23 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""
+Generic event description.
+
+This is a dummy backend to establish appropriate frontend/backend compatibility
+checks.
+"""
+
+__author__     = "Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__  = "Copyright 2012, 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"
+
+
+def events_h(events):
+    pass
+
+def events_c(events):
+    pass
diff --git a/scripts/tracetool/format/events_c.py b/scripts/tracetool/format/events_c.py
new file mode 100644
index 0000000..f395e82
--- /dev/null
+++ b/scripts/tracetool/format/events_c.py
@@ -0,0 +1,39 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""
+Generate .c for event description.
+"""
+
+__author__     = "Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__  = "Copyright 2012, 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 begin(events):
+    out('/* This file is autogenerated by tracetool, do not edit. */',
+        '',
+        '#include "trace.h"',
+        '#include "trace-events.h"',
+        '#include "trace/control.h"',
+        '',
+        )
+
+    out('TraceEvent trace_events[TRACE_EVENT_COUNT] = {')
+
+    for e in events:
+        out('    { .id = %(id)s, .name = \"%(name)s\", .sstate = %(sstate)s, .dstate = 0 },',
+            id = "TRACE_" + e.name.upper(),
+            name = e.name,
+            sstate = "TRACE_%s_ENABLED" % e.name.upper(),
+            )
+
+    out('};',
+        '',
+        )
diff --git a/scripts/tracetool/format/events_h.py b/scripts/tracetool/format/events_h.py
new file mode 100644
index 0000000..643a119
--- /dev/null
+++ b/scripts/tracetool/format/events_h.py
@@ -0,0 +1,50 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""
+Generate .h for event description.
+"""
+
+__author__     = "Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__  = "Copyright 2012, 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 begin(events):
+    out('/* This file is autogenerated by tracetool, do not edit. */',
+        '',
+        '#ifndef TRACE_EVENTS_H',
+        '#define TRACE_EVENTS_H',
+        '',
+        '#include <stdbool.h>',
+        ''
+        )
+
+    # event identifiers
+    out('typedef enum {')
+
+    for e in events:
+        out('    TRACE_%s,' % e.name.upper())
+
+    out('    TRACE_EVENT_COUNT',
+        '} TraceEventID;',
+        )
+
+    # static state
+    for e in events:
+        if 'disable' in e.properties:
+            enabled = 0
+        else:
+            enabled = 1
+        out('#define TRACE_%s_ENABLED %d' % (e.name.upper(), enabled))
+
+    out('#include "trace/event-internal.h"',
+        '',
+        '#endif  /* TRACE_EVENTS_H */',
+        )
diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
index 6ffb3c2..80a1a05 100644
--- a/scripts/tracetool/format/h.py
+++ b/scripts/tracetool/format/h.py
@@ -25,14 +25,7 @@ def begin(events):
         '#include "qemu-common.h"')
 
 def end(events):
-    for e in events:
-        if "disable" in e.properties:
-            enabled = 0
-        else:
-            enabled = 1
-        out('#define TRACE_%s_ENABLED %d' % (e.name.upper(), enabled))
-    out('',
-        '#endif /* TRACE_H */')
+    out('#endif /* TRACE_H */')
 
 def nop(events):
     for e in events:
diff --git a/trace/event-internal.h b/trace/event-internal.h
new file mode 100644
index 0000000..a26c57f
--- /dev/null
+++ b/trace/event-internal.h
@@ -0,0 +1,33 @@
+/*
+ * Interface for configuring and controlling the state of tracing events.
+ *
+ * Copyright (C) 2012 Lluís Vilanova <vilanova@ac.upc.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef TRACE__EVENT_H
+#define TRACE__EVENT_H
+
+#include "trace-events.h"
+
+
+/**
+ * TraceEvent:
+ * @id: Unique event identifier.
+ * @name: Event name.
+ * @sstate: Static instrumentation state.
+ * @dstate: Dynamic instrumentation state.
+ *
+ * Opaque generic description of a tracing event.
+ */
+typedef struct TraceEvent {
+    TraceEventID id;
+    const char * name;
+    const bool sstate;
+    bool dstate;
+} TraceEvent;
+
+
+#endif  /* TRACE__EVENT_H */

[Qemu-devel] [PATCH v4 3/7] trace: Provide a detailed event control interface

[Lluís Vilanova]

This interface decouples event obtention from interaction.

Events can be obtained through three different methods:

* identifier
* name
* simple wildcard pattern

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 docs/tracing.txt         |   42 ++++------
 trace/control-internal.h |   60 +++++++++++++++
 trace/control.c          |   92 +++++++++++++++++++++--
 trace/control.h          |  185 +++++++++++++++++++++++++++++++++++++++++-----
 4 files changed, 328 insertions(+), 51 deletions(-)
 create mode 100644 trace/control-internal.h

diff --git a/docs/tracing.txt b/docs/tracing.txt
index c541133..5fa2446 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -100,44 +100,32 @@ respectively.  This ensures portability between 32- and 64-bit platforms.
 
 == Generic interface and monitor commands ==
 
-You can programmatically query and control the dynamic state of trace events
-through a backend-agnostic interface:
+You can programmatically query and control the state of trace events through a
+backend-agnostic interface provided by the file "trace/control.h".
 
-* trace_print_events
+Note that some of the backends do not provide an implementation for some parts
+of this interface, in which case QEMU will just print a warning (please refer to
+header "trace/control.h" to see which routines are backend-dependent).
 
-* trace_event_set_state
-  Enables or disables trace events at runtime inside QEMU.
-  The function returns "true" if the state of the event has been successfully
-  changed, or "false" otherwise:
-
-    #include "trace/control.h"
-    
-    trace_event_set_state("virtio_irq", true); /* enable */
-    [...]
-    trace_event_set_state("virtio_irq", false); /* disable */
-
-Note that some of the backends do not provide an implementation for this
-interface, in which case QEMU will just print a warning.
-
-This functionality is also provided through monitor commands:
+The state of events can also be queried and modified through monitor commands:
 
 * info trace-events
   View available trace events and their state.  State 1 means enabled, state 0
   means disabled.
 
 * trace-event NAME on|off
-  Enable/disable a given trace event or a group of events having common prefix
-  through wildcard.
+  Enable/disable a given trace event or a group of events (using wildcards).
 
 The "-trace events=<file>" command line argument can be used to enable the
 events listed in <file> from the very beginning of the program. This file must
 contain one event name per line.
 
-A basic wildcard matching is supported in both the monitor command "trace
--event" and the events list file. That means you can enable/disable the events
-having a common prefix in a batch. For example, virtio-blk trace events could
-be enabled using:
-  trace-event virtio_blk_* on
+Wildcard matching is supported in both the monitor command "trace-event" and the
+events list file. That means you can enable/disable the events having a common
+prefix in a batch. For example, virtio-blk trace events could be enabled using
+the following monitor command:
+
+    trace-event virtio_blk_* on
 
 == Trace backends ==
 
@@ -268,3 +256,7 @@ guard such computations and avoid its compilation when the event is disabled:
         }
         return ptr;
     }
+
+You can check both if the event has been disabled and is dynamically enabled at
+the same time using the 'trace_event_get_state' routine (see header
+"trace/control.h" for more information).
diff --git a/trace/control-internal.h b/trace/control-internal.h
new file mode 100644
index 0000000..81db761
--- /dev/null
+++ b/trace/control-internal.h
@@ -0,0 +1,60 @@
+/*
+ * Interface for configuring and controlling the state of tracing events.
+ *
+ * Copyright (C) 2011, 2012 Lluís Vilanova <vilanova@ac.upc.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+
+extern TraceEvent trace_events[];
+
+
+static inline TraceEvent *trace_event_id(TraceEventID id)
+{
+    assert(id < trace_event_count());
+    return &trace_events[id];
+}
+
+static inline TraceEventID trace_event_count(void)
+{
+    return TRACE_EVENT_COUNT;
+}
+
+static inline bool trace_event_is_pattern(const char *str)
+{
+    assert(str != NULL);
+
+    while (*str != '\0') {
+        if (*str == '*') {
+            return true;
+        }
+        str++;
+    }
+    return false;
+}
+
+static inline TraceEventID trace_event_get_id(TraceEvent *ev)
+{
+    assert(ev != NULL);
+    return ev->id;
+}
+
+static inline const char * trace_event_get_name(TraceEvent *ev)
+{
+    assert(ev != NULL);
+    return ev->name;
+}
+
+static inline bool trace_event_get_state_static(TraceEvent *ev)
+{
+    assert(ev != NULL);
+    return ev->sstate;
+}
+
+static inline bool trace_event_get_state_dynamic(TraceEvent *ev)
+{
+    assert(ev != NULL);
+    return ev->dstate;
+}
diff --git a/trace/control.c b/trace/control.c
index 4c5527d..78973ce 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -1,15 +1,84 @@
 /*
  * Interface for configuring and controlling the state of tracing events.
  *
- * Copyright (C) 2011 Lluís Vilanova <vilanova@ac.upc.edu>
+ * Copyright (C) 2011, 2012 Lluís Vilanova <vilanova@ac.upc.edu>
  *
- * This work is licensed under the terms of the GNU GPL, version 2.  See
- * the COPYING file in the top-level directory.
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
  */
 
 #include "trace/control.h"
 
 
+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);
+        if (strcmp(trace_event_get_name(ev), name) == 0) {
+            return ev;
+        }
+    }
+    return NULL;
+}
+
+static bool glob(const char *pat, const char *ev)
+{
+    while (*pat != '\0' && *ev != '\0') {
+        if (*pat == *ev) {
+            pat++;
+            ev++;
+        }
+        else if (*pat == '*') {
+            if (glob(pat, ev+1)) {
+                return true;
+            } else if (glob(pat+1, ev)) {
+                return true;
+            } else {
+                return false;
+            }
+        } else {
+            return false;
+        }
+    }
+
+    while (*pat == '*') {
+        pat++;
+    }
+
+    if (*pat == '\0' && *ev == '\0') {
+        return true;
+    } else {
+        return false;
+    }
+}
+
+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 (glob(pat, trace_event_get_name(res))) {
+            return res;
+        }
+        i++;
+    }
+
+    return NULL;
+}
+
 void trace_backend_init_events(const char *fname)
 {
     if (fname == NULL) {
@@ -27,10 +96,19 @@ void trace_backend_init_events(const char *fname)
         size_t len = strlen(line_buf);
         if (len > 1) {              /* skip empty lines */
             line_buf[len - 1] = '\0';
-            if (!trace_event_set_state(line_buf, true)) {
-                fprintf(stderr,
-                        "error: trace event '%s' does not exist\n", line_buf);
-                exit(1);
+            if (trace_event_is_pattern(line_buf)) {
+                TraceEvent *ev;
+                while ((ev = trace_event_pattern(line_buf, ev)) != NULL) {
+                    trace_event_set_state_dynamic(ev, true);
+                }
+            } else {
+                TraceEvent *ev = trace_event_name(line_buf);
+                if (ev == NULL) {
+                    fprintf(stderr,
+                            "error: trace event '%s' does not exist\n", line_buf);
+                    exit(1);
+                }
+                trace_event_set_state_dynamic(ev, true);
             }
         }
     }
diff --git a/trace/control.h b/trace/control.h
index 2acaa42..1ef8f14 100644
--- a/trace/control.h
+++ b/trace/control.h
@@ -1,41 +1,188 @@
 /*
  * Interface for configuring and controlling the state of tracing events.
  *
- * Copyright (C) 2011 Lluís Vilanova <vilanova@ac.upc.edu>
+ * Copyright (C) 2011, 2012 Lluís Vilanova <vilanova@ac.upc.edu>
  *
- * This work is licensed under the terms of the GNU GPL, version 2.  See
- * the COPYING file in the top-level directory.
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
  */
 
-#ifndef TRACE_CONTROL_H
-#define TRACE_CONTROL_H
+#ifndef TRACE__CONTROL_H
+#define TRACE__CONTROL_H
 
 #include "qemu-common.h"
+#include "trace-events.h"
 
 
-/** Print the state of all events. */
-void trace_print_events(FILE *stream, fprintf_function stream_printf);
-/** Set the state of an event.
+/**
+ * TraceEventID:
+ *
+ * Unique tracing event identifier.
+ *
+ * These are named as 'TRACE_${EVENT_NAME}'.
+ *
+ * See also: "trace-events.h"
+ */
+enum TraceEventID;
+
+/**
+ * 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:
+ * @id: Event name.
+ *
+ * Search an event by its name.
+ *
+ * Returns: pointer to #TraceEvent or NULL if not found.
+ */
+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:
+ *
+ * Get the identifier of an event.
+ */
+static TraceEventID trace_event_get_id(TraceEvent *ev);
+
+/**
+ * trace_event_get_name:
+ *
+ * Get the name of an event.
+ */
+static const char * trace_event_get_name(TraceEvent *ev);
+
+/**
+ * trace_event_get_state:
+ * @id: Event identifier.
+ *
+ * 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(trace_event_id(id)))
+
+/**
+ * trace_event_get_state_static:
+ * @id: Event identifier.
+ *
+ * Get the static tracing state of an event.
+ *
+ * Use the define 'TRACE_${EVENT_NAME}_ENABLED' for compile-time checks (it will
+ * be set to 1 or 0 according to the presence of the disabled property).
+ */
+static bool trace_event_get_state_static(TraceEvent *ev);
+
+/**
+ * trace_event_get_state_dynamic:
  *
- * @return Whether the state changed.
+ * Get the dynamic tracing state of an event.
  */
-bool trace_event_set_state(const char *name, bool state);
+static bool trace_event_get_state_dynamic(TraceEvent *ev);
 
+/**
+ * trace_event_set_state:
+ *
+ * Set the tracing state of an event.
+ */
+#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)
 
-/** Initialize the tracing backend.
+/**
+ * trace_event_set_state_dynamic:
+ *
+ * Set the dynamic tracing state of an event.
  *
- * @events Name of file with events to be enabled at startup; may be NULL.
- *         Corresponds to commandline option "-trace events=...".
- * @file   Name of trace output file; may be NULL.
- *         Corresponds to commandline option "-trace file=...".
- * @return Whether the backend could be successfully initialized.
+ * Warning: This function must be implemented by each tracing backend.
+ */
+void trace_event_set_state_dynamic(TraceEvent *ev, bool state);
+
+
+
+/**
+ * trace_print_events:
+ *
+ * Print the state of all events.
+ *
+ * Warning: This function must be implemented by each tracing backend.
+ *
+ * TODO: Should this be moved to generic code?
+ */
+void trace_print_events(FILE *stream, fprintf_function stream_printf);
+
+/**
+ * trace_backend_init:
+ * @events: Name of file with events to be enabled at startup; may be NULL.
+ *          Corresponds to commandline option "-trace events=...".
+ * @file:   Name of trace output file; may be NULL.
+ *          Corresponds to commandline option "-trace file=...".
+ *
+ * Initialize the tracing backend.
+ *
+ * Warning: This function must be implemented by each tracing backend.
+ *
+ * Returns: Whether the backend could be successfully initialized.
  */
 bool trace_backend_init(const char *events, const char *file);
 
-/** Generic function to initialize the state of events.
+/**
+ * trace_backend_init_events:
+ * @fname: Name of file with events to enable; may be NULL.
  *
- * @fname Name of file with events to enable; may be NULL.
+ * Generic function to initialize the state of events.
  */
 void trace_backend_init_events(const char *fname);
 
-#endif  /* TRACE_CONTROL_H */
+
+#include "trace/control-internal.h"
+
+#endif  /* TRACE__CONTROL_H */

[Qemu-devel] [PATCH v4 4/7] trace: [monitor] Use new event control interface

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 monitor.c |   15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/monitor.c b/monitor.c
index 8946a10..86c2538 100644
--- a/monitor.c
+++ b/monitor.c
@@ -625,10 +625,19 @@ static void do_trace_event_set_state(Monitor *mon, const QDict *qdict)
 {
     const char *tp_name = qdict_get_str(qdict, "name");
     bool new_state = qdict_get_bool(qdict, "option");
-    int ret = trace_event_set_state(tp_name, new_state);
 
-    if (!ret) {
-        monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
+    if (trace_event_is_pattern(tp_name)) {
+        TraceEvent *ev = NULL;
+        while ((ev = trace_event_pattern(tp_name, ev)) != NULL) {
+            trace_event_set_state_dynamic(ev, new_state);
+        }
+    } else {
+        TraceEvent *ev = trace_event_name(tp_name);
+        if (ev == NULL) {
+            monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
+        } else {
+            trace_event_set_state_dynamic(ev, new_state);
+        }
     }
 }
 

[Qemu-devel] [PATCH v4 5/7] trace: [default] Use new event control interface

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 trace/default.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/trace/default.c b/trace/default.c
index c9b27a2..3c41990 100644
--- a/trace/default.c
+++ b/trace/default.c
@@ -1,7 +1,7 @@
 /*
  * Default implementation for backend initialization from commandline.
  *
- * Copyright (C) 2011 Lluís Vilanova <vilanova@ac.upc.edu>
+ * Copyright (C) 2011, 2012 Lluís Vilanova <vilanova@ac.upc.edu>
  *
  * This work is licensed under the terms of the GNU GPL, version 2.  See
  * the COPYING file in the top-level directory.
@@ -18,11 +18,11 @@ void trace_print_events(FILE *stream, fprintf_function stream_printf)
                   "operation not supported with the current backend\n");
 }
 
-bool trace_event_set_state(const char *name, bool state)
+void trace_event_set_state_dynamic(TraceEvent *ev, bool state)
 {
+    assert(ev != NULL);
     fprintf(stderr, "warning: "
             "cannot set the state of a trace event with the current backend\n");
-    return false;
 }
 
 bool trace_backend_init(const char *events, const char *file)

[Qemu-devel] [PATCH v4 6/7] trace: [simple] Port to generic event information and new control interface

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 scripts/tracetool/backend/simple.py |   12 +----------
 trace/simple.c                      |   37 +++++++++--------------------------
 trace/simple.h                      |    6 +-----
 trace/stderr.h                      |   11 ----------
 4 files changed, 11 insertions(+), 55 deletions(-)
 delete mode 100644 trace/stderr.h

diff --git a/scripts/tracetool/backend/simple.py b/scripts/tracetool/backend/simple.py
index 9a4f512..eca17c6 100644
--- a/scripts/tracetool/backend/simple.py
+++ b/scripts/tracetool/backend/simple.py
@@ -22,14 +22,7 @@ PUBLIC = True
 def c(events):
     out('#include "trace.h"',
         '',
-        'TraceEvent trace_list[] = {')
-
-    for e in events:
-        out('{.tp_name = "%(name)s", .state=0},',
-            name = e.name,
-            )
-
-    out('};')
+        )
 
 def h(events):
     out('#include "trace/simple.h"',
@@ -53,6 +46,3 @@ def h(events):
             argc = len(e.args),
             trace_args = simple_args,
             )
-
-    out('#define NR_TRACE_EVENTS %d' % len(events))
-    out('extern TraceEvent trace_list[NR_TRACE_EVENTS];')
diff --git a/trace/simple.c b/trace/simple.c
index 33ae486..79e9adb 100644
--- a/trace/simple.c
+++ b/trace/simple.c
@@ -156,7 +156,9 @@ static void trace(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3,
     unsigned int idx;
     uint64_t timestamp;
 
-    if (!trace_list[event].state) {
+    TraceEvent *eventp = trace_event_id(event);
+    bool _state = trace_event_get_state_dynamic(eventp);
+    if (!_state) {
         return;
     }
 
@@ -315,38 +317,17 @@ void trace_print_events(FILE *stream, fprintf_function stream_printf)
 {
     unsigned int i;
 
-    for (i = 0; i < NR_TRACE_EVENTS; i++) {
+    for (i = 0; i < trace_event_count(); i++) {
+        TraceEvent *ev = trace_event_id(i);
         stream_printf(stream, "%s [Event ID %u] : state %u\n",
-                      trace_list[i].tp_name, i, trace_list[i].state);
+                      trace_event_get_name(ev), i, trace_event_get_state_dynamic(ev));
     }
 }
 
-bool trace_event_set_state(const char *name, bool state)
+void trace_event_set_state_dynamic(TraceEvent *ev, bool state)
 {
-    unsigned int i;
-    unsigned int len;
-    bool wildcard = false;
-    bool matched = false;
-
-    len = strlen(name);
-    if (len > 0 && name[len - 1] == '*') {
-        wildcard = true;
-        len -= 1;
-    }
-    for (i = 0; i < NR_TRACE_EVENTS; i++) {
-        if (wildcard) {
-            if (!strncmp(trace_list[i].tp_name, name, len)) {
-                trace_list[i].state = state;
-                matched = true;
-            }
-            continue;
-        }
-        if (!strcmp(trace_list[i].tp_name, name)) {
-            trace_list[i].state = state;
-            return true;
-        }
-    }
-    return matched;
+    assert(ev != NULL);
+    ev->dstate = state;
 }
 
 /* Helper function to create a thread with signals blocked.  Use glib's
diff --git a/trace/simple.h b/trace/simple.h
index 466e75b..059752b 100644
--- a/trace/simple.h
+++ b/trace/simple.h
@@ -15,12 +15,8 @@
 #include <stdbool.h>
 #include <stdio.h>
 
-typedef uint64_t TraceEventID;
+#include "trace-events.h"
 
-typedef struct {
-    const char *tp_name;
-    bool state;
-} TraceEvent;
 
 void trace0(TraceEventID event);
 void trace1(TraceEventID event, uint64_t x1);
diff --git a/trace/stderr.h b/trace/stderr.h
deleted file mode 100644
index d575b61..0000000
--- a/trace/stderr.h
+++ /dev/null
@@ -1,11 +0,0 @@
-#ifndef TRACE_STDERR_H
-#define TRACE_STDERR_H
-
-typedef uint64_t TraceEventID;
-
-typedef struct {
-    const char *tp_name;
-    bool state;
-} TraceEvent;
-
-#endif /* ! TRACE_STDERR_H */

[Qemu-devel] [PATCH v4 7/7] trace: [stderr] Port to generic event information and new control interface

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 scripts/tracetool/backend/stderr.py |   27 ++++++++-------------------
 trace/stderr.c                      |   35 +++++++----------------------------
 2 files changed, 15 insertions(+), 47 deletions(-)

diff --git a/scripts/tracetool/backend/stderr.py b/scripts/tracetool/backend/stderr.py
index a10fbb8..6f93dbd 100644
--- a/scripts/tracetool/backend/stderr.py
+++ b/scripts/tracetool/backend/stderr.py
@@ -20,40 +20,29 @@ PUBLIC = True
 
 
 def c(events):
-    out('#include "trace.h"',
-        '',
-        'TraceEvent trace_list[] = {')
-
-    for e in events:
-        out('{.tp_name = "%(name)s", .state=0},',
-            name = e.name,
-            )
-
-    out('};')
+    pass
 
 def h(events):
     out('#include <stdio.h>',
-        '#include "trace/stderr.h"',
+        '#include "trace/control.h"',
         '',
-        'extern TraceEvent trace_list[];')
+        )
 
-    for num, e in enumerate(events):
+    for e in events:
         argnames = ", ".join(e.args.names())
         if len(e.args) > 0:
             argnames = ", " + argnames
 
         out('static inline void trace_%(name)s(%(args)s)',
             '{',
-            '    if (trace_list[%(event_num)s].state != 0) {',
+            '    bool _state = trace_event_get_state(%(event_id)s);',
+            '    if (_state) {',
             '        fprintf(stderr, "%(name)s " %(fmt)s "\\n" %(argnames)s);',
             '    }',
             '}',
             name = e.name,
             args = e.args,
-            event_num = num,
-            fmt = e.fmt,
+            event_id = "TRACE_" + e.name.upper(),
+            fmt = e.fmt.rstrip("\n"),
             argnames = argnames,
             )
-
-    out('',
-        '#define NR_TRACE_EVENTS %d' % len(events))
diff --git a/trace/stderr.c b/trace/stderr.c
index 0810d6f..3d5f313 100644
--- a/trace/stderr.c
+++ b/trace/stderr.c
@@ -4,40 +4,19 @@
 
 void trace_print_events(FILE *stream, fprintf_function stream_printf)
 {
-    unsigned int i;
+    TraceEventID i;
 
-    for (i = 0; i < NR_TRACE_EVENTS; i++) {
+    for (i = 0; i < trace_event_count(); i++) {
+        TraceEvent *ev = trace_event_id(i);
         stream_printf(stream, "%s [Event ID %u] : state %u\n",
-                      trace_list[i].tp_name, i, trace_list[i].state);
+                      trace_event_get_name(ev), i, trace_event_get_state_dynamic(ev));
     }
 }
 
-bool trace_event_set_state(const char *name, bool state)
+void trace_event_set_state_dynamic(TraceEvent *ev, bool state)
 {
-    unsigned int i;
-    unsigned int len;
-    bool wildcard = false;
-    bool matched = false;
-
-    len = strlen(name);
-    if (len > 0 && name[len - 1] == '*') {
-        wildcard = true;
-        len -= 1;
-    }
-    for (i = 0; i < NR_TRACE_EVENTS; i++) {
-        if (wildcard) {
-            if (!strncmp(trace_list[i].tp_name, name, len)) {
-                trace_list[i].state = state;
-                matched = true;
-            }
-            continue;
-        }
-        if (!strcmp(trace_list[i].tp_name, name)) {
-            trace_list[i].state = state;
-            return true;
-        }
-    }
-    return matched;
+    assert(ev != NULL);
+    ev->dstate = state;
 }
 
 bool trace_backend_init(const char *events, const char *file)

Re: [Qemu-devel] [PATCH v4 0/7] trace: Generic event state description

[Lluís Vilanova]

Lluís Vilanova writes:

> Provides a generic event state description and a more detailed event control and
> query interface.

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

ping


> Changes in v4:

> * Documentation fixes and (re)formatting.

> Changes in v3:

> * Add some assertions.

> * Remove debugging printf's.

> * Improve documentation.

> * Make 'trace_event_get_state_static' use run-time information, and leave
>   TRACE_*_ENABLED for compile-time checks.


> Changes in v2:

> * Minor compilation fixes.


> Lluís Vilanova (7):
>       tracetool: Explicitly identify public backends
>       trace: Provide a generic tracing event descriptor
>       trace: Provide a detailed event control interface
>       trace: [monitor] Use new event control interface
>       trace: [default] Use new event control interface
>       trace: [simple] Port to generic event information and new control interface
>       trace: [stderr] Port to generic event information and new control interface


>  Makefile                              |    5 +
>  Makefile.objs                         |   21 ++++
>  docs/tracing.txt                      |   42 +++----
>  monitor.c                             |   15 ++-
>  scripts/tracetool.py                  |    4 -
>  scripts/tracetool/backend/__init__.py |   16 +++
>  scripts/tracetool/backend/dtrace.py   |    3 +
>  scripts/tracetool/backend/events.py   |   23 ++++
>  scripts/tracetool/backend/simple.py   |   15 +--
>  scripts/tracetool/backend/stderr.py   |   28 ++---
>  scripts/tracetool/backend/ust.py      |    3 +
>  scripts/tracetool/format/events_c.py  |   39 +++++++
>  scripts/tracetool/format/events_h.py  |   50 +++++++++
>  scripts/tracetool/format/h.py         |    9 --
>  trace/control-internal.h              |   60 +++++++++++
>  trace/control.c                       |   92 +++++++++++++++-
>  trace/control.h                       |  185 ++++++++++++++++++++++++++++++---
>  trace/default.c                       |    6 +
>  trace/event-internal.h                |   33 ++++++
>  trace/simple.c                        |   37 ++-----
>  trace/simple.h                        |    6 -
>  trace/stderr.c                        |   35 +-----
>  trace/stderr.h                        |   11 --
>  23 files changed, 568 insertions(+), 170 deletions(-)
>  create mode 100644 scripts/tracetool/backend/events.py
>  create mode 100644 scripts/tracetool/format/events_c.py
>  create mode 100644 scripts/tracetool/format/events_h.py
>  create mode 100644 trace/control-internal.h
>  create mode 100644 trace/event-internal.h
>  delete mode 100644 trace/stderr.h


> To: qemu-devel@nongnu.org
> Cc: Stefan Hajnoczi <stefanha@gmail.com>
> Cc: Blue Swirl <blauwirbel@gmail.com>


-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Re: [Qemu-devel] [PATCH v4 3/7] trace: Provide a detailed event control interface

[Stefan Hajnoczi]

On Tue, May 8, 2012 at 3:38 PM, Lluís Vilanova <vilanova@ac.upc.edu> wrote:
> @@ -27,10 +96,19 @@ void trace_backend_init_events(const char *fname)
>         size_t len = strlen(line_buf);
>         if (len > 1) {              /* skip empty lines */
>             line_buf[len - 1] = '\0';
> -            if (!trace_event_set_state(line_buf, true)) {
> -                fprintf(stderr,
> -                        "error: trace event '%s' does not exist\n", line_buf);
> -                exit(1);
> +            if (trace_event_is_pattern(line_buf)) {
> +                TraceEvent *ev;
> +                while ((ev = trace_event_pattern(line_buf, ev)) != NULL) {

ev must be initialized to NULL.

Stefan

Re: [Qemu-devel] [PATCH v4 4/7] trace: [monitor] Use new event control interface

[Stefan Hajnoczi]

On Tue, May 8, 2012 at 3:38 PM, Lluís Vilanova <vilanova@ac.upc.edu> wrote:
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
>  monitor.c |   15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 8946a10..86c2538 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -625,10 +625,19 @@ static void do_trace_event_set_state(Monitor *mon, const QDict *qdict)
>  {
>     const char *tp_name = qdict_get_str(qdict, "name");
>     bool new_state = qdict_get_bool(qdict, "option");
> -    int ret = trace_event_set_state(tp_name, new_state);
>
> -    if (!ret) {
> -        monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
> +    if (trace_event_is_pattern(tp_name)) {
> +        TraceEvent *ev = NULL;
> +        while ((ev = trace_event_pattern(tp_name, ev)) != NULL) {
> +            trace_event_set_state_dynamic(ev, new_state);
> +        }
> +    } else {
> +        TraceEvent *ev = trace_event_name(tp_name);
> +        if (ev == NULL) {
> +            monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
> +        } else {
> +            trace_event_set_state_dynamic(ev, new_state);
> +        }

Why check for a pattern and split the code in two?  How about just:

while ((ev = trace_event_pattern(tp_name, ev)) != NULL) {
    ...
}

That should cover both the single trace event name case and the wildcard case.

Stefan

Re: [Qemu-devel] [PATCH v4 4/7] trace: [monitor] Use new event control interface

[Lluís Vilanova]

Stefan Hajnoczi writes:

> On Tue, May 8, 2012 at 3:38 PM, Lluís Vilanova <vilanova@ac.upc.edu> wrote:
>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>> ---
>>  monitor.c |   15 ++++++++++++---
>>  1 file changed, 12 insertions(+), 3 deletions(-)
>> 
>> diff --git a/monitor.c b/monitor.c
>> index 8946a10..86c2538 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -625,10 +625,19 @@ static void do_trace_event_set_state(Monitor *mon, const QDict *qdict)
>>  {
>>     const char *tp_name = qdict_get_str(qdict, "name");
>>     bool new_state = qdict_get_bool(qdict, "option");
>> -    int ret = trace_event_set_state(tp_name, new_state);
>> 
>> -    if (!ret) {
>> -        monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
>> +    if (trace_event_is_pattern(tp_name)) {
>> +        TraceEvent *ev = NULL;
>> +        while ((ev = trace_event_pattern(tp_name, ev)) != NULL) {
>> +            trace_event_set_state_dynamic(ev, new_state);
>> +        }
>> +    } else {
>> +        TraceEvent *ev = trace_event_name(tp_name);
>> +        if (ev == NULL) {
>> +            monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
>> +        } else {
>> +            trace_event_set_state_dynamic(ev, new_state);
>> +        }

> Why check for a pattern and split the code in two?  How about just:

> while ((ev = trace_event_pattern(tp_name, ev)) != NULL) {
>     ...
> }

> That should cover both the single trace event name case and the wildcard case.

That's true... it's just that somehow I thought it was abusive to use pattern
matching on a string without patterns :)



Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Re: [Qemu-devel] [PATCH v4 1/7] tracetool: Explicitly identify public backends

[Stefan Hajnoczi]

On Tue, May 8, 2012 at 3:38 PM, Lluís Vilanova <vilanova@ac.upc.edu> wrote:
> Public backends are those printed by "--list-backends" and thus considered valid
> by the configure script.
>
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
>  scripts/tracetool.py                  |    4 ++--
>  scripts/tracetool/backend/__init__.py |   16 +++++++++++++++-
>  scripts/tracetool/backend/dtrace.py   |    3 +++
>  scripts/tracetool/backend/simple.py   |    3 +++
>  scripts/tracetool/backend/stderr.py   |    3 +++
>  scripts/tracetool/backend/ust.py      |    3 +++
>  6 files changed, 29 insertions(+), 3 deletions(-)

Is there any use for PUBLIC in qemu.git/master at this time?

If there is no real user in mainline yet then I'd rather not take this code.

Stefan

Re: [Qemu-devel] [PATCH v4 0/7] trace: Generic event state description

[Stefan Hajnoczi]

On Tue, May 8, 2012 at 3:38 PM, Lluís Vilanova <vilanova@ac.upc.edu> wrote:
> Provides a generic event state description and a more detailed event control and
> query interface.
>
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
>
> Changes in v4:
>
> * Documentation fixes and (re)formatting.
>
> Changes in v3:
>
> * Add some assertions.
>
> * Remove debugging printf's.
>
> * Improve documentation.
>
> * Make 'trace_event_get_state_static' use run-time information, and leave
>  TRACE_*_ENABLED for compile-time checks.
>
>
> Changes in v2:
>
> * Minor compilation fixes.
>
>
> Lluís Vilanova (7):
>      tracetool: Explicitly identify public backends
>      trace: Provide a generic tracing event descriptor
>      trace: Provide a detailed event control interface
>      trace: [monitor] Use new event control interface
>      trace: [default] Use new event control interface
>      trace: [simple] Port to generic event information and new control interface
>      trace: [stderr] Port to generic event information and new control interface
>
>
>  Makefile                              |    5 +
>  Makefile.objs                         |   21 ++++
>  docs/tracing.txt                      |   42 +++----
>  monitor.c                             |   15 ++-
>  scripts/tracetool.py                  |    4 -
>  scripts/tracetool/backend/__init__.py |   16 +++
>  scripts/tracetool/backend/dtrace.py   |    3 +
>  scripts/tracetool/backend/events.py   |   23 ++++
>  scripts/tracetool/backend/simple.py   |   15 +--
>  scripts/tracetool/backend/stderr.py   |   28 ++---
>  scripts/tracetool/backend/ust.py      |    3 +
>  scripts/tracetool/format/events_c.py  |   39 +++++++
>  scripts/tracetool/format/events_h.py  |   50 +++++++++
>  scripts/tracetool/format/h.py         |    9 --
>  trace/control-internal.h              |   60 +++++++++++
>  trace/control.c                       |   92 +++++++++++++++-
>  trace/control.h                       |  185 ++++++++++++++++++++++++++++++---
>  trace/default.c                       |    6 +
>  trace/event-internal.h                |   33 ++++++
>  trace/simple.c                        |   37 ++-----
>  trace/simple.h                        |    6 -
>  trace/stderr.c                        |   35 +-----
>  trace/stderr.h                        |   11 --
>  23 files changed, 568 insertions(+), 170 deletions(-)
>  create mode 100644 scripts/tracetool/backend/events.py
>  create mode 100644 scripts/tracetool/format/events_c.py
>  create mode 100644 scripts/tracetool/format/events_h.py
>  create mode 100644 trace/control-internal.h
>  create mode 100644 trace/event-internal.h
>  delete mode 100644 trace/stderr.h

What is the point of sstate vs dstate?  It seems the dynamic state is
what can be toggled and sstate is the "disable" keyword.  Why have
sstate since there is already a macro?

Stefan

Re: [Qemu-devel] [PATCH v4 1/7] tracetool: Explicitly identify public backends

[Lluís Vilanova]

Stefan Hajnoczi writes:

> On Tue, May 8, 2012 at 3:38 PM, Lluís Vilanova <vilanova@ac.upc.edu> wrote:
>> Public backends are those printed by "--list-backends" and thus considered valid
>> by the configure script.
>> 
>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>> ---
>>  scripts/tracetool.py                  |    4 ++--
>>  scripts/tracetool/backend/__init__.py |   16 +++++++++++++++-
>>  scripts/tracetool/backend/dtrace.py   |    3 +++
>>  scripts/tracetool/backend/simple.py   |    3 +++
>>  scripts/tracetool/backend/stderr.py   |    3 +++
>>  scripts/tracetool/backend/ust.py      |    3 +++
>>  6 files changed, 29 insertions(+), 3 deletions(-)

> Is there any use for PUBLIC in qemu.git/master at this time?

> If there is no real user in mainline yet then I'd rather not take this code.

Yes, next patch adds a new backend that should not be selectable by the user at
configure time.


Lluis


-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Re: [Qemu-devel] [PATCH v4 0/7] trace: Generic event state description

[Lluís Vilanova]

Stefan Hajnoczi writes:

> On Tue, May 8, 2012 at 3:38 PM, Lluís Vilanova <vilanova@ac.upc.edu> wrote:
>> Provides a generic event state description and a more detailed event control and
>> query interface.
>> 
>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>> ---
[...]
> What is the point of sstate vs dstate?  It seems the dynamic state is
> what can be toggled and sstate is the "disable" keyword.  Why have
> sstate since there is already a macro?

'sstate' is there just in case you query the tracing state of an event through
the 'TraceEvent' structure instead of through the corresponding macro.


Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Re: [Qemu-devel] [PATCH v4 0/7] trace: Generic event state description

[Stefan Hajnoczi]

On Mon, Jun 11, 2012 at 12:12 PM, Lluís Vilanova <vilanova@ac.upc.edu> wrote:
> Stefan Hajnoczi writes:
>
>> On Tue, May 8, 2012 at 3:38 PM, Lluís Vilanova <vilanova@ac.upc.edu> wrote:
>>> Provides a generic event state description and a more detailed event control and
>>> query interface.
>>>
>>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>>> ---
> [...]
>> What is the point of sstate vs dstate?  It seems the dynamic state is
>> what can be toggled and sstate is the "disable" keyword.  Why have
>> sstate since there is already a macro?
>
> 'sstate' is there just in case you query the tracing state of an event through
> the 'TraceEvent' structure instead of through the corresponding macro.

If this is not used by a tracer today we should hold off until it's needed.

Stefan

Re: [Qemu-devel] [PATCH v4 0/7] trace: Generic event state description

[Lluís Vilanova]

Stefan Hajnoczi writes:

> On Mon, Jun 11, 2012 at 12:12 PM, Lluís Vilanova <vilanova@ac.upc.edu> wrote:
>> Stefan Hajnoczi writes:
>> 
>>> On Tue, May 8, 2012 at 3:38 PM, Lluís Vilanova <vilanova@ac.upc.edu> wrote:
>>>> Provides a generic event state description and a more detailed event control and
>>>> query interface.
>>>> 
>>>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>>>> ---
>> [...]
>>> What is the point of sstate vs dstate?  It seems the dynamic state is
>>> what can be toggled and sstate is the "disable" keyword.  Why have
>>> sstate since there is already a macro?
>> 
>> 'sstate' is there just in case you query the tracing state of an event through
>> the 'TraceEvent' structure instead of through the corresponding macro.

> If this is not used by a tracer today we should hold off until it's needed.

I double-checked the code for uses of that and it appears to be there just for
the sake of completeness.

Still, I realized I should have added a check in 'trace_event_set_state_dynamic'
and assert that the event is statically enabled (otherwise dynamically enabling
an event that is statically disabled just does not make sense).

Whatever you prefer, although the code is pretty simple.


Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Re: [Qemu-devel] [PATCH v4 0/7] trace: Generic event state description

[Stefan Hajnoczi]

On Mon, Jun 11, 2012 at 2:52 PM, Lluís Vilanova <vilanova@ac.upc.edu> wrote:
> Stefan Hajnoczi writes:
>
>> On Mon, Jun 11, 2012 at 12:12 PM, Lluís Vilanova <vilanova@ac.upc.edu> wrote:
>>> Stefan Hajnoczi writes:
>>>
>>>> On Tue, May 8, 2012 at 3:38 PM, Lluís Vilanova <vilanova@ac.upc.edu> wrote:
>>>>> Provides a generic event state description and a more detailed event control and
>>>>> query interface.
>>>>>
>>>>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>>>>> ---
>>> [...]
>>>> What is the point of sstate vs dstate?  It seems the dynamic state is
>>>> what can be toggled and sstate is the "disable" keyword.  Why have
>>>> sstate since there is already a macro?
>>>
>>> 'sstate' is there just in case you query the tracing state of an event through
>>> the 'TraceEvent' structure instead of through the corresponding macro.
>
>> If this is not used by a tracer today we should hold off until it's needed.
>
> I double-checked the code for uses of that and it appears to be there just for
> the sake of completeness.
>
> Still, I realized I should have added a check in 'trace_event_set_state_dynamic'
> and assert that the event is statically enabled (otherwise dynamically enabling
> an event that is statically disabled just does not make sense).
>
> Whatever you prefer, although the code is pretty simple.

I'd like to avoid adding things that are unused - even if they are small.

Stefan

Re: [Qemu-devel] [PATCH v4 4/7] trace: [monitor] Use new event control interface

[Lluís Vilanova]

Lluís Vilanova writes:

> Stefan Hajnoczi writes:
>> On Tue, May 8, 2012 at 3:38 PM, Lluís Vilanova <vilanova@ac.upc.edu> wrote:
>>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>>> ---
>>>  monitor.c |   15 ++++++++++++---
>>>  1 file changed, 12 insertions(+), 3 deletions(-)
>>> 
>>> diff --git a/monitor.c b/monitor.c
>>> index 8946a10..86c2538 100644
>>> --- a/monitor.c
>>> +++ b/monitor.c
>>> @@ -625,10 +625,19 @@ static void do_trace_event_set_state(Monitor *mon, const QDict *qdict)
>>>  {
>>>     const char *tp_name = qdict_get_str(qdict, "name");
>>>     bool new_state = qdict_get_bool(qdict, "option");
>>> -    int ret = trace_event_set_state(tp_name, new_state);
>>> 
>>> -    if (!ret) {
>>> -        monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
>>> +    if (trace_event_is_pattern(tp_name)) {
>>> +        TraceEvent *ev = NULL;
>>> +        while ((ev = trace_event_pattern(tp_name, ev)) != NULL) {
>>> +            trace_event_set_state_dynamic(ev, new_state);
>>> +        }
>>> +    } else {
>>> +        TraceEvent *ev = trace_event_name(tp_name);
>>> +        if (ev == NULL) {
>>> +            monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
>>> +        } else {
>>> +            trace_event_set_state_dynamic(ev, new_state);
>>> +        }

>> Why check for a pattern and split the code in two?  How about just:

>> while ((ev = trace_event_pattern(tp_name, ev)) != NULL) {
>> ...
>> }

>> That should cover both the single trace event name case and the wildcard case.

> That's true... it's just that somehow I thought it was abusive to use pattern
> matching on a string without patterns :)

When going through the code to add your comments I realized why it made sense to
use 'trace_event_name' instead of 'trace_event_pattern'.

In the case the string contains no pattern, not finding a result is an error in
'trace_backend_init_events' (when reading the list of events given in the
commandline file), as well as in 'do_trace_event_set_state' (when setting the
state from the monitor, although here the error is not fatal).

In comparison, setting by pattern never fails.


Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Re: [Qemu-devel] [PATCH v4 4/7] trace: [monitor] Use new event control interface

[Stefan Hajnoczi]

On Mon, Jun 11, 2012 at 6:20 PM, Lluís Vilanova <vilanova@ac.upc.edu> wrote:
> Lluís Vilanova writes:
>
>> Stefan Hajnoczi writes:
>>> On Tue, May 8, 2012 at 3:38 PM, Lluís Vilanova <vilanova@ac.upc.edu> wrote:
>>>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>>>> ---
>>>>  monitor.c |   15 ++++++++++++---
>>>>  1 file changed, 12 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/monitor.c b/monitor.c
>>>> index 8946a10..86c2538 100644
>>>> --- a/monitor.c
>>>> +++ b/monitor.c
>>>> @@ -625,10 +625,19 @@ static void do_trace_event_set_state(Monitor *mon, const QDict *qdict)
>>>>  {
>>>>     const char *tp_name = qdict_get_str(qdict, "name");
>>>>     bool new_state = qdict_get_bool(qdict, "option");
>>>> -    int ret = trace_event_set_state(tp_name, new_state);
>>>>
>>>> -    if (!ret) {
>>>> -        monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
>>>> +    if (trace_event_is_pattern(tp_name)) {
>>>> +        TraceEvent *ev = NULL;
>>>> +        while ((ev = trace_event_pattern(tp_name, ev)) != NULL) {
>>>> +            trace_event_set_state_dynamic(ev, new_state);
>>>> +        }
>>>> +    } else {
>>>> +        TraceEvent *ev = trace_event_name(tp_name);
>>>> +        if (ev == NULL) {
>>>> +            monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
>>>> +        } else {
>>>> +            trace_event_set_state_dynamic(ev, new_state);
>>>> +        }
>
>>> Why check for a pattern and split the code in two?  How about just:
>
>>> while ((ev = trace_event_pattern(tp_name, ev)) != NULL) {
>>> ...
>>> }
>
>>> That should cover both the single trace event name case and the wildcard case.
>
>> That's true... it's just that somehow I thought it was abusive to use pattern
>> matching on a string without patterns :)
>
> When going through the code to add your comments I realized why it made sense to
> use 'trace_event_name' instead of 'trace_event_pattern'.
>
> In the case the string contains no pattern, not finding a result is an error in
> 'trace_backend_init_events' (when reading the list of events given in the
> commandline file), as well as in 'do_trace_event_set_state' (when setting the
> state from the monitor, although here the error is not fatal).
>
> In comparison, setting by pattern never fails.

I see.  Then the single code-path implementation looks like this:

bool found = false;
while ((ev = trace_event_pattern(tp_name, ev)) != NULL) {
   found = true;
   ...
}
if (!found && !trace_event_is_pattern(tp_name)) {
   fprintf(stderr, "ERROR!");
}

It's up to you which you prefer.

Stefan