[Qemu-devel] [PATCH 00/14] trace: Add static tracing to QEMU

[Qemu-devel] [PATCH 00/14] trace: Add static tracing to QEMU

[Stefan Hajnoczi]

This patch series adds static tracing to QEMU.  It can be used to instrument
QEMU code by means of lightweight logging called trace events.

Prerna and I are now posting the entire patch series with a serious eye towards
checking we meet users' and developers' tracing needs and with the goal of
getting this functionality merged into qemu.git.

Tracing infrastructure allows debugging, performance analysis, and observation
to be performed.  Right now there are ad-hoc logging calls in some source files
which require rebuilding QEMU with certain #defines and perform poorly.  This
patch series introduces a single tracing infrastructure which is easy to use
and can replace ad-hoc techniques.

Two key points:

1. The trace-events file contains the set of defined trace events which can be
   called from a source file.  For example, trace-events has:

    qemu_malloc(size_t size, void *ptr) "size %zu ptr %p"

   and qemu-malloc.c uses this trace event like this:

    #include "trace.h"  /* needed for trace event prototype */

    void *qemu_malloc(size_t size)
    {
        void *ptr;
        if (!size && !allow_zero_malloc()) {
            abort();
        }
        ptr = oom_check(malloc(size ? size : 1));
        trace_qemu_malloc(size, ptr);  /* <-- trace event */
        return ptr;
    }

2. The built-in 'simple' trace backend writes binary traces to
   /tmp/trace-<pid>.  They can be pretty-printed like this:

    ./simpletrace.py trace-events /tmp/trace-*

   Although you can also try LTTng Userspace Tracer ('ust' trace backend), the
   'simple' trace backend provides commands within the QEMU monitor to
   enable/disable trace events at runtime.  It is easy to use and should serve
   as a good default trace backend.

   The 'simple' trace backend's limitation is that it isn't thread-safe and can
   therefore only trace correctly when the QEMU global mutex is held.

For full documentation, see:
http://repo.or.cz/w/qemu/stefanha.git/blob_plain/8d9f8f62d5af986fab6636770450f57fbd0d8b66:/docs/tracing.txt

The git branch is here:
http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/tracing

Stefan

[Qemu-devel] [PATCH 01/14] trace: Add trace-events file for declaring trace events

[Stefan Hajnoczi]

This patch introduces the trace-events file where trace events can be
declared like so:

qemu_malloc(size_t size) "size %zu"
qemu_free(void *ptr) "ptr %p"

These trace event declarations are processed by a new tool called
tracetool to generate code for the trace events.  Trace event
declarations are independent of the backend tracing system (LTTng User
Space Tracing, ftrace markers, DTrace).

The default "nop" backend generates empty trace event functions.
Therefore trace events are disabled by default.

The trace-events file serves two purposes:

1. Adding trace events is easy.  It is not necessary to understand the
   details of a backend tracing system.  The trace-events file is a
   single location where trace events can be declared without code
   duplication.

2. QEMU is not tightly coupled to one particular backend tracing system.
   In order to support tracing across QEMU host platforms and to
   anticipate new backend tracing systems that are currently maturing,
   it is important to be flexible and not tied to one system.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

trace: Rebuild when switching trace backends

Fix to ensure rebuild is properly triggered when switching trace backends
using ./configure.

Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 .gitignore      |    2 +
 Makefile        |   17 ++++-
 Makefile.objs   |    5 ++
 Makefile.target |    1 +
 configure       |   18 ++++++
 trace-events    |   24 ++++++++
 tracetool       |  175 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 238 insertions(+), 4 deletions(-)
 create mode 100644 trace-events
 create mode 100755 tracetool

diff --git a/.gitignore b/.gitignore
index ec6f89f..f3f2bb7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -2,6 +2,8 @@ config-devices.*
 config-all-devices.*
 config-host.*
 config-target.*
+trace.h
+trace.c
 *-softmmu
 *-darwin-user
 *-linux-user
diff --git a/Makefile b/Makefile
index f95cc2f..3c5e6a0 100644
--- a/Makefile
+++ b/Makefile
@@ -1,6 +1,6 @@
 # Makefile for QEMU.
 
-GENERATED_HEADERS = config-host.h
+GENERATED_HEADERS = config-host.h trace.h
 
 ifneq ($(wildcard config-host.mak),)
 # Put the all: rule here so that config-host.mak can contain dependencies.
@@ -104,16 +104,24 @@ ui/vnc.o: QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
 
 bt-host.o: QEMU_CFLAGS += $(BLUEZ_CFLAGS)
 
+trace.h: $(SRC_PATH)/trace-events config-host.mak
+	$(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -h < $< > $@,"  GEN   $@")
+
+trace.c: $(SRC_PATH)/trace-events config-host.mak
+	$(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -c < $< > $@,"  GEN   $@")
+
+trace.o: trace.c $(GENERATED_HEADERS)
+
 ######################################################################
 
 qemu-img.o: qemu-img-cmds.h
 qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o: $(GENERATED_HEADERS)
 
-qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-error.o $(block-obj-y) $(qobject-obj-y)
+qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-error.o $(trace-obj-y) $(block-obj-y) $(qobject-obj-y)
 
-qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-error.o $(block-obj-y) $(qobject-obj-y)
+qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-error.o $(trace-obj-y) $(block-obj-y) $(qobject-obj-y)
 
-qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o $(block-obj-y) $(qobject-obj-y)
+qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o $(trace-obj-y) $(block-obj-y) $(qobject-obj-y)
 
 qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
 	$(call quiet-command,sh $(SRC_PATH)/hxtool -h < $< > $@,"  GEN   $@")
@@ -133,6 +141,7 @@ clean:
 	rm -f *.o *.d *.a $(TOOLS) TAGS cscope.* *.pod *~ */*~
 	rm -f slirp/*.o slirp/*.d audio/*.o audio/*.d block/*.o block/*.d net/*.o net/*.d fsdev/*.o fsdev/*.d ui/*.o ui/*.d
 	rm -f qemu-img-cmds.h
+	rm -f trace.c trace.h
 	$(MAKE) -C tests clean
 	for d in $(ALL_SUBDIRS) libhw32 libhw64 libuser libdis libdis-user; do \
 	if test -d $$d; then $(MAKE) -C $$d $@ || exit 1; fi; \
diff --git a/Makefile.objs b/Makefile.objs
index 4a1eaa1..c61332d 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -265,6 +265,11 @@ libdis-$(CONFIG_S390_DIS) += s390-dis.o
 libdis-$(CONFIG_SH4_DIS) += sh4-dis.o
 libdis-$(CONFIG_SPARC_DIS) += sparc-dis.o
 
+######################################################################
+# trace
+
+trace-obj-y = trace.o
+
 vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
 
 vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS)
diff --git a/Makefile.target b/Makefile.target
index b791492..d3f5878 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -310,6 +310,7 @@ obj-y += $(addprefix $(HWDIR)/, $(hw-obj-y))
 
 endif # CONFIG_SOFTMMU
 
+obj-y += $(addprefix ../, $(trace-obj-y))
 obj-$(CONFIG_GDBSTUB_XML) += gdbstub-xml.o
 
 $(QEMU_PROG): $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y)
diff --git a/configure b/configure
index a20371c..d3648b2 100755
--- a/configure
+++ b/configure
@@ -315,6 +315,7 @@ pkgversion=""
 check_utests="no"
 user_pie="no"
 zero_malloc=""
+trace_backend="nop"
 
 # OS specific
 if check_define __linux__ ; then
@@ -517,6 +518,8 @@ for opt do
   ;;
   --target-list=*) target_list="$optarg"
   ;;
+  --trace-backend=*) trace_backend="$optarg"
+  ;;
   --enable-gprof) gprof="yes"
   ;;
   --static)
@@ -895,6 +898,7 @@ echo "  --enable-docs            enable documentation build"
 echo "  --disable-docs           disable documentation build"
 echo "  --disable-vhost-net      disable vhost-net acceleration support"
 echo "  --enable-vhost-net       enable vhost-net acceleration support"
+echo "  --trace-backend=B        Trace backend nop"
 echo ""
 echo "NOTE: The object files are built at the place where configure is launched"
 exit 1
@@ -2057,6 +2061,18 @@ if compile_prog "" "" ; then
     fdatasync=yes
 fi
 
+##########################################
+# check if trace backend exists
+
+sh "$source_path/tracetool" "--$trace_backend" --check-backend > /dev/null 2> /dev/null
+if test "$?" -ne 0 ; then
+  echo
+  echo "Error: invalid trace backend"
+  echo "Please choose a supported trace backend."
+  echo
+  exit 1
+fi
+
 # End of CC checks
 # After here, no more $cc or $ld runs
 
@@ -2187,6 +2203,7 @@ echo "preadv support    $preadv"
 echo "fdatasync         $fdatasync"
 echo "uuid support      $uuid"
 echo "vhost-net support $vhost_net"
+echo "Trace backend     $trace_backend"
 
 if test $sdl_too_old = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -2448,6 +2465,7 @@ bsd)
 ;;
 esac
 
+echo "TRACE_BACKEND=$trace_backend" >> $config_host_mak
 echo "TOOLS=$tools" >> $config_host_mak
 echo "ROMS=$roms" >> $config_host_mak
 echo "MAKE=$make" >> $config_host_mak
diff --git a/trace-events b/trace-events
new file mode 100644
index 0000000..a37d3cc
--- /dev/null
+++ b/trace-events
@@ -0,0 +1,24 @@
+# Trace events for debugging and performance instrumentation
+#
+# This file is processed by the tracetool script during the build.
+#
+# To add a new trace event:
+#
+# 1. Choose a name for the trace event.  Declare its arguments and format
+#    string.
+#
+# 2. Call the trace event from code using trace_##name, e.g. multiwrite_cb() ->
+#    trace_multiwrite_cb().  The source file must #include "trace.h".
+#
+# Format of a trace event:
+#
+# <name>(<type1> <arg1>[, <type2> <arg2>] ...) "<format-string>"
+#
+# Example: qemu_malloc(size_t size) "size %zu"
+#
+# The <name> must be a valid as a C function name.
+#
+# Types should be standard C types.  Use void * for pointers because the trace
+# system may not have the necessary headers included.
+#
+# The <format-string> should be a sprintf()-compatible format string.
diff --git a/tracetool b/tracetool
new file mode 100755
index 0000000..86b6446
--- /dev/null
+++ b/tracetool
@@ -0,0 +1,175 @@
+#!/bin/sh
+#
+# Code generator for trace events
+#
+# Copyright IBM, Corp. 2010
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+
+# Disable pathname expansion, makes processing text with '*' characters simpler
+set -f
+
+usage()
+{
+    cat >&2 <<EOF
+usage: $0 --nop [-h | -c]
+Generate tracing code for a file on stdin.
+
+Backends:
+  --nop Tracing disabled
+
+Output formats:
+  -h    Generate .h file
+  -c    Generate .c file
+EOF
+    exit 1
+}
+
+# Get the name of a trace event
+get_name()
+{
+    echo ${1%%(*}
+}
+
+# Get the argument list of a trace event, including types and names
+get_args()
+{
+    local args
+    args=${1#*(}
+    args=${args%)*}
+    echo "$args"
+}
+
+# Get the argument name list of a trace event
+get_argnames()
+{
+    local nfields field name
+    nfields=0
+    for field in $(get_args "$1"); do
+        nfields=$((nfields + 1))
+
+        # Drop pointer star
+        field=${field#\*}
+
+        # Only argument names have commas at the end
+        name=${field%,}
+        test "$field" = "$name" && continue
+
+        echo -n "$name, "
+    done
+
+    # Last argument name
+    if [ "$nfields" -gt 1 ]
+    then
+        echo -n "$name"
+    fi
+}
+
+# Get the format string for a trace event
+get_fmt()
+{
+    local fmt
+    fmt=${1#*\"}
+    fmt=${fmt%\"*}
+    echo "$fmt"
+}
+
+linetoh_begin_nop()
+{
+    return
+}
+
+linetoh_nop()
+{
+    local name args
+    name=$(get_name "$1")
+    args=$(get_args "$1")
+
+    # Define an empty function for the trace event
+    cat <<EOF
+static inline void trace_$name($args)
+{
+}
+EOF
+}
+
+linetoh_end_nop()
+{
+    return
+}
+
+linetoc_begin_nop()
+{
+    return
+}
+
+linetoc_nop()
+{
+    # No need for function definitions in nop backend
+    return
+}
+
+linetoc_end_nop()
+{
+    return
+}
+
+# Process stdin by calling begin, line, and end functions for the backend
+convert()
+{
+    local begin process_line end
+    begin="lineto$1_begin_$backend"
+    process_line="lineto$1_$backend"
+    end="lineto$1_end_$backend"
+
+    "$begin"
+
+    while read -r str; do
+        # Skip comments and empty lines
+        str=${str%%#*}
+        test -z "$str" && continue
+
+        echo
+        "$process_line" "$str"
+    done
+
+    echo
+    "$end"
+}
+
+tracetoh()
+{
+    cat <<EOF
+#ifndef TRACE_H
+#define TRACE_H
+
+/* This file is autogenerated by tracetool, do not edit. */
+
+#include "qemu-common.h"
+EOF
+    convert h
+    echo "#endif /* TRACE_H */"
+}
+
+tracetoc()
+{
+    echo "/* This file is autogenerated by tracetool, do not edit. */"
+    convert c
+}
+
+# Choose backend
+case "$1" in
+"--nop") backend="${1#--}" ;;
+*) usage ;;
+esac
+shift
+
+case "$1" in
+"-h") tracetoh ;;
+"-c") tracetoc ;;
+"--check-backend") exit 0 ;; # used by ./configure to test for backend
+*) usage ;;
+esac
+
+exit 0
-- 
1.7.1

[Qemu-devel] [PATCH 02/14] trace: Trace qemu_malloc() and qemu_vmalloc()

[Stefan Hajnoczi]

It is often useful to instrument memory management functions in order to
find leaks or performance problems.  This patch adds trace events for
the memory allocation primitives.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 osdep.c       |   24 ++++++++++++++++++------
 qemu-malloc.c |   12 ++++++++++--
 trace-events  |   10 ++++++++++
 3 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/osdep.c b/osdep.c
index dbf872a..30426ff 100644
--- a/osdep.c
+++ b/osdep.c
@@ -50,6 +50,7 @@
 #endif
 
 #include "qemu-common.h"
+#include "trace.h"
 #include "sysemu.h"
 #include "qemu_socket.h"
 
@@ -71,25 +72,34 @@ static void *oom_check(void *ptr)
 #if defined(_WIN32)
 void *qemu_memalign(size_t alignment, size_t size)
 {
+    void *ptr;
+
     if (!size) {
         abort();
     }
-    return oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
+    ptr = oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
+    trace_qemu_memalign(alignment, size, ptr);
+    return ptr;
 }
 
 void *qemu_vmalloc(size_t size)
 {
+    void *ptr;
+
     /* FIXME: this is not exactly optimal solution since VirtualAlloc
        has 64Kb granularity, but at least it guarantees us that the
        memory is page aligned. */
     if (!size) {
         abort();
     }
-    return oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
+    ptr = oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
+    trace_qemu_vmalloc(size, ptr);
+    return ptr;
 }
 
 void qemu_vfree(void *ptr)
 {
+    trace_qemu_vfree(ptr);
     VirtualFree(ptr, 0, MEM_RELEASE);
 }
 
@@ -97,21 +107,22 @@ void qemu_vfree(void *ptr)
 
 void *qemu_memalign(size_t alignment, size_t size)
 {
+    void *ptr;
 #if defined(_POSIX_C_SOURCE) && !defined(__sun__)
     int ret;
-    void *ptr;
     ret = posix_memalign(&ptr, alignment, size);
     if (ret != 0) {
         fprintf(stderr, "Failed to allocate %zu B: %s\n",
                 size, strerror(ret));
         abort();
     }
-    return ptr;
 #elif defined(CONFIG_BSD)
-    return oom_check(valloc(size));
+    ptr = oom_check(valloc(size));
 #else
-    return oom_check(memalign(alignment, size));
+    ptr = oom_check(memalign(alignment, size));
 #endif
+    trace_qemu_memalign(alignment, size, ptr);
+    return ptr;
 }
 
 /* alloc shared memory pages */
@@ -122,6 +133,7 @@ void *qemu_vmalloc(size_t size)
 
 void qemu_vfree(void *ptr)
 {
+    trace_qemu_vfree(ptr);
     free(ptr);
 }
 
diff --git a/qemu-malloc.c b/qemu-malloc.c
index 36b0b36..ecffb67 100644
--- a/qemu-malloc.c
+++ b/qemu-malloc.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 #include "qemu-common.h"
+#include "trace.h"
 #include <stdlib.h>
 
 static void *oom_check(void *ptr)
@@ -34,6 +35,7 @@ static void *oom_check(void *ptr)
 
 void qemu_free(void *ptr)
 {
+    trace_qemu_free(ptr);
     free(ptr);
 }
 
