On Fri, May 09, 2014 at 04:38:51PM +0200, Lluís Vilanova wrote:
Nice. A few minor points but overall it looks close.
> @@ -260,13 +260,13 @@ def generate(fevents, format, backend,
> raise TracetoolError("unknown format: %s" % format)
> format = format.replace("-", "_")
>
> - backend = str(backend)
> - if len(backend) is 0:
> - raise TracetoolError("backend not set")
> - if not tracetool.backend.exists(backend):
> - raise TracetoolError("unknown backend: %s" % backend)
> - backend = backend.replace("-", "_")
> - backend = tracetool.backend.Wrapper(backend, format)
> + if len(backends) is 0:
> + raise TracetoolError("no backends specified")
> + for backend in backends:
> + if not tracetool.backend.exists(backend):
> + raise TracetoolError("unknown backend: %s" % backend)
> + backends = [backend.replace("-", "_") for backend in backends]
This seems to be duplicated down...
> + backend = tracetool.backend.Wrapper(backends, format)
>
> import tracetool.backend.dtrace
> tracetool.backend.dtrace.BINARY = binary
> diff --git a/scripts/tracetool/backend/__init__.py
> b/scripts/tracetool/backend/__init__.py
> index 5e36f04..5bfa1ef 100644
> --- a/scripts/tracetool/backend/__init__.py
> +++ b/scripts/tracetool/backend/__init__.py
> @@ -99,17 +99,18 @@ def exists(name):
>
>
> class Wrapper:
> - def __init__(self, backend, format):
> - self._backend = backend.replace("-", "_")
> + def __init__(self, backends, format):
> + self._backends = [backend.replace("-", "_") for backend in backends]
...here.
> @@ -130,3 +149,29 @@ void trace_backend_init_events(const char *fname)
> exit(1);
> }
> }
> +
> +bool trace_init_backends(const char *events, const char *file)
> +{
> +#ifdef CONFIG_TRACE_SIMPLE
> + if (!st_init(file)) {
> + fprintf(stderr, "failed to initialize simple tracing
> backend.\n");
> + return false;
> + }
> +#else
> + if (file) {
> + fprintf(stderr, "error: -trace file=...: "
> + "option not supported by the selected tracing backends\n");
> + return false;
> + }
> +#endif
> +
> +#ifdef CONFIG_TRACE_FTRACE
> + if (!ftrace_init()) {
> + fprintf(stderr, "failed to initialize ftrace backend.\n");
> + return false;
> + }
> +#endif
Please use scripts/checkpatch.pl to scan patches for coding style
issues. QEMU uses 4-space indentation.
> diff --git a/trace/ftrace.h b/trace/ftrace.h
> index 94cb8d5..ad18ccb 100644
> --- a/trace/ftrace.h
> +++ b/trace/ftrace.h
> @@ -1,10 +1,15 @@
> #ifndef TRACE_FTRACE_H
> #define TRACE_FTRACE_H
>
> +#include <stdint.h>
Why do you need this include?
> +
> +
> #define MAX_TRACE_STRLEN 512
> #define _STR(x) #x
> #define STR(x) _STR(x)
>
> extern int trace_marker_fd;
>
> +bool ftrace_init(void);
Missing #include <stdbool.h> for the bool type.
