Re: [Qemu-devel] [PATCH v5 2/5] Modified the tracetool framework for LTTng 2.x

[Stefan Hajnoczi]

On Thu, Oct 24, 2013 at 08:46:04PM -0400, Mohamad Gebai wrote:
> * A new format is required to generate definitions for ust tracepoints.
>   Files ust_events_h.py and ust_events_c.py define common macros, while
>   new function ust_events_h in events.py does the actual definition of
>   each tracepoint.
> * ust.py generates the new interface for calling userspace tracepoints
>   with LTTng 2.x, replacing trace_name(args) to tracepoint(name, args).
> * As explained in ust_events_c.py, -Wredundant-decls gives a warning
>   when compiling with gcc 4.7 or older. This is specific to lttng-ust so
>   for now use a pragma clause to avoid getting a warning.
> 
> Signed-off-by: Mohamad Gebai <address@hidden>
> Reviewed-by: Alex Bennée <address@hidden>
> ---
>  scripts/tracetool/backend/events.py      | 44 +++++++++++++++++
>  scripts/tracetool/backend/ust.py         | 82 
> +++++++-------------------------
>  scripts/tracetool/format/ust_events_c.py | 30 ++++++++++++
>  scripts/tracetool/format/ust_events_h.py | 57 ++++++++++++++++++++++
>  4 files changed, 147 insertions(+), 66 deletions(-)
>  create mode 100644 scripts/tracetool/format/ust_events_c.py
>  create mode 100644 scripts/tracetool/format/ust_events_h.py

I tried to understand why the extra ust .c and .h are needed.  It seems
they are really part of how LTTng 2.0+ works by #including your header
back from its header (?!).  So I'm okay with this.

> +def ust_events_h(events):
> +    for e in events:
> +        if len(e.args) > 0:
> +            out('TRACEPOINT_EVENT(',
> +                '   qemu,',
> +                '   %(name)s,',
> +                '   TP_ARGS(%(args)s),',
> +                '   TP_FIELDS(',
> +                name = e.name,
> +                args = ", ".join(", ".join(i) for i in e.args),
> +                )
> +
> +            for t,n in e.args:
> +                if ('int' in t) or ('long' in t) or ('unsigned' in t) or 
> ('size_t' in t):
> +                    out('       ctf_integer(' + t + ', ' + n + ', ' + n + 
> ')')
> +                elif ('double' in t) or ('float' in t):
> +                    out('       ctf_float(' + t + ', ' + n + ', ' + n + ')')
> +                elif ('char *' in t) or ('char*' in t):
> +                    out('       ctf_string(' + n + ', ' + n + ')')
> +                elif ('void *' in t) or ('void*' in t):
> +                    out('       ctf_integer_hex(unsigned long, ' + n + ', ' 
> + n + ')')
> +
> +            out('   )',
> +                ')',
> +                '')
> +
> +        else:
> +            out('TRACEPOINT_EVENT(',
> +                '   qemu,',
> +                '   %(name)s,',
> +                '   TP_ARGS(void),',
> +                '   TP_FIELDS()',
> +                ')',
> +                '',
> +                name = e.name,
> +                )

This file is generic, it should not contain UST backend-specific code.
Let the ust.py backend override ust_events_h() so the code can live
there, and just leave a nop ust_events_h() in events.py.

> +            argnames = ", " + argnames
> +
> +        out('static inline void trace_%(name)s(%(args)s)',
> +            '{',
> +            '    tracepoint(qemu, %(name)s%(tp_args)s);'
> +            '',

Any reason for this empty string?

> diff --git a/scripts/tracetool/format/ust_events_c.py 
> b/scripts/tracetool/format/ust_events_c.py
> new file mode 100644
> index 0000000..71328f3
> --- /dev/null
> +++ b/scripts/tracetool/format/ust_events_c.py
> @@ -0,0 +1,30 @@
> +#!/usr/bin/env python
> +# -*- coding: utf-8 -*-
> +
> +"""
> +Generate .c for LTTng ust event description.
> +"""
> +
> +__author__     = "Lluís Vilanova <address@hidden>"
> +__copyright__  = "Copyright 2012, Lluís Vilanova <address@hidden>"

Should you be the author?

> +__license__    = "GPL version 2 or (at your option) any later version"
> +
> +__maintainer__ = "Stefan Hajnoczi"
> +__email__      = "address@hidden"

My email address is address@hidden now.

> +__author__     = "Lluís Vilanova <address@hidden>"
> +__copyright__  = "Copyright 2012, Lluís Vilanova <address@hidden>"
> +__license__    = "GPL version 2 or (at your option) any later version"
> +
> +__maintainer__ = "Stefan Hajnoczi"
> +__email__      = "address@hidden"

Same here.