@@ -48,18 +50,24 @@ static int allow_zero_malloc(void)
 
 void *qemu_malloc(size_t size)
 {
+    void *ptr;
     if (!size && !allow_zero_malloc()) {
         abort();
     }
-    return oom_check(malloc(size ? size : 1));
+    ptr = oom_check(malloc(size ? size : 1));
+    trace_qemu_malloc(size, ptr);
+    return ptr;
 }
 
 void *qemu_realloc(void *ptr, size_t size)
 {
+    void *newptr;
     if (!size && !allow_zero_malloc()) {
         abort();
     }
-    return oom_check(realloc(ptr, size ? size : 1));
+    newptr = oom_check(realloc(ptr, size ? size : 1));
+    trace_qemu_realloc(ptr, size, newptr);
+    return newptr;
 }
 
 void *qemu_mallocz(size_t size)
diff --git a/trace-events b/trace-events
index a37d3cc..98250e9 100644
--- a/trace-events
+++ b/trace-events
@@ -22,3 +22,13 @@
 # system may not have the necessary headers included.
 #
 # The <format-string> should be a sprintf()-compatible format string.
+
+# qemu-malloc.c
+qemu_malloc(size_t size, void *ptr) "size %zu ptr %p"
+qemu_realloc(void *ptr, size_t size, void *newptr) "ptr %p size %zu newptr %p"
+qemu_free(void *ptr) "ptr %p"
+
+# osdep.c
+qemu_memalign(size_t alignment, size_t size, void *ptr) "alignment %zu size %zu ptr %p"
+qemu_valloc(size_t size, void *ptr) "size %zu ptr %p"
+qemu_vfree(void *ptr) "ptr %p"
-- 
1.7.1

[Qemu-devel] [PATCH 03/14] trace: Trace virtio-blk, multiwrite, and paio_submit

[Stefan Hajnoczi]

This patch adds trace events that make it possible to observe
virtio-blk.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block.c            |    8 ++++++++
 hw/virtio-blk.c    |    7 +++++++
 posix-aio-compat.c |    2 ++
 trace-events       |   14 ++++++++++++++
 4 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index da70f29..603dd1f 100644
--- a/block.c
+++ b/block.c
@@ -23,6 +23,7 @@
  */
 #include "config-host.h"
 #include "qemu-common.h"
+#include "trace.h"
 #include "monitor.h"
 #include "block_int.h"
 #include "module.h"
@@ -2062,6 +2063,8 @@ static void multiwrite_cb(void *opaque, int ret)
 {
     MultiwriteCB *mcb = opaque;
 
+    trace_multiwrite_cb(mcb, ret);
+
     if (ret < 0 && !mcb->error) {
         mcb->error = ret;
     }
@@ -2202,6 +2205,8 @@ int bdrv_aio_multiwrite(BlockDriverState *bs, BlockRequest *reqs, int num_reqs)
     // Check for mergable requests
     num_reqs = multiwrite_merge(bs, reqs, num_reqs, mcb);
 
+    trace_bdrv_aio_multiwrite(mcb, mcb->num_callbacks, num_reqs);
+
     /*
      * Run the aio requests. As soon as one request can't be submitted
      * successfully, fail all requests that are not yet submitted (we must
@@ -2223,6 +2228,7 @@ int bdrv_aio_multiwrite(BlockDriverState *bs, BlockRequest *reqs, int num_reqs)
      */
     mcb->num_requests = 1;
 
+    // Run the aio requests
     for (i = 0; i < num_reqs; i++) {
         mcb->num_requests++;
         acb = bdrv_aio_writev(bs, reqs[i].sector, reqs[i].qiov,
@@ -2233,8 +2239,10 @@ int bdrv_aio_multiwrite(BlockDriverState *bs, BlockRequest *reqs, int num_reqs)
             // submitted yet. Otherwise we'll wait for the submitted AIOs to
             // complete and report the error in the callback.
             if (i == 0) {
+                trace_bdrv_aio_multiwrite_earlyfail(mcb);
                 goto fail;
             } else {
+                trace_bdrv_aio_multiwrite_latefail(mcb, i);
                 multiwrite_cb(mcb, -EIO);
                 break;
             }
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 490cd41..2acc208 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -13,6 +13,7 @@
 
 #include <qemu-common.h>
 #include "qemu-error.h"
+#include "trace.h"
 #include "virtio-blk.h"
 #ifdef __linux__
 # include <scsi/sg.h>
@@ -51,6 +52,8 @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, int status)
 {
     VirtIOBlock *s = req->dev;
 
+    trace_virtio_blk_req_complete(req, status);
+
     req->in->status = status;
     virtqueue_push(s->vq, &req->elem, req->qiov.size + sizeof(*req->in));
     virtio_notify(&s->vdev, s->vq);
@@ -87,6 +90,8 @@ static void virtio_blk_rw_complete(void *opaque, int ret)
 {
     VirtIOBlockReq *req = opaque;
 
+    trace_virtio_blk_rw_complete(req, ret);
+
     if (ret) {
         int is_read = !(req->out->type & VIRTIO_BLK_T_OUT);
         if (virtio_blk_handle_rw_error(req, -ret, is_read))
@@ -269,6 +274,8 @@ static void virtio_blk_handle_write(VirtIOBlockReq *req, MultiReqBuffer *mrb)
 {
     BlockRequest *blkreq;
 
+    trace_virtio_blk_handle_write(req, req->out->sector, req->qiov.size / 512);
+
     if (req->out->sector & req->dev->sector_mask) {
         virtio_blk_rw_complete(req, -EIO);
         return;
diff --git a/posix-aio-compat.c b/posix-aio-compat.c
index a67ffe3..2e13184 100644
--- a/posix-aio-compat.c
+++ b/posix-aio-compat.c
@@ -25,6 +25,7 @@
 #include "qemu-queue.h"
 #include "osdep.h"
 #include "qemu-common.h"
+#include "trace.h"
 #include "block_int.h"
 
 #include "block/raw-posix-aio.h"
@@ -583,6 +584,7 @@ BlockDriverAIOCB *paio_submit(BlockDriverState *bs, int fd,
     acb->next = posix_aio_state->first_aio;
     posix_aio_state->first_aio = acb;
 
+    trace_paio_submit(acb, opaque, sector_num, nb_sectors, type);
     qemu_paio_submit(acb);
     return &acb->common;
 }
diff --git a/trace-events b/trace-events
index 98250e9..63037a1 100644
--- a/trace-events
+++ b/trace-events
@@ -32,3 +32,17 @@ qemu_free(void *ptr) "ptr %p"
 qemu_memalign(size_t alignment, size_t size, void *ptr) "alignment %zu size %zu ptr %p"
 qemu_valloc(size_t size, void *ptr) "size %zu ptr %p"
 qemu_vfree(void *ptr) "ptr %p"
+
+# block.c
+multiwrite_cb(void *mcb, int ret) "mcb %p ret %d"
+bdrv_aio_multiwrite(void *mcb, int num_callbacks, int num_reqs) "mcb %p num_callbacks %d num_reqs %d"
+bdrv_aio_multiwrite_earlyfail(void *mcb) "mcb %p"
+bdrv_aio_multiwrite_latefail(void *mcb, int i) "mcb %p i %d"
+
+# hw/virtio-blk.c
+virtio_blk_req_complete(void *req, int status) "req %p status %d"
+virtio_blk_rw_complete(void *req, int ret) "req %p ret %d"
+virtio_blk_handle_write(void *req, unsigned long sector, unsigned long nsectors) "req %p sector %lu nsectors %lu"
+
+# posix-aio-compat.c
+paio_submit(void *acb, void *opaque, unsigned long sector_num, unsigned long nb_sectors, unsigned long type) "acb %p opaque %p sector_num %lu nb_sectors %lu type %lu"
-- 
1.7.1

[Qemu-devel] [PATCH 04/14] trace: Trace virtqueue operations

[Stefan Hajnoczi]

This patch adds trace events for virtqueue operations including
adding/removing buffers, notifying the guest, and receiving a notify
from the guest.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 hw/virtio.c  |    8 ++++++++
 trace-events |    8 ++++++++
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/hw/virtio.c b/hw/virtio.c
index 4475bb3..a5741ae 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -13,6 +13,7 @@
 
 #include <inttypes.h>
 
+#include "trace.h"
 #include "virtio.h"
 #include "sysemu.h"
 
@@ -205,6 +206,8 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
     unsigned int offset;
     int i;
 
+    trace_virtqueue_fill(vq, elem, len, idx);
+
     offset = 0;
     for (i = 0; i < elem->in_num; i++) {
         size_t size = MIN(len - offset, elem->in_sg[i].iov_len);
@@ -232,6 +235,7 @@ void virtqueue_flush(VirtQueue *vq, unsigned int count)
 {
     /* Make sure buffer is written before we update index. */
     wmb();
+    trace_virtqueue_flush(vq, count);
     vring_used_idx_increment(vq, count);
     vq->inuse -= count;
 }
@@ -422,6 +426,7 @@ int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem)
 
     vq->inuse++;
 
+    trace_virtqueue_pop(vq, elem, elem->in_num, elem->out_num);
     return elem->in_num + elem->out_num;
 }
 
@@ -560,6 +565,7 @@ int virtio_queue_get_num(VirtIODevice *vdev, int n)
 void virtio_queue_notify(VirtIODevice *vdev, int n)
 {
     if (n < VIRTIO_PCI_QUEUE_MAX && vdev->vq[n].vring.desc) {
+        trace_virtio_queue_notify(vdev, n, &vdev->vq[n]);
         vdev->vq[n].handle_output(vdev, &vdev->vq[n]);
     }
 }
@@ -597,6 +603,7 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
 
 void virtio_irq(VirtQueue *vq)
 {
+    trace_virtio_irq(vq);
     vq->vdev->isr |= 0x01;
     virtio_notify_vector(vq->vdev, vq->vector);
 }
@@ -609,6 +616,7 @@ void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
          (vq->inuse || vring_avail_idx(vq) != vq->last_avail_idx)))
         return;
 
+    trace_virtio_notify(vdev, vq);
     vdev->isr |= 0x01;
     virtio_notify_vector(vdev, vq->vector);
 }
diff --git a/trace-events b/trace-events
index 63037a1..9a3f6a6 100644
--- a/trace-events
+++ b/trace-events
@@ -33,6 +33,14 @@ qemu_memalign(size_t alignment, size_t size, void *ptr) "alignment %zu size %zu
 qemu_valloc(size_t size, void *ptr) "size %zu ptr %p"
 qemu_vfree(void *ptr) "ptr %p"
 
+# hw/virtio.c
+virtqueue_fill(void *vq, const void *elem, unsigned int len, unsigned int idx) "vq %p elem %p len %u idx %u"
+virtqueue_flush(void *vq, unsigned int count) "vq %p count %u"
+virtqueue_pop(void *vq, void *elem, unsigned int in_num, unsigned int out_num) "vq %p elem %p in_num %u out_num %u"
+virtio_queue_notify(void *vdev, int n, void *vq) "vdev %p n %d vq %p"
+virtio_irq(void *vq) "vq %p"
+virtio_notify(void *vdev, void *vq) "vdev %p vq %p"
+
 # block.c
 multiwrite_cb(void *mcb, int ret) "mcb %p ret %d"
 bdrv_aio_multiwrite(void *mcb, int num_callbacks, int num_reqs) "mcb %p num_callbacks %d num_reqs %d"
-- 
1.7.1

[Qemu-devel] [PATCH 05/14] trace: Trace port IO

[Stefan Hajnoczi]

From: Prerna Saxena <prerna@linux.vnet.ibm.com>

Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 ioport.c     |    7 +++++++
 trace-events |    4 ++++
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/ioport.c b/ioport.c
index 53dd87a..ec3dc65 100644
--- a/ioport.c
+++ b/ioport.c
@@ -26,6 +26,7 @@
  */
 
 #include "ioport.h"
+#include "trace.h"
 
 /***********************************************************/
 /* IO Port */
@@ -195,18 +196,21 @@ void isa_unassign_ioport(pio_addr_t start, int length)
 void cpu_outb(pio_addr_t addr, uint8_t val)
 {
     LOG_IOPORT("outb: %04"FMT_pioaddr" %02"PRIx8"\n", addr, val);
+    trace_cpu_out(addr, val);
     ioport_write(0, addr, val);
 }
 
 void cpu_outw(pio_addr_t addr, uint16_t val)
 {
     LOG_IOPORT("outw: %04"FMT_pioaddr" %04"PRIx16"\n", addr, val);
+    trace_cpu_out(addr, val);
     ioport_write(1, addr, val);
 }
 
 void cpu_outl(pio_addr_t addr, uint32_t val)
 {
     LOG_IOPORT("outl: %04"FMT_pioaddr" %08"PRIx32"\n", addr, val);
+    trace_cpu_out(addr, val);
     ioport_write(2, addr, val);
 }
 
@@ -214,6 +218,7 @@ uint8_t cpu_inb(pio_addr_t addr)
 {
     uint8_t val;
     val = ioport_read(0, addr);
+    trace_cpu_in(addr, val);
     LOG_IOPORT("inb : %04"FMT_pioaddr" %02"PRIx8"\n", addr, val);
     return val;
 }
@@ -222,6 +227,7 @@ uint16_t cpu_inw(pio_addr_t addr)
 {
     uint16_t val;
     val = ioport_read(1, addr);
+    trace_cpu_in(addr, val);
     LOG_IOPORT("inw : %04"FMT_pioaddr" %04"PRIx16"\n", addr, val);
     return val;
 }
@@ -230,6 +236,7 @@ uint32_t cpu_inl(pio_addr_t addr)
 {
     uint32_t val;
     val = ioport_read(2, addr);
+    trace_cpu_in(addr, val);
     LOG_IOPORT("inl : %04"FMT_pioaddr" %08"PRIx32"\n", addr, val);
     return val;
 }
diff --git a/trace-events b/trace-events
index 9a3f6a6..c1f3e28 100644
--- a/trace-events
+++ b/trace-events
@@ -54,3 +54,7 @@ virtio_blk_handle_write(void *req, unsigned long sector, unsigned long nsectors)
 
 # posix-aio-compat.c
 paio_submit(void *acb, void *opaque, unsigned long sector_num, unsigned long nb_sectors, unsigned long type) "acb %p opaque %p sector_num %lu nb_sectors %lu type %lu"
+
+# ioport.c
+cpu_in(unsigned int addr, unsigned int val) "addr %#x value %u"
+cpu_out(unsigned int addr, unsigned int val) "addr %#x value %u"
-- 
1.7.1

[Qemu-devel] [PATCH 06/14] trace: Trace entry point of balloon request handler

[Stefan Hajnoczi]

From: Prerna Saxena <prerna@linux.vnet.ibm.com>

Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 balloon.c    |    2 ++
 trace-events |    4 ++++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/balloon.c b/balloon.c
index 8e0b7f1..0021fef 100644
--- a/balloon.c
+++ b/balloon.c
@@ -29,6 +29,7 @@
 #include "cpu-common.h"
 #include "kvm.h"
 #include "balloon.h"
+#include "trace.h"
 
 
 static QEMUBalloonEvent *qemu_balloon_event;
@@ -43,6 +44,7 @@ void qemu_add_balloon_handler(QEMUBalloonEvent *func, void *opaque)
 int qemu_balloon(ram_addr_t target, MonitorCompletion cb, void *opaque)
 {
     if (qemu_balloon_event) {
+        trace_balloon_event(qemu_balloon_event_opaque, target);
         qemu_balloon_event(qemu_balloon_event_opaque, target, cb, opaque);
         return 1;
     } else {
diff --git a/trace-events b/trace-events
index c1f3e28..eae62da 100644
--- a/trace-events
+++ b/trace-events
@@ -58,3 +58,7 @@ paio_submit(void *acb, void *opaque, unsigned long sector_num, unsigned long nb_
 # ioport.c
 cpu_in(unsigned int addr, unsigned int val) "addr %#x value %u"
 cpu_out(unsigned int addr, unsigned int val) "addr %#x value %u"
+
+# balloon.c
+# Since requests are raised via monitor, not many tracepoints are needed.
+balloon_event(void *opaque, unsigned long addr) "opaque %p addr %lu"
-- 
1.7.1

[Qemu-devel] [PATCH 07/14] trace: Add simple built-in tracing backend

[Stefan Hajnoczi]

This patch adds a simple tracer which produces binary trace files.  To
try out the simple backend:

$ ./configure --trace-backend=simple
$ make

After running QEMU you can pretty-print the trace:

$ ./simpletrace.py trace-events /tmp/trace.log

The output of simpletrace.py looks like this:

  qemu_realloc 0.699 ptr=0x24363f0 size=0x3 newptr=0x24363f0
  qemu_free 0.768 ptr=0x24363f0
  ^           ^---- timestamp delta (us)
  |____ trace event name

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

trace: Make trace record fields 64-bit

Explicitly use 64-bit fields in trace records so that timestamps and
magic numbers work for 32-bit host builds.

Includes fixes from Prerna Saxena <prerna@linux.vnet.ibm.com>.

Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 .gitignore     |    1 +
 Makefile       |    2 +
 Makefile.objs  |    3 +
 configure      |    2 +-
 simpletrace.c  |  127 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 simpletrace.h  |   26 +++++++++++
 simpletrace.py |   96 ++++++++++++++++++++++++++++++++++++++++++
 tracetool      |   82 ++++++++++++++++++++++++++++++++++--
 8 files changed, 333 insertions(+), 6 deletions(-)
 create mode 100644 simpletrace.c
 create mode 100644 simpletrace.h
 create mode 100755 simpletrace.py

diff --git a/.gitignore b/.gitignore
index f3f2bb7..72bff98 100644
--- a/.gitignore
+++ b/.gitignore
@@ -42,6 +42,7 @@ QMP/qmp-commands.txt
 *.log
 *.pdf
 *.pg
+*.pyc
 *.toc
 *.tp
 *.vr
diff --git a/Makefile b/Makefile
index 3c5e6a0..ab91d42 100644
--- a/Makefile
+++ b/Makefile
@@ -112,6 +112,8 @@ trace.c: $(SRC_PATH)/trace-events config-host.mak
 
 trace.o: trace.c $(GENERATED_HEADERS)
 
+simpletrace.o: simpletrace.c $(GENERATED_HEADERS)
+
 ######################################################################
 
 qemu-img.o: qemu-img-cmds.h
diff --git a/Makefile.objs b/Makefile.objs
index c61332d..3ef6d80 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -269,6 +269,9 @@ libdis-$(CONFIG_SPARC_DIS) += sparc-dis.o
 # trace
 
 trace-obj-y = trace.o
+ifeq ($(TRACE_BACKEND),simple)
+trace-obj-y += simpletrace.o
+endif
 
 vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
 
diff --git a/configure b/configure
index d3648b2..62dd10d 100755
--- a/configure
+++ b/configure
@@ -898,7 +898,7 @@ echo "  --enable-docs            enable documentation build"
 echo "  --disable-docs           disable documentation build"
 echo "  --disable-vhost-net      disable vhost-net acceleration support"
 echo "  --enable-vhost-net       enable vhost-net acceleration support"
-echo "  --trace-backend=B        Trace backend nop"
+echo "  --trace-backend=B        Trace backend nop simple"
 echo ""
 echo "NOTE: The object files are built at the place where configure is launched"
 exit 1
diff --git a/simpletrace.c b/simpletrace.c
new file mode 100644
index 0000000..a6afc51
--- /dev/null
+++ b/simpletrace.c
@@ -0,0 +1,127 @@
+/*
+ * Simple trace backend
+ *
+ * Copyright IBM, Corp. 2010
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include <stdlib.h>
+#include <stdint.h>
+#include <stdio.h>
+#include "trace.h"
+
+/** Trace file header event ID */
+#define HEADER_EVENT_ID (~(uint64_t)0) /* avoids conflicting with TraceEventIDs */
+
+/** Trace file magic number */
+#define HEADER_MAGIC 0xf2b177cb0aa429b4ULL
+
+/** Trace file version number, bump if format changes */
+#define HEADER_VERSION 0
+
+/** Trace buffer entry */
+typedef struct {
+    uint64_t event;
+    uint64_t timestamp_ns;
+    uint64_t x1;
+    uint64_t x2;
+    uint64_t x3;
+    uint64_t x4;
+    uint64_t x5;
+} TraceRecord;
+
+enum {
+    TRACE_BUF_LEN = 64 * 1024 / sizeof(TraceRecord),
+};
+
+static TraceRecord trace_buf[TRACE_BUF_LEN];
+static unsigned int trace_idx;
+static FILE *trace_fp;
+
+static bool write_header(FILE *fp)
+{
+    TraceRecord header = {
+        .event = HEADER_EVENT_ID,
+        .timestamp_ns = HEADER_MAGIC,
+        .x1 = HEADER_VERSION,
+    };
+
+    return fwrite(&header, sizeof header, 1, fp) == 1;
+}
+
+static void flush_trace_buffer(void)
+{
+    if (!trace_fp) {
+        trace_fp = fopen("/tmp/trace.log", "w");
+        if (trace_fp) {
+            write_header(trace_fp);
+            atexit(flush_trace_buffer);
+        }
+    }
+    if (trace_fp) {
+        size_t unused; /* for when fwrite(3) is declared warn_unused_result */
+        unused = fwrite(trace_buf, trace_idx * sizeof(trace_buf[0]), 1, trace_fp);
+    }
+
+    /* Discard written trace records */
+    trace_idx = 0;
+}
+
+static void trace(TraceEventID event, unsigned long x1,
+                  unsigned long x2, unsigned long x3,
+                  unsigned long x4, unsigned long x5)
+{
+    TraceRecord *rec = &trace_buf[trace_idx];
+    struct timespec ts;
+
+    /* TODO Windows?  It would be good to use qemu-timer here but that isn't
+     * linked into qemu-tools.  Also we should avoid recursion in the tracing
+     * code, therefore it is useful to be self-contained.
+     */
+    clock_gettime(CLOCK_MONOTONIC, &ts);
+
+    rec->event = event;
+    rec->timestamp_ns = ts.tv_sec * 1000000000LL + ts.tv_nsec;
+    rec->x1 = x1;
+    rec->x2 = x2;
+    rec->x3 = x3;
+    rec->x4 = x4;
+    rec->x5 = x5;
+
+    if (++trace_idx == TRACE_BUF_LEN) {
+        flush_trace_buffer();
+    }
+}
+
+void trace0(TraceEventID event)
+{
+    trace(event, 0, 0, 0, 0, 0);
+}
+
+void trace1(TraceEventID event, uint64_t x1)
+{
+    trace(event, x1, 0, 0, 0, 0);
+}
+
+void trace2(TraceEventID event, uint64_t x1, uint64_t x2)
+{
+    trace(event, x1, x2, 0, 0, 0);
+}
+
+void trace3(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3)
+{
+    trace(event, x1, x2, x3, 0, 0);
+}
+
+void trace4(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t x4)
+{
+    trace(event, x1, x2, x3, x4, 0);
+}
+
+void trace5(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t x4, uint64_t x5)
+{
+    trace(event, x1, x2, x3, x4, x5);
+}
diff --git a/simpletrace.h b/simpletrace.h
new file mode 100644
index 0000000..e4e9759
--- /dev/null
+++ b/simpletrace.h
@@ -0,0 +1,26 @@
+/*
+ * Simple trace backend
+ *
+ * Copyright IBM, Corp. 2010
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef SIMPLETRACE_H
+#define SIMPLETRACE_H
+
+#include <stdbool.h>
+#include <stdint.h>
+
+typedef uint64_t TraceEventID;
+
+void trace0(TraceEventID event);
+void trace1(TraceEventID event, uint64_t x1);
+void trace2(TraceEventID event, uint64_t x1, uint64_t x2);
+void trace3(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3);
+void trace4(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t x4);
+void trace5(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t x4, uint64_t x5);
+
+#endif /* SIMPLETRACE_H */
diff --git a/simpletrace.py b/simpletrace.py
new file mode 100755
index 0000000..fdf0eb5
--- /dev/null
+++ b/simpletrace.py
@@ -0,0 +1,96 @@
+#!/usr/bin/env python
+#
+# Pretty-printer for simple trace backend binary trace files
+#
+# Copyright IBM, Corp. 2010
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+# For help see docs/tracing.txt
+
+import sys
+import struct
+import re
+
+header_event_id = 0xffffffffffffffff
+header_magic    = 0xf2b177cb0aa429b4
+header_version  = 0
+
+trace_fmt = '=QQQQQQQ'
+trace_len = struct.calcsize(trace_fmt)
+event_re  = re.compile(r'(disable\s+)?([a-zA-Z0-9_]+)\(([^)]*)\)\s+"([^"]*)"')
+
+def err(msg):
+    sys.stderr.write(msg + '\n')
+    sys.exit(1)
+
+def parse_events(fobj):
+    """Parse a trace-events file."""
+
+    def get_argnames(args):
+        """Extract argument names from a parameter list."""
+        return tuple(arg.split()[-1].lstrip('*') for arg in args.split(','))
+
+    events = {}
+    event_num = 0
+    for line in fobj:
+        m = event_re.match(line.strip())
+        if m is None:
+            continue
+
+        disable, name, args, fmt = m.groups()
+        if disable:
+            continue
+
+        events[event_num] = (name,) + get_argnames(args)
+        event_num += 1
+    return events
+
+def read_record(fobj):
+    """Deserialize a trace record from a file."""
+    s = fobj.read(trace_len)
+    if len(s) != trace_len:
+        return None
+    return struct.unpack(trace_fmt, s)
+
+def read_trace_file(fobj):
+    """Deserialize trace records from a file."""
+    header = read_record(fobj)
+    if header is None or \
+       header[0] != header_event_id or \
+       header[1] != header_magic or \
+       header[2] != header_version:
+        err('not a trace file or incompatible version')
+
+    while True:
+        rec = read_record(fobj)
+        if rec is None:
+            break
+
+        yield rec
+
+class Formatter(object):
+    def __init__(self, events):
+        self.events = events
+        self.last_timestamp = None
+
+    def format_record(self, rec):
+        if self.last_timestamp is None:
+            self.last_timestamp = rec[1]
+        delta_ns = rec[1] - self.last_timestamp
+        self.last_timestamp = rec[1]
+
+        event = self.events[rec[0]]
+        fields = [event[0], '%0.3f' % (delta_ns / 1000.0)]
+        for i in xrange(1, len(event)):
+            fields.append('%s=0x%x' % (event[i], rec[i + 1]))
+        return ' '.join(fields)
+
+if len(sys.argv) != 3:
+    err('usage: %s <trace-events> <trace-file>' % sys.argv[0])
+
+events = parse_events(open(sys.argv[1], 'r'))
+formatter = Formatter(events)
+for rec in read_trace_file(open(sys.argv[2], 'rb')):
+    print formatter.format_record(rec)
diff --git a/tracetool b/tracetool
index 86b6446..66df685 100755
--- a/tracetool
+++ b/tracetool
@@ -13,11 +13,12 @@ set -f
 usage()
 {
     cat >&2 <<EOF
-usage: $0 --nop [-h | -c]
+usage: $0 [--nop | --simple] [-h | -c]
 Generate tracing code for a file on stdin.
 
 Backends:
-  --nop Tracing disabled
+  --nop     Tracing disabled
+  --simple  Simple built-in backend
 
 Output formats:
   -h    Generate .h file
@@ -56,16 +57,27 @@ get_argnames()
         name=${field%,}
         test "$field" = "$name" && continue
 
-        echo -n "$name, "
+        printf "%s" "$name, "
     done
 
     # Last argument name
     if [ "$nfields" -gt 1 ]
     then
-        echo -n "$name"
+        printf "%s" "$name"
     fi
 }
 
+# Get the number of arguments to a trace event
+get_argc()
+{
+    local name argc
+    argc=0
+    for name in $(get_argnames "$1"); do
+        argc=$((argc + 1))
+    done
+    echo $argc
+}
+
 # Get the format string for a trace event
 get_fmt()
 {
@@ -115,6 +127,66 @@ linetoc_end_nop()
     return
 }
 
+linetoh_begin_simple()
+{
+    cat <<EOF
+#include "simpletrace.h"
+EOF
+
+    simple_event_num=0
+}
+
+cast_args_to_uint64_t()
+{
+    local arg
+    for arg in $(get_argnames "$1"); do
+        printf "%s" "(uint64_t)(uintptr_t)$arg"
+    done
+}
+
+linetoh_simple()
+{
+    local name args argc trace_args
+    name=$(get_name "$1")
+    args=$(get_args "$1")
+    argc=$(get_argc "$1")
+
+    trace_args="$simple_event_num"
+    if [ "$argc" -gt 0 ]
+    then
+        trace_args="$trace_args, $(cast_args_to_uint64_t "$1")"
+    fi
+
+    cat <<EOF
+static inline void trace_$name($args)
+{
+    trace$argc($trace_args);
+}
+EOF
+
+    simple_event_num=$((simple_event_num + 1))
+}
+
+linetoh_end_simple()
+{
+    return
+}
+
+linetoc_begin_simple()
+{
+    return
+}
+
+linetoc_simple()
+{
+    return
+}
+
+linetoc_end_simple()
+{
+    return
+}
+
 # Process stdin by calling begin, line, and end functions for the backend
 convert()
 {
@@ -160,7 +232,7 @@ tracetoc()
 
 # Choose backend
 case "$1" in
-"--nop") backend="${1#--}" ;;
+"--nop" | "--simple") backend="${1#--}" ;;
 *) usage ;;
 esac
 shift
-- 
1.7.1

[Qemu-devel] [PATCH 08/14] trace: Support for dynamically enabling/disabling trace events.

[Stefan Hajnoczi]

From: Prerna Saxena <prerna@linux.vnet.ibm.com>

This patch adds support for dynamically enabling/disabling of trace events.
This is done by internally maintaining each trace event's state, and
permitting logging of data from a trace event only if it is in an
'active' state.

Monitor commands added :
1) info trace-events 		: to view all available trace events and
				  their state.
2) trace-event NAME on|off 	: to enable/disable data logging from a
				  given trace event.
				  Eg, trace-event paio_submit off
				  	disables logging of data when
					paio_submit is hit.

By default, all trace-events are disabled. One can enable desired trace-events
via the monitor.

Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

trace: Monitor command 'info trace'

Monitor command 'info trace' to display contents of trace buffer

Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

trace: Remove monitor.h dependency from simpletrace

User-mode targets don't have a monitor so the simple trace backend
currently does not build on those targets.  This patch abstracts the
monitor printing interface so there is no direct coupling between
simpletrace and the monitor.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 configure       |    3 +++
 monitor.c       |   40 ++++++++++++++++++++++++++++++++++++++++
 qemu-monitor.hx |   25 +++++++++++++++++++++++++
 simpletrace.c   |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 simpletrace.h   |    9 +++++++++
 tracetool       |   24 ++++++++++++++++++++----
 6 files changed, 149 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index 62dd10d..830e49e 100755
--- a/configure
+++ b/configure
@@ -2466,6 +2466,9 @@ bsd)
 esac
 
 echo "TRACE_BACKEND=$trace_backend" >> $config_host_mak
+if test "$trace_backend" = "simple"; then
+  echo "CONFIG_SIMPLE_TRACE=y" >> $config_host_mak
+fi
 echo "TOOLS=$tools" >> $config_host_mak
 echo "ROMS=$roms" >> $config_host_mak
 echo "MAKE=$make" >> $config_host_mak
diff --git a/monitor.c b/monitor.c
index c313d5a..4f1639d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -56,6 +56,9 @@
 #include "json-parser.h"
 #include "osdep.h"
 #include "exec-all.h"
+#ifdef CONFIG_SIMPLE_TRACE
+#include "trace.h"
+#endif
 
 //#define DEBUG
 //#define DEBUG_COMPLETION
@@ -539,6 +542,15 @@ static void do_help_cmd(Monitor *mon, const QDict *qdict)
     help_cmd(mon, qdict_get_try_str(qdict, "name"));
 }
 
