* [Qemu-devel] [PATCH 01/12] Converting tracetool.sh to tracetool.py
2012-03-13 20:02 [Qemu-devel] [PATCH 00/12] Rewrite tracetool using python Lluís Vilanova
@ 2012-03-13 20:02 ` Lluís Vilanova
2012-03-19 16:51 ` Stefan Hajnoczi
2012-03-13 20:02 ` [Qemu-devel] [PATCH 02/12] trace: [tracetool] Remove unused 'sizestr' attribute in 'Event' Lluís Vilanova
` (11 subsequent siblings)
12 siblings, 1 reply; 25+ messages in thread
From: Lluís Vilanova @ 2012-03-13 20:02 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, harsh, aneesh.kumar
Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
Makefile.objs | 6
Makefile.target | 10 -
configure | 7 -
scripts/tracetool | 648 --------------------------------------------------
scripts/tracetool.py | 534 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 546 insertions(+), 659 deletions(-)
delete mode 100755 scripts/tracetool
create mode 100755 scripts/tracetool.py
diff --git a/Makefile.objs b/Makefile.objs
index 5f0b3f7..a718963 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -357,12 +357,12 @@ else
trace.h: trace.h-timestamp
endif
trace.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
- $(call quiet-command,sh $(SRC_PATH)/scripts/tracetool --$(TRACE_BACKEND) -h < $< > $@," GEN trace.h")
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/tracetool.py --backend=$(TRACE_BACKEND) -h < $< > $@," GEN trace.h")
@cmp -s $@ trace.h || cp $@ trace.h
trace.c: trace.c-timestamp
trace.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
- $(call quiet-command,sh $(SRC_PATH)/scripts/tracetool --$(TRACE_BACKEND) -c < $< > $@," GEN trace.c")
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/tracetool.py --backend=$(TRACE_BACKEND) -c < $< > $@," GEN trace.c")
@cmp -s $@ trace.c || cp $@ trace.c
trace.o: trace.c $(GENERATED_HEADERS)
@@ -375,7 +375,7 @@ trace-dtrace.h: trace-dtrace.dtrace
# rule file. So we use '.dtrace' instead
trace-dtrace.dtrace: trace-dtrace.dtrace-timestamp
trace-dtrace.dtrace-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
- $(call quiet-command,sh $(SRC_PATH)/scripts/tracetool --$(TRACE_BACKEND) -d < $< > $@," GEN trace-dtrace.dtrace")
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/tracetool.py --backend=$(TRACE_BACKEND) -d < $< > $@," GEN trace-dtrace.dtrace")
@cmp -s $@ trace-dtrace.dtrace || cp $@ trace-dtrace.dtrace
trace-dtrace.o: trace-dtrace.dtrace $(GENERATED_HEADERS)
diff --git a/Makefile.target b/Makefile.target
index 1bd25a8..3e42e5a 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -59,11 +59,11 @@ TARGET_TYPE=system
endif
$(QEMU_PROG).stp:
- $(call quiet-command,sh $(SRC_PATH)/scripts/tracetool \
- --$(TRACE_BACKEND) \
- --binary $(bindir)/$(QEMU_PROG) \
- --target-arch $(TARGET_ARCH) \
- --target-type $(TARGET_TYPE) \
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/tracetool.py \
+ --backend=$(TRACE_BACKEND) \
+ --binary=$(bindir)/$(QEMU_PROG) \
+ --target-arch=$(TARGET_ARCH) \
+ --target-type=$(TARGET_TYPE) \
--stap < $(SRC_PATH)/trace-events > $(QEMU_PROG).stp," GEN $(QEMU_PROG).stp")
else
stap:
diff --git a/configure b/configure
index d7631ed..629e740 100755
--- a/configure
+++ b/configure
@@ -1097,7 +1097,7 @@ 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 " --enable-trace-backend=B Set trace backend"
-echo " Available backends:" $("$source_path"/scripts/tracetool --list-backends)
+echo " Available backends:" $("$source_path"/scripts/tracetool.py --list-backends)
echo " --with-trace-file=NAME Full PATH,NAME of file to store traces"
echo " Default:trace-<pid>"
echo " --disable-spice disable spice"
@@ -2654,7 +2654,7 @@ fi
##########################################
# check if trace backend exists
-sh "$source_path/scripts/tracetool" "--$trace_backend" --check-backend > /dev/null 2> /dev/null
+$python "$source_path/scripts/tracetool.py" "--backend=$trace_backend" --check-backend > /dev/null 2> /dev/null
if test "$?" -ne 0 ; then
echo
echo "Error: invalid trace backend"
@@ -2672,7 +2672,8 @@ if test "$trace_backend" = "ust"; then
int main(void) { return 0; }
EOF
if compile_prog "" "" ; then
- LIBS="-lust $LIBS"
+ LIBS="-lust -lurcu-bp $LIBS"
+ libs_qga+="-lust -lurcu-bp"
else
echo
echo "Error: Trace backend 'ust' missing libust header files"
diff --git a/scripts/tracetool b/scripts/tracetool
deleted file mode 100755
index 65bd0a1..0000000
--- a/scripts/tracetool
+++ /dev/null
@@ -1,648 +0,0 @@
-#!/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 | --simple | --stderr | --ust | --dtrace] [-h | -c]
-Generate tracing code for a file on stdin.
-
-Backends:
- --nop Tracing disabled
- --simple Simple built-in backend
- --stderr Stderr built-in backend
- --ust LTTng User Space Tracing backend
- --dtrace DTrace/SystemTAP backend
-
-Output formats:
- -h Generate .h file
- -c Generate .c file
- -d Generate .d file (DTrace only)
- --stap Generate .stp file (DTrace with SystemTAP only)
-
-Options:
- --binary [path] Full path to QEMU binary
- --target-arch [arch] QEMU emulator target arch
- --target-type [type] QEMU emulator target type ('system' or 'user')
- --probe-prefix [prefix] Prefix for dtrace probe names
- (default: qemu-\$targettype-\$targetarch)
-
-EOF
- exit 1
-}
-
-# Print a line without interpreting backslash escapes
-#
-# The built-in echo command may interpret backslash escapes without an option
-# to disable this behavior.
-puts()
-{
- printf "%s\n" "$1"
-}
-
-# Get the name of a trace event
-get_name()
-{
- local name
- name=${1%%\(*}
- echo "${name##* }"
-}
-
-# Get the given property of a trace event
-# 1: trace-events line
-# 2: property name
-# -> return 0 if property is present, or 1 otherwise
-has_property()
-{
- local props prop
- props=${1%%\(*}
- props=${props% *}
- for prop in $props; do
- if [ "$prop" = "$2" ]; then
- return 0
- fi
- done
- return 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 sep
- nfields=0
- sep="$2"
- 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
-
- printf "%s%s " $name $sep
- done
-
- # Last argument name
- if [ "$nfields" -gt 1 ]
- then
- 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 including double quotes for a trace event
-get_fmt()
-{
- puts "${1#*)}"
-}
-
-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
-}
-
-linetoh_begin_simple()
-{
- cat <<EOF
-#include "trace/simple.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()
-{
- cat <<EOF
-#define NR_TRACE_EVENTS $simple_event_num
-extern TraceEvent trace_list[NR_TRACE_EVENTS];
-EOF
-}
-
-linetoc_begin_simple()
-{
- cat <<EOF
-#include "trace.h"
-
-TraceEvent trace_list[] = {
-EOF
- simple_event_num=0
-
-}
-
-linetoc_simple()
-{
- local name
- name=$(get_name "$1")
- cat <<EOF
-{.tp_name = "$name", .state=0},
-EOF
- simple_event_num=$((simple_event_num + 1))
-}
-
-linetoc_end_simple()
-{
- cat <<EOF
-};
-EOF
-}
-
-#STDERR
-linetoh_begin_stderr()
-{
- cat <<EOF
-#include <stdio.h>
-#include "trace/stderr.h"
-
-extern TraceEvent trace_list[];
-EOF
-
- stderr_event_num=0
-}
-
-linetoh_stderr()
-{
- local name args argnames argc fmt
- name=$(get_name "$1")
- args=$(get_args "$1")
- argnames=$(get_argnames "$1" ",")
- argc=$(get_argc "$1")
- fmt=$(get_fmt "$1")
-
- if [ "$argc" -gt 0 ]; then
- argnames=", $argnames"
- fi
-
- cat <<EOF
-static inline void trace_$name($args)
-{
- if (trace_list[$stderr_event_num].state != 0) {
- fprintf(stderr, "$name " $fmt "\n" $argnames);
- }
-}
-EOF
- stderr_event_num=$((stderr_event_num + 1))
-
-}
-
-linetoh_end_stderr()
-{
- cat <<EOF
-#define NR_TRACE_EVENTS $stderr_event_num
-EOF
-}
-
-linetoc_begin_stderr()
-{
- cat <<EOF
-#include "trace.h"
-
-TraceEvent trace_list[] = {
-EOF
- stderr_event_num=0
-}
-
-linetoc_stderr()
-{
- local name
- name=$(get_name "$1")
- cat <<EOF
-{.tp_name = "$name", .state=0},
-EOF
- stderr_event_num=$(($stderr_event_num + 1))
-}
-
-linetoc_end_stderr()
-{
- cat <<EOF
-};
-EOF
-}
-#END OF STDERR
-
-# 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, TP_PROTO($args), TP_ARGS($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", ",")
- [ -z "$argnames" ] || argnames=", $argnames"
- 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 "}"
-}
-
-linetoh_begin_dtrace()
-{
- cat <<EOF
-#include "trace-dtrace.h"
-EOF
-}
-
-linetoh_dtrace()
-{
- local name args argnames nameupper
- name=$(get_name "$1")
- args=$(get_args "$1")
- argnames=$(get_argnames "$1", ",")
-
- nameupper=`echo $name | tr '[:lower:]' '[:upper:]'`
-
- # Define an empty function for the trace event
- cat <<EOF
-static inline void trace_$name($args) {
- QEMU_${nameupper}($argnames);
-}
-EOF
-}
-
-linetoh_end_dtrace()
-{
- return
-}
-
-linetoc_begin_dtrace()
-{
- return
-}
-
-linetoc_dtrace()
-{
- # No need for function definitions in dtrace backend
- return
-}
-
-linetoc_end_dtrace()
-{
- return
-}
-
-linetod_begin_dtrace()
-{
- cat <<EOF
-provider qemu {
-EOF
-}
-
-linetod_dtrace()
-{
- local name args
- name=$(get_name "$1")
- args=$(get_args "$1")
-
- # DTrace provider syntax expects foo() for empty
- # params, not foo(void)
- if [ "$args" = "void" ]; then
- args=""
- fi
-
- # Define prototype for probe arguments
- cat <<EOF
- probe $name($args);
-EOF
-}
-
-linetod_end_dtrace()
-{
- cat <<EOF
-};
-EOF
-}
-
-linetostap_begin_dtrace()
-{
- return
-}
-
-linetostap_dtrace()
-{
- local i arg name args arglist
- name=$(get_name "$1")
- args=$(get_args "$1")
- arglist=$(get_argnames "$1", "")
-
- # Define prototype for probe arguments
- cat <<EOF
-probe $probeprefix.$name = process("$binary").mark("$name")
-{
-EOF
-
- i=1
- for arg in $arglist
- do
- # 'limit' is a reserved keyword
- if [ "$arg" = "limit" ]; then
- arg="_limit"
- fi
- cat <<EOF
- $arg = \$arg$i;
-EOF
- i="$((i+1))"
- done
-
- cat <<EOF
-}
-EOF
-}
-
-linetostap_end_dtrace()
-{
- return
-}
-
-# Process stdin by calling begin, line, and end functions for the backend
-convert()
-{
- local begin process_line end str name NAME enabled
- 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
- test -z "${str%%#*}" && continue
-
- echo
- # Process the line. The nop backend handles disabled lines.
- if has_property "$str" "disable"; then
- "lineto$1_nop" "$str"
- enabled=0
- else
- "$process_line" "$str"
- enabled=1
- fi
- if [ "$1" = "h" ]; then
- name=$(get_name "$str")
- NAME=$(echo $name | tr '[:lower:]' '[:upper:]')
- echo "#define TRACE_${NAME}_ENABLED ${enabled}"
- fi
- 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
-}
-
-tracetod()
-{
- if [ $backend != "dtrace" ]; then
- echo "DTrace probe generator not applicable to $backend backend"
- exit 1
- fi
- echo "/* This file is autogenerated by tracetool, do not edit. */"
- convert d
-}
-
-tracetostap()
-{
- if [ $backend != "dtrace" ]; then
- echo "SystemTAP tapset generator not applicable to $backend backend"
- exit 1
- fi
- if [ -z "$binary" ]; then
- echo "--binary is required for SystemTAP tapset generator"
- exit 1
- fi
- if [ -z "$probeprefix" -a -z "$targettype" ]; then
- echo "--target-type is required for SystemTAP tapset generator"
- exit 1
- fi
- if [ -z "$probeprefix" -a -z "$targetarch" ]; then
- echo "--target-arch is required for SystemTAP tapset generator"
- exit 1
- fi
- if [ -z "$probeprefix" ]; then
- probeprefix="qemu.$targettype.$targetarch";
- fi
- echo "/* This file is autogenerated by tracetool, do not edit. */"
- convert stap
-}
-
-
-backend=
-output=
-binary=
-targettype=
-targetarch=
-probeprefix=
-
-
-until [ -z "$1" ]
-do
- case "$1" in
- "--nop" | "--simple" | "--stderr" | "--ust" | "--dtrace") backend="${1#--}" ;;
-
- "--binary") shift ; binary="$1" ;;
- "--target-arch") shift ; targetarch="$1" ;;
- "--target-type") shift ; targettype="$1" ;;
- "--probe-prefix") shift ; probeprefix="$1" ;;
-
- "-h" | "-c" | "-d") output="${1#-}" ;;
- "--stap") output="${1#--}" ;;
-
- "--check-backend") exit 0 ;; # used by ./configure to test for backend
-
- "--list-backends") # used by ./configure to list available backends
- echo "nop simple stderr ust dtrace"
- exit 0
- ;;
-
- *)
- usage;;
- esac
- shift
-done
-
-if [ "$backend" = "" -o "$output" = "" ]; then
- usage
-fi
-
-gen="traceto$output"
-"$gen"
-
-exit 0
diff --git a/scripts/tracetool.py b/scripts/tracetool.py
new file mode 100755
index 0000000..079ec7a
--- /dev/null
+++ b/scripts/tracetool.py
@@ -0,0 +1,534 @@
+#!/usr/bin/env python
+# Code generator for trace events
+#
+# Copyright IBM, Corp. 2011
+#
+# This work is licensed under the terms of the GNU GPL, version 2. See
+# the COPYING file in the top-level directory.
+#
+#
+
+import sys
+import getopt
+
+def usage():
+ print "Tracetool: Generate tracing code for trace events file on stdin"
+ print "Usage:"
+ print sys.argv[0], "--backend=[nop|simple|stderr|dtrace|ust] [-h|-c|-d|--stap]"
+ print '''
+Backends:
+ --nop Tracing disabled
+ --simple Simple built-in backend
+ --stderr Stderr built-in backend
+ --dtrace DTrace/SystemTAP backend
+ --ust LTTng User Space Tracing backend
+
+Output formats:
+ -h Generate .h file
+ -c Generate .c file
+ -d Generate .d file (DTrace only)
+ --stap Generate .stp file (DTrace with SystemTAP only)
+
+Options:
+ --binary [path] Full path to QEMU binary
+ --target-arch [arch] QEMU emulator target arch
+ --target-type [type] QEMU emulator target type ('system' or 'user')
+ --probe-prefix [prefix] Prefix for dtrace probe names
+ (default: qemu-targettype-targetarch)
+'''
+ sys.exit(1)
+
+def get_name(line, sep='('):
+ head, sep, tail = line.partition(sep)
+ property, sep, name = head.rpartition(' ')
+ return name
+
+def get_properties(line, sep='('):
+ head, sep, tail = line.partition(sep)
+ property, sep, name = head.rpartition(' ')
+ return property.split()
+
+def get_args(line, sep1='(', sep2=')'):
+ head, sep1, tail = line.partition(sep1)
+ args, sep2, fmt_str = tail.partition(sep2)
+ return args
+
+def get_argnames(line, sep=','):
+ nfields = 0
+ str = []
+ args = get_args(line)
+ for field in args.split():
+ nfields = nfields + 1
+ # Drop pointer star
+ type, ptr, tail = field.partition('*')
+ if type != field:
+ field = tail
+
+ name, sep, tail = field.partition(',')
+
+ if name == field:
+ continue
+ str.append(name)
+ str.append(", ")
+
+ if nfields > 1:
+ str.append(name)
+ return ''.join(str)
+ else:
+ return ''
+
+def get_argc(line):
+ argc = 0
+ argnames = get_argnames(line)
+ if argnames:
+ for name in argnames.split(','):
+ argc = argc + 1
+ return argc
+
+def get_fmt(line, sep=')'):
+ event, sep, fmt = line.partition(sep)
+ return fmt.rstrip('\n')
+
+def calc_sizeofargs(line):
+ args = get_args(line)
+ argc = get_argc(line)
+ str = []
+ newstr = ""
+ if argc > 0:
+ str = args.split(',')
+ for elem in str:
+ if is_string(elem): #string
+ type, sep, var = elem.rpartition('*')
+ newstr = newstr+"4 + strlen("+var.lstrip()+") + "
+ else:
+ newstr = newstr + '8 + '
+ newstr = newstr + '0' # takes care of last +
+ return newstr
+
+
+def trace_h_begin():
+ print '''#ifndef TRACE_H
+#define TRACE_H
+
+/* This file is autogenerated by tracetool, do not edit. */
+
+#include "qemu-common.h"'''
+
+
+def trace_h_end():
+ print '#endif /* TRACE_H */'
+
+
+def trace_c_begin():
+ print '/* This file is autogenerated by tracetool, do not edit. */'
+
+def trace_c_end():
+ pass # nop, required for trace_gen
+
+def nop_h(events):
+ print
+ for event in events:
+ print '''static inline void trace_%(name)s(%(args)s)
+{
+}
+''' % {
+ 'name': event.name,
+ 'args': event.args
+}
+ return
+
+def nop_c(events):
+ pass # nop, reqd for converters
+
+def simple_h(events):
+ print '#include "trace/simple.h"'
+ print
+
+ for event in events:
+ if event.argc:
+ argstr = event.argnames.split()
+ arg_prefix = '(uint64_t)(uintptr_t)'
+ cast_args = arg_prefix + arg_prefix.join(argstr)
+ simple_args = (str(event.num) + ', ' + cast_args)
+ else:
+ simple_args = str(event.num)
+
+ print '''static inline void trace_%(name)s(%(args)s)
+{
+ trace%(argc)d(%(trace_args)s);
+}''' % {
+ 'name': event.name,
+ 'args': event.args,
+ 'argc': event.argc,
+ 'trace_args': simple_args
+}
+ print
+ print '#define NR_TRACE_EVENTS %d' % (event.num + 1)
+ print 'extern TraceEvent trace_list[NR_TRACE_EVENTS];'
+
+
+def is_string(arg):
+ strtype = ('const char*', 'char*', 'const char *', 'char *')
+ if arg.lstrip().startswith(strtype):
+ return True
+ else:
+ return False
+
+def simple_c(events):
+ print '#include "trace.h"'
+ print
+ print 'TraceEvent trace_list[] = {'
+ print
+
+ for event in events:
+ print '{.tp_name = "%(name)s", .state=0},' % {
+ 'name': event.name,
+}
+ print
+ print '};'
+
+def stderr_h(events):
+ print '''#include <stdio.h>
+#include "trace/stderr.h"
+
+extern TraceEvent trace_list[];'''
+ for event in events:
+ argnames = event.argnames
+ if event.argc > 0:
+ argnames = ', ' + event.argnames
+ else:
+ argnames = ''
+ print '''
+static inline void trace_%(name)s(%(args)s)
+{
+ if (trace_list[%(event_num)s].state != 0) {
+ fprintf(stderr, "%(name)s " %(fmt)s "\\n" %(argnames)s);
+ }
+}''' % {
+ 'name': event.name,
+ 'args': event.args,
+ 'event_num': event.num,
+ 'fmt': event.fmt,
+ 'argnames': argnames
+}
+ print
+ print '#define NR_TRACE_EVENTS %d' % (event.num + 1)
+
+def stderr_c(events):
+ print '''#include "trace.h"
+
+TraceEvent trace_list[] = {
+'''
+ for event in events:
+ print '{.tp_name = "%(name)s", .state=0},' % {
+ 'name': event.name
+}
+ print
+ print '};'
+
+def ust_h(events):
+ print '''#include <ust/tracepoint.h>
+#undef mutex_lock
+#undef mutex_unlock
+#undef inline
+#undef wmb'''
+
+ for event in events:
+ if event.argc > 0:
+ print '''
+DECLARE_TRACE(ust_%(name)s, TP_PROTO(%(args)s), TP_ARGS(%(argnames)s));
+#define trace_%(name)s trace_ust_%(name)s''' % {
+ 'name': event.name,
+ 'args': event.args,
+ 'argnames': event.argnames
+}
+ else:
+ print '''
+_DECLARE_TRACEPOINT_NOARGS(ust_%(name)s);
+#define trace_%(name)s trace_ust_%(name)s''' % {
+ 'name': event.name,
+}
+ print
+
+def ust_c(events):
+ print '''#include <ust/marker.h>
+#undef mutex_lock
+#undef mutex_unlock
+#undef inline
+#undef wmb
+#include "trace.h"'''
+ eventlist = list(events)
+ for event in eventlist:
+ argnames = event.argnames
+ if event.argc > 0:
+ argnames = ', ' + event.argnames
+ print '''
+DEFINE_TRACE(ust_%(name)s);
+
+static void ust_%(name)s_probe(%(args)s)
+{
+ trace_mark(ust, %(name)s, %(fmt)s%(argnames)s);
+}''' % {
+ 'name': event.name,
+ 'args': event.args,
+ 'fmt': event.fmt,
+ 'argnames': argnames
+}
+ else:
+ print '''
+DEFINE_TRACE(ust_%(name)s);
+
+static void ust_%(name)s_probe(%(args)s)
+{
+ trace_mark(ust, %(name)s, UST_MARKER_NOARGS);
+}''' % {
+ 'name': event.name,
+ 'args': event.args,
+}
+
+ # register probes
+ print '''
+static void __attribute__((constructor)) trace_init(void)
+{'''
+ for event in eventlist:
+ print ' register_trace_ust_%(name)s(ust_%(name)s_probe);' % {
+ 'name': event.name
+}
+ print '}'
+
+def dtrace_h(events):
+ print '#include "trace-dtrace.h"'
+ print
+ for event in events:
+ print '''static inline void trace_%(name)s(%(args)s) {
+ if (QEMU_%(uppername)s_ENABLED()) {
+ QEMU_%(uppername)s(%(argnames)s);
+ }
+}
+''' % {
+ 'name': event.name,
+ 'args': event.args,
+ 'uppername': event.name.upper(),
+ 'argnames': event.argnames,
+}
+
+def dtrace_c(events):
+ pass # No need for function definitions in dtrace backend
+
+def dtrace_d(events):
+ print 'provider qemu {'
+ for event in events:
+ args = event.args
+
+ # DTrace provider syntax expects foo() for empty
+ # params, not foo(void)
+ if args == 'void':
+ args = ''
+
+ # Define prototype for probe arguments
+ print '''
+ probe %(name)s(%(args)s);''' % {
+ 'name': event.name,
+ 'args': args
+}
+ print
+ print '};'
+
+def dtrace_stp(events):
+ for event in events:
+ # Define prototype for probe arguments
+ print '''
+probe %(probeprefix)s.%(name)s = process("%(binary)s").mark("%(name)s")
+{''' % {
+ 'probeprefix': probeprefix,
+ 'name': event.name,
+ 'binary': binary
+}
+ i = 1
+ if event.argc > 0:
+ for arg in event.argnames.split(','):
+ # 'limit' is a reserved keyword
+ if arg == 'limit':
+ arg = '_limit'
+ print ' %s = $arg%d;' % (arg.lstrip(), i)
+ i += 1
+ print '}'
+ print
+
+def trace_stap_begin():
+ print '/* This file is autogenerated by tracetool, do not edit. */'
+
+def trace_stap_end():
+ pass #nop, reqd for trace_gen
+
+def trace_d_begin():
+ print '/* This file is autogenerated by tracetool, do not edit. */'
+
+def trace_d_end():
+ pass #nop, reqd for trace_gen
+
+
+# Registry of backends and their converter functions
+converters = {
+ 'simple': {
+ 'h': simple_h,
+ 'c': simple_c,
+ },
+
+ 'nop': {
+ 'h': nop_h,
+ 'c': nop_c,
+ },
+
+ 'stderr': {
+ 'h': stderr_h,
+ 'c': stderr_c,
+ },
+
+ 'dtrace': {
+ 'h': dtrace_h,
+ 'c': dtrace_c,
+ 'd': dtrace_d,
+ 'stap': dtrace_stp
+ },
+
+ 'ust': {
+ 'h': ust_h,
+ 'c': ust_c,
+ },
+
+}
+
+# Trace file header and footer code generators
+trace_gen = {
+ 'h': {
+ 'begin': trace_h_begin,
+ 'end': trace_h_end,
+ },
+ 'c': {
+ 'begin': trace_c_begin,
+ 'end': trace_c_end,
+ },
+ 'd': {
+ 'begin': trace_d_begin,
+ 'end': trace_d_end,
+ },
+ 'stap': {
+ 'begin': trace_stap_begin,
+ 'end': trace_stap_end,
+ },
+}
+
+# A trace event
+class Event(object):
+ def __init__(self, num, line):
+ self.num = num
+ self.args = get_args(line)
+ self.arglist = self.args.split(',')
+ self.name = get_name(line)
+ self.argc = get_argc(line)
+ self.argnames = get_argnames(line)
+ self.sizestr = calc_sizeofargs(line)
+ self.fmt = get_fmt(line)
+ self.properties = get_properties(line)
+
+# Generator that yields Event objects given a trace-events file object
+def read_events(fobj):
+ event_num = 0
+ for line in fobj:
+ if not line.strip():
+ continue
+ if line.lstrip().startswith('#'):
+ continue
+ yield Event(event_num, line)
+ event_num += 1
+
+binary = ""
+probeprefix = ""
+
+def main():
+ global binary, probeprefix
+ targettype = ""
+ targetarch = ""
+ supported_backends = ["simple", "nop", "stderr", "dtrace", "ust"]
+ short_options = "hcd"
+ long_options = ["stap", "backend=", "binary=", "target-arch=", "target-type=", "probe-prefix=", "list-backends", "check-backend"]
+ try:
+ opts, args = getopt.getopt(sys.argv[1:], short_options, long_options)
+ except getopt.GetoptError, err:
+ # print help information and exit:
+ print str(err) # will print something like "option -a not recognized"
+ usage()
+ sys.exit(2)
+ for opt, arg in opts:
+ if opt == '-h':
+ output = 'h'
+ elif opt == '-c':
+ output = 'c'
+ elif opt == '-d':
+ output = 'd'
+ elif opt == '--stap':
+ output = 'stap'
+ elif opt == '--backend':
+ backend = arg
+ elif opt == '--binary':
+ binary = arg
+ elif opt == '--target-arch':
+ targetarch = arg
+ elif opt == '--target-type':
+ targettype = arg
+ elif opt == '--probe-prefix':
+ probeprefix = arg
+ elif opt == '--list-backends':
+ print 'simple, nop, stderr, dtrace, ust'
+ sys.exit(0)
+ elif opt == "--check-backend":
+ if any(backend in s for s in supported_backends):
+ sys.exit(0)
+ else:
+ sys.exit(1)
+ else:
+ #assert False, "unhandled option"
+ print "unhandled option: ", opt
+ usage()
+
+ if backend == "" or output == "":
+ usage()
+ sys.exit(0)
+
+ if backend != 'dtrace' and output == 'd':
+ print 'DTrace probe generator not applicable to %s backend' % backend
+ sys.exit(1)
+
+ if output == 'stap':
+ if backend != "dtrace":
+ print 'SystemTAP tapset generator not applicable to %s backend' % backend
+ sys.exit(1)
+ if binary == "":
+ print '--binary is required for SystemTAP tapset generator'
+ sys.exit(1)
+ if not probeprefix and not targettype:
+ print '--target-type is required for SystemTAP tapset generator'
+ sys.exit(1)
+ if not probeprefix and not targetarch:
+ print '--target-arch is required for SystemTAP tapset generator'
+ sys.exit(1)
+ if probeprefix == "":
+ probeprefix = 'qemu.' + targettype + '.' + targetarch
+
+ disabled_events, enabled_events = [], []
+ for e in read_events(sys.stdin):
+ if 'disable' in e.properties:
+ disabled_events.append(e)
+ else:
+ enabled_events.append(e)
+
+ trace_gen[output]['begin']()
+ if output == 'h': # disabled events
+ converters['nop'][output](disabled_events)
+ converters[backend][output](enabled_events)
+ trace_gen[output]['end']()
+
+if __name__ == "__main__":
+ main()
+
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 01/12] Converting tracetool.sh to tracetool.py
2012-03-13 20:02 ` [Qemu-devel] [PATCH 01/12] Converting tracetool.sh to tracetool.py Lluís Vilanova
@ 2012-03-19 16:51 ` Stefan Hajnoczi
0 siblings, 0 replies; 25+ messages in thread
From: Stefan Hajnoczi @ 2012-03-19 16:51 UTC (permalink / raw)
To: Lluís Vilanova; +Cc: harsh, qemu-devel, aneesh.kumar
2012/3/13 Lluís Vilanova <vilanova@ac.upc.edu>:
> diff --git a/configure b/configure
> index d7631ed..629e740 100755
> --- a/configure
> +++ b/configure
> @@ -1097,7 +1097,7 @@ 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 " --enable-trace-backend=B Set trace backend"
> -echo " Available backends:" $("$source_path"/scripts/tracetool --list-backends)
> +echo " Available backends:" $("$source_path"/scripts/tracetool.py --list-backends)
We need to use $python.
> echo " --with-trace-file=NAME Full PATH,NAME of file to store traces"
> echo " Default:trace-<pid>"
> echo " --disable-spice disable spice"
> @@ -2654,7 +2654,7 @@ fi
> ##########################################
> # check if trace backend exists
>
> -sh "$source_path/scripts/tracetool" "--$trace_backend" --check-backend > /dev/null 2> /dev/null
> +$python "$source_path/scripts/tracetool.py" "--backend=$trace_backend" --check-backend > /dev/null 2> /dev/null
> if test "$?" -ne 0 ; then
> echo
> echo "Error: invalid trace backend"
> @@ -2672,7 +2672,8 @@ if test "$trace_backend" = "ust"; then
> int main(void) { return 0; }
> EOF
> if compile_prog "" "" ; then
> - LIBS="-lust $LIBS"
> + LIBS="-lust -lurcu-bp $LIBS"
> + libs_qga+="-lust -lurcu-bp"
This should be part of a separate commit because it's unrelated to
converting tracetool to Python. Also, how about "$pkg_config --libs
ust" instead of hardcoding the libraries?
> +def get_argnames(line, sep=','):
> + nfields = 0
> + str = []
> + args = get_args(line)
> + for field in args.split():
> + nfields = nfields + 1
2-space indentation. Please use 4 spaces.
> +def dtrace_h(events):
> + print '#include "trace-dtrace.h"'
> + print
> + for event in events:
> + print '''static inline void trace_%(name)s(%(args)s) {
> + if (QEMU_%(uppername)s_ENABLED()) {
> + QEMU_%(uppername)s(%(argnames)s);
> + }
This has changed in qemu.git/master - we don't check
QEMU_$name_ENABLED() anymore. Just calling the trace event is fine.
> + elif opt == "--check-backend":
> + if any(backend in s for s in supported_backends):
> + sys.exit(0)
> + else:
> + sys.exit(1)
Why not just:
if backend in supported_backends:
sys.exit(0)
else:
sys.exit(1)
Also backend is only assigned from --backend. So it's possible to
trigger a Python NameError here if you didn't specify --backend first
- that's ugly and not user-friendly. It would be better to set
backend to None by default.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH 02/12] trace: [tracetool] Remove unused 'sizestr' attribute in 'Event'
2012-03-13 20:02 [Qemu-devel] [PATCH 00/12] Rewrite tracetool using python Lluís Vilanova
2012-03-13 20:02 ` [Qemu-devel] [PATCH 01/12] Converting tracetool.sh to tracetool.py Lluís Vilanova
@ 2012-03-13 20:02 ` Lluís Vilanova
2012-03-13 20:02 ` [Qemu-devel] [PATCH 03/12] trace: [tracetool] Do not rebuild event list in backend code Lluís Vilanova
` (10 subsequent siblings)
12 siblings, 0 replies; 25+ messages in thread
From: Lluís Vilanova @ 2012-03-13 20:02 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, harsh, aneesh.kumar
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
scripts/tracetool.py | 18 ------------------
1 files changed, 0 insertions(+), 18 deletions(-)
diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index 079ec7a..26a4c43 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -89,23 +89,6 @@ def get_fmt(line, sep=')'):
event, sep, fmt = line.partition(sep)
return fmt.rstrip('\n')
-def calc_sizeofargs(line):
- args = get_args(line)
- argc = get_argc(line)
- str = []
- newstr = ""
- if argc > 0:
- str = args.split(',')
- for elem in str:
- if is_string(elem): #string
- type, sep, var = elem.rpartition('*')
- newstr = newstr+"4 + strlen("+var.lstrip()+") + "
- else:
- newstr = newstr + '8 + '
- newstr = newstr + '0' # takes care of last +
- return newstr
-
-
def trace_h_begin():
print '''#ifndef TRACE_H
#define TRACE_H
@@ -428,7 +411,6 @@ class Event(object):
self.name = get_name(line)
self.argc = get_argc(line)
self.argnames = get_argnames(line)
- self.sizestr = calc_sizeofargs(line)
self.fmt = get_fmt(line)
self.properties = get_properties(line)
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PATCH 03/12] trace: [tracetool] Do not rebuild event list in backend code
2012-03-13 20:02 [Qemu-devel] [PATCH 00/12] Rewrite tracetool using python Lluís Vilanova
2012-03-13 20:02 ` [Qemu-devel] [PATCH 01/12] Converting tracetool.sh to tracetool.py Lluís Vilanova
2012-03-13 20:02 ` [Qemu-devel] [PATCH 02/12] trace: [tracetool] Remove unused 'sizestr' attribute in 'Event' Lluís Vilanova
@ 2012-03-13 20:02 ` Lluís Vilanova
2012-03-13 20:02 ` [Qemu-devel] [PATCH 04/12] trace: [tracetool] Simplify event line parsing Lluís Vilanova
` (9 subsequent siblings)
12 siblings, 0 replies; 25+ messages in thread
From: Lluís Vilanova @ 2012-03-13 20:02 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, harsh, aneesh.kumar
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
---
scripts/tracetool.py | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index 26a4c43..71ef16c 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -240,8 +240,7 @@ def ust_c(events):
#undef inline
#undef wmb
#include "trace.h"'''
- eventlist = list(events)
- for event in eventlist:
+ for event in events:
argnames = event.argnames
if event.argc > 0:
argnames = ', ' + event.argnames
@@ -273,7 +272,7 @@ static void ust_%(name)s_probe(%(args)s)
print '''
static void __attribute__((constructor)) trace_init(void)
{'''
- for event in eventlist:
+ for event in events:
print ' register_trace_ust_%(name)s(ust_%(name)s_probe);' % {
'name': event.name
}
@@ -417,13 +416,15 @@ class Event(object):
# Generator that yields Event objects given a trace-events file object
def read_events(fobj):
event_num = 0
+ res = []
for line in fobj:
if not line.strip():
continue
if line.lstrip().startswith('#'):
continue
- yield Event(event_num, line)
+ res.append(Event(event_num, line))
event_num += 1
+ return res
binary = ""
probeprefix = ""
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PATCH 04/12] trace: [tracetool] Simplify event line parsing
2012-03-13 20:02 [Qemu-devel] [PATCH 00/12] Rewrite tracetool using python Lluís Vilanova
` (2 preceding siblings ...)
2012-03-13 20:02 ` [Qemu-devel] [PATCH 03/12] trace: [tracetool] Do not rebuild event list in backend code Lluís Vilanova
@ 2012-03-13 20:02 ` Lluís Vilanova
2012-03-13 20:03 ` [Qemu-devel] [PATCH 05/12] trace: [tracetool] Do not precompute the event number Lluís Vilanova
` (8 subsequent siblings)
12 siblings, 0 replies; 25+ messages in thread
From: Lluís Vilanova @ 2012-03-13 20:02 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, harsh, aneesh.kumar
Using regular expressions yields more compact and error-proof code when breaking
the lines into pieces.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
---
scripts/tracetool.py | 43 +++++++++++++++----------------------------
1 files changed, 15 insertions(+), 28 deletions(-)
diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index 71ef16c..51c2106 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -10,6 +10,7 @@
import sys
import getopt
+import re
def usage():
print "Tracetool: Generate tracing code for trace events file on stdin"
@@ -38,25 +39,14 @@ Options:
'''
sys.exit(1)
-def get_name(line, sep='('):
- head, sep, tail = line.partition(sep)
- property, sep, name = head.rpartition(' ')
- return name
-
def get_properties(line, sep='('):
head, sep, tail = line.partition(sep)
property, sep, name = head.rpartition(' ')
return property.split()
-def get_args(line, sep1='(', sep2=')'):
- head, sep1, tail = line.partition(sep1)
- args, sep2, fmt_str = tail.partition(sep2)
- return args
-
-def get_argnames(line, sep=','):
+def get_argnames(args):
nfields = 0
str = []
- args = get_args(line)
for field in args.split():
nfields = nfields + 1
# Drop pointer star
@@ -77,17 +67,6 @@ def get_argnames(line, sep=','):
else:
return ''
-def get_argc(line):
- argc = 0
- argnames = get_argnames(line)
- if argnames:
- for name in argnames.split(','):
- argc = argc + 1
- return argc
-
-def get_fmt(line, sep=')'):
- event, sep, fmt = line.partition(sep)
- return fmt.rstrip('\n')
def trace_h_begin():
print '''#ifndef TRACE_H
@@ -402,15 +381,23 @@ trace_gen = {
}
# A trace event
+cre = re.compile("(?P<name>[^(\s]+)\((?P<args>[^)]*)\)\s*(?P<fmt>\".*)?")
+
class Event(object):
def __init__(self, num, line):
self.num = num
- self.args = get_args(line)
+ m = cre.match(line)
+ assert m is not None
+ groups = m.groupdict('')
+ self.args = groups["args"]
self.arglist = self.args.split(',')
- self.name = get_name(line)
- self.argc = get_argc(line)
- self.argnames = get_argnames(line)
- self.fmt = get_fmt(line)
+ self.name = groups["name"]
+ if len(self.arglist) == 1 and self.arglist[0] == "void":
+ self.argc = 0
+ else:
+ self.argc = len(self.arglist)
+ self.argnames = get_argnames(self.args)
+ self.fmt = groups["fmt"]
self.properties = get_properties(line)
# Generator that yields Event objects given a trace-events file object
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PATCH 05/12] trace: [tracetool] Do not precompute the event number
2012-03-13 20:02 [Qemu-devel] [PATCH 00/12] Rewrite tracetool using python Lluís Vilanova
` (3 preceding siblings ...)
2012-03-13 20:02 ` [Qemu-devel] [PATCH 04/12] trace: [tracetool] Simplify event line parsing Lluís Vilanova
@ 2012-03-13 20:03 ` Lluís Vilanova
2012-03-13 20:03 ` [Qemu-devel] [PATCH 06/12] trace: [tracetool] Add support for event properties Lluís Vilanova
` (7 subsequent siblings)
12 siblings, 0 replies; 25+ messages in thread
From: Lluís Vilanova @ 2012-03-13 20:03 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, harsh, aneesh.kumar
An event number is implicit in its position on the list of events.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
---
scripts/tracetool.py | 21 +++++++++------------
1 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index 51c2106..4a85f26 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -106,14 +106,14 @@ def simple_h(events):
print '#include "trace/simple.h"'
print
- for event in events:
+ for num, event in enumerate(events):
if event.argc:
argstr = event.argnames.split()
arg_prefix = '(uint64_t)(uintptr_t)'
cast_args = arg_prefix + arg_prefix.join(argstr)
- simple_args = (str(event.num) + ', ' + cast_args)
+ simple_args = (str(num) + ', ' + cast_args)
else:
- simple_args = str(event.num)
+ simple_args = str(num)
print '''static inline void trace_%(name)s(%(args)s)
{
@@ -125,7 +125,7 @@ def simple_h(events):
'trace_args': simple_args
}
print
- print '#define NR_TRACE_EVENTS %d' % (event.num + 1)
+ print '#define NR_TRACE_EVENTS %d' % len(events)
print 'extern TraceEvent trace_list[NR_TRACE_EVENTS];'
@@ -154,7 +154,7 @@ def stderr_h(events):
#include "trace/stderr.h"
extern TraceEvent trace_list[];'''
- for event in events:
+ for num, event in enumerate(events):
argnames = event.argnames
if event.argc > 0:
argnames = ', ' + event.argnames
@@ -169,12 +169,12 @@ static inline void trace_%(name)s(%(args)s)
}''' % {
'name': event.name,
'args': event.args,
- 'event_num': event.num,
+ 'event_num': num,
'fmt': event.fmt,
'argnames': argnames
}
print
- print '#define NR_TRACE_EVENTS %d' % (event.num + 1)
+ print '#define NR_TRACE_EVENTS %d' % len(events)
def stderr_c(events):
print '''#include "trace.h"
@@ -384,8 +384,7 @@ trace_gen = {
cre = re.compile("(?P<name>[^(\s]+)\((?P<args>[^)]*)\)\s*(?P<fmt>\".*)?")
class Event(object):
- def __init__(self, num, line):
- self.num = num
+ def __init__(self, line):
m = cre.match(line)
assert m is not None
groups = m.groupdict('')
@@ -402,15 +401,13 @@ class Event(object):
# Generator that yields Event objects given a trace-events file object
def read_events(fobj):
- event_num = 0
res = []
for line in fobj:
if not line.strip():
continue
if line.lstrip().startswith('#'):
continue
- res.append(Event(event_num, line))
- event_num += 1
+ res.append(Event(line))
return res
binary = ""
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PATCH 06/12] trace: [tracetool] Add support for event properties
2012-03-13 20:02 [Qemu-devel] [PATCH 00/12] Rewrite tracetool using python Lluís Vilanova
` (4 preceding siblings ...)
2012-03-13 20:03 ` [Qemu-devel] [PATCH 05/12] trace: [tracetool] Do not precompute the event number Lluís Vilanova
@ 2012-03-13 20:03 ` Lluís Vilanova
2012-03-13 20:03 ` [Qemu-devel] [PATCH 07/12] trace: [tracetool] Process the "disable" event property Lluís Vilanova
` (6 subsequent siblings)
12 siblings, 0 replies; 25+ messages in thread
From: Lluís Vilanova @ 2012-03-13 20:03 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, harsh, aneesh.kumar
Using regular expressions yields more compact and error-proof code when breaking
the event properties into pieces.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
---
scripts/tracetool.py | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index 4a85f26..fa646bc 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -39,11 +39,6 @@ Options:
'''
sys.exit(1)
-def get_properties(line, sep='('):
- head, sep, tail = line.partition(sep)
- property, sep, name = head.rpartition(' ')
- return property.split()
-
def get_argnames(args):
nfields = 0
str = []
@@ -381,7 +376,9 @@ trace_gen = {
}
# A trace event
-cre = re.compile("(?P<name>[^(\s]+)\((?P<args>[^)]*)\)\s*(?P<fmt>\".*)?")
+cre = re.compile("((?P<props>.*)\s+)?(?P<name>[^(\s]+)\((?P<args>[^)]*)\)\s*(?P<fmt>\".*)?")
+
+VALID_PROPS = set(["disable"])
class Event(object):
def __init__(self, line):
@@ -397,7 +394,10 @@ class Event(object):
self.argc = len(self.arglist)
self.argnames = get_argnames(self.args)
self.fmt = groups["fmt"]
- self.properties = get_properties(line)
+ self.properties = groups["props"].split()
+ unknown_props = set(self.properties) - VALID_PROPS
+ if len(unknown_props) > 0:
+ raise ValueError("Unknown properties: %s" % ", ".join(unknown_props))
# Generator that yields Event objects given a trace-events file object
def read_events(fobj):
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PATCH 07/12] trace: [tracetool] Process the "disable" event property
2012-03-13 20:02 [Qemu-devel] [PATCH 00/12] Rewrite tracetool using python Lluís Vilanova
` (5 preceding siblings ...)
2012-03-13 20:03 ` [Qemu-devel] [PATCH 06/12] trace: [tracetool] Add support for event properties Lluís Vilanova
@ 2012-03-13 20:03 ` Lluís Vilanova
2012-03-13 20:03 ` [Qemu-devel] [PATCH 08/12] trace: [tracetool] Rewrite event argument parsing Lluís Vilanova
` (5 subsequent siblings)
12 siblings, 0 replies; 25+ messages in thread
From: Lluís Vilanova @ 2012-03-13 20:03 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, harsh, aneesh.kumar
Use different converters (depending on the 'disabled' property) regardless of
the output format.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
---
scripts/tracetool.py | 20 +++++++++++---------
1 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index fa646bc..d7c04b4 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -290,6 +290,9 @@ def dtrace_d(events):
print
print '};'
+def dtrace_nop_d(events):
+ pass
+
def dtrace_stp(events):
for event in events:
# Define prototype for probe arguments
@@ -311,6 +314,9 @@ probe %(probeprefix)s.%(name)s = process("%(binary)s").mark("%(name)s")
print '}'
print
+def dtrace_nop_stp(events):
+ pass
+
def trace_stap_begin():
print '/* This file is autogenerated by tracetool, do not edit. */'
@@ -334,6 +340,8 @@ converters = {
'nop': {
'h': nop_h,
'c': nop_c,
+ 'd': dtrace_nop_d,
+ 'stap': dtrace_nop_stp,
},
'stderr': {
@@ -483,17 +491,11 @@ def main():
if probeprefix == "":
probeprefix = 'qemu.' + targettype + '.' + targetarch
- disabled_events, enabled_events = [], []
- for e in read_events(sys.stdin):
- if 'disable' in e.properties:
- disabled_events.append(e)
- else:
- enabled_events.append(e)
+ events = read_events(sys.stdin)
trace_gen[output]['begin']()
- if output == 'h': # disabled events
- converters['nop'][output](disabled_events)
- converters[backend][output](enabled_events)
+ converters[backend][output]([ e for e in events if 'disable' not in e.properties ])
+ converters['nop'][output]([ e for e in events if 'disable' in e.properties ])
trace_gen[output]['end']()
if __name__ == "__main__":
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PATCH 08/12] trace: [tracetool] Rewrite event argument parsing
2012-03-13 20:02 [Qemu-devel] [PATCH 00/12] Rewrite tracetool using python Lluís Vilanova
` (6 preceding siblings ...)
2012-03-13 20:03 ` [Qemu-devel] [PATCH 07/12] trace: [tracetool] Process the "disable" event property Lluís Vilanova
@ 2012-03-13 20:03 ` Lluís Vilanova
2012-03-13 20:03 ` [Qemu-devel] [PATCH 09/12] trace: [tracetool] Make format-specific code optional and with access to events Lluís Vilanova
` (4 subsequent siblings)
12 siblings, 0 replies; 25+ messages in thread
From: Lluís Vilanova @ 2012-03-13 20:03 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, harsh, aneesh.kumar
Adds an 'Arguments' class to describe event arguments.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
---
scripts/tracetool.py | 113 ++++++++++++++++++++++++--------------------------
1 files changed, 54 insertions(+), 59 deletions(-)
diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index d7c04b4..f017053 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -39,30 +39,6 @@ Options:
'''
sys.exit(1)
-def get_argnames(args):
- nfields = 0
- str = []
- for field in args.split():
- nfields = nfields + 1
- # Drop pointer star
- type, ptr, tail = field.partition('*')
- if type != field:
- field = tail
-
- name, sep, tail = field.partition(',')
-
- if name == field:
- continue
- str.append(name)
- str.append(", ")
-
- if nfields > 1:
- str.append(name)
- return ''.join(str)
- else:
- return ''
-
-
def trace_h_begin():
print '''#ifndef TRACE_H
#define TRACE_H
@@ -102,11 +78,11 @@ def simple_h(events):
print
for num, event in enumerate(events):
- if event.argc:
- argstr = event.argnames.split()
- arg_prefix = '(uint64_t)(uintptr_t)'
+ if len(event.args):
+ argstr = event.args.names()
+ arg_prefix = ', (uint64_t)(uintptr_t)'
cast_args = arg_prefix + arg_prefix.join(argstr)
- simple_args = (str(num) + ', ' + cast_args)
+ simple_args = (str(num) + cast_args)
else:
simple_args = str(num)
@@ -116,7 +92,7 @@ def simple_h(events):
}''' % {
'name': event.name,
'args': event.args,
- 'argc': event.argc,
+ 'argc': len(event.args),
'trace_args': simple_args
}
print
@@ -124,13 +100,6 @@ def simple_h(events):
print 'extern TraceEvent trace_list[NR_TRACE_EVENTS];'
-def is_string(arg):
- strtype = ('const char*', 'char*', 'const char *', 'char *')
- if arg.lstrip().startswith(strtype):
- return True
- else:
- return False
-
def simple_c(events):
print '#include "trace.h"'
print
@@ -150,11 +119,9 @@ def stderr_h(events):
extern TraceEvent trace_list[];'''
for num, event in enumerate(events):
- argnames = event.argnames
- if event.argc > 0:
- argnames = ', ' + event.argnames
- else:
- argnames = ''
+ argnames = ", ".join(event.args.names())
+ if len(event.args) > 0:
+ argnames = ", " + argnames
print '''
static inline void trace_%(name)s(%(args)s)
{
@@ -191,13 +158,13 @@ def ust_h(events):
#undef wmb'''
for event in events:
- if event.argc > 0:
+ if len(event.args) > 0:
print '''
DECLARE_TRACE(ust_%(name)s, TP_PROTO(%(args)s), TP_ARGS(%(argnames)s));
#define trace_%(name)s trace_ust_%(name)s''' % {
'name': event.name,
'args': event.args,
- 'argnames': event.argnames
+ 'argnames': ", ".join(event.args.names())
}
else:
print '''
@@ -215,9 +182,9 @@ def ust_c(events):
#undef wmb
#include "trace.h"'''
for event in events:
- argnames = event.argnames
- if event.argc > 0:
- argnames = ', ' + event.argnames
+ argnames = ", ".join(event.args.names())
+ if len(event.args) > 0:
+ argnames = ', ' + argnames
print '''
DEFINE_TRACE(ust_%(name)s);
@@ -265,7 +232,7 @@ def dtrace_h(events):
'name': event.name,
'args': event.args,
'uppername': event.name.upper(),
- 'argnames': event.argnames,
+ 'argnames': ", ".join(event.args.names()),
}
def dtrace_c(events):
@@ -304,12 +271,12 @@ probe %(probeprefix)s.%(name)s = process("%(binary)s").mark("%(name)s")
'binary': binary
}
i = 1
- if event.argc > 0:
- for arg in event.argnames.split(','):
+ if len(event.args) > 0:
+ for name in event.args.names():
# 'limit' is a reserved keyword
- if arg == 'limit':
- arg = '_limit'
- print ' %s = $arg%d;' % (arg.lstrip(), i)
+ if name == 'limit':
+ name = '_limit'
+ print ' %s = $arg%d;' % (name.lstrip(), i)
i += 1
print '}'
print
@@ -383,6 +350,39 @@ trace_gen = {
},
}
+# Event arguments
+class Arguments:
+ def __init__ (self, arg_str):
+ self._args = []
+ for arg in arg_str.split(","):
+ arg = arg.strip()
+ parts = arg.split()
+ head, sep, tail = parts[-1].rpartition("*")
+ parts = parts[:-1]
+ if tail == "void":
+ assert len(parts) == 0 and sep == ""
+ continue
+ arg_type = " ".join(parts + [ " ".join([head, sep]).strip() ]).strip()
+ self._args.append((arg_type, tail))
+
+ def __iter__(self):
+ return iter(self._args)
+
+ def __len__(self):
+ return len(self._args)
+
+ def __str__(self):
+ if len(self._args) == 0:
+ return "void"
+ else:
+ return ", ".join([ " ".join([t, n]) for t,n in self._args ])
+
+ def names(self):
+ return [ name for _, name in self._args ]
+
+ def types(self):
+ return [ type_ for type_, _ in self._args ]
+
# A trace event
cre = re.compile("((?P<props>.*)\s+)?(?P<name>[^(\s]+)\((?P<args>[^)]*)\)\s*(?P<fmt>\".*)?")
@@ -393,16 +393,11 @@ class Event(object):
m = cre.match(line)
assert m is not None
groups = m.groupdict('')
- self.args = groups["args"]
- self.arglist = self.args.split(',')
self.name = groups["name"]
- if len(self.arglist) == 1 and self.arglist[0] == "void":
- self.argc = 0
- else:
- self.argc = len(self.arglist)
- self.argnames = get_argnames(self.args)
self.fmt = groups["fmt"]
self.properties = groups["props"].split()
+ self.args = Arguments(groups["args"])
+
unknown_props = set(self.properties) - VALID_PROPS
if len(unknown_props) > 0:
raise ValueError("Unknown properties: %s" % ", ".join(unknown_props))
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PATCH 09/12] trace: [tracetool] Make format-specific code optional and with access to events
2012-03-13 20:02 [Qemu-devel] [PATCH 00/12] Rewrite tracetool using python Lluís Vilanova
` (7 preceding siblings ...)
2012-03-13 20:03 ` [Qemu-devel] [PATCH 08/12] trace: [tracetool] Rewrite event argument parsing Lluís Vilanova
@ 2012-03-13 20:03 ` Lluís Vilanova
2012-03-13 20:03 ` [Qemu-devel] [PATCH 10/12] trace: [tracetool] Automatically establish available backends and formats Lluís Vilanova
` (3 subsequent siblings)
12 siblings, 0 replies; 25+ messages in thread
From: Lluís Vilanova @ 2012-03-13 20:03 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, harsh, aneesh.kumar
First, routines for format-specific code are not mandatory.
Second, all format-specific routines get the list of events as an argument.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
---
scripts/tracetool.py | 32 ++++++++++----------------------
1 files changed, 10 insertions(+), 22 deletions(-)
diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index f017053..377c683 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -39,7 +39,7 @@ Options:
'''
sys.exit(1)
-def trace_h_begin():
+def trace_h_begin(events):
print '''#ifndef TRACE_H
#define TRACE_H
@@ -47,17 +47,12 @@ def trace_h_begin():
#include "qemu-common.h"'''
-
-def trace_h_end():
+def trace_h_end(events):
print '#endif /* TRACE_H */'
-
-def trace_c_begin():
+def trace_c_begin(events):
print '/* This file is autogenerated by tracetool, do not edit. */'
-def trace_c_end():
- pass # nop, required for trace_gen
-
def nop_h(events):
print
for event in events:
@@ -284,18 +279,12 @@ probe %(probeprefix)s.%(name)s = process("%(binary)s").mark("%(name)s")
def dtrace_nop_stp(events):
pass
-def trace_stap_begin():
+def trace_stap_begin(events):
print '/* This file is autogenerated by tracetool, do not edit. */'
-def trace_stap_end():
- pass #nop, reqd for trace_gen
-
-def trace_d_begin():
+def trace_d_begin(events):
print '/* This file is autogenerated by tracetool, do not edit. */'
-def trace_d_end():
- pass #nop, reqd for trace_gen
-
# Registry of backends and their converter functions
converters = {
@@ -331,22 +320,19 @@ converters = {
}
# Trace file header and footer code generators
-trace_gen = {
+formats = {
'h': {
'begin': trace_h_begin,
'end': trace_h_end,
},
'c': {
'begin': trace_c_begin,
- 'end': trace_c_end,
},
'd': {
'begin': trace_d_begin,
- 'end': trace_d_end,
},
'stap': {
'begin': trace_stap_begin,
- 'end': trace_stap_end,
},
}
@@ -488,10 +474,12 @@ def main():
events = read_events(sys.stdin)
- trace_gen[output]['begin']()
+ if 'begin' in formats[output]:
+ formats[output]['begin'](events)
converters[backend][output]([ e for e in events if 'disable' not in e.properties ])
converters['nop'][output]([ e for e in events if 'disable' in e.properties ])
- trace_gen[output]['end']()
+ if 'end' in formats[output]:
+ formats[output]['end'](events)
if __name__ == "__main__":
main()
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PATCH 10/12] trace: [tracetool] Automatically establish available backends and formats
2012-03-13 20:02 [Qemu-devel] [PATCH 00/12] Rewrite tracetool using python Lluís Vilanova
` (8 preceding siblings ...)
2012-03-13 20:03 ` [Qemu-devel] [PATCH 09/12] trace: [tracetool] Make format-specific code optional and with access to events Lluís Vilanova
@ 2012-03-13 20:03 ` Lluís Vilanova
2012-03-20 9:22 ` Stefan Hajnoczi
2012-03-13 20:03 ` [Qemu-devel] [PATCH 11/12] trace: Provide a per-event status define for conditional compilation Lluís Vilanova
` (2 subsequent siblings)
12 siblings, 1 reply; 25+ messages in thread
From: Lluís Vilanova @ 2012-03-13 20:03 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, harsh, aneesh.kumar
Adds decorators to establish which backend and/or format each routine is meant
to process.
With this, tables enumerating format and backend routines can be eliminated and
part of the usage message can be computed in a more generic way.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
---
Makefile.objs | 6 -
Makefile.target | 3
scripts/tracetool.py | 320 ++++++++++++++++++++++++++++++++------------------
3 files changed, 211 insertions(+), 118 deletions(-)
diff --git a/Makefile.objs b/Makefile.objs
index a718963..228a756 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -357,12 +357,12 @@ else
trace.h: trace.h-timestamp
endif
trace.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/tracetool.py --backend=$(TRACE_BACKEND) -h < $< > $@," GEN trace.h")
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/tracetool.py --format=h --backend=$(TRACE_BACKEND) < $< > $@," GEN trace.h")
@cmp -s $@ trace.h || cp $@ trace.h
trace.c: trace.c-timestamp
trace.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/tracetool.py --backend=$(TRACE_BACKEND) -c < $< > $@," GEN trace.c")
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/tracetool.py --format=c --backend=$(TRACE_BACKEND) < $< > $@," GEN trace.c")
@cmp -s $@ trace.c || cp $@ trace.c
trace.o: trace.c $(GENERATED_HEADERS)
@@ -375,7 +375,7 @@ trace-dtrace.h: trace-dtrace.dtrace
# rule file. So we use '.dtrace' instead
trace-dtrace.dtrace: trace-dtrace.dtrace-timestamp
trace-dtrace.dtrace-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/tracetool.py --backend=$(TRACE_BACKEND) -d < $< > $@," GEN trace-dtrace.dtrace")
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/tracetool.py --format=d --backend=$(TRACE_BACKEND) < $< > $@," GEN trace-dtrace.dtrace")
@cmp -s $@ trace-dtrace.dtrace || cp $@ trace-dtrace.dtrace
trace-dtrace.o: trace-dtrace.dtrace $(GENERATED_HEADERS)
diff --git a/Makefile.target b/Makefile.target
index 3e42e5a..c1e58fb 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -60,11 +60,12 @@ endif
$(QEMU_PROG).stp:
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/tracetool.py \
+ --format=stap \
--backend=$(TRACE_BACKEND) \
--binary=$(bindir)/$(QEMU_PROG) \
--target-arch=$(TARGET_ARCH) \
--target-type=$(TARGET_TYPE) \
- --stap < $(SRC_PATH)/trace-events > $(QEMU_PROG).stp," GEN $(QEMU_PROG).stp")
+ < $(SRC_PATH)/trace-events > $(QEMU_PROG).stp," GEN $(QEMU_PROG).stp")
else
stap:
endif
diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index 377c683..6ef4374 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -12,33 +12,107 @@ import sys
import getopt
import re
-def usage():
- print "Tracetool: Generate tracing code for trace events file on stdin"
- print "Usage:"
- print sys.argv[0], "--backend=[nop|simple|stderr|dtrace|ust] [-h|-c|-d|--stap]"
- print '''
-Backends:
- --nop Tracing disabled
- --simple Simple built-in backend
- --stderr Stderr built-in backend
- --dtrace DTrace/SystemTAP backend
- --ust LTTng User Space Tracing backend
-
-Output formats:
- -h Generate .h file
- -c Generate .c file
- -d Generate .d file (DTrace only)
- --stap Generate .stp file (DTrace with SystemTAP only)
+######################################################################
+# format auto-registration
-Options:
- --binary [path] Full path to QEMU binary
- --target-arch [arch] QEMU emulator target arch
- --target-type [type] QEMU emulator target type ('system' or 'user')
- --probe-prefix [prefix] Prefix for dtrace probe names
- (default: qemu-targettype-targetarch)
-'''
- sys.exit(1)
+class _Tag:
+ pass
+
+_formats = {}
+
+BEGIN = _Tag()
+END = _Tag()
+_DESCR = _Tag()
+
+def for_format(format_, when, descr = None):
+ """Decorator for format generator functions."""
+
+ if when is not BEGIN and when is not END:
+ raise ValueError("Invalid 'when' tag")
+ if format_ in _formats and when in _formats[format_]:
+ raise ValueError("Format '%s' already set for given 'when' tag" % format_)
+
+ if format_ not in _formats:
+ _formats[format_] = {}
+ if descr is not None:
+ if _DESCR in _formats[format_]:
+ raise ValueError("Description already set")
+ _formats[format_][_DESCR] = descr
+
+ def func(f):
+ _formats[format_][when] = f
+ return f
+ return func
+
+def get_format(format_, when):
+ """Get a format generator function."""
+
+ def nop(*args, **kwargs):
+ pass
+ if format_ in _formats and when in _formats[format_]:
+ return _formats[format_][when]
+ else:
+ return nop
+
+def get_format_descr(format_):
+ """Get the description of a format generator."""
+
+ if format_ in _formats and _DESCR in _formats[format_]:
+ return _formats[format_][_DESCR]
+ else:
+ return ""
+
+
+######################################################################
+# backend auto-registration and format compatibility
+
+_backends = {}
+
+def for_backend(backend, format_, descr = None):
+ if backend not in _backends:
+ _backends[backend] = {}
+ if format_ in _backends[backend]:
+ raise ValueError("Backend '%s' already set for backend '%s'" % (backend, format_))
+ if format_ not in _formats:
+ raise ValueError("Unknown format '%s'" % format_)
+
+ if descr is not None:
+ if _DESCR in _backends[backend]:
+ raise ValueError("Description already set")
+ _backends[backend][_DESCR] = descr
+
+ def func(f):
+ _backends[backend][format_] = f
+ return f
+ return func
+
+def get_backend(format_, backend):
+ if backend not in _backends:
+ raise ValueError("Unknown backend '%s'" % backend)
+ if format_ not in _formats:
+ raise ValueError("Unknown format '%s'" % format_)
+ if format_ not in _backends[backend]:
+ raise ValueError("Format '%s' not supported with backend '%s'" % (format_, backend))
+ return _backends[backend][format_]
+
+def get_backend_descr(backend):
+ """Get the description of a backend."""
+
+ if backend in _backends and _DESCR in _backends[backend]:
+ return _backends[backend][_DESCR]
+ else:
+ return ""
+
+
+
+######################################################################
+# formats
+
+##################################################
+# format: h
+
+@for_format("h", BEGIN, "Generate .h file")
def trace_h_begin(events):
print '''#ifndef TRACE_H
#define TRACE_H
@@ -47,12 +121,27 @@ def trace_h_begin(events):
#include "qemu-common.h"'''
+@for_format("h", END)
def trace_h_end(events):
print '#endif /* TRACE_H */'
+
+##################################################
+# format: c
+
+@for_format("c", BEGIN, "Generate .c file")
def trace_c_begin(events):
print '/* This file is autogenerated by tracetool, do not edit. */'
+
+
+######################################################################
+# backends
+
+##################################################
+# backend: nop
+
+@for_backend("nop", "h", "Tracing disabled")
def nop_h(events):
print
for event in events:
@@ -63,11 +152,15 @@ def nop_h(events):
'name': event.name,
'args': event.args
}
- return
+@for_backend("nop", "c")
def nop_c(events):
pass # nop, reqd for converters
+##################################################
+# backend: simple
+
+@for_backend("simple", "h", "Simple built-in backend")
def simple_h(events):
print '#include "trace/simple.h"'
print
@@ -95,6 +188,7 @@ def simple_h(events):
print 'extern TraceEvent trace_list[NR_TRACE_EVENTS];'
+@for_backend("simple", "c")
def simple_c(events):
print '#include "trace.h"'
print
@@ -108,6 +202,10 @@ def simple_c(events):
print
print '};'
+##################################################
+# backend: stderr
+
+@for_backend("stderr", "h", "Stderr built-in backend")
def stderr_h(events):
print '''#include <stdio.h>
#include "trace/stderr.h"
@@ -133,6 +231,7 @@ static inline void trace_%(name)s(%(args)s)
print
print '#define NR_TRACE_EVENTS %d' % len(events)
+@for_backend("stderr", "c")
def stderr_c(events):
print '''#include "trace.h"
@@ -145,6 +244,11 @@ TraceEvent trace_list[] = {
print
print '};'
+
+##################################################
+# backend: ust
+
+@for_backend("ust", "h", "LTTng User Space Tracing backend")
def ust_h(events):
print '''#include <ust/tracepoint.h>
#undef mutex_lock
@@ -169,6 +273,7 @@ _DECLARE_TRACEPOINT_NOARGS(ust_%(name)s);
}
print
+@for_backend("ust", "c")
def ust_c(events):
print '''#include <ust/marker.h>
#undef mutex_lock
@@ -214,6 +319,10 @@ static void __attribute__((constructor)) trace_init(void)
}
print '}'
+##################################################
+# backend: dtrace
+
+@for_backend("dtrace", "h", "DTrace/SystemTAP backend")
def dtrace_h(events):
print '#include "trace-dtrace.h"'
print
@@ -230,9 +339,15 @@ def dtrace_h(events):
'argnames': ", ".join(event.args.names()),
}
+@for_backend("dtrace", "c")
def dtrace_c(events):
pass # No need for function definitions in dtrace backend
+@for_format("d", BEGIN, "Generate .d file (DTrace probes)")
+def trace_d_begin(events):
+ print '/* This file is autogenerated by tracetool, do not edit. */'
+
+@for_backend("dtrace", "d")
def dtrace_d(events):
print 'provider qemu {'
for event in events:
@@ -252,9 +367,15 @@ def dtrace_d(events):
print
print '};'
+@for_backend("nop", "d")
def dtrace_nop_d(events):
pass
+
+@for_format("stap", BEGIN, "Generate .stp file (SystemTAP tapsets)")
+def trace_stap_begin(events):
+ print '/* This file is autogenerated by tracetool, do not edit. */'
+
def dtrace_stp(events):
for event in events:
# Define prototype for probe arguments
@@ -279,64 +400,10 @@ probe %(probeprefix)s.%(name)s = process("%(binary)s").mark("%(name)s")
def dtrace_nop_stp(events):
pass
-def trace_stap_begin(events):
- print '/* This file is autogenerated by tracetool, do not edit. */'
-
-def trace_d_begin(events):
- print '/* This file is autogenerated by tracetool, do not edit. */'
-
-
-# Registry of backends and their converter functions
-converters = {
- 'simple': {
- 'h': simple_h,
- 'c': simple_c,
- },
-
- 'nop': {
- 'h': nop_h,
- 'c': nop_c,
- 'd': dtrace_nop_d,
- 'stap': dtrace_nop_stp,
- },
-
- 'stderr': {
- 'h': stderr_h,
- 'c': stderr_c,
- },
-
- 'dtrace': {
- 'h': dtrace_h,
- 'c': dtrace_c,
- 'd': dtrace_d,
- 'stap': dtrace_stp
- },
-
- 'ust': {
- 'h': ust_h,
- 'c': ust_c,
- },
-
-}
-
-# Trace file header and footer code generators
-formats = {
- 'h': {
- 'begin': trace_h_begin,
- 'end': trace_h_end,
- },
- 'c': {
- 'begin': trace_c_begin,
- },
- 'd': {
- 'begin': trace_d_begin,
- },
- 'stap': {
- 'begin': trace_stap_begin,
- },
-}
+######################################################################
# Event arguments
+
class Arguments:
def __init__ (self, arg_str):
self._args = []
@@ -369,7 +436,10 @@ class Arguments:
def types(self):
return [ type_ for type_, _ in self._args ]
+
+######################################################################
# A trace event
+
cre = re.compile("((?P<props>.*)\s+)?(?P<name>[^(\s]+)\((?P<args>[^)]*)\)\s*(?P<fmt>\".*)?")
VALID_PROPS = set(["disable"])
@@ -399,32 +469,51 @@ def read_events(fobj):
res.append(Event(line))
return res
+######################################################################
+# Main
+
+format_ = ""
binary = ""
probeprefix = ""
+def usage():
+ print "Tracetool: Generate tracing code for trace events file on stdin"
+ print "Usage:"
+ print sys.argv[0], " --format=<format> --backend=<backend>"
+ print
+ print "Output formats:"
+ for f in _formats:
+ print " %-10s %s" % (f, get_format_descr(f))
+ print
+ print "Backends:"
+ for b in _backends:
+ print " %-10s %s" % (b, get_backend_descr(b))
+ print """
+Options:
+ --binary [path] Full path to QEMU binary
+ --target-arch [arch] QEMU emulator target arch
+ --target-type [type] QEMU emulator target type ('system' or 'user')
+ --probe-prefix [prefix] Prefix for dtrace probe names
+ (default: qemu-targettype-targetarch)
+"""
+
+ sys.exit(1)
+
def main():
- global binary, probeprefix
+ global format_, binary, probeprefix
targettype = ""
targetarch = ""
- supported_backends = ["simple", "nop", "stderr", "dtrace", "ust"]
- short_options = "hcd"
- long_options = ["stap", "backend=", "binary=", "target-arch=", "target-type=", "probe-prefix=", "list-backends", "check-backend"]
+ long_options = ["format=", "backend=", "binary=", "target-arch=", "target-type=", "probe-prefix=", "list-backends", "check-backend"]
try:
- opts, args = getopt.getopt(sys.argv[1:], short_options, long_options)
+ opts, args = getopt.getopt(sys.argv[1:], "", long_options)
except getopt.GetoptError, err:
# print help information and exit:
print str(err) # will print something like "option -a not recognized"
usage()
sys.exit(2)
for opt, arg in opts:
- if opt == '-h':
- output = 'h'
- elif opt == '-c':
- output = 'c'
- elif opt == '-d':
- output = 'd'
- elif opt == '--stap':
- output = 'stap'
+ if opt == '--format':
+ format_ = arg
elif opt == '--backend':
backend = arg
elif opt == '--binary':
@@ -436,30 +525,27 @@ def main():
elif opt == '--probe-prefix':
probeprefix = arg
elif opt == '--list-backends':
- print 'simple, nop, stderr, dtrace, ust'
+ print ', '.join(_backends)
sys.exit(0)
elif opt == "--check-backend":
- if any(backend in s for s in supported_backends):
+ if backend in _backends:
sys.exit(0)
else:
sys.exit(1)
else:
- #assert False, "unhandled option"
print "unhandled option: ", opt
usage()
- if backend == "" or output == "":
+ if format_ not in _formats:
+ print "Unknown format: %s" % format_
+ print
+ usage()
+ if backend not in _backends:
+ print "Unknown backend: %s" % backend
+ print
usage()
- sys.exit(0)
-
- if backend != 'dtrace' and output == 'd':
- print 'DTrace probe generator not applicable to %s backend' % backend
- sys.exit(1)
- if output == 'stap':
- if backend != "dtrace":
- print 'SystemTAP tapset generator not applicable to %s backend' % backend
- sys.exit(1)
+ if format_ == 'stap':
if binary == "":
print '--binary is required for SystemTAP tapset generator'
sys.exit(1)
@@ -474,12 +560,18 @@ def main():
events = read_events(sys.stdin)
- if 'begin' in formats[output]:
- formats[output]['begin'](events)
- converters[backend][output]([ e for e in events if 'disable' not in e.properties ])
- converters['nop'][output]([ e for e in events if 'disable' in e.properties ])
- if 'end' in formats[output]:
- formats[output]['end'](events)
+ try:
+ # just force format/backend compatibility check
+ bfun = get_backend(format_, backend)
+ bnop = get_backend(format_, "nop")
+ except Exception as e:
+ sys.stderr.write(str(e) + "\n\n")
+ usage()
+
+ get_format(format_, BEGIN)(events)
+ bfun([ e for e in events if "disable" not in e.properties ])
+ bnop([ e for e in events if "disable" in e.properties ])
+ get_format(format_, END)(events)
if __name__ == "__main__":
main()
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 10/12] trace: [tracetool] Automatically establish available backends and formats
2012-03-13 20:03 ` [Qemu-devel] [PATCH 10/12] trace: [tracetool] Automatically establish available backends and formats Lluís Vilanova
@ 2012-03-20 9:22 ` Stefan Hajnoczi
2012-03-20 14:00 ` Lluís Vilanova
0 siblings, 1 reply; 25+ messages in thread
From: Stefan Hajnoczi @ 2012-03-20 9:22 UTC (permalink / raw)
To: Lluís Vilanova; +Cc: harsh, qemu-devel, aneesh.kumar
2012/3/13 Lluís Vilanova <vilanova@ac.upc.edu>:
> Adds decorators to establish which backend and/or format each routine is meant
> to process.
>
> With this, tables enumerating format and backend routines can be eliminated and
> part of the usage message can be computed in a more generic way.
>
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
> ---
> Makefile.objs | 6 -
> Makefile.target | 3
> scripts/tracetool.py | 320 ++++++++++++++++++++++++++++++++------------------
> 3 files changed, 211 insertions(+), 118 deletions(-)
I find the decorators are overkill and we miss the chance to use more
straightforward approaches that Python supports. The decorators build
structures behind the scenes instead of using classes in an open coded
way. I think this makes it more difficult for people to modify the
code - they will need to dig in to what exactly the decorators do -
what they do is pretty similar to what you get from a class.
I've tried out an alternative approach which works very nicely for
formats. For backends it's not a perfect fit because it creates
instances when we don't really need them, but I think it's still an
overall cleaner approach:
https://github.com/stefanha/qemu/commit/3500eb43f80f3c9200107aa0ca19a1b57300ef8a
What do you think?
Stefan
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 10/12] trace: [tracetool] Automatically establish available backends and formats
2012-03-20 9:22 ` Stefan Hajnoczi
@ 2012-03-20 14:00 ` Lluís Vilanova
2012-03-20 16:33 ` Stefan Hajnoczi
0 siblings, 1 reply; 25+ messages in thread
From: Lluís Vilanova @ 2012-03-20 14:00 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: harsh, qemu-devel, aneesh.kumar
Stefan Hajnoczi writes:
> 2012/3/13 Lluís Vilanova <vilanova@ac.upc.edu>:
>> Adds decorators to establish which backend and/or format each routine is meant
>> to process.
>>
>> With this, tables enumerating format and backend routines can be eliminated and
>> part of the usage message can be computed in a more generic way.
>>
>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>> Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
>> ---
>> Makefile.objs | 6 -
>> Makefile.target | 3
>> scripts/tracetool.py | 320 ++++++++++++++++++++++++++++++++------------------
>> 3 files changed, 211 insertions(+), 118 deletions(-)
> I find the decorators are overkill and we miss the chance to use more
> straightforward approaches that Python supports. The decorators build
> structures behind the scenes instead of using classes in an open coded
> way. I think this makes it more difficult for people to modify the
> code - they will need to dig in to what exactly the decorators do -
> what they do is pretty similar to what you get from a class.
> I've tried out an alternative approach which works very nicely for
> formats. For backends it's not a perfect fit because it creates
> instances when we don't really need them, but I think it's still an
> overall cleaner approach:
> https://github.com/stefanha/qemu/commit/3500eb43f80f3c9200107aa0ca19a1b57300ef8a
> What do you think?
I don't like it:
1) Format and backend tables must be manually filled.
2) The base Backend class has empty methods for each possible format.
3) There is no control on format/backend compatibility.
But I do like the idea of having a single per-backend class having methods for
each possible format.
The main reason for automatically establishing formats, backends and their
compatibility is that the instrumentation patches add some extra formats and
backends, which makes the process much more tedious if it's not automated.
Whether you use decorators or classes is not that important.
Auto-registration can be accomplished using metaclasses and special
per-format/backend "special" attributes the metaclasses will look for (e.g. NAME
to set the commandline-visible name of a format/backend.).
Format/backend compatibility can be established by introspecting into the
methods on each backend child class, matched against the NAMEs of the registered
formats.
But going this way does not sound to me like it will be much clearer than
decorators.
Do you agree? (I'll wait on solving this before fixing the rest of your concerns
in this series). Do you still prefer classes over decorators?
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 10/12] trace: [tracetool] Automatically establish available backends and formats
2012-03-20 14:00 ` Lluís Vilanova
@ 2012-03-20 16:33 ` Stefan Hajnoczi
2012-03-20 18:45 ` Lluís Vilanova
0 siblings, 1 reply; 25+ messages in thread
From: Stefan Hajnoczi @ 2012-03-20 16:33 UTC (permalink / raw)
To: Lluís Vilanova; +Cc: harsh, qemu-devel, aneesh.kumar
2012/3/20 Lluís Vilanova <vilanova@ac.upc.edu>:
> Stefan Hajnoczi writes:
>
>> 2012/3/13 Lluís Vilanova <vilanova@ac.upc.edu>:
>>> Adds decorators to establish which backend and/or format each routine is meant
>>> to process.
>>>
>>> With this, tables enumerating format and backend routines can be eliminated and
>>> part of the usage message can be computed in a more generic way.
>>>
>>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>>> Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
>>> ---
>>> Makefile.objs | 6 -
>>> Makefile.target | 3
>>> scripts/tracetool.py | 320 ++++++++++++++++++++++++++++++++------------------
>>> 3 files changed, 211 insertions(+), 118 deletions(-)
>
>> I find the decorators are overkill and we miss the chance to use more
>> straightforward approaches that Python supports. The decorators build
>> structures behind the scenes instead of using classes in an open coded
>> way. I think this makes it more difficult for people to modify the
>> code - they will need to dig in to what exactly the decorators do -
>> what they do is pretty similar to what you get from a class.
>
>> I've tried out an alternative approach which works very nicely for
>> formats. For backends it's not a perfect fit because it creates
>> instances when we don't really need them, but I think it's still an
>> overall cleaner approach:
>
>> https://github.com/stefanha/qemu/commit/3500eb43f80f3c9200107aa0ca19a1b57300ef8a
>
>> What do you think?
>
> I don't like it:
>
> 1) Format and backend tables must be manually filled.
>
> 2) The base Backend class has empty methods for each possible format.
>
> 3) There is no control on format/backend compatibility.
>
> But I do like the idea of having a single per-backend class having methods for
> each possible format.
>
> The main reason for automatically establishing formats, backends and their
> compatibility is that the instrumentation patches add some extra formats and
> backends, which makes the process much more tedious if it's not automated.
>
> Whether you use decorators or classes is not that important.
>
> Auto-registration can be accomplished using metaclasses and special
> per-format/backend "special" attributes the metaclasses will look for (e.g. NAME
> to set the commandline-visible name of a format/backend.).
>
> Format/backend compatibility can be established by introspecting into the
> methods on each backend child class, matched against the NAMEs of the registered
> formats.
>
> But going this way does not sound to me like it will be much clearer than
> decorators.
>
> Do you agree? (I'll wait on solving this before fixing the rest of your concerns
> in this series). Do you still prefer classes over decorators?
For formats the Format class plus a dict approach is much nicer than
decorators. The code is short and easy to understand.
For backends it becomes a little tougher and I was wondering whether
splitting the code into modules would buy us something. The fact that
you've added '####...' section delimeters shows that tracetool.py is
growing to long and we're putting too many concepts into one file. If
each backend is a module then we have a natural way of containing
backend-specific code. Perhaps the module can register itself when
tracetool.py wildcard imports them all. I think this will approach
the level of magic that decorators involve but with the bonus that we
begin to split the code instead of growing a blob. What do you think
of putting each backend in its own module?
Do you have a link to your latest code that adds formats/backends?
I'd like to take a quick look to make sure I understand where you're
going with this - I've only been thinking of the current set of
formats/backends.
As the next step with this patch series we could drop this patch. It
doesn't make an external difference. Then we can continue to discuss
how to best handle backends as a separate patch.
Stefan
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 10/12] trace: [tracetool] Automatically establish available backends and formats
2012-03-20 16:33 ` Stefan Hajnoczi
@ 2012-03-20 18:45 ` Lluís Vilanova
2012-03-21 9:29 ` Stefan Hajnoczi
0 siblings, 1 reply; 25+ messages in thread
From: Lluís Vilanova @ 2012-03-20 18:45 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: harsh, qemu-devel, aneesh.kumar
Stefan Hajnoczi writes:
> 2012/3/20 Lluís Vilanova <vilanova@ac.upc.edu>:
>> Stefan Hajnoczi writes:
>>
>>> 2012/3/13 Lluís Vilanova <vilanova@ac.upc.edu>:
>>>> Adds decorators to establish which backend and/or format each routine is meant
>>>> to process.
>>>>
>>>> With this, tables enumerating format and backend routines can be eliminated and
>>>> part of the usage message can be computed in a more generic way.
>>>>
>>>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>>>> Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
>>>> ---
>>>> Makefile.objs | 6 -
>>>> Makefile.target | 3
>>>> scripts/tracetool.py | 320 ++++++++++++++++++++++++++++++++------------------
>>>> 3 files changed, 211 insertions(+), 118 deletions(-)
>>
>>> I find the decorators are overkill and we miss the chance to use more
>>> straightforward approaches that Python supports. The decorators build
>>> structures behind the scenes instead of using classes in an open coded
>>> way. I think this makes it more difficult for people to modify the
>>> code - they will need to dig in to what exactly the decorators do -
>>> what they do is pretty similar to what you get from a class.
>>
>>> I've tried out an alternative approach which works very nicely for
>>> formats. For backends it's not a perfect fit because it creates
>>> instances when we don't really need them, but I think it's still an
>>> overall cleaner approach:
>>
>>> https://github.com/stefanha/qemu/commit/3500eb43f80f3c9200107aa0ca19a1b57300ef8a
>>
>>> What do you think?
>>
>> I don't like it:
>>
>> 1) Format and backend tables must be manually filled.
>>
>> 2) The base Backend class has empty methods for each possible format.
>>
>> 3) There is no control on format/backend compatibility.
>>
>> But I do like the idea of having a single per-backend class having methods for
>> each possible format.
>>
>> The main reason for automatically establishing formats, backends and their
>> compatibility is that the instrumentation patches add some extra formats and
>> backends, which makes the process much more tedious if it's not automated.
>>
>> Whether you use decorators or classes is not that important.
>>
>> Auto-registration can be accomplished using metaclasses and special
>> per-format/backend "special" attributes the metaclasses will look for (e.g. NAME
>> to set the commandline-visible name of a format/backend.).
>>
>> Format/backend compatibility can be established by introspecting into the
>> methods on each backend child class, matched against the NAMEs of the registered
>> formats.
>>
>> But going this way does not sound to me like it will be much clearer than
>> decorators.
>>
>> Do you agree? (I'll wait on solving this before fixing the rest of your concerns
>> in this series). Do you still prefer classes over decorators?
> For formats the Format class plus a dict approach is much nicer than
> decorators. The code is short and easy to understand.
Well, I prefer to automate this kind of things so that new features get
automatically registered and the changes are localized; but it really doesn't
matter that much :)
> For backends it becomes a little tougher and I was wondering whether
> splitting the code into modules would buy us something. The fact that
> you've added '####...' section delimeters shows that tracetool.py is
> growing to long and we're putting too many concepts into one file. If
> each backend is a module then we have a natural way of containing
> backend-specific code. Perhaps the module can register itself when
> tracetool.py wildcard imports them all. I think this will approach
> the level of magic that decorators involve but with the bonus that we
> begin to split the code instead of growing a blob. What do you think
> of putting each backend in its own module?
Sure, the script is getting too large. I just tried to get what I needed with
minimal changes on top of the existing code.
> Do you have a link to your latest code that adds formats/backends?
> I'd like to take a quick look to make sure I understand where you're
> going with this - I've only been thinking of the current set of
> formats/backends.
There's no public repo, sorry. Still, some of "my backends" need registration of
intermediate backends *and* formats (e.g., not available through
--list-backends) that are specific to instrumentation.
Maybe this would work nice for everybody:
tracetool.py # main program (just parse cmdline opts and call tracetool module)
tracetool/__init__.py # common boilerplate code (e.g., event parsing and call dispatching)
tracetool/format/__init__.py # format auto-registration/lookup code
tracetool/format/h.py # code for the 'h' format
# __doc__ [mandatory, format description]
# def begin(events) [optional]
# def end(events) [optional]
tracetool/backend/__init__.py # backend auto-registration/lookup code
tracetool/backend/simple.py # code for the 'simple' backend
# __doc__ [mandatory, backend description]
# PUBLIC = [True|False] [optional,
# defaults to False,
# indicates it's listed on --list-backends]
# def format(events) [optional,
# backend-specific code for given format,
# implicitly indicates format compatibility]
Note that new formats will need to add new format routines in
'tracetool/backend/nop.py' to accomodate whatever action has to be taken on
disabled events.
> As the next step with this patch series we could drop this patch. It
> doesn't make an external difference. Then we can continue to discuss
> how to best handle backends as a separate patch.
WDYT of the organization above? Given the current code it's pretty simple to
split it into different modules. If everybody agrees on the above, I can make it
happen.
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
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 10/12] trace: [tracetool] Automatically establish available backends and formats
2012-03-20 18:45 ` Lluís Vilanova
@ 2012-03-21 9:29 ` Stefan Hajnoczi
2012-03-21 14:10 ` Lluís Vilanova
2012-03-21 19:59 ` Lluís Vilanova
0 siblings, 2 replies; 25+ messages in thread
From: Stefan Hajnoczi @ 2012-03-21 9:29 UTC (permalink / raw)
To: Lluís Vilanova; +Cc: harsh, qemu-devel, aneesh.kumar
2012/3/20 Lluís Vilanova <vilanova@ac.upc.edu>:
> Stefan Hajnoczi writes:
>
>> 2012/3/20 Lluís Vilanova <vilanova@ac.upc.edu>:
>>> Stefan Hajnoczi writes:
>>>
>>>> 2012/3/13 Lluís Vilanova <vilanova@ac.upc.edu>:
>>>>> Adds decorators to establish which backend and/or format each routine is meant
>>>>> to process.
>>>>>
>>>>> With this, tables enumerating format and backend routines can be eliminated and
>>>>> part of the usage message can be computed in a more generic way.
>>>>>
>>>>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>>>>> Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
>>>>> ---
>>>>> Makefile.objs | 6 -
>>>>> Makefile.target | 3
>>>>> scripts/tracetool.py | 320 ++++++++++++++++++++++++++++++++------------------
>>>>> 3 files changed, 211 insertions(+), 118 deletions(-)
>>>
>>>> I find the decorators are overkill and we miss the chance to use more
>>>> straightforward approaches that Python supports. The decorators build
>>>> structures behind the scenes instead of using classes in an open coded
>>>> way. I think this makes it more difficult for people to modify the
>>>> code - they will need to dig in to what exactly the decorators do -
>>>> what they do is pretty similar to what you get from a class.
>>>
>>>> I've tried out an alternative approach which works very nicely for
>>>> formats. For backends it's not a perfect fit because it creates
>>>> instances when we don't really need them, but I think it's still an
>>>> overall cleaner approach:
>>>
>>>> https://github.com/stefanha/qemu/commit/3500eb43f80f3c9200107aa0ca19a1b57300ef8a
>>>
>>>> What do you think?
>>>
>>> I don't like it:
>>>
>>> 1) Format and backend tables must be manually filled.
>>>
>>> 2) The base Backend class has empty methods for each possible format.
>>>
>>> 3) There is no control on format/backend compatibility.
>>>
>>> But I do like the idea of having a single per-backend class having methods for
>>> each possible format.
>>>
>>> The main reason for automatically establishing formats, backends and their
>>> compatibility is that the instrumentation patches add some extra formats and
>>> backends, which makes the process much more tedious if it's not automated.
>>>
>>> Whether you use decorators or classes is not that important.
>>>
>>> Auto-registration can be accomplished using metaclasses and special
>>> per-format/backend "special" attributes the metaclasses will look for (e.g. NAME
>>> to set the commandline-visible name of a format/backend.).
>>>
>>> Format/backend compatibility can be established by introspecting into the
>>> methods on each backend child class, matched against the NAMEs of the registered
>>> formats.
>>>
>>> But going this way does not sound to me like it will be much clearer than
>>> decorators.
>>>
>>> Do you agree? (I'll wait on solving this before fixing the rest of your concerns
>>> in this series). Do you still prefer classes over decorators?
>
>> For formats the Format class plus a dict approach is much nicer than
>> decorators. The code is short and easy to understand.
>
> Well, I prefer to automate this kind of things so that new features get
> automatically registered and the changes are localized; but it really doesn't
> matter that much :)
Formats seem so simple to me that the cost of any infrastructure is
higher than throwing a Format() instance in a dict.
> Maybe this would work nice for everybody:
>
> tracetool.py # main program (just parse cmdline opts and call tracetool module)
> tracetool/__init__.py # common boilerplate code (e.g., event parsing and call dispatching)
> tracetool/format/__init__.py # format auto-registration/lookup code
> tracetool/format/h.py # code for the 'h' format
> # __doc__ [mandatory, format description]
> # def begin(events) [optional]
> # def end(events) [optional]
> tracetool/backend/__init__.py # backend auto-registration/lookup code
> tracetool/backend/simple.py # code for the 'simple' backend
> # __doc__ [mandatory, backend description]
> # PUBLIC = [True|False] [optional,
> # defaults to False,
> # indicates it's listed on --list-backends]
> # def format(events) [optional,
> # backend-specific code for given format,
> # implicitly indicates format compatibility]
Let's call this function backend.generate(events) instead of "format"
since we already use that term and it's a Python builtin too. This is
a code generation function.
>
> Note that new formats will need to add new format routines in
> 'tracetool/backend/nop.py' to accomodate whatever action has to be taken on
> disabled events.
I think it's more convenient to drop the nop backend and introduce a
nop() code generation function for each format.
The .h format would nop() function would emit a static inline empty C
function that does nothing.
The other formats could probably do nothing in nop().
>> As the next step with this patch series we could drop this patch. It
>> doesn't make an external difference. Then we can continue to discuss
>> how to best handle backends as a separate patch.
>
> WDYT of the organization above? Given the current code it's pretty simple to
> split it into different modules. If everybody agrees on the above, I can make it
> happen.
I like the organization you have proposed.
In order to avoid rebasing and porting too many fixes from tracetool
to tracetool.py I suggest you resend this series without the
format/backend consolidation code. I can merge this series quickly
and we can handle the format/backend consolidation code in a separate
patch series.
Stefan
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 10/12] trace: [tracetool] Automatically establish available backends and formats
2012-03-21 9:29 ` Stefan Hajnoczi
@ 2012-03-21 14:10 ` Lluís Vilanova
2012-03-22 8:07 ` Stefan Hajnoczi
2012-03-21 19:59 ` Lluís Vilanova
1 sibling, 1 reply; 25+ messages in thread
From: Lluís Vilanova @ 2012-03-21 14:10 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: harsh, qemu-devel, aneesh.kumar
Stefan Hajnoczi writes:
[...]
>> Maybe this would work nice for everybody:
>>
>> tracetool.py # main program (just parse cmdline opts and call tracetool module)
>> tracetool/__init__.py # common boilerplate code (e.g., event parsing and call dispatching)
>> tracetool/format/__init__.py # format auto-registration/lookup code
>> tracetool/format/h.py # code for the 'h' format
>> # __doc__ [mandatory, format description]
>> # def begin(events) [optional]
>> # def end(events) [optional]
>> tracetool/backend/__init__.py # backend auto-registration/lookup code
>> tracetool/backend/simple.py # code for the 'simple' backend
>> # __doc__ [mandatory, backend description]
>> # PUBLIC = [True|False] [optional,
>> # defaults to False,
>> # indicates it's listed on --list-backends]
>> # def format(events) [optional,
>> # backend-specific code for given format,
>> # implicitly indicates format compatibility]
> Let's call this function backend.generate(events) instead of "format"
> since we already use that term and it's a Python builtin too. This is
> a code generation function.
Maybe I wasn't clear. I meant that for tracetool/backend/simple.py to work with
the format 'h', it must have the following routine:
def h (events):
...
That is, there must exist a routine with the same name of the format (with '-'
replaced with '_', when necessary) for the backend to be compatible with that
format.
>> Note that new formats will need to add new format routines in
>> 'tracetool/backend/nop.py' to accomodate whatever action has to be taken on
>> disabled events.
> I think it's more convenient to drop the nop backend and introduce a
> nop() code generation function for each format.
> The .h format would nop() function would emit a static inline empty C
> function that does nothing.
> The other formats could probably do nothing in nop().
Sounds good:
tracetool/format/f.py:
def begin (events):
...
def nop (events):
...
def end (events):
...
tracetool/__init__.py:
def generate (events, format, backend, force_nop = False):
if force_nop:
events = [ e.properties.add("disabled") for e in events ]
tracetool.format.begin(events)
tracetool.backend.<backend>.<format>(non-disabled events events)
tracetool.format.<format>.nop(disabled events events)
tracetool.format.end(events)
>>> As the next step with this patch series we could drop this patch. It
>>> doesn't make an external difference. Then we can continue to discuss
>>> how to best handle backends as a separate patch.
>>
>> WDYT of the organization above? Given the current code it's pretty simple to
>> split it into different modules. If everybody agrees on the above, I can make it
>> happen.
> I like the organization you have proposed.
> In order to avoid rebasing and porting too many fixes from tracetool
> to tracetool.py I suggest you resend this series without the
> format/backend consolidation code. I can merge this series quickly
> and we can handle the format/backend consolidation code in a separate
> patch series.
Sure, I'll try to do it later today or tomorrow otherwise.
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
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 10/12] trace: [tracetool] Automatically establish available backends and formats
2012-03-21 14:10 ` Lluís Vilanova
@ 2012-03-22 8:07 ` Stefan Hajnoczi
0 siblings, 0 replies; 25+ messages in thread
From: Stefan Hajnoczi @ 2012-03-22 8:07 UTC (permalink / raw)
To: Lluís Vilanova; +Cc: harsh, qemu-devel, aneesh.kumar
On Wed, Mar 21, 2012 at 03:10:49PM +0100, Lluís Vilanova wrote:
> Stefan Hajnoczi writes:
> [...]
> >> Maybe this would work nice for everybody:
> >>
> >> tracetool.py # main program (just parse cmdline opts and call tracetool module)
> >> tracetool/__init__.py # common boilerplate code (e.g., event parsing and call dispatching)
> >> tracetool/format/__init__.py # format auto-registration/lookup code
> >> tracetool/format/h.py # code for the 'h' format
> >> # __doc__ [mandatory, format description]
> >> # def begin(events) [optional]
> >> # def end(events) [optional]
> >> tracetool/backend/__init__.py # backend auto-registration/lookup code
> >> tracetool/backend/simple.py # code for the 'simple' backend
> >> # __doc__ [mandatory, backend description]
> >> # PUBLIC = [True|False] [optional,
> >> # defaults to False,
> >> # indicates it's listed on --list-backends]
> >> # def format(events) [optional,
> >> # backend-specific code for given format,
> >> # implicitly indicates format compatibility]
>
> > Let's call this function backend.generate(events) instead of "format"
> > since we already use that term and it's a Python builtin too. This is
> > a code generation function.
>
> Maybe I wasn't clear. I meant that for tracetool/backend/simple.py to work with
> the format 'h', it must have the following routine:
>
> def h (events):
> ...
>
> That is, there must exist a routine with the same name of the format (with '-'
> replaced with '_', when necessary) for the backend to be compatible with that
> format.
Ah, yes. That makes sense, I misunderstood.
Stefan
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 10/12] trace: [tracetool] Automatically establish available backends and formats
2012-03-21 9:29 ` Stefan Hajnoczi
2012-03-21 14:10 ` Lluís Vilanova
@ 2012-03-21 19:59 ` Lluís Vilanova
2012-03-21 21:22 ` Lluís Vilanova
1 sibling, 1 reply; 25+ messages in thread
From: Lluís Vilanova @ 2012-03-21 19:59 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: harsh, qemu-devel, aneesh.kumar
Stefan Hajnoczi writes:
[...]
> In order to avoid rebasing and porting too many fixes from tracetool
> to tracetool.py I suggest you resend this series without the
> format/backend consolidation code. I can merge this series quickly
> and we can handle the format/backend consolidation code in a separate
> patch series.
Does this mean removing from this series the patch adding the decorators? Or
just re-send all the series after addressing the other concerns?
Thanks,
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 10/12] trace: [tracetool] Automatically establish available backends and formats
2012-03-21 19:59 ` Lluís Vilanova
@ 2012-03-21 21:22 ` Lluís Vilanova
0 siblings, 0 replies; 25+ messages in thread
From: Lluís Vilanova @ 2012-03-21 21:22 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: harsh, qemu-devel, aneesh.kumar
Lluís Vilanova writes:
> Stefan Hajnoczi writes:
> [...]
>> In order to avoid rebasing and porting too many fixes from tracetool
>> to tracetool.py I suggest you resend this series without the
>> format/backend consolidation code. I can merge this series quickly
>> and we can handle the format/backend consolidation code in a separate
>> patch series.
> Does this mean removing from this series the patch adding the decorators? Or
> just re-send all the series after addressing the other concerns?
My bad, never mind.
--
"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
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH 11/12] trace: Provide a per-event status define for conditional compilation
2012-03-13 20:02 [Qemu-devel] [PATCH 00/12] Rewrite tracetool using python Lluís Vilanova
` (9 preceding siblings ...)
2012-03-13 20:03 ` [Qemu-devel] [PATCH 10/12] trace: [tracetool] Automatically establish available backends and formats Lluís Vilanova
@ 2012-03-13 20:03 ` Lluís Vilanova
2012-03-13 20:03 ` [Qemu-devel] [PATCH 12/12] trace: [tracetool] Add error-reporting functions Lluís Vilanova
2012-03-20 9:24 ` [Qemu-devel] [PATCH 00/12] Rewrite tracetool using python Stefan Hajnoczi
12 siblings, 0 replies; 25+ messages in thread
From: Lluís Vilanova @ 2012-03-13 20:03 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, harsh, aneesh.kumar
Revives the functionality introduced by commit b7d66a76, which was temporarily
removed on the tracetool conversion performed during this series.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
---
scripts/tracetool.py | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index 6ef4374..0f76b50 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -123,6 +123,13 @@ def trace_h_begin(events):
@for_format("h", END)
def trace_h_end(events):
+ for e in events:
+ if 'disable' in e.properties:
+ enabled = 0
+ else:
+ enabled = 1
+ print "#define TRACE_%s_ENABLED %d" % (e.name.upper(), enabled)
+ print
print '#endif /* TRACE_H */'
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PATCH 12/12] trace: [tracetool] Add error-reporting functions
2012-03-13 20:02 [Qemu-devel] [PATCH 00/12] Rewrite tracetool using python Lluís Vilanova
` (10 preceding siblings ...)
2012-03-13 20:03 ` [Qemu-devel] [PATCH 11/12] trace: Provide a per-event status define for conditional compilation Lluís Vilanova
@ 2012-03-13 20:03 ` Lluís Vilanova
2012-03-19 17:45 ` Stefan Hajnoczi
2012-03-20 9:24 ` [Qemu-devel] [PATCH 00/12] Rewrite tracetool using python Stefan Hajnoczi
12 siblings, 1 reply; 25+ messages in thread
From: Lluís Vilanova @ 2012-03-13 20:03 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, harsh, aneesh.kumar
Unifies the print+exit sequence into a single 'error' call.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
---
scripts/tracetool.py | 27 +++++++++++++++------------
1 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index 0f76b50..6714466 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -63,6 +63,13 @@ def get_format_descr(format_):
return ""
+def error_write(*lines):
+ sys.stderr.writelines(lines)
+
+def error(*lines):
+ error_write(*lines)
+ sys.exit(1)
+
######################################################################
# backend auto-registration and format compatibility
@@ -514,8 +521,9 @@ def main():
try:
opts, args = getopt.getopt(sys.argv[1:], "", long_options)
except getopt.GetoptError, err:
- # print help information and exit:
- print str(err) # will print something like "option -a not recognized"
+ # print help information and exit
+ # will print something like "option -a not recognized"
+ error_write(str(err)+"\n")
usage()
sys.exit(2)
for opt, arg in opts:
@@ -544,24 +552,19 @@ def main():
usage()
if format_ not in _formats:
- print "Unknown format: %s" % format_
- print
+ error_write("Unknown format: %s\n\n" % format_)
usage()
if backend not in _backends:
- print "Unknown backend: %s" % backend
- print
+ error_write("Unknown backend: %s\n\n" % backend)
usage()
if format_ == 'stap':
if binary == "":
- print '--binary is required for SystemTAP tapset generator'
- sys.exit(1)
+ error("--binary is required for SystemTAP tapset generator\n")
if not probeprefix and not targettype:
- print '--target-type is required for SystemTAP tapset generator'
- sys.exit(1)
+ error("--target-type is required for SystemTAP tapset generator\n")
if not probeprefix and not targetarch:
- print '--target-arch is required for SystemTAP tapset generator'
- sys.exit(1)
+ error("--target-arch is required for SystemTAP tapset generator\n")
if probeprefix == "":
probeprefix = 'qemu.' + targettype + '.' + targetarch
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 12/12] trace: [tracetool] Add error-reporting functions
2012-03-13 20:03 ` [Qemu-devel] [PATCH 12/12] trace: [tracetool] Add error-reporting functions Lluís Vilanova
@ 2012-03-19 17:45 ` Stefan Hajnoczi
0 siblings, 0 replies; 25+ messages in thread
From: Stefan Hajnoczi @ 2012-03-19 17:45 UTC (permalink / raw)
To: Lluís Vilanova; +Cc: harsh, qemu-devel, aneesh.kumar
On Tue, Mar 13, 2012 at 09:03:43PM +0100, Lluís Vilanova wrote:
> @@ -514,8 +521,9 @@ def main():
> try:
> opts, args = getopt.getopt(sys.argv[1:], "", long_options)
> except getopt.GetoptError, err:
> - # print help information and exit:
> - print str(err) # will print something like "option -a not recognized"
> + # print help information and exit
> + # will print something like "option -a not recognized"
> + error_write(str(err)+"\n")
Please use whitespace in expressions:
error_write(str(err) + "\n")
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 00/12] Rewrite tracetool using python
2012-03-13 20:02 [Qemu-devel] [PATCH 00/12] Rewrite tracetool using python Lluís Vilanova
` (11 preceding siblings ...)
2012-03-13 20:03 ` [Qemu-devel] [PATCH 12/12] trace: [tracetool] Add error-reporting functions Lluís Vilanova
@ 2012-03-20 9:24 ` Stefan Hajnoczi
12 siblings, 0 replies; 25+ messages in thread
From: Stefan Hajnoczi @ 2012-03-20 9:24 UTC (permalink / raw)
To: Lluís Vilanova; +Cc: harsh, qemu-devel, aneesh.kumar
2012/3/13 Lluís Vilanova <vilanova@ac.upc.edu>:
> The first patch in the series (by Harsh Prateek) is a rewrite of the tracetool
> shell script in python, which is easier to handle given the current complexity
> of the script.
>
> The following patches (by Lluís Vilanova) add a series of random code cleanups
> and generalizations.
>
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
>
> NOTE: This series contains the first 11 patches from Harsh's v5 series, which
> are the ones required for the language conversion.
>
> Version History:
>
> v6:
> - Rebase on cb72b758 from master.
> - Revive documentation whitespace deletions.
> - Split off this series the patches regarding the new simpletrace trace format.
>
> v5:
> - trace/simple.c: Introduced new struct TraceRecordHeader for log header
> consistency
> - Addressed Stefan's review comments for scripts/simpletrace.py
>
> v4:
> - Addressed Stefan's review comments for tracetool, trace/simple.*
> - Adressed Fast producer, Slow consumer problem
> - Isolated tracetool python conversion patch from simpletrace v2 changes.
> - Included improvements and fixes from Lluis Vilanova
> TODO: Update simpletrace.py for simpletrace v2 log format.
>
> v3:
> - Added support for LTTng ust trace backend in tracetool.py
>
> v2:
> - Updated tracetool.py to support nop, stderr, dtrace backend
>
> v1:
> - Working protoype with tracetool.py converted only for simpletrace backend
>
> Harsh Prateek Bora (1):
> Converting tracetool.sh to tracetool.py
>
> Lluís Vilanova (11):
> trace: [tracetool] Remove unused 'sizestr' attribute in 'Event'
> trace: [tracetool] Do not rebuild event list in backend code
> trace: [tracetool] Simplify event line parsing
> trace: [tracetool] Do not precompute the event number
> trace: [tracetool] Add support for event properties
> trace: [tracetool] Process the "disable" event property
> trace: [tracetool] Rewrite event argument parsing
> trace: [tracetool] Make format-specific code optional and with access to events
> trace: [tracetool] Automatically establish available backends and formats
> trace: Provide a per-event status define for conditional compilation
> trace: [tracetool] Add error-reporting functions
>
>
> Makefile.objs | 6
> Makefile.target | 13 +
> configure | 7 -
> scripts/tracetool | 648 --------------------------------------------------
> scripts/tracetool.py | 588 +++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 602 insertions(+), 660 deletions(-)
> delete mode 100755 scripts/tracetool
> create mode 100755 scripts/tracetool.py
Looks very close. I diffed the tracetool output for all backends and
verified that there is no semantic difference.
The only real point I want to reach agreement with you on is how to
consolidate formats/backends. I left a reply on that patch because I
prefer an alternative to the decorators approach.
Stefan
^ permalink raw reply [flat|nested] 25+ messages in thread