Re: [Qemu-devel] [PATCH 1/5] trace: Add trace-events file for declaring trace events

[Anthony Liguori <aliguori@linux.vnet.ibm.com>]

On 05/22/2010 04:08 PM, Stefan Hajnoczi wrote:
> This patch introduces the trace-events file where trace events can be
> declared like so:
>
> qemu_malloc(size_t size) "size %zu"
> qemu_free(void *ptr) "ptr %p"
>
> These trace event declarations are processed by a new tool called
> tracetool to generate code for the trace events.  Trace event
> declarations are independent of the backend tracing system (LTTng User
> Space Tracing, kernel markers, DTrace).
>
> The default "nop" backend generates empty trace event functions.
> Therefore trace events are disabled by default.
>
> The trace-events file serves to purposes:
>
> 1. Adding trace events is easy.  It is not necessary to understand the
>     details of a backend tracing system.  The trace-events file is a
>     single location where trace events can be declared without code
>     duplication.
>
> 2. QEMU is not tightly coupled to one particular backend tracing system.
>     In order to support tracing across QEMU host platforms and to
>     anticipate new backend tracing systems that are currently maturing,
>     it is important to be flexible and not tied to one system.
>
> Signed-off-by: Stefan Hajnoczi<stefanha@linux.vnet.ibm.com>
> ---
>   .gitignore      |    2 +
>   Makefile        |   17 +++++--
>   Makefile.objs   |    5 ++
>   Makefile.target |    1 +
>   configure       |   19 +++++++
>   trace-events    |   24 ++++++++
>   tracetool       |  162 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   7 files changed, 226 insertions(+), 4 deletions(-)
>   create mode 100644 trace-events
>   create mode 100755 tracetool
>
> diff --git a/.gitignore b/.gitignore
> index fdfe2f0..4644557 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -2,6 +2,8 @@ config-devices.*
>   config-all-devices.*
>   config-host.*
>   config-target.*
> +trace.h
> +trace.c
>   *-softmmu
>   *-darwin-user
>   *-linux-user
> diff --git a/Makefile b/Makefile
> index 306a1a4..ff57845 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1,6 +1,6 @@
>   # Makefile for QEMU.
>
> -GENERATED_HEADERS = config-host.h
> +GENERATED_HEADERS = config-host.h trace.h
>
>   ifneq ($(wildcard config-host.mak),)
>   # Put the all: rule here so that config-host.mak can contain dependencies.
> @@ -130,16 +130,24 @@ bt-host.o: QEMU_CFLAGS += $(BLUEZ_CFLAGS)
>
>   iov.o: iov.c iov.h
>
> +trace.h: trace-events
> +	$(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -h<  $<  >  $@,"  GEN   $@")
> +
> +trace.c: trace-events
> +	$(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -c<  $<  >  $@,"  GEN   $@")
> +
> +trace.o: trace.c
> +
>   ######################################################################
>
>   qemu-img.o: qemu-img-cmds.h
>   qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o: $(GENERATED_HEADERS)
>
> -qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-error.o $(block-obj-y) $(qobject-obj-y)
> +qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-error.o $(trace-obj-y) $(block-obj-y) $(qobject-obj-y)
>
> -qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-error.o $(block-obj-y) $(qobject-obj-y)
> +qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-error.o $(trace-obj-y) $(block-obj-y) $(qobject-obj-y)
>
> -qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o $(block-obj-y) $(qobject-obj-y)
> +qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o $(trace-obj-y) $(block-obj-y) $(qobject-obj-y)
>
>   qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
>   	$(call quiet-command,sh $(SRC_PATH)/hxtool -h<  $<  >  $@,"  GEN   $@")
> @@ -157,6 +165,7 @@ clean:
>   	rm -f *.o *.d *.a $(TOOLS) TAGS cscope.* *.pod *~ */*~
>   	rm -f slirp/*.o slirp/*.d audio/*.o audio/*.d block/*.o block/*.d net/*.o net/*.d
>   	rm -f qemu-img-cmds.h
> +	rm -f trace.c trace.h
>   	$(MAKE) -C tests clean
>   	for d in $(ALL_SUBDIRS) libhw32 libhw64 libuser libdis libdis-user; do \
>   	if test -d $$d; then $(MAKE) -C $$d $@ || exit 1; fi; \
> diff --git a/Makefile.objs b/Makefile.objs
> index acbaf22..9bbdf6f 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -248,6 +248,11 @@ libdis-$(CONFIG_S390_DIS) += s390-dis.o
>   libdis-$(CONFIG_SH4_DIS) += sh4-dis.o
>   libdis-$(CONFIG_SPARC_DIS) += sparc-dis.o
>
> +######################################################################
> +# trace
> +
> +trace-obj-y = trace.o
> +
>   vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
>
>   vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS)
> diff --git a/Makefile.target b/Makefile.target
> index a22484e..4e63c02 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -294,6 +294,7 @@ $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y): $(GENERATED_HEADERS)
>
>   obj-y += $(addprefix ../, $(common-obj-y))
>   obj-y += $(addprefix ../libdis/, $(libdis-y))
> +obj-y += $(addprefix ../, $(trace-obj-y))
>   obj-y += $(libobj-y)
>   obj-y += $(addprefix $(HWDIR)/, $(hw-obj-y))
>
> diff --git a/configure b/configure
> index 3cd2c5f..5e66f3a 100755
> --- a/configure
> +++ b/configure
> @@ -299,6 +299,7 @@ pkgversion=""
>   check_utests="no"
>   user_pie="no"
>   zero_malloc=""
> +trace_backend="nop"
>
>   # OS specific
>   if check_define __linux__ ; then
> @@ -494,6 +495,8 @@ for opt do
>     ;;
>     --target-list=*) target_list="$optarg"
>     ;;
> +  --trace-backend=*) trace_backend="$optarg"
> +  ;;
>     --enable-gprof) gprof="yes"
>     ;;
>     --static)
> @@ -826,6 +829,7 @@ echo "  --enable-docs            enable documentation build"
>   echo "  --disable-docs           disable documentation build"
>   echo "  --disable-vhost-net      disable vhost-net acceleration support"
>   echo "  --enable-vhost-net       enable vhost-net acceleration support"
> +echo "  --trace-backend=B        Trace backend nop"
>   echo ""
>   echo "NOTE: The object files are built at the place where configure is launched"
>   exit 1
> @@ -1914,6 +1918,18 @@ if compile_prog "" "" ; then
>       fdatasync=yes
>   fi
>
> +##########################################
> +# check if trace backend exists
> +
> +sh tracetool "--$trace_backend" --check-backend>  /dev/null 2>  /dev/null
>    

This will fail if objdir != srcdir.  You have to qualify tracetool with 
the path to srcdir.

Regards,

Anthony Liguori

> +if test "$?" -ne 0 ; then
> +  echo
> +  echo "Error: invalid trace backend"
> +  echo "Please choose a supported trace backend."
> +  echo
> +  exit 1
> +fi
> +
>   # End of CC checks
>   # After here, no more $cc or $ld runs
>
> @@ -2041,6 +2057,7 @@ echo "preadv support    $preadv"
>   echo "fdatasync         $fdatasync"
>   echo "uuid support      $uuid"
>   echo "vhost-net support $vhost_net"
> +echo "Trace backend     $trace_backend"
>
>   if test $sdl_too_old = "yes"; then
>   echo "->  Your SDL version is too old - please upgrade to have SDL support"
> @@ -2284,6 +2301,8 @@ bsd)
>   ;;
>   esac
>
> +echo "TRACE_BACKEND=$trace_backend">>  $config_host_mak
> +
>   tools=
>   if test `expr "$target_list" : ".*softmmu.*"` != 0 ; then
>     tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) $tools"
> diff --git a/trace-events b/trace-events
> new file mode 100644
> index 0000000..a37d3cc
> --- /dev/null
> +++ b/trace-events
> @@ -0,0 +1,24 @@
> +# Trace events for debugging and performance instrumentation
> +#
> +# This file is processed by the tracetool script during the build.
> +#
> +# To add a new trace event:
> +#
> +# 1. Choose a name for the trace event.  Declare its arguments and format
> +#    string.
> +#
> +# 2. Call the trace event from code using trace_##name, e.g. multiwrite_cb() ->
> +#    trace_multiwrite_cb().  The source file must #include "trace.h".
> +#
> +# Format of a trace event:
> +#
> +#<name>(<type1>  <arg1>[,<type2>  <arg2>] ...) "<format-string>"
> +#
> +# Example: qemu_malloc(size_t size) "size %zu"
> +#
> +# The<name>  must be a valid as a C function name.
> +#
> +# Types should be standard C types.  Use void * for pointers because the trace
> +# system may not have the necessary headers included.
> +#
> +# The<format-string>  should be a sprintf()-compatible format string.
> diff --git a/tracetool b/tracetool
> new file mode 100755
> index 0000000..e243d42
> --- /dev/null
> +++ b/tracetool
> @@ -0,0 +1,162 @@
> +#!/bin/sh
> +
> +usage()
> +{
> +    cat>&2<<EOF
> +usage: $0 --nop [-h | -c]
> +Generate tracing code for a file on stdin.
> +
> +Backends:
> +  --nop Tracing disabled
> +
> +Output formats:
> +  -h    Generate .h file
> +  -c    Generate .c file
> +EOF
> +    exit 1
> +}
> +
> +# Get the name of a trace event
> +get_name()
> +{
> +    echo ${1%%(*}
> +}
> +
> +# Get the argument list of a trace event, including types and names
> +get_args()
> +{
> +    local args
> +    args=${1#*(}
> +    args=${args%)*}
> +    echo "$args"
> +}
> +
> +# Get the argument name list of a trace event
> +get_argnames()
> +{
> +    local first field name
> +    for field in $(get_args "$1"); do
> +        # Drop pointer star
> +        field=${field#\*}
> +
> +        # Only argument names have commas at the end
> +        name=${field%,}
> +        test "$field" = "$name"&&  continue
> +
> +        echo -n "$name, "
> +    done
> +    echo -n "$name"
> +}
> +
> +# Get the format string for a trace event
> +get_fmt()
> +{
> +    local fmt
> +    fmt=${1#*\"}
> +    fmt=${fmt%\"*}
> +    echo "$fmt"
> +}
> +
> +warn_autogen_c()
> +{
> +    echo "/* This file is autogenerated by tracetool, do not edit. */"
> +}
> +
> +warn_autogen_h()
> +{
> +    echo "/* This file is autogenerated by tracetool, do not edit. */"
> +}
> +
> +linetoh_begin_nop()
> +{
> +    return
> +}
> +
> +linetoh_nop()
> +{
> +    local name args
> +    name=$(get_name "$1")
> +    args=$(get_args "$1")
> +
> +    # Define an empty function for the trace event
> +    cat<<EOF
> +static inline void trace_$name($args)
> +{
> +}
> +EOF
> +}
> +
> +linetoh_end_nop()
> +{
> +    return
> +}
> +
> +linetoc_begin_nop()
> +{
> +    return
> +}
> +
> +linetoc_nop()
> +{
> +    # No need for function definitions in nop backend
> +    return
> +}
> +
> +linetoc_end_nop()
> +{
> +    return
> +}
> +
> +# Process stdin by calling begin, line, and end functions for the backend
> +convert()
> +{
> +    local begin process_line end
> +    begin="lineto$1_begin_$backend"
> +    process_line="lineto$1_$backend"
> +    end="lineto$1_end_$backend"
> +
> +    warn_autogen_$1
> +    "$begin"
> +
> +    while read -r str; do
> +        # Skip comments and empty lines
> +        str=${str%%#*}
> +        test -z "$str"&&  continue
> +
> +        echo
> +        "$process_line" "$str"
> +    done
> +
> +    echo
> +    "$end"
> +}
> +
> +tracetoh()
> +{
> +    echo "#ifndef TRACE_H"
> +    echo "#define TRACE_H"
> +    echo
> +    convert h
> +    echo "#endif /* TRACE_H */"
> +}
> +
> +tracetoc()
> +{
> +    convert c
> +}
> +
> +# Choose backend
> +case "$1" in
> +"--nop") backend="${1#--}" ;;
> +*) usage ;;
> +esac
> +shift
> +
> +case "$1" in
> +"-h") tracetoh ;;
> +"-c") tracetoc ;;
> +"--check-backend") exit 0 ;; # used by ./configure to test for backend
> +*) usage ;;
> +esac
> +
> +exit 0
>