+#ifdef CONFIG_SIMPLE_TRACE
+static void do_change_trace_event_state(Monitor *mon, const QDict *qdict)
+{
+    const char *tp_name = qdict_get_str(qdict, "name");
+    bool new_state = qdict_get_bool(qdict, "option");
+    st_change_trace_event_state(tp_name, new_state);
+}
+#endif
+
 static void user_monitor_complete(void *opaque, QObject *ret_data)
 {
     MonitorCompletionData *data = (MonitorCompletionData *)opaque; 
@@ -923,6 +935,18 @@ static void do_info_cpu_stats(Monitor *mon)
 }
 #endif
 
+#if defined(CONFIG_SIMPLE_TRACE)
+static void do_info_trace(Monitor *mon)
+{
+    st_print_trace((FILE *)mon, &monitor_fprintf);
+}
+
+static void do_info_trace_events(Monitor *mon)
+{
+    st_print_trace_events((FILE *)mon, &monitor_fprintf);
+}
+#endif
+
 /**
  * do_quit(): Quit QEMU execution
  */
@@ -2579,6 +2603,22 @@ static const mon_cmd_t info_cmds[] = {
         .help       = "show roms",
         .mhandler.info = do_info_roms,
     },
+#if defined(CONFIG_SIMPLE_TRACE)
+    {
+        .name       = "trace",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show current contents of trace buffer",
+        .mhandler.info = do_info_trace,
+    },
+    {
+        .name       = "trace-events",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show available trace-events & their state",
+        .mhandler.info = do_info_trace_events,
+    },
+#endif
     {
         .name       = NULL,
     },
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 2af3de6..a9f4221 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -259,6 +259,22 @@ STEXI
 Output logs to @var{filename}.
 ETEXI
 
+#ifdef CONFIG_SIMPLE_TRACE
+    {
+        .name       = "trace-event",
+        .args_type  = "name:s,option:b",
+        .params     = "name on|off",
+        .help       = "changes status of a specific trace event",
+        .mhandler.cmd = do_change_trace_event_state,
+    },
+
+STEXI
+@item trace-event
+@findex trace-event
+changes status of a trace event
+ETEXI
+#endif
+
     {
         .name       = "log",
         .args_type  = "items:s",
@@ -2495,6 +2511,15 @@ show roms
 @end table
 ETEXI
 
+#ifdef CONFIG_SIMPLE_TRACE
+STEXI
+@item info trace
+show contents of trace buffer
+@item info trace-events
+show available trace events and their state
+ETEXI
+#endif
+
 HXCOMM DO NOT add new commands after 'info', move your addition before it!
 
 STEXI
diff --git a/simpletrace.c b/simpletrace.c
index a6afc51..311fa44 100644
--- a/simpletrace.c
+++ b/simpletrace.c
@@ -11,6 +11,7 @@
 #include <stdlib.h>
 #include <stdint.h>
 #include <stdio.h>
+#include <time.h>
 #include "trace.h"
 
 /** Trace file header event ID */
@@ -83,6 +84,10 @@ static void trace(TraceEventID event, unsigned long x1,
      */
     clock_gettime(CLOCK_MONOTONIC, &ts);
 
+    if (!trace_list[event].state) {
+        return;
+    }
+
     rec->event = event;
     rec->timestamp_ns = ts.tv_sec * 1000000000LL + ts.tv_nsec;
     rec->x1 = x1;
@@ -125,3 +130,50 @@ void trace5(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t
 {
     trace(event, x1, x2, x3, x4, x5);
 }
+
+void st_print_trace(FILE *stream, int (*stream_printf)(FILE *stream, const char *fmt, ...))
+{
+    unsigned int i;
+
+    for (i = 0; i < trace_idx; i++) {
+        stream_printf(stream, "Event %lu : %lx %lx %lx %lx %lx\n",
+                      trace_buf[i].event, trace_buf[i].x1, trace_buf[i].x2,
+                      trace_buf[i].x3, trace_buf[i].x4, trace_buf[i].x5);
+    }
+}
+
+void st_print_trace_events(FILE *stream, int (*stream_printf)(FILE *stream, const char *fmt, ...))
+{
+    unsigned int i;
+
+    for (i = 0; i < NR_TRACE_EVENTS; i++) {
+        stream_printf(stream, "%s [Event ID %u] : state %u\n",
+                      trace_list[i].tp_name, i, trace_list[i].state);
+    }
+}
+
+static TraceEvent* find_trace_event_by_name(const char *tname)
+{
+    unsigned int i;
+
+    if (!tname) {
+        return NULL;
+    }
+
+    for (i = 0; i < NR_TRACE_EVENTS; i++) {
+        if (!strcmp(trace_list[i].tp_name, tname)) {
+            return &trace_list[i];
+        }
+    }
+    return NULL; /* indicates end of list reached without a match */
+}
+
+void st_change_trace_event_state(const char *tname, bool tstate)
+{
+    TraceEvent *tp;
+
+    tp = find_trace_event_by_name(tname);
+    if (tp) {
+        tp->state = tstate;
+    }
+}
diff --git a/simpletrace.h b/simpletrace.h
index e4e9759..ee30ae9 100644
--- a/simpletrace.h
+++ b/simpletrace.h
@@ -13,14 +13,23 @@
 
 #include <stdbool.h>
 #include <stdint.h>
+#include <stdio.h>
 
 typedef uint64_t TraceEventID;
 
+typedef struct {
+    const char *tp_name;
+    bool state;
+} TraceEvent;
+
 void trace0(TraceEventID event);
 void trace1(TraceEventID event, uint64_t x1);
 void trace2(TraceEventID event, uint64_t x1, uint64_t x2);
 void trace3(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3);
 void trace4(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t x4);
 void trace5(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t x4, uint64_t x5);
+void st_print_trace(FILE *stream, int (*stream_printf)(FILE *stream, const char *fmt, ...));
+void st_print_trace_events(FILE *stream, int (*stream_printf)(FILE *stream, const char *fmt, ...));
+void st_change_trace_event_state(const char *tname, bool tstate);
 
 #endif /* SIMPLETRACE_H */
diff --git a/tracetool b/tracetool
index 66df685..30dc812 100755
--- a/tracetool
+++ b/tracetool
@@ -169,22 +169,38 @@ EOF
 
 linetoh_end_simple()
 {
-    return
+    cat <<EOF
+#define NR_TRACE_EVENTS $simple_event_num
+extern TraceEvent trace_list[NR_TRACE_EVENTS];
+EOF
 }
 
 linetoc_begin_simple()
 {
-    return
+    cat <<EOF
+#include "trace.h"
+
+TraceEvent trace_list[] = {
+EOF
+    simple_event_num=0
+
 }
 
 linetoc_simple()
 {
-    return
+    local name
+    name=$(get_name "$1")
+    cat <<EOF
+{.tp_name = "$name", .state=0},
+EOF
+    simple_event_num=$((simple_event_num + 1))
 }
 
 linetoc_end_simple()
 {
-    return
+    cat <<EOF
+};
+EOF
 }
 
 # Process stdin by calling begin, line, and end functions for the backend
-- 
1.7.1

[Qemu-devel] [PATCH 09/14] trace: Support disabled events in trace-events

[Stefan Hajnoczi]

Sometimes it is useful to disable a trace event.  Removing the event
from trace-events is not enough since source code will call the
trace_*() function for the event.

This patch makes it easy to build without specific trace events by
marking them disabled in trace-events:

disable multiwrite_cb(void *mcb, int ret) "mcb %p ret %d"

This builds without the multiwrite_cb trace event.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

trace: Allow bulk enabling/disabling of trace events at compile time

For 'simple' trace backend, allow bulk enabling/disabling of trace
events at compile time.  Trace events that are preceded by 'disable'
keyword are compiled in, but turned off by default. These can
individually be turned on using the monitor.  All other trace events are
enabled by default.

TODO :
This could be enhanced when the trace-event namespace is partitioned into a
group and an ID within that group. In such a case, marking a group as enabled
would automatically enable all trace-events listed under it.

Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 trace-events |    7 ++++++-
 tracetool    |   44 +++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/trace-events b/trace-events
index eae62da..e96d1f3 100644
--- a/trace-events
+++ b/trace-events
@@ -12,10 +12,15 @@
 #
 # Format of a trace event:
 #
-# <name>(<type1> <arg1>[, <type2> <arg2>] ...) "<format-string>"
+# [disable] <name>(<type1> <arg1>[, <type2> <arg2>] ...) "<format-string>"
 #
 # Example: qemu_malloc(size_t size) "size %zu"
 #
+# The "disable" keyword will build without the trace event.
+# In case of 'simple' trace backend, it will allow the trace event to be
+# compiled, but this would be turned off by default. It can be toggled on via
+# the monitor.
+#
 # The <name> must be a valid as a C function name.
 #
 # Types should be standard C types.  Use void * for pointers because the trace
diff --git a/tracetool b/tracetool
index 30dc812..d840e6f 100755
--- a/tracetool
+++ b/tracetool
@@ -87,6 +87,20 @@ get_fmt()
     echo "$fmt"
 }
 
+# Get the state of a trace event
+get_state()
+{
+    local str disable state
+    str=$(get_name "$1")
+    disable=${str##disable }
+    if [ "$disable" = "$str" ] ; then
+        state=1
+    else
+        state=0
+    fi
+    echo "$state"
+}
+
 linetoh_begin_nop()
 {
     return
@@ -146,10 +160,14 @@ cast_args_to_uint64_t()
 
 linetoh_simple()
 {
-    local name args argc trace_args
+    local name args argc trace_args state
     name=$(get_name "$1")
     args=$(get_args "$1")
     argc=$(get_argc "$1")
+    state=$(get_state "$1")
+    if [ "$state" = "0" ]; then
+        name=${name##disable }
+    fi
 
     trace_args="$simple_event_num"
     if [ "$argc" -gt 0 ]
@@ -188,10 +206,14 @@ EOF
 
 linetoc_simple()
 {
-    local name
+    local name state
     name=$(get_name "$1")
+    state=$(get_state "$1")
+    if [ "$state" = "0" ] ; then
+        name=${name##disable }
+    fi
     cat <<EOF
-{.tp_name = "$name", .state=0},
+{.tp_name = "$name", .state=$state},
 EOF
     simple_event_num=$((simple_event_num + 1))
 }
@@ -206,7 +228,7 @@ EOF
 # Process stdin by calling begin, line, and end functions for the backend
 convert()
 {
-    local begin process_line end
+    local begin process_line end str disable
     begin="lineto$1_begin_$backend"
     process_line="lineto$1_$backend"
     end="lineto$1_end_$backend"
@@ -218,8 +240,20 @@ convert()
         str=${str%%#*}
         test -z "$str" && continue
 
+        # Process the line.  The nop backend handles disabled lines.
+        disable=${str%%disable *}
         echo
-        "$process_line" "$str"
+        if test -z "$disable"; then
+            # Pass the disabled state as an arg to lineto$1_simple().
+            # For all other cases, call lineto$1_nop()
+            if [ $backend = "simple" ]; then
+                "$process_line" "$str"
+            else
+                "lineto$1_nop" "${str##disable }"
+            fi
+        else
+            "$process_line" "$str"
+        fi
     done
 
     echo
-- 
1.7.1

[Qemu-devel] [PATCH 10/14] trace: Specify trace file name

[Stefan Hajnoczi]

From: Prerna Saxena <prerna@linux.vnet.ibm.com>

Allow users to specify a file for trace-outputs at configuration.
Also, allow trace files to be annotated by <pid> so each qemu instance has
unique traces.

The trace file name can be passed as a config option:
--trace-file=/path/to/file
(Default : /tmp/trace )
At runtime, the pid of the qemu process is appended to the filename so
that mutiple qemu instances do not have overlapping logs.

Eg : /tmp/trace-1234 for qemu launched with pid 1234.

I have yet to test this on windows. getpid() is used at many places
in code(including vnc.c), so I'm hoping this would be okay too.

Edited-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 configure     |   14 ++++++++++++++
 simpletrace.c |   20 +++++++++++++++++---
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 830e49e..5c48a90 100755
--- a/configure
+++ b/configure
@@ -316,6 +316,7 @@ check_utests="no"
 user_pie="no"
 zero_malloc=""
 trace_backend="nop"
+trace_file="/tmp/trace"
 
 # OS specific
 if check_define __linux__ ; then
@@ -473,6 +474,7 @@ if test "$mingw32" = "yes" ; then
   bindir="\${prefix}"
   sysconfdir="\${prefix}"
   confsuffix=""
+  trace_file="trace"
 fi
 
 # find source path
@@ -520,6 +522,8 @@ for opt do
   ;;
   --trace-backend=*) trace_backend="$optarg"
   ;;
+  --trace-file=*) trace_file="$optarg"
+  ;;
   --enable-gprof) gprof="yes"
   ;;
   --static)
@@ -899,6 +903,9 @@ echo "  --disable-docs           disable documentation build"
 echo "  --disable-vhost-net      disable vhost-net acceleration support"
 echo "  --enable-vhost-net       enable vhost-net acceleration support"
 echo "  --trace-backend=B        Trace backend nop simple"
+echo "  --trace-file=NAME        Full PATH,NAME of file to store traces"
+echo "                           Default:/tmp/trace-<pid>"
+echo "                           Default:trace-<pid> on Windows"
 echo ""
 echo "NOTE: The object files are built at the place where configure is launched"
 exit 1
@@ -2204,6 +2211,7 @@ echo "fdatasync         $fdatasync"
 echo "uuid support      $uuid"
 echo "vhost-net support $vhost_net"
 echo "Trace backend     $trace_backend"
+echo "Trace output file $trace_file-<pid>"
 
 if test $sdl_too_old = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -2469,6 +2477,12 @@ echo "TRACE_BACKEND=$trace_backend" >> $config_host_mak
 if test "$trace_backend" = "simple"; then
   echo "CONFIG_SIMPLE_TRACE=y" >> $config_host_mak
 fi
+# Set the appropriate trace file.
+if test "$trace_backend" = "simple"; then
+  trace_file="\"$trace_file-%u\""
+fi
+echo "CONFIG_TRACE_FILE=$trace_file" >> $config_host_mak
+
 echo "TOOLS=$tools" >> $config_host_mak
 echo "ROMS=$roms" >> $config_host_mak
 echo "MAKE=$make" >> $config_host_mak
diff --git a/simpletrace.c b/simpletrace.c
index 311fa44..c3a4b73 100644
--- a/simpletrace.c
+++ b/simpletrace.c
@@ -53,12 +53,26 @@ static bool write_header(FILE *fp)
     return fwrite(&header, sizeof header, 1, fp) == 1;
 }
 
+static bool open_trace_file(void)
+{
+    char *filename;
+
+    if (asprintf(&filename, CONFIG_TRACE_FILE, getpid()) < 0) {
+        return false;
+    }
+
+    trace_fp = fopen(filename, "w");
+    free(filename);
+    if (!trace_fp) {
+        return false;
+    }
+    return write_header(trace_fp);
+}
+
 static void flush_trace_buffer(void)
 {
     if (!trace_fp) {
-        trace_fp = fopen("/tmp/trace.log", "w");
-        if (trace_fp) {
-            write_header(trace_fp);
+        if (open_trace_file()) {
             atexit(flush_trace_buffer);
         }
     }
-- 
1.7.1

[Qemu-devel] [PATCH 11/14] trace: Add trace-file command to open/close/flush trace file

[Stefan Hajnoczi]

This patch adds the trace-file command:

  trace-file [on|off|flush]

  Open, close, or flush the trace file.  If no argument is given,
  the status of the trace file is displayed.

The trace file is turned on by default but is only written out when the
trace buffer becomes full.  The flush operation can be used to force
write out at any time.

Turning off the trace file does not change the state of trace events;
tracing will continue to the trace buffer.  When the trace file is off,
use "info trace" to display the contents of the trace buffer in memory.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

This commit also contains the trace-file sub-command from the following
commit:

commit 5ce8d1a957afae2c52ad748944ce72848ccf57bd
Author: Prerna Saxena <prerna@linux.vnet.ibm.com>
Date:   Wed Aug 4 16:23:54 2010 +0530

    trace: Add options to specify trace file name at startup and runtime

    This patch adds an optional command line switch '-trace' to specify the
    filename to write traces to, when qemu starts.
    Eg, If compiled with the 'simple' trace backend,
    [temp@system]$ qemu -trace FILENAME IMAGE
    Allows the binary traces to be written to FILENAME instead of the option
    set at config-time.

    Also, this adds monitor sub-command 'set' to trace-file commands to
    dynamically change trace log file at runtime.
    Eg,
    (qemu)trace-file set FILENAME
    This allows one to set trace outputs to FILENAME from the default
    specified at startup.

    Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
    Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 monitor.c       |   23 ++++++++++++++++
 qemu-monitor.hx |   14 ++++++++++
 simpletrace.c   |   79 +++++++++++++++++++++++++++++++++++++++++++++----------
 simpletrace.h   |    4 +++
 4 files changed, 106 insertions(+), 14 deletions(-)

diff --git a/monitor.c b/monitor.c
index 4f1639d..24d122f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -549,6 +549,29 @@ static void do_change_trace_event_state(Monitor *mon, const QDict *qdict)
     bool new_state = qdict_get_bool(qdict, "option");
     st_change_trace_event_state(tp_name, new_state);
 }
+
+static void do_trace_file(Monitor *mon, const QDict *qdict)
+{
+    const char *op = qdict_get_try_str(qdict, "op");
+    const char *arg = qdict_get_try_str(qdict, "arg");
+
+    if (!op) {
+        st_print_trace_file_status((FILE *)mon, &monitor_fprintf);
+    } else if (!strcmp(op, "on")) {
+        st_set_trace_file_enabled(true);
+    } else if (!strcmp(op, "off")) {
+        st_set_trace_file_enabled(false);
+    } else if (!strcmp(op, "flush")) {
+        st_flush_trace_buffer();
+    } else if (!strcmp(op, "set")) {
+        if (arg) {
+            st_set_trace_file(arg);
+        }
+    } else {
+        monitor_printf(mon, "unexpected argument \"%s\"\n", op);
+        help_cmd(mon, "trace-file");
+    }
+}
 #endif
 
 static void user_monitor_complete(void *opaque, QObject *ret_data)
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index a9f4221..592c6a8 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -273,6 +273,20 @@ STEXI
 @findex trace-event
 changes status of a trace event
 ETEXI
+
+    {
+        .name       = "trace-file",
+        .args_type  = "op:s?,arg:F?",
+        .params     = "on|off|flush|set [arg]",
+        .help       = "open, close, or flush trace file, or set a new file name",
+        .mhandler.cmd = do_trace_file,
+    },
+
+STEXI
+@item trace-file on|off|flush
+@findex trace-file
+Open, close, or flush the trace file.  If no argument is given, the status of the trace file is displayed.
+ETEXI
 #endif
 
     {
diff --git a/simpletrace.c b/simpletrace.c
index c3a4b73..27b0cab 100644
--- a/simpletrace.c
+++ b/simpletrace.c
@@ -41,6 +41,14 @@ enum {
 static TraceRecord trace_buf[TRACE_BUF_LEN];
 static unsigned int trace_idx;
 static FILE *trace_fp;
+static char *trace_file_name = NULL;
+static bool trace_file_enabled = false;
+
+void st_print_trace_file_status(FILE *stream, int (*stream_printf)(FILE *stream, const char *fmt, ...))
+{
+    stream_printf(stream, "Trace file \"%s\" %s.\n",
+                  trace_file_name, trace_file_enabled ? "on" : "off");
+}
 
 static bool write_header(FILE *fp)
 {
@@ -53,38 +61,81 @@ static bool write_header(FILE *fp)
     return fwrite(&header, sizeof header, 1, fp) == 1;
 }
 
-static bool open_trace_file(void)
+/**
+ * set_trace_file : To set the name of a trace file.
+ * @file : pointer to the name to be set.
+ *         If NULL, set to the default name-<pid> set at config time.
+ */
+bool st_set_trace_file(const char *file)
 {
-    char *filename;
+    st_set_trace_file_enabled(false);
 
-    if (asprintf(&filename, CONFIG_TRACE_FILE, getpid()) < 0) {
-        return false;
-    }
+    free(trace_file_name);
 
-    trace_fp = fopen(filename, "w");
-    free(filename);
-    if (!trace_fp) {
-        return false;
+    if (!file) {
+        if (asprintf(&trace_file_name, CONFIG_TRACE_FILE, getpid()) < 0) {
+            trace_file_name = NULL;
+            return false;
+        }
+    } else {
+        if (asprintf(&trace_file_name, "%s", file) < 0) {
+            trace_file_name = NULL;
+            return false;
+        }
     }
-    return write_header(trace_fp);
+
+    st_set_trace_file_enabled(true);
+    return true;
 }
 
-static void flush_trace_buffer(void)
+static void flush_trace_file(void)
 {
+    /* If the trace file is not open yet, open it now */
     if (!trace_fp) {
-        if (open_trace_file()) {
-            atexit(flush_trace_buffer);
+        trace_fp = fopen(trace_file_name, "w");
+        if (!trace_fp) {
+            /* Avoid repeatedly trying to open file on failure */
+            trace_file_enabled = false;
+            return;
         }
+        write_header(trace_fp);
+        atexit(st_flush_trace_buffer);
     }
+
     if (trace_fp) {
         size_t unused; /* for when fwrite(3) is declared warn_unused_result */
         unused = fwrite(trace_buf, trace_idx * sizeof(trace_buf[0]), 1, trace_fp);
     }
+}
+
+void st_flush_trace_buffer(void)
+{
+    if (trace_file_enabled) {
+        flush_trace_file();
+    }
 
     /* Discard written trace records */
     trace_idx = 0;
 }
 
+void st_set_trace_file_enabled(bool enable)
+{
+    if (enable == trace_file_enabled) {
+        return; /* no change */
+    }
+
+    /* Flush/discard trace buffer */
+    st_flush_trace_buffer();
+
+    /* To disable, close trace file */
+    if (!enable) {
+        fclose(trace_fp);
+        trace_fp = NULL;
+    }
+
+    trace_file_enabled = enable;
+}
+
 static void trace(TraceEventID event, unsigned long x1,
                   unsigned long x2, unsigned long x3,
                   unsigned long x4, unsigned long x5)
@@ -111,7 +162,7 @@ static void trace(TraceEventID event, unsigned long x1,
     rec->x5 = x5;
 
     if (++trace_idx == TRACE_BUF_LEN) {
-        flush_trace_buffer();
+        st_flush_trace_buffer();
     }
 }
 
diff --git a/simpletrace.h b/simpletrace.h
index ee30ae9..ecb7eb9 100644
--- a/simpletrace.h
+++ b/simpletrace.h
@@ -31,5 +31,9 @@ void trace5(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t
 void st_print_trace(FILE *stream, int (*stream_printf)(FILE *stream, const char *fmt, ...));
 void st_print_trace_events(FILE *stream, int (*stream_printf)(FILE *stream, const char *fmt, ...));
 void st_change_trace_event_state(const char *tname, bool tstate);
+void st_print_trace_file_status(FILE *stream, int (*stream_printf)(FILE *stream, const char *fmt, ...));
+void st_set_trace_file_enabled(bool enable);
+bool st_set_trace_file(const char *file);
+void st_flush_trace_buffer(void);
 
 #endif /* SIMPLETRACE_H */
-- 
1.7.1

[Qemu-devel] [PATCH 12/14] trace: Add trace file name command-line option

[Stefan Hajnoczi]

From: Prerna Saxena <prerna@linux.vnet.ibm.com>

This patch adds an optional command line switch '-trace' to specify the
filename to write traces to, when qemu starts.
Eg, If compiled with the 'simple' trace backend,
[temp@system]$ qemu -trace FILENAME IMAGE
Allows the binary traces to be written to FILENAME instead of the option
set at config-time.

Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 qemu-options.hx |   11 +++++++++++
 vl.c            |   18 ++++++++++++++++++
 2 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index db86feb..a0c091f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2234,6 +2234,17 @@ Normally QEMU loads a configuration file from @var{sysconfdir}/qemu.conf and
 @var{sysconfdir}/target-@var{ARCH}.conf on startup.  The @code{-nodefconfig}
 option will prevent QEMU from loading these configuration files at startup.
 ETEXI
+#ifdef CONFIG_SIMPLE_TRACE
+DEF("trace", HAS_ARG, QEMU_OPTION_trace,
+    "-trace\n"
+    "                Specify a trace file to log traces to\n",
+    QEMU_ARCH_ALL)
+STEXI
+@item -trace
+@findex -trace
+Specify a trace file to log output traces to.
+ETEXI
+#endif
 
 HXCOMM This is the last statement. Insert new options before this line!
 STEXI
diff --git a/vl.c b/vl.c
index b3e3676..99664e9 100644
--- a/vl.c
+++ b/vl.c
@@ -47,6 +47,10 @@
 #include <dirent.h>
 #include <netdb.h>
 #include <sys/select.h>
+#ifdef CONFIG_SIMPLE_TRACE
+#include "trace.h"
+#endif
+
 #ifdef CONFIG_BSD
 #include <sys/stat.h>
 #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__DragonFly__)
@@ -1821,6 +1825,9 @@ int main(int argc, char **argv, char **envp)
     int show_vnc_port = 0;
     int defconfig = 1;
 
+#ifdef CONFIG_SIMPLE_TRACE
+    const char *trace_file = NULL;
+#endif
     atexit(qemu_run_exit_notifiers);
     error_set_progname(argv[0]);
 
@@ -2590,6 +2597,11 @@ int main(int argc, char **argv, char **envp)
                 }
                 xen_mode = XEN_ATTACH;
                 break;
+#ifdef CONFIG_SIMPLE_TRACE
+            case QEMU_OPTION_trace:
+                trace_file = optarg;
+                break;
+#endif
             case QEMU_OPTION_readconfig:
                 {
                     int ret = qemu_read_config_file(optarg);
@@ -2633,6 +2645,12 @@ int main(int argc, char **argv, char **envp)
         data_dir = CONFIG_QEMU_DATADIR;
     }
 
+#ifdef CONFIG_SIMPLE_TRACE
+    /*
+     * Set the trace file name, if specified.
+     */
+    st_set_trace_file(trace_file);
+#endif
     /*
      * Default to max_cpus = smp_cpus, in case the user doesn't
      * specify a max_cpus value.
-- 
1.7.1

[Qemu-devel] [PATCH 13/14] trace: Add LTTng Userspace Tracer backend

[Stefan Hajnoczi]

This patch adds LTTng Userspace Tracer (UST) backend support.  The UST
system requires no kernel support but libust and liburcu must be
installed.

$ ./configure --trace-backend ust
$ make

Start the UST daemon:
$ ustd &

List available tracepoints and enable some:
$ ustctl --list-markers $(pgrep qemu)
[...]
{PID: 5458, channel/marker: ust/paio_submit, state: 0, fmt: "acb %p
opaque %p sector_num %lu nb_sectors %lu type %lu" 0x4b32ba}
$ ustctl --enable-marker "ust/paio_submit" $(pgrep qemu)

Run the trace:
$ ustctl --create-trace $(pgrep qemu)
$ ustctl --start-trace $(pgrep qemu)
[...]
$ ustctl --stop-trace $(pgrep qemu)
$ ustctl --destroy-trace $(pgrep qemu)

Trace results can be viewed using lttv-gui.

More information about UST:
http://lttng.org/ust

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

trace: Check for LTTng Userspace Tracer headers

When using the 'ust' backend, check if the relevant headers are
available at host.

Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 configure |   22 +++++++++++++++-
 tracetool |   85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 104 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 5c48a90..3d6dfd4 100755
--- a/configure
+++ b/configure
@@ -902,7 +902,7 @@ echo "  --enable-docs            enable documentation build"
 echo "  --disable-docs           disable documentation build"
 echo "  --disable-vhost-net      disable vhost-net acceleration support"
 echo "  --enable-vhost-net       enable vhost-net acceleration support"
-echo "  --trace-backend=B        Trace backend nop simple"
+echo "  --trace-backend=B        Trace backend nop simple ust"
 echo "  --trace-file=NAME        Full PATH,NAME of file to store traces"
 echo "                           Default:/tmp/trace-<pid>"
 echo "                           Default:trace-<pid> on Windows"
@@ -2080,6 +2080,23 @@ if test "$?" -ne 0 ; then
   exit 1
 fi
 
+##########################################
+# For 'ust' backend, test if ust headers are present
+if test "$trace_backend" = "ust"; then
+  cat > $TMPC << EOF
+#include <ust/tracepoint.h>
+#include <ust/marker.h>
+int main(void) { return 0; }
+EOF
+  if compile_prog "" "" ; then
+    LIBS="-lust $LIBS"
+  else
+    echo "ERROR: Trace backend 'ust' does not have relevant headers available"
+    echo "       on the host. Pls choose a different backend."
+    exit 1
+  fi
+fi
+##########################################
 # End of CC checks
 # After here, no more $cc or $ld runs
 
@@ -2482,6 +2499,9 @@ if test "$trace_backend" = "simple"; then
   trace_file="\"$trace_file-%u\""
 fi
 echo "CONFIG_TRACE_FILE=$trace_file" >> $config_host_mak
+if test "$trace_backend" = "ust"; then
+  LIBS="-lust $LIBS"
+fi
 
 echo "TOOLS=$tools" >> $config_host_mak
 echo "ROMS=$roms" >> $config_host_mak
diff --git a/tracetool b/tracetool
index d840e6f..d10c9ee 100755
--- a/tracetool
+++ b/tracetool
@@ -13,12 +13,13 @@ set -f
 usage()
 {
     cat >&2 <<EOF
-usage: $0 [--nop | --simple] [-h | -c]
+usage: $0 [--nop | --simple | --ust] [-h | -c]
 Generate tracing code for a file on stdin.
 
 Backends:
   --nop     Tracing disabled
   --simple  Simple built-in backend
+  --ust     LTTng User Space Tracing backend
 
 Output formats:
   -h    Generate .h file
@@ -225,6 +226,86 @@ linetoc_end_simple()
 EOF
 }
 
+# Clean up after UST headers which pollute the namespace
+ust_clean_namespace() {
+    cat <<EOF
+#undef mutex_lock
+#undef mutex_unlock
+#undef inline
+#undef wmb
+EOF
+}
+
+linetoh_begin_ust()
+{
+    echo "#include <ust/tracepoint.h>"
+    ust_clean_namespace
+}
+
+linetoh_ust()
+{
+    local name args argnames
+    name=$(get_name "$1")
+    args=$(get_args "$1")
+    argnames=$(get_argnames "$1")
+
+    cat <<EOF
+DECLARE_TRACE(ust_$name, TPPROTO($args), TPARGS($argnames));
+#define trace_$name trace_ust_$name
+EOF
+}
+
+linetoh_end_ust()
+{
+    return
+}
+
+linetoc_begin_ust()
+{
+    cat <<EOF
+#include <ust/marker.h>
+$(ust_clean_namespace)
+#include "trace.h"
+EOF
+}
+
+linetoc_ust()
+{
+    local name args argnames fmt
+    name=$(get_name "$1")
+    args=$(get_args "$1")
+    argnames=$(get_argnames "$1")
+    fmt=$(get_fmt "$1")
+
+    cat <<EOF
+DEFINE_TRACE(ust_$name);
+
+static void ust_${name}_probe($args)
+{
+    trace_mark(ust, $name, "$fmt", $argnames);
+}
+EOF
+
+    # Collect names for later
+    names="$names $name"
+}
+
+linetoc_end_ust()
+{
+    cat <<EOF
+static void __attribute__((constructor)) trace_init(void)
+{
+EOF
+
+    for name in $names; do
+        cat <<EOF
+    register_trace_ust_$name(ust_${name}_probe);
+EOF
+    done
+
+    echo "}"
+}
+
 # Process stdin by calling begin, line, and end functions for the backend
 convert()
 {
@@ -282,7 +363,7 @@ tracetoc()
 
 # Choose backend
 case "$1" in
-"--nop" | "--simple") backend="${1#--}" ;;
+"--nop" | "--simple" | "--ust") backend="${1#--}" ;;
 *) usage ;;
 esac
 shift
-- 
1.7.1

[Qemu-devel] [PATCH 14/14] trace: Add user documentation

[Stefan Hajnoczi]

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 docs/tracing.txt |  149 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 149 insertions(+), 0 deletions(-)
 create mode 100644 docs/tracing.txt

diff --git a/docs/tracing.txt b/docs/tracing.txt
new file mode 100644
index 0000000..fe3c6ac
--- /dev/null
+++ b/docs/tracing.txt
@@ -0,0 +1,149 @@
+= Tracing =
+
+== Introduction ==
+
+This document describes the tracing infrastructure in QEMU and how to use it
+for debugging, profiling, and observing execution.
+
+== Quickstart ==
+
+1. Build with the 'simple' trace backend:
+
+    ./configure --trace-backend=simple
+    make
+
+2. Run the virtual machine to produce a trace file:
+
+    qemu ... # your normal QEMU invocation
+
+3. Pretty-print the binary trace file:
+
+    ./simpletrace.py trace-events /tmp/trace-*
+
+== Trace events ==
+
+There is a set of static trace events declared in the trace-events source
+file.  Each trace event declaration names the event, its arguments, and the
+format string which can be used for pretty-printing:
+
+    qemu_malloc(size_t size, void *ptr) "size %zu ptr %p"
+    qemu_free(void *ptr) "ptr %p"
+
+The trace-events file is processed by the tracetool script during build to
+generate code for the trace events.  Trace events are invoked directly from
+source code like this:
+
+    #include "trace.h"  /* needed for trace event prototype */
+
+    void *qemu_malloc(size_t size)
+    {
+        void *ptr;
+        if (!size && !allow_zero_malloc()) {
+            abort();
+        }
+        ptr = oom_check(malloc(size ? size : 1));
+        trace_qemu_malloc(size, ptr);  /* <-- trace event */
+        return ptr;
+    }
+
+This is all that needs to be known from a user perspective for adding new
+trace events.
+
+=== Hints for adding new trace events ===
+
+1. Trace state changes in the code.  Interesting points in the code usually
+   involve a state change like starting, stopping, allocating, freeing.  State
+   changes are good trace events because they can be used to understand the
+   execution of the system.
+
+2. Trace guest operations.  Guest I/O accesses like reading device registers
+   are good trace events because they can be used to understand guest
+   interactions.
+
+3. Use correlator fields so the context of an individual line of trace output
+   can be understood.  For example, trace the pointer returned by malloc and
+   used as an argument to free.  This way mallocs and frees can be matched up.
+   Trace events with no context are not very useful.
+
+4. Name trace events after their function.  If there are multiple trace events
+   in one function, append a unique distinguisher at the end of the name.
+
+== Trace backends ==
+
+The tracetool script automates tedious trace event code generation and also
+keeps the trace event declarations independent of the trace backend.  The trace
+events are not tightly coupled to a specific trace backend, such as LTTng or
+SystemTap.  Support for trace backends can be added by extending the tracetool
+script.
+
+The trace backend is chosen at configure time and only one trace backend can
+be built into the binary:
+
+    ./configure --trace-backend=simple
+
+For a list of supported trace backends, try ./configure --help or see below.
+
+The following subsections describe the supported trace backends.
+
+=== Nop ===
+
+The "nop" backend generates empty trace event functions so that the compiler
+can optimize out trace events completely.  This is the default and imposes no
+performance penalty.
+
+=== Simpletrace ===
+
+The "simple" backend supports common use cases and comes as part of the QEMU
+source tree.  It may not be as powerful as platform-specific or third-party
+trace backends but it is portable.  This is the recommended trace backend
+unless you have specific needs for more advanced backends.
+
+==== Monitor commands ====
+
+* info trace
+  Display the contents of trace buffer.  This command dumps the trace buffer
+  with simple formatting.  For full pretty-printing, use the simpletrace.py
+  script on a binary trace file.
+
+  The trace buffer is written into until full.  The full trace buffer is
+  flushed and emptied.  This means the 'info trace' will display few or no
+  entries if the buffer has just been flushed.
+
+* 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.
+
+* trace-file on|off|flush|set <path>
+  Enable/disable/flush the trace file or set the trace file name.
+
+==== Enabling/disabling trace events programmatically ====
+
+The st_change_trace_event_state() function can be used to enable or disable trace
+events at runtime inside QEMU:
+
+    #include "trace.h"
+    
+    st_change_trace_event_state("virtio_irq", true); /* enable */
+    [...]
+    st_change_trace_event_state("virtio_irq", false); /* disable */
+
+==== Analyzing trace files ====
+
+The "simple" backend produces binary trace files that can be formatted with the
+simpletrace.py script.  The script takes the trace-events file and the binary
+trace:
+
+    ./simpletrace.py trace-events /tmp/trace.log
+
+You must ensure that the same trace-events file was used to build QEMU,
+otherwise trace event declarations may have changed and output will not be
+consistent.
+
+=== LTTng Userspace Tracer ===
+
+The "ust" backend uses the LTTng Userspace Tracer library.  There are no
+monitor commands built into QEMU, instead UST utilities should be used to list,
+enable/disable, and dump traces.
-- 
1.7.1

Re: [Qemu-devel] [PATCH 01/14] trace: Add trace-events file for declaring trace events

[malc]

On Thu, 12 Aug 2010, Stefan Hajnoczi wrote:

> This patch introduces the trace-events file where trace events can be
> declared like so:
> 
> qemu_malloc(size_t size) "size %zu"
> qemu_free(void *ptr) "ptr %p"
> 

[..snip..]

> +        echo -n "$name, "

Once again: standard echo doesn't have any command line arguments, please
use printf instead.

[..snip..]

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH 07/14] trace: Add simple built-in tracing backend

[Blue Swirl]

On Thu, Aug 12, 2010 at 10:36 AM, Stefan Hajnoczi
<stefanha@linux.vnet.ibm.com> wrote:
> This patch adds a simple tracer which produces binary trace files.  To
> try out the simple backend:
>
> $ ./configure --trace-backend=simple
> $ make
>
> After running QEMU you can pretty-print the trace:
>
> $ ./simpletrace.py trace-events /tmp/trace.log
>
> The output of simpletrace.py looks like this:
>
>  qemu_realloc 0.699 ptr=0x24363f0 size=0x3 newptr=0x24363f0
>  qemu_free 0.768 ptr=0x24363f0
>  ^           ^---- timestamp delta (us)
>  |____ trace event name
>
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>
> trace: Make trace record fields 64-bit
>
> Explicitly use 64-bit fields in trace records so that timestamps and
> magic numbers work for 32-bit host builds.
>
> Includes fixes from Prerna Saxena <prerna@linux.vnet.ibm.com>.
>
> Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> ---
>  .gitignore     |    1 +
>  Makefile       |    2 +
>  Makefile.objs  |    3 +
>  configure      |    2 +-
>  simpletrace.c  |  127 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  simpletrace.h  |   26 +++++++++++
>  simpletrace.py |   96 ++++++++++++++++++++++++++++++++++++++++++
>  tracetool      |   82 ++++++++++++++++++++++++++++++++++--
>  8 files changed, 333 insertions(+), 6 deletions(-)
>  create mode 100644 simpletrace.c
>  create mode 100644 simpletrace.h
>  create mode 100755 simpletrace.py
>
> diff --git a/.gitignore b/.gitignore
> index f3f2bb7..72bff98 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -42,6 +42,7 @@ QMP/qmp-commands.txt
>  *.log
>  *.pdf
>  *.pg
> +*.pyc
>  *.toc
>  *.tp
>  *.vr
> diff --git a/Makefile b/Makefile
> index 3c5e6a0..ab91d42 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -112,6 +112,8 @@ trace.c: $(SRC_PATH)/trace-events config-host.mak
>
>  trace.o: trace.c $(GENERATED_HEADERS)
>
> +simpletrace.o: simpletrace.c $(GENERATED_HEADERS)
> +
>  ######################################################################
>
>  qemu-img.o: qemu-img-cmds.h
> diff --git a/Makefile.objs b/Makefile.objs
> index c61332d..3ef6d80 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -269,6 +269,9 @@ libdis-$(CONFIG_SPARC_DIS) += sparc-dis.o
>  # trace
>
>  trace-obj-y = trace.o
> +ifeq ($(TRACE_BACKEND),simple)
> +trace-obj-y += simpletrace.o
> +endif
>
>  vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
>
> diff --git a/configure b/configure
> index d3648b2..62dd10d 100755
> --- a/configure
> +++ b/configure
> @@ -898,7 +898,7 @@ echo "  --enable-docs            enable documentation build"
>  echo "  --disable-docs           disable documentation build"
>  echo "  --disable-vhost-net      disable vhost-net acceleration support"
>  echo "  --enable-vhost-net       enable vhost-net acceleration support"
> -echo "  --trace-backend=B        Trace backend nop"
> +echo "  --trace-backend=B        Trace backend nop simple"
>  echo ""
>  echo "NOTE: The object files are built at the place where configure is launched"
>  exit 1
> diff --git a/simpletrace.c b/simpletrace.c
> new file mode 100644
> index 0000000..a6afc51
> --- /dev/null
> +++ b/simpletrace.c
> @@ -0,0 +1,127 @@
> +/*
> + * Simple trace backend
> + *
> + * Copyright IBM, Corp. 2010
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include "trace.h"
> +
> +/** Trace file header event ID */
> +#define HEADER_EVENT_ID (~(uint64_t)0) /* avoids conflicting with TraceEventIDs */
> +
> +/** Trace file magic number */
> +#define HEADER_MAGIC 0xf2b177cb0aa429b4ULL
> +
> +/** Trace file version number, bump if format changes */
> +#define HEADER_VERSION 0
> +
> +/** Trace buffer entry */
> +typedef struct {
> +    uint64_t event;
> +    uint64_t timestamp_ns;
> +    uint64_t x1;
> +    uint64_t x2;
> +    uint64_t x3;
> +    uint64_t x4;
> +    uint64_t x5;
> +} TraceRecord;
> +
> +enum {
> +    TRACE_BUF_LEN = 64 * 1024 / sizeof(TraceRecord),
> +};
> +
> +static TraceRecord trace_buf[TRACE_BUF_LEN];
> +static unsigned int trace_idx;
> +static FILE *trace_fp;
> +
> +static bool write_header(FILE *fp)
> +{
> +    TraceRecord header = {

This should be 'static const'.

> +        .event = HEADER_EVENT_ID,
> +        .timestamp_ns = HEADER_MAGIC,
> +        .x1 = HEADER_VERSION,
> +    };
> +
> +    return fwrite(&header, sizeof header, 1, fp) == 1;
> +}
> +
> +static void flush_trace_buffer(void)
> +{
> +    if (!trace_fp) {
> +        trace_fp = fopen("/tmp/trace.log", "w");
> +        if (trace_fp) {
> +            write_header(trace_fp);
> +            atexit(flush_trace_buffer);
> +        }
> +    }
> +    if (trace_fp) {
> +        size_t unused; /* for when fwrite(3) is declared warn_unused_result */
> +        unused = fwrite(trace_buf, trace_idx * sizeof(trace_buf[0]), 1, trace_fp);
> +    }
> +
> +    /* Discard written trace records */
> +    trace_idx = 0;
> +}
> +
> +static void trace(TraceEventID event, unsigned long x1,
> +                  unsigned long x2, unsigned long x3,
> +                  unsigned long x4, unsigned long x5)
> +{
> +    TraceRecord *rec = &trace_buf[trace_idx];
> +    struct timespec ts;
> +
> +    /* TODO Windows?  It would be good to use qemu-timer here but that isn't
> +     * linked into qemu-tools.  Also we should avoid recursion in the tracing
> +     * code, therefore it is useful to be self-contained.
> +     */
> +    clock_gettime(CLOCK_MONOTONIC, &ts);
> +
> +    rec->event = event;
> +    rec->timestamp_ns = ts.tv_sec * 1000000000LL + ts.tv_nsec;
> +    rec->x1 = x1;
> +    rec->x2 = x2;
> +    rec->x3 = x3;
> +    rec->x4 = x4;
> +    rec->x5 = x5;
> +
> +    if (++trace_idx == TRACE_BUF_LEN) {
> +        flush_trace_buffer();
> +    }
> +}
> +
> +void trace0(TraceEventID event)
> +{
> +    trace(event, 0, 0, 0, 0, 0);
> +}
> +
> +void trace1(TraceEventID event, uint64_t x1)
> +{
> +    trace(event, x1, 0, 0, 0, 0);
> +}
> +
> +void trace2(TraceEventID event, uint64_t x1, uint64_t x2)
> +{
> +    trace(event, x1, x2, 0, 0, 0);
> +}
> +
> +void trace3(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3)
> +{
> +    trace(event, x1, x2, x3, 0, 0);
> +}
> +
> +void trace4(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t x4)
> +{
> +    trace(event, x1, x2, x3, x4, 0);
> +}
> +
> +void trace5(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t x4, uint64_t x5)
> +{
> +    trace(event, x1, x2, x3, x4, x5);
> +}
> diff --git a/simpletrace.h b/simpletrace.h
> new file mode 100644
> index 0000000..e4e9759
> --- /dev/null
> +++ b/simpletrace.h
> @@ -0,0 +1,26 @@
> +/*
> + * Simple trace backend
> + *
> + * Copyright IBM, Corp. 2010
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef SIMPLETRACE_H
> +#define SIMPLETRACE_H
> +
> +#include <stdbool.h>

Why?

> +#include <stdint.h>
> +
> +typedef uint64_t TraceEventID;
> +
> +void trace0(TraceEventID event);
> +void trace1(TraceEventID event, uint64_t x1);
> +void trace2(TraceEventID event, uint64_t x1, uint64_t x2);
> +void trace3(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3);
> +void trace4(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t x4);
> +void trace5(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t x4, uint64_t x5);
> +
> +#endif /* SIMPLETRACE_H */
> diff --git a/simpletrace.py b/simpletrace.py
> new file mode 100755
> index 0000000..fdf0eb5
> --- /dev/null
> +++ b/simpletrace.py
> @@ -0,0 +1,96 @@
> +#!/usr/bin/env python
> +#
> +# Pretty-printer for simple trace backend binary trace files
> +#
> +# Copyright IBM, Corp. 2010
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2.  See
> +# the COPYING file in the top-level directory.
> +#
> +# For help see docs/tracing.txt
> +
> +import sys
> +import struct
> +import re
> +
> +header_event_id = 0xffffffffffffffff
> +header_magic    = 0xf2b177cb0aa429b4
> +header_version  = 0
> +
> +trace_fmt = '=QQQQQQQ'
> +trace_len = struct.calcsize(trace_fmt)
> +event_re  = re.compile(r'(disable\s+)?([a-zA-Z0-9_]+)\(([^)]*)\)\s+"([^"]*)"')
> +
> +def err(msg):
> +    sys.stderr.write(msg + '\n')
> +    sys.exit(1)
> +
> +def parse_events(fobj):
> +    """Parse a trace-events file."""
> +
> +    def get_argnames(args):
> +        """Extract argument names from a parameter list."""
> +        return tuple(arg.split()[-1].lstrip('*') for arg in args.split(','))
> +
> +    events = {}
> +    event_num = 0
> +    for line in fobj:
> +        m = event_re.match(line.strip())
> +        if m is None:
> +            continue
> +
> +        disable, name, args, fmt = m.groups()
> +        if disable:
> +            continue
> +
> +        events[event_num] = (name,) + get_argnames(args)
> +        event_num += 1
> +    return events
> +
> +def read_record(fobj):
> +    """Deserialize a trace record from a file."""
> +    s = fobj.read(trace_len)
> +    if len(s) != trace_len:
> +        return None
> +    return struct.unpack(trace_fmt, s)
> +
> +def read_trace_file(fobj):
> +    """Deserialize trace records from a file."""
> +    header = read_record(fobj)
> +    if header is None or \
> +       header[0] != header_event_id or \
> +       header[1] != header_magic or \
> +       header[2] != header_version:
> +        err('not a trace file or incompatible version')
> +
> +    while True:
> +        rec = read_record(fobj)
> +        if rec is None:
> +            break
> +
> +        yield rec
> +
> +class Formatter(object):
> +    def __init__(self, events):
> +        self.events = events
> +        self.last_timestamp = None
> +
> +    def format_record(self, rec):
> +        if self.last_timestamp is None:
> +            self.last_timestamp = rec[1]
> +        delta_ns = rec[1] - self.last_timestamp
> +        self.last_timestamp = rec[1]
> +
> +        event = self.events[rec[0]]
> +        fields = [event[0], '%0.3f' % (delta_ns / 1000.0)]
> +        for i in xrange(1, len(event)):
> +            fields.append('%s=0x%x' % (event[i], rec[i + 1]))
> +        return ' '.join(fields)
> +
> +if len(sys.argv) != 3:
> +    err('usage: %s <trace-events> <trace-file>' % sys.argv[0])
> +
> +events = parse_events(open(sys.argv[1], 'r'))
> +formatter = Formatter(events)
> +for rec in read_trace_file(open(sys.argv[2], 'rb')):
> +    print formatter.format_record(rec)
> diff --git a/tracetool b/tracetool
> index 86b6446..66df685 100755
> --- a/tracetool
> +++ b/tracetool
> @@ -13,11 +13,12 @@ set -f
>  usage()
>  {
>     cat >&2 <<EOF
> -usage: $0 --nop [-h | -c]
> +usage: $0 [--nop | --simple] [-h | -c]
>  Generate tracing code for a file on stdin.
>
>  Backends:
> -  --nop Tracing disabled
> +  --nop     Tracing disabled
> +  --simple  Simple built-in backend
>
>  Output formats:
>   -h    Generate .h file
> @@ -56,16 +57,27 @@ get_argnames()
>         name=${field%,}
>         test "$field" = "$name" && continue
>
> -        echo -n "$name, "
> +        printf "%s" "$name, "
>     done
>
>     # Last argument name
>     if [ "$nfields" -gt 1 ]
>     then
> -        echo -n "$name"
> +        printf "%s" "$name"
>     fi
>  }
>
> +# Get the number of arguments to a trace event
> +get_argc()
> +{
> +    local name argc
> +    argc=0
> +    for name in $(get_argnames "$1"); do
> +        argc=$((argc + 1))
> +    done
> +    echo $argc
> +}
> +
>  # Get the format string for a trace event
>  get_fmt()
>  {
> @@ -115,6 +127,66 @@ linetoc_end_nop()
>     return
>  }
>
> +linetoh_begin_simple()
> +{
> +    cat <<EOF
> +#include "simpletrace.h"
> +EOF
> +
> +    simple_event_num=0
> +}
> +
> +cast_args_to_uint64_t()
> +{
> +    local arg
> +    for arg in $(get_argnames "$1"); do
> +        printf "%s" "(uint64_t)(uintptr_t)$arg"
> +    done
> +}
> +
> +linetoh_simple()
> +{
> +    local name args argc trace_args
> +    name=$(get_name "$1")
> +    args=$(get_args "$1")
> +    argc=$(get_argc "$1")
> +
> +    trace_args="$simple_event_num"
> +    if [ "$argc" -gt 0 ]
> +    then
> +        trace_args="$trace_args, $(cast_args_to_uint64_t "$1")"
> +    fi
> +
> +    cat <<EOF
> +static inline void trace_$name($args)
> +{
> +    trace$argc($trace_args);
> +}
> +EOF
> +
> +    simple_event_num=$((simple_event_num + 1))
> +}
> +
> +linetoh_end_simple()
> +{
> +    return
> +}
> +
> +linetoc_begin_simple()
> +{
> +    return
> +}
> +
> +linetoc_simple()
> +{
> +    return
> +}
> +
> +linetoc_end_simple()
> +{
> +    return
> +}
> +
>  # Process stdin by calling begin, line, and end functions for the backend
>  convert()
>  {
> @@ -160,7 +232,7 @@ tracetoc()
>
>  # Choose backend
>  case "$1" in
> -"--nop") backend="${1#--}" ;;
> +"--nop" | "--simple") backend="${1#--}" ;;
>  *) usage ;;
>  esac
>  shift
> --
> 1.7.1
>
>
>

Re: [Qemu-devel] [PATCH 08/14] trace: Support for dynamically enabling/disabling trace events.

[Blue Swirl]

On Thu, Aug 12, 2010 at 10:36 AM, Stefan Hajnoczi
<stefanha@linux.vnet.ibm.com> wrote:
> From: Prerna Saxena <prerna@linux.vnet.ibm.com>
>
> This patch adds support for dynamically enabling/disabling of trace events.
> This is done by internally maintaining each trace event's state, and
> permitting logging of data from a trace event only if it is in an
> 'active' state.
>
> Monitor commands added :
> 1) info trace-events            : to view all available trace events and
>                                  their state.
> 2) trace-event NAME on|off      : to enable/disable data logging from a
>                                  given trace event.
>                                  Eg, trace-event paio_submit off
>                                        disables logging of data when
>                                        paio_submit is hit.
>
> By default, all trace-events are disabled. One can enable desired trace-events
> via the monitor.
>
> Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>
> trace: Monitor command 'info trace'
>
> Monitor command 'info trace' to display contents of trace buffer
>
> Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>
> trace: Remove monitor.h dependency from simpletrace
>
> User-mode targets don't have a monitor so the simple trace backend
> currently does not build on those targets.  This patch abstracts the
> monitor printing interface so there is no direct coupling between
> simpletrace and the monitor.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> ---
>  configure       |    3 +++
>  monitor.c       |   40 ++++++++++++++++++++++++++++++++++++++++
>  qemu-monitor.hx |   25 +++++++++++++++++++++++++
>  simpletrace.c   |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  simpletrace.h   |    9 +++++++++
>  tracetool       |   24 ++++++++++++++++++++----
>  6 files changed, 149 insertions(+), 4 deletions(-)
>
> diff --git a/configure b/configure
> index 62dd10d..830e49e 100755
> --- a/configure
> +++ b/configure
> @@ -2466,6 +2466,9 @@ bsd)
>  esac
>
>  echo "TRACE_BACKEND=$trace_backend" >> $config_host_mak
> +if test "$trace_backend" = "simple"; then
> +  echo "CONFIG_SIMPLE_TRACE=y" >> $config_host_mak
> +fi
>  echo "TOOLS=$tools" >> $config_host_mak
>  echo "ROMS=$roms" >> $config_host_mak
>  echo "MAKE=$make" >> $config_host_mak
> diff --git a/monitor.c b/monitor.c
> index c313d5a..4f1639d 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -56,6 +56,9 @@
>  #include "json-parser.h"
>  #include "osdep.h"
>  #include "exec-all.h"
> +#ifdef CONFIG_SIMPLE_TRACE
> +#include "trace.h"
> +#endif
>
>  //#define DEBUG
>  //#define DEBUG_COMPLETION
> @@ -539,6 +542,15 @@ static void do_help_cmd(Monitor *mon, const QDict *qdict)
>     help_cmd(mon, qdict_get_try_str(qdict, "name"));
>  }
>
> +#ifdef CONFIG_SIMPLE_TRACE
> +static void do_change_trace_event_state(Monitor *mon, const QDict *qdict)
> +{
> +    const char *tp_name = qdict_get_str(qdict, "name");
> +    bool new_state = qdict_get_bool(qdict, "option");
> +    st_change_trace_event_state(tp_name, new_state);
> +}
> +#endif
> +
>  static void user_monitor_complete(void *opaque, QObject *ret_data)
>  {
>     MonitorCompletionData *data = (MonitorCompletionData *)opaque;
> @@ -923,6 +935,18 @@ static void do_info_cpu_stats(Monitor *mon)
>  }
>  #endif
>
> +#if defined(CONFIG_SIMPLE_TRACE)
> +static void do_info_trace(Monitor *mon)
> +{
> +    st_print_trace((FILE *)mon, &monitor_fprintf);

The cast is ugly. Is there no other way than passing falsified types?

> +}
> +
> +static void do_info_trace_events(Monitor *mon)
> +{
> +    st_print_trace_events((FILE *)mon, &monitor_fprintf);
> +}
> +#endif
> +
>  /**
>  * do_quit(): Quit QEMU execution
>  */
> @@ -2579,6 +2603,22 @@ static const mon_cmd_t info_cmds[] = {
>         .help       = "show roms",
>         .mhandler.info = do_info_roms,
>     },
> +#if defined(CONFIG_SIMPLE_TRACE)
> +    {
> +        .name       = "trace",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show current contents of trace buffer",
> +        .mhandler.info = do_info_trace,
> +    },
> +    {
> +        .name       = "trace-events",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show available trace-events & their state",
> +        .mhandler.info = do_info_trace_events,
> +    },
> +#endif
>     {
>         .name       = NULL,
>     },
> diff --git a/qemu-monitor.hx b/qemu-monitor.hx
> index 2af3de6..a9f4221 100644
> --- a/qemu-monitor.hx
> +++ b/qemu-monitor.hx
> @@ -259,6 +259,22 @@ STEXI
>  Output logs to @var{filename}.
>  ETEXI
>
> +#ifdef CONFIG_SIMPLE_TRACE
> +    {
> +        .name       = "trace-event",
> +        .args_type  = "name:s,option:b",
> +        .params     = "name on|off",
> +        .help       = "changes status of a specific trace event",
> +        .mhandler.cmd = do_change_trace_event_state,
> +    },
> +
> +STEXI
> +@item trace-event
> +@findex trace-event
> +changes status of a trace event
> +ETEXI
> +#endif
> +
>     {
>         .name       = "log",
>         .args_type  = "items:s",
> @@ -2495,6 +2511,15 @@ show roms
>  @end table
>  ETEXI
>
> +#ifdef CONFIG_SIMPLE_TRACE
> +STEXI
> +@item info trace
> +show contents of trace buffer
> +@item info trace-events
> +show available trace events and their state
> +ETEXI
> +#endif
> +
>  HXCOMM DO NOT add new commands after 'info', move your addition before it!
>
>  STEXI
> diff --git a/simpletrace.c b/simpletrace.c
> index a6afc51..311fa44 100644
> --- a/simpletrace.c
> +++ b/simpletrace.c
> @@ -11,6 +11,7 @@
>  #include <stdlib.h>
>  #include <stdint.h>
>  #include <stdio.h>
> +#include <time.h>

The changes below don't seem to require this addition.

>  #include "trace.h"
>
>  /** Trace file header event ID */
> @@ -83,6 +84,10 @@ static void trace(TraceEventID event, unsigned long x1,
>      */
>     clock_gettime(CLOCK_MONOTONIC, &ts);
>
> +    if (!trace_list[event].state) {
> +        return;
> +    }
> +
>     rec->event = event;
>     rec->timestamp_ns = ts.tv_sec * 1000000000LL + ts.tv_nsec;
>     rec->x1 = x1;
> @@ -125,3 +130,50 @@ void trace5(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t
>  {
>     trace(event, x1, x2, x3, x4, x5);
>  }
> +
> +void st_print_trace(FILE *stream, int (*stream_printf)(FILE *stream, const char *fmt, ...))
> +{
> +    unsigned int i;
> +
> +    for (i = 0; i < trace_idx; i++) {
> +        stream_printf(stream, "Event %lu : %lx %lx %lx %lx %lx\n",
> +                      trace_buf[i].event, trace_buf[i].x1, trace_buf[i].x2,
> +                      trace_buf[i].x3, trace_buf[i].x4, trace_buf[i].x5);
> +    }
> +}
> +
> +void st_print_trace_events(FILE *stream, int (*stream_printf)(FILE *stream, const char *fmt, ...))
> +{
> +    unsigned int i;
> +
> +    for (i = 0; i < NR_TRACE_EVENTS; i++) {
> +        stream_printf(stream, "%s [Event ID %u] : state %u\n",
> +                      trace_list[i].tp_name, i, trace_list[i].state);
> +    }
> +}
> +
> +static TraceEvent* find_trace_event_by_name(const char *tname)
> +{
> +    unsigned int i;
> +
> +    if (!tname) {
> +        return NULL;
> +    }
> +
> +    for (i = 0; i < NR_TRACE_EVENTS; i++) {
> +        if (!strcmp(trace_list[i].tp_name, tname)) {
> +            return &trace_list[i];
> +        }
> +    }
> +    return NULL; /* indicates end of list reached without a match */
> +}
> +
> +void st_change_trace_event_state(const char *tname, bool tstate)
> +{
> +    TraceEvent *tp;
> +
> +    tp = find_trace_event_by_name(tname);
> +    if (tp) {
> +        tp->state = tstate;
> +    }
> +}
> diff --git a/simpletrace.h b/simpletrace.h
> index e4e9759..ee30ae9 100644
> --- a/simpletrace.h
> +++ b/simpletrace.h
> @@ -13,14 +13,23 @@
>
>  #include <stdbool.h>
>  #include <stdint.h>
> +#include <stdio.h>
>
>  typedef uint64_t TraceEventID;
>
> +typedef struct {
> +    const char *tp_name;
> +    bool state;
> +} TraceEvent;
> +
>  void trace0(TraceEventID event);
>  void trace1(TraceEventID event, uint64_t x1);
>  void trace2(TraceEventID event, uint64_t x1, uint64_t x2);
>  void trace3(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3);
>  void trace4(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t x4);
>  void trace5(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t x4, uint64_t x5);
> +void st_print_trace(FILE *stream, int (*stream_printf)(FILE *stream, const char *fmt, ...));
> +void st_print_trace_events(FILE *stream, int (*stream_printf)(FILE *stream, const char *fmt, ...));
> +void st_change_trace_event_state(const char *tname, bool tstate);
>
>  #endif /* SIMPLETRACE_H */
> diff --git a/tracetool b/tracetool
> index 66df685..30dc812 100755
> --- a/tracetool
> +++ b/tracetool
> @@ -169,22 +169,38 @@ EOF
>
>  linetoh_end_simple()
>  {
> -    return
> +    cat <<EOF
> +#define NR_TRACE_EVENTS $simple_event_num
> +extern TraceEvent trace_list[NR_TRACE_EVENTS];
> +EOF
>  }
>
>  linetoc_begin_simple()
>  {
> -    return
> +    cat <<EOF
> +#include "trace.h"
> +
> +TraceEvent trace_list[] = {
> +EOF
> +    simple_event_num=0
> +
>  }
>
>  linetoc_simple()
>  {
> -    return
> +    local name
> +    name=$(get_name "$1")
> +    cat <<EOF
> +{.tp_name = "$name", .state=0},
> +EOF
> +    simple_event_num=$((simple_event_num + 1))
>  }
>
>  linetoc_end_simple()
>  {
> -    return
> +    cat <<EOF
> +};
> +EOF
>  }
>
>  # Process stdin by calling begin, line, and end functions for the backend
> --
> 1.7.1
>
>
>

Re: [Qemu-devel] [PATCH 00/14] trace: Add static tracing to QEMU

[Blue Swirl]

On Thu, Aug 12, 2010 at 10:36 AM, Stefan Hajnoczi
<stefanha@linux.vnet.ibm.com> wrote:
> This patch series adds static tracing to QEMU.  It can be used to instrument
> QEMU code by means of lightweight logging called trace events.
>
> Prerna and I are now posting the entire patch series with a serious eye towards
> checking we meet users' and developers' tracing needs and with the goal of
> getting this functionality merged into qemu.git.

I sent a few comments about some patches, others looked fine to me.

Re: [Qemu-devel] [PATCH 01/14] trace: Add trace-events file for declaring trace events

[Stefan Hajnoczi]

On Thu, Aug 12, 2010 at 5:07 PM, malc <av1474@comtv.ru> wrote:
> On Thu, 12 Aug 2010, Stefan Hajnoczi wrote:
>
>> This patch introduces the trace-events file where trace events can be
>> declared like so:
>>
>> qemu_malloc(size_t size) "size %zu"
>> qemu_free(void *ptr) "ptr %p"
>>
>
> [..snip..]
>
>> +        echo -n "$name, "
>
> Once again: standard echo doesn't have any command line arguments, please
> use printf instead.

Sorry about this.  The s/echo -n/printf "%s"/ fix is in the patchset
in "[PATCH 07/14] trace: Add simple built-in tracing backend".  I
didn't push the change back far enough in the commit history.

Will fix for v2.

Stefan

Re: [Qemu-devel] [PATCH 07/14] trace: Add simple built-in tracing backend

[Stefan Hajnoczi]

On Thu, Aug 12, 2010 at 6:56 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Thu, Aug 12, 2010 at 10:36 AM, Stefan Hajnoczi
>> +static bool write_header(FILE *fp)
>> +{
>> +    TraceRecord header = {
>
> This should be 'static const'.

Okay.

>> diff --git a/simpletrace.h b/simpletrace.h
>> new file mode 100644
>> index 0000000..e4e9759
>> --- /dev/null
>> +++ b/simpletrace.h
>> @@ -0,0 +1,26 @@
>> +/*
>> + * Simple trace backend
>> + *
>> + * Copyright IBM, Corp. 2010
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2.  See
>> + * the COPYING file in the top-level directory.
>> + *
>> + */
>> +
>> +#ifndef SIMPLETRACE_H
>> +#define SIMPLETRACE_H
>> +
>> +#include <stdbool.h>
>
> Why?

The stdbool.h header is not needed here.  This is an artifact of
rebasing the tracing branch to squash and order commits logically.
The header is only needed starting in "[PATCH 08/14] trace: Support
for dynamically enabling/disabling trace events".

Will fix in v2.

Stefan

Re: [Qemu-devel] [PATCH 08/14] trace: Support for dynamically enabling/disabling trace events.

[Stefan Hajnoczi]

On Thu, Aug 12, 2010 at 7:02 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Thu, Aug 12, 2010 at 10:36 AM, Stefan Hajnoczi
> <stefanha@linux.vnet.ibm.com> wrote:
>> From: Prerna Saxena <prerna@linux.vnet.ibm.com>
>> +#if defined(CONFIG_SIMPLE_TRACE)
>> +static void do_info_trace(Monitor *mon)
>> +{
>> +    st_print_trace((FILE *)mon, &monitor_fprintf);
>
> The cast is ugly. Is there no other way than passing falsified types?

Yes.  This approach is the solution that monitor.c currently uses for
decoupling subsystem code from monitor code:

static void do_info_registers(Monitor *mon)
{
    CPUState *env;
    env = mon_get_cpu();
#ifdef TARGET_I386
    cpu_dump_state(env, (FILE *)mon, monitor_fprintf,
                   X86_DUMP_FPU);
#else
    cpu_dump_state(env, (FILE *)mon, monitor_fprintf,
                   0);
#endif
}

...and others do it in order to avoid telling subsystems about
Monitor.  There are 4 other places in monitor.c before this patch that
use this technique.  I suggest leaving it and if someone implements a
better approach, a search-replace will update monitor.c in one patch.

>> diff --git a/simpletrace.c b/simpletrace.c
>> index a6afc51..311fa44 100644
>> --- a/simpletrace.c
>> +++ b/simpletrace.c
>> @@ -11,6 +11,7 @@
>>  #include <stdlib.h>
>>  #include <stdint.h>
>>  #include <stdio.h>
>> +#include <time.h>
>
> The changes below don't seem to require this addition.

You are right, this is a rebase/squash issue.  Sorry, will fix in v2.

Stefan

Re: [Qemu-devel] [PATCH 00/14] trace: Add static tracing to QEMU

[Stefan Hajnoczi]

On Thu, Aug 12, 2010 at 7:10 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Thu, Aug 12, 2010 at 10:36 AM, Stefan Hajnoczi
> <stefanha@linux.vnet.ibm.com> wrote:
>> This patch series adds static tracing to QEMU.  It can be used to instrument
>> QEMU code by means of lightweight logging called trace events.
>>
>> Prerna and I are now posting the entire patch series with a serious eye towards
>> checking we meet users' and developers' tracing needs and with the goal of
>> getting this functionality merged into qemu.git.
>
> I sent a few comments about some patches, others looked fine to me.

I appreciate your reviews!  Thank you.

Two outstanding issues that I'd like to discuss:

1. Writing the trace buffer is currently synchronous - fwrite(3) is
used by the unlucky caller that fills up the trace buffer.  Ideally
traces should never block and write-out would be asynchronous.  With a
regular file I'm not sure how much of an issue this is to performance,
since the kernel could copy the trace buffer to the page cache and
write it out sometime later.  I've avoided adding a dedicated thread
because it will make the code more complex - any thoughts?

2. The timestamping code uses clock_gettime() and will not build on
Windows.  I'd like to reuse qemu-timer.c but there are two issues.
First, calling QEMU code from the trace backend risks infinite
recursion so we need to be limit that or add a counter to prevent
recursion.  Second, the qemu-timer.c code isn't build into qemu-tools,
whereas tracing should work across the whole codebase (usermode,
softmmu, tools, etc).  The ugly solution would be to add the Windows
timestamp code into simpletrace.c too.

While we're at it I'm going to add a 6th argument to the trace record
so we don't need to change the simple trace backend when someone needs
to trace 6 arguments ;).  Right now it support 0-5 arguments, but 6
seems like a reasonable limit.

Stefan

Re: [Qemu-devel] [PATCH 00/14] trace: Add static tracing to QEMU

[Lluís]

Stefan Hajnoczi writes:

> 1. Writing the trace buffer is currently synchronous - fwrite(3) is
> used by the unlucky caller that fills up the trace buffer.  Ideally
> traces should never block and write-out would be asynchronous.  With a
> regular file I'm not sure how much of an issue this is to performance,
> since the kernel could copy the trace buffer to the page cache and
> write it out sometime later.  I've avoided adding a dedicated thread
> because it will make the code more complex - any thoughts?

I think the only reliable and non-multithreaded way to do it would be to use
aio_write(3). Then, total buffer size and the threshold to trigger an aio_write
should be tuned to match the current disk throughput in order to eliminate
emulation stalls.

But this doesn't sound much easier than a producer-consumer threading scheme.

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 01/14] trace: Add trace-events file for declaring trace events

[Anthony Liguori]

On 08/12/2010 05:36 AM, Stefan Hajnoczi wrote:
> +linetoh_nop()
> +{
> +    local name args
> +    name=$(get_name "$1")
> +    args=$(get_args "$1")
> +
> +    # Define an empty function for the trace event
> +    cat<<EOF
> +static inline void trace_$name($args)
> +{
> +}
>    

This seems like it can potentially create a single header that ends up 
needed to know about every type of structure QEMU implements, no?  If 
so, this would be challenging for improving modularity.

Or would we restrict trace points to not allow passing of anything but 
void *s?

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 06/14] trace: Trace entry point of balloon request handler

[Anthony Liguori]

On 08/12/2010 05:36 AM, Stefan Hajnoczi wrote:
> From: Prerna Saxena<prerna@linux.vnet.ibm.com>
>
> Signed-off-by: Prerna Saxena<prerna@linux.vnet.ibm.com>
> Signed-off-by: Stefan Hajnoczi<stefanha@linux.vnet.ibm.com>
> ---
>   balloon.c    |    2 ++
>   trace-events |    4 ++++
>   2 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/balloon.c b/balloon.c
> index 8e0b7f1..0021fef 100644
> --- a/balloon.c
> +++ b/balloon.c
> @@ -29,6 +29,7 @@
>   #include "cpu-common.h"
>   #include "kvm.h"
>   #include "balloon.h"
> +#include "trace.h"
>
>
>   static QEMUBalloonEvent *qemu_balloon_event;
> @@ -43,6 +44,7 @@ void qemu_add_balloon_handler(QEMUBalloonEvent *func, void *opaque)
>   int qemu_balloon(ram_addr_t target, MonitorCompletion cb, void *opaque)
>   {
>       if (qemu_balloon_event) {
> +        trace_balloon_event(qemu_balloon_event_opaque, target);
>           qemu_balloon_event(qemu_balloon_event_opaque, target, cb, opaque);
>           return 1;
>       } else {
> diff --git a/trace-events b/trace-events
> index c1f3e28..eae62da 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -58,3 +58,7 @@ paio_submit(void *acb, void *opaque, unsigned long sector_num, unsigned long nb_
>   # ioport.c
>   cpu_in(unsigned int addr, unsigned int val) "addr %#x value %u"
>   cpu_out(unsigned int addr, unsigned int val) "addr %#x value %u"
> +
> +# balloon.c
> +# Since requests are raised via monitor, not many tracepoints are needed.
> +balloon_event(void *opaque, unsigned long addr) "opaque %p addr %lu"
>    

ram_addr_t is not necessarily an unsigned long.  It's actually a 
uint64_t.   Integer promotion should handle this in most cases but 
having the definition in two different places worries me a bit.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 12/14] trace: Add trace file name command-line option

[Anthony Liguori]

On 08/12/2010 05:36 AM, Stefan Hajnoczi wrote:
> From: Prerna Saxena<prerna@linux.vnet.ibm.com>
>
> This patch adds an optional command line switch '-trace' to specify the
> filename to write traces to, when qemu starts.
> Eg, If compiled with the 'simple' trace backend,
> [temp@system]$ qemu -trace FILENAME IMAGE
> Allows the binary traces to be written to FILENAME instead of the option
> set at config-time.
>
> Signed-off-by: Prerna Saxena<prerna@linux.vnet.ibm.com>
> Signed-off-by: Stefan Hajnoczi<stefanha@linux.vnet.ibm.com>
> ---
>   qemu-options.hx |   11 +++++++++++
>   vl.c            |   18 ++++++++++++++++++
>   2 files changed, 29 insertions(+), 0 deletions(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index db86feb..a0c091f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2234,6 +2234,17 @@ Normally QEMU loads a configuration file from @var{sysconfdir}/qemu.conf and
>   @var{sysconfdir}/target-@var{ARCH}.conf on startup.  The @code{-nodefconfig}
>   option will prevent QEMU from loading these configuration files at startup.
>   ETEXI
> +#ifdef CONFIG_SIMPLE_TRACE
> +DEF("trace", HAS_ARG, QEMU_OPTION_trace,
> +    "-trace\n"
> +    "                Specify a trace file to log traces to\n",
> +    QEMU_ARCH_ALL)
> +STEXI
> +@item -trace
> +@findex -trace
> +Specify a trace file to log output traces to.
> +ETEXI
> +#endif
>
>   HXCOMM This is the last statement. Insert new options before this line!
>   STEXI
> diff --git a/vl.c b/vl.c
> index b3e3676..99664e9 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -47,6 +47,10 @@
>   #include<dirent.h>
>   #include<netdb.h>
>   #include<sys/select.h>
> +#ifdef CONFIG_SIMPLE_TRACE
> +#include "trace.h"
> +#endif
> +
>   #ifdef CONFIG_BSD
>   #include<sys/stat.h>
>   #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__DragonFly__)
> @@ -1821,6 +1825,9 @@ int main(int argc, char **argv, char **envp)
>       int show_vnc_port = 0;
>       int defconfig = 1;
>
> +#ifdef CONFIG_SIMPLE_TRACE
> +    const char *trace_file = NULL;
> +#endif
>       atexit(qemu_run_exit_notifiers);
>       error_set_progname(argv[0]);
>
> @@ -2590,6 +2597,11 @@ int main(int argc, char **argv, char **envp)
>                   }
>                   xen_mode = XEN_ATTACH;
>                   break;
> +#ifdef CONFIG_SIMPLE_TRACE
> +            case QEMU_OPTION_trace:
> +                trace_file = optarg;
> +                break;
> +#endif
>    

You'll save yourself some future headaches by using a QemuOptsList to 
start out with.

Regards,

Anthony Liguori

>               case QEMU_OPTION_readconfig:
>                   {
>                       int ret = qemu_read_config_file(optarg);
> @@ -2633,6 +2645,12 @@ int main(int argc, char **argv, char **envp)
>           data_dir = CONFIG_QEMU_DATADIR;
>       }
>
> +#ifdef CONFIG_SIMPLE_TRACE
> +    /*
> +     * Set the trace file name, if specified.
> +     */
> +    st_set_trace_file(trace_file);
> +#endif
>       /*
>        * Default to max_cpus = smp_cpus, in case the user doesn't
>        * specify a max_cpus value.
>    

Re: [Qemu-devel] [PATCH 14/14] trace: Add user documentation

[Anthony Liguori]

On 08/12/2010 05:36 AM, Stefan Hajnoczi wrote:
> Signed-off-by: Stefan Hajnoczi<stefanha@linux.vnet.ibm.com>
>    

I very much like that you use wiki formatting.

> ---
>   docs/tracing.txt |  149 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 files changed, 149 insertions(+), 0 deletions(-)
>   create mode 100644 docs/tracing.txt
>
> diff --git a/docs/tracing.txt b/docs/tracing.txt
> new file mode 100644
> index 0000000..fe3c6ac
> --- /dev/null
> +++ b/docs/tracing.txt
> @@ -0,0 +1,149 @@
> += Tracing =
> +
> +== Introduction ==
> +
> +This document describes the tracing infrastructure in QEMU and how to use it
> +for debugging, profiling, and observing execution.
> +
> +== Quickstart ==
> +
> +1. Build with the 'simple' trace backend:
> +
> +    ./configure --trace-backend=simple
> +    make
> +
> +2. Run the virtual machine to produce a trace file:
> +
> +    qemu ... # your normal QEMU invocation
> +
> +3. Pretty-print the binary trace file:
> +
> +    ./simpletrace.py trace-events /tmp/trace-*
> +
> +== Trace events ==
> +
> +There is a set of static trace events declared in the trace-events source
> +file.  Each trace event declaration names the event, its arguments, and the
> +format string which can be used for pretty-printing:
> +
> +    qemu_malloc(size_t size, void *ptr) "size %zu ptr %p"
> +    qemu_free(void *ptr) "ptr %p"
> +
> +The trace-events file is processed by the tracetool script during build to
> +generate code for the trace events.  Trace events are invoked directly from
> +source code like this:
> +
> +    #include "trace.h"  /* needed for trace event prototype */
> +
> +    void *qemu_malloc(size_t size)
> +    {
> +        void *ptr;
> +        if (!size&&  !allow_zero_malloc()) {
> +            abort();
> +        }
> +        ptr = oom_check(malloc(size ? size : 1));
> +        trace_qemu_malloc(size, ptr);  /*<-- trace event */
> +        return ptr;
> +    }
> +
> +This is all that needs to be known from a user perspective for adding new
> +trace events.
>    

Please add some information about the restrictions on types.  Clearly, 
you can't specify just any type in the trace file so what set of types 
are allowed?

Regards,

Anthony Liguori

> +=== Hints for adding new trace events ===
> +
> +1. Trace state changes in the code.  Interesting points in the code usually
> +   involve a state change like starting, stopping, allocating, freeing.  State
> +   changes are good trace events because they can be used to understand the
> +   execution of the system.
> +
> +2. Trace guest operations.  Guest I/O accesses like reading device registers
> +   are good trace events because they can be used to understand guest
> +   interactions.
> +
> +3. Use correlator fields so the context of an individual line of trace output
> +   can be understood.  For example, trace the pointer returned by malloc and
> +   used as an argument to free.  This way mallocs and frees can be matched up.
> +   Trace events with no context are not very useful.
> +
> +4. Name trace events after their function.  If there are multiple trace events
> +   in one function, append a unique distinguisher at the end of the name.
> +
> +== Trace backends ==
> +
> +The tracetool script automates tedious trace event code generation and also
> +keeps the trace event declarations independent of the trace backend.  The trace
> +events are not tightly coupled to a specific trace backend, such as LTTng or
> +SystemTap.  Support for trace backends can be added by extending the tracetool
> +script.
> +
> +The trace backend is chosen at configure time and only one trace backend can
> +be built into the binary:
> +
> +    ./configure --trace-backend=simple
> +
> +For a list of supported trace backends, try ./configure --help or see below.
> +
> +The following subsections describe the supported trace backends.
> +
> +=== Nop ===
> +
> +The "nop" backend generates empty trace event functions so that the compiler
> +can optimize out trace events completely.  This is the default and imposes no
> +performance penalty.
> +
> +=== Simpletrace ===
> +
> +The "simple" backend supports common use cases and comes as part of the QEMU
> +source tree.  It may not be as powerful as platform-specific or third-party
> +trace backends but it is portable.  This is the recommended trace backend
> +unless you have specific needs for more advanced backends.
> +
> +==== Monitor commands ====
> +
> +* info trace
> +  Display the contents of trace buffer.  This command dumps the trace buffer
> +  with simple formatting.  For full pretty-printing, use the simpletrace.py
> +  script on a binary trace file.
> +
> +  The trace buffer is written into until full.  The full trace buffer is
> +  flushed and emptied.  This means the 'info trace' will display few or no
> +  entries if the buffer has just been flushed.
> +
> +* 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.
> +
> +* trace-file on|off|flush|set<path>
> +  Enable/disable/flush the trace file or set the trace file name.
> +
> +==== Enabling/disabling trace events programmatically ====
> +
> +The st_change_trace_event_state() function can be used to enable or disable trace
> +events at runtime inside QEMU:
> +
> +    #include "trace.h"
> +
> +    st_change_trace_event_state("virtio_irq", true); /* enable */
> +    [...]
> +    st_change_trace_event_state("virtio_irq", false); /* disable */
> +
> +==== Analyzing trace files ====
> +
> +The "simple" backend produces binary trace files that can be formatted with the
> +simpletrace.py script.  The script takes the trace-events file and the binary
> +trace:
> +
> +    ./simpletrace.py trace-events /tmp/trace.log
> +
> +You must ensure that the same trace-events file was used to build QEMU,
> +otherwise trace event declarations may have changed and output will not be
> +consistent.
> +
> +=== LTTng Userspace Tracer ===
> +
> +The "ust" backend uses the LTTng Userspace Tracer library.  There are no
> +monitor commands built into QEMU, instead UST utilities should be used to list,
> +enable/disable, and dump traces.
>    

Re: [Qemu-devel] [PATCH 00/14] trace: Add static tracing to QEMU

[Anthony Liguori]

On 08/12/2010 05:36 AM, Stefan Hajnoczi wrote:
> This patch series adds static tracing to QEMU.  It can be used to instrument
> QEMU code by means of lightweight logging called trace events.
>
> Prerna and I are now posting the entire patch series with a serious eye towards
> checking we meet users' and developers' tracing needs and with the goal of
> getting this functionality merged into qemu.git.
>
> Tracing infrastructure allows debugging, performance analysis, and observation
> to be performed.  Right now there are ad-hoc logging calls in some source files
> which require rebuilding QEMU with certain #defines and perform poorly.  This
> patch series introduces a single tracing infrastructure which is easy to use
> and can replace ad-hoc techniques.
>
> Two key points:
>
> 1. The trace-events file contains the set of defined trace events which can be
>     called from a source file.  For example, trace-events has:
>
>      qemu_malloc(size_t size, void *ptr) "size %zu ptr %p"
>
>     and qemu-malloc.c uses this trace event like this:
>
>      #include "trace.h"  /* needed for trace event prototype */
>
>      void *qemu_malloc(size_t size)
>      {
>          void *ptr;
>          if (!size&&  !allow_zero_malloc()) {
>              abort();
>          }
>          ptr = oom_check(malloc(size ? size : 1));
>          trace_qemu_malloc(size, ptr);  /*<-- trace event */
>          return ptr;
>      }
>
> 2. The built-in 'simple' trace backend writes binary traces to
>     /tmp/trace-<pid>.  They can be pretty-printed like this:
>
>      ./simpletrace.py trace-events /tmp/trace-*
>
>     Although you can also try LTTng Userspace Tracer ('ust' trace backend), the
>     'simple' trace backend provides commands within the QEMU monitor to
>     enable/disable trace events at runtime.  It is easy to use and should serve
>     as a good default trace backend.
>
>     The 'simple' trace backend's limitation is that it isn't thread-safe and can
>     therefore only trace correctly when the QEMU global mutex is held.
>
> For full documentation, see:
> http://repo.or.cz/w/qemu/stefanha.git/blob_plain/8d9f8f62d5af986fab6636770450f57fbd0d8b66:/docs/tracing.txt
>
> The git branch is here:
> http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/tracing
>    

This turned out very nice.  Good work!

Regards,

Anthony Liguori

> Stefan
>
>
>
>    

Re: [Qemu-devel] [PATCH 01/14] trace: Add trace-events file for declaring trace events

[Stefan Hajnoczi]

On Sun, Aug 22, 2010 at 04:38:36PM -0500, Anthony Liguori wrote:
> On 08/12/2010 05:36 AM, Stefan Hajnoczi wrote:
> >+linetoh_nop()
> >+{
> >+    local name args
> >+    name=$(get_name "$1")
> >+    args=$(get_args "$1")
> >+
> >+    # Define an empty function for the trace event
> >+    cat<<EOF
> >+static inline void trace_$name($args)
> >+{
> >+}
> 
> This seems like it can potentially create a single header that ends
> up needed to know about every type of structure QEMU implements, no?
> If so, this would be challenging for improving modularity.
> 
> Or would we restrict trace points to not allow passing of anything
> but void *s?

Trace events use void *.  It would be nicer to use the specific struct
pointer type but that is not possible for the reason you mentioned.

Stefan

Re: [Qemu-devel] [PATCH 06/14] trace: Trace entry point of balloon request handler

[Stefan Hajnoczi]

On Sun, Aug 22, 2010 at 04:41:44PM -0500, Anthony Liguori wrote:
> ram_addr_t is not necessarily an unsigned long.  It's actually a
> uint64_t.   Integer promotion should handle this in most cases but
> having the definition in two different places worries me a bit.

I thought it is unsigned long:

cpu-common.h:typedef unsigned long ram_addr_t;

ram_addr_t cannot be used explicitly because it is only defined for
softmmu targets.  User-only builds do not define ram_addr_t and the
generated trace.h header would break the compile.

This is a more general weakness of the tracing system: it is fragile
under #ifdefed builds because it tries to work for softmmu, user-only,
and qemu-tools!  Perhaps we should split the trace-events file.

Stefan

Re: [Qemu-devel] [PATCH 14/14] trace: Add user documentation

[Stefan Hajnoczi]

On Sun, Aug 22, 2010 at 04:47:17PM -0500, Anthony Liguori wrote:
> Please add some information about the restrictions on types.
> Clearly, you can't specify just any type in the trace file so what
> set of types are allowed?

Sure, will fix in v2.

Stefan

Re: [Qemu-devel] [PATCH 06/14] trace: Trace entry point of balloon request handler

[Anthony Liguori]

On 08/23/2010 05:10 AM, Stefan Hajnoczi wrote:
> On Sun, Aug 22, 2010 at 04:41:44PM -0500, Anthony Liguori wrote:
>    
>> ram_addr_t is not necessarily an unsigned long.  It's actually a
>> uint64_t.   Integer promotion should handle this in most cases but
>> having the definition in two different places worries me a bit.
>>      
> I thought it is unsigned long:
>
> cpu-common.h:typedef unsigned long ram_addr_t;
>    

Yeah, the definition has changed I think so you're right.

> ram_addr_t cannot be used explicitly because it is only defined for
> softmmu targets.  User-only builds do not define ram_addr_t and the
> generated trace.h header would break the compile.
>
> This is a more general weakness of the tracing system: it is fragile
> under #ifdefed builds because it tries to work for softmmu, user-only,
> and qemu-tools!  Perhaps we should split the trace-events file.
>    

I think documentation could go a long way to addressing this.

Regards,

Anthony Liguori

> Stefan
>    

[Qemu-devel] Re: [PATCH 00/14] trace: Add static tracing to QEMU

[Michael S. Tsirkin]

On Thu, Aug 12, 2010 at 11:36:21AM +0100, Stefan Hajnoczi wrote:
> 2. The built-in 'simple' trace backend writes binary traces to
>    /tmp/trace-<pid>.

Saving files with predictable names in /tmp is usually not a good idea,
see e.g. http://en.wikipedia.org/wiki/Symlink_race.
Put them in $HOME or something like that.

-- 
MST

Re: [Qemu-devel] Re: [PATCH 00/14] trace: Add static tracing to QEMU

[Stefan Hajnoczi]

On Sun, Sep 5, 2010 at 8:30 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Thu, Aug 12, 2010 at 11:36:21AM +0100, Stefan Hajnoczi wrote:
>> 2. The built-in 'simple' trace backend writes binary traces to
>>    /tmp/trace-<pid>.
>
> Saving files with predictable names in /tmp is usually not a good idea,
> see e.g. http://en.wikipedia.org/wiki/Symlink_race.
> Put them in $HOME or something like that.

Let's default to the current working directory (which it already does
on Windows).  If QEMU is run as a daemon or in another environment
where this doesn't make sense, then it needs to be configured
appropriately:
$ ./configure --trace-file=/var/qemu/trace

Stefan