Re: [Qemu-devel] [PATCH] trace: [qmp] Add QAPI/QMP commands to query and control event tracing state

[Lluís Vilanova]

Lluís Vilanova writes:

> Also removes old "trace-event", "trace-file" and "info trace-events" HMP
> commands.

Please ignore, there's a silly bug.


> Signed-off-by: Lluís Vilanova <address@hidden>
> ---
>  hmp-commands.hx     |   35 -----------------------
>  monitor.c           |   61 ----------------------------------------
>  qapi-schema.json    |    3 ++
>  qmp-commands.hx     |   27 ++++++++++++++++++
>  trace/Makefile.objs |    1 +
>  trace/commands.json |   44 +++++++++++++++++++++++++++++
>  trace/control.c     |   13 ---------
>  trace/control.h     |    7 -----
>  trace/qmp.c         |   77 
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  trace/simple.c      |   10 +------
>  trace/simple.h      |    3 --
>  11 files changed, 154 insertions(+), 127 deletions(-)
>  create mode 100644 trace/commands.json
>  create mode 100644 trace/qmp.c

> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index d0943b1..a0d8999 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -269,36 +269,6 @@ Output logs to @var{filename}.
>  ETEXI
 
>      {
> -        .name       = "trace-event",
> -        .args_type  = "name:s,option:b",
> -        .params     = "name on|off",
> -        .help       = "changes status of a specific trace event",
> -        .mhandler.cmd = do_trace_event_set_state,
> -    },
> -
> -STEXI
> address@hidden trace-event
> address@hidden trace-event
> -changes status of a trace event
> -ETEXI
> -
> -#if defined(CONFIG_TRACE_SIMPLE)
> -    {
> -        .name       = "trace-file",
> -        .args_type  = "op:s?,arg:F?",
> -        .params     = "on|off|flush|set [arg]",
> -        .help       = "open, close, or flush trace file, or set a new file 
> name",
> -        .mhandler.cmd = do_trace_file,
> -    },
> -
> -STEXI
> address@hidden trace-file on|off|flush
> address@hidden trace-file
> -Open, close, or flush the trace file.  If no argument is given, the status 
> of the trace file is displayed.
> -ETEXI
> -#endif
> -
> -    {
>          .name       = "log",
>          .args_type  = "items:s",
>          .params     = "item1[,...]",
> @@ -1784,10 +1754,5 @@ show the TPM device
>  ETEXI
 
>  STEXI
> address@hidden info trace-events
> -show available trace events and their state
> -ETEXI
> -
> -STEXI
>  @end table
>  ETEXI
> diff --git a/monitor.c b/monitor.c
> index cdbaa60..d856d84 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -61,10 +61,6 @@
>  #include "qemu/osdep.h"
>  #include "cpu.h"
>  #include "trace.h"
> -#include "trace/control.h"
> -#ifdef CONFIG_TRACE_SIMPLE
> -#include "trace/simple.h"
> -#endif
>  #include "exec/memory.h"
>  #include "exec/cpu_ldst.h"
>  #include "qmp-commands.h"
> @@ -882,51 +878,6 @@ static void do_help_cmd(Monitor *mon, const QDict *qdict)
>      help_cmd(mon, qdict_get_try_str(qdict, "name"));
>  }
 
> -static void do_trace_event_set_state(Monitor *mon, const QDict *qdict)
> -{
> -    const char *tp_name = qdict_get_str(qdict, "name");
> -    bool new_state = qdict_get_bool(qdict, "option");
> -
> -    bool found = false;
> -    TraceEvent *ev = NULL;
> -    while ((ev = trace_event_pattern(tp_name, ev)) != NULL) {
> -        found = true;
> -        if (!trace_event_get_state_static(ev)) {
> -            monitor_printf(mon, "event \"%s\" is not traceable\n", tp_name);
> -        } else {
> -            trace_event_set_state_dynamic(ev, new_state);
> -        }
> -    }
> -    if (!trace_event_is_pattern(tp_name) && !found) {
> -        monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
> -    }
> -}
> -
> -#ifdef CONFIG_TRACE_SIMPLE
> -static void do_trace_file(Monitor *mon, const QDict *qdict)
> -{
> -    const char *op = qdict_get_try_str(qdict, "op");
> -    const char *arg = qdict_get_try_str(qdict, "arg");
> -
> -    if (!op) {
> -        st_print_trace_file_status((FILE *)mon, &monitor_fprintf);
> -    } else if (!strcmp(op, "on")) {
> -        st_set_trace_file_enabled(true);
> -    } else if (!strcmp(op, "off")) {
> -        st_set_trace_file_enabled(false);
> -    } else if (!strcmp(op, "flush")) {
> -        st_flush_trace_buffer();
> -    } else if (!strcmp(op, "set")) {
> -        if (arg) {
> -            st_set_trace_file(arg);
> -        }
> -    } else {
> -        monitor_printf(mon, "unexpected argument \"%s\"\n", op);
> -        help_cmd(mon, "trace-file");
> -    }
> -}
> -#endif
> -
>  static void user_monitor_complete(void *opaque, QObject *ret_data)
>  {
>      MonitorCompletionData *data = (MonitorCompletionData *)opaque; 
> @@ -1077,11 +1028,6 @@ static void do_info_cpu_stats(Monitor *mon, const 
> QDict *qdict)
>      cpu_dump_statistics(cpu, (FILE *)mon, &monitor_fprintf, 0);
>  }
 
> -static void do_trace_print_events(Monitor *mon, const QDict *qdict)
> -{
> -    trace_print_events((FILE *)mon, &monitor_fprintf);
> -}
> -
>  static int client_migrate_info(Monitor *mon, const QDict *qdict,
>                                 MonitorCompletion cb, void *opaque)
>  {
> @@ -2898,13 +2844,6 @@ static mon_cmd_t info_cmds[] = {
>          .mhandler.cmd = do_info_roms,
>      },
>      {
> -        .name       = "trace-events",
> -        .args_type  = "",
> -        .params     = "",
> -        .help       = "show available trace-events & their state",
> -        .mhandler.cmd = do_trace_print_events,
> -    },
> -    {
>          .name       = "tpm",
>          .args_type  = "",
>          .params     = "",
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 341f417..0b90f60 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -11,6 +11,9 @@
>  # QAPI event definitions
>  { 'include': 'qapi/event.json' }
 
> +# Tracing commands
> +{ 'include': 'trace/commands.json' }
> +
>  ##
>  # LostTickPolicy:
>  #
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 4be4765..443dd16 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -3753,5 +3753,32 @@ Example:
 
-> { "execute": "rtc-reset-reinjection" }
>  <- { "return": {} }
> +EQMP
> +
> +    {
> +        .name       = "trace-event-get-state",
> +        .args_type  = "name:s",
> +        .mhandler.cmd_new = qmp_marshal_input_trace_event_get_state,
> +    },
> +
> +SQMP
> +trace-event-get-state
> +---------------------
> +
> +Query the state of events.
> +
> +EQMP
> +
> +    {
> +        .name       = "trace-event-set-state",
> +        .args_type  = "name:s,state:b,keepgoing:b?",
> +        .mhandler.cmd_new = qmp_marshal_input_trace_event_set_state,
> +    },
> +
> +SQMP
> +trace-event-set-state
> +---------------------
> +
> +Set the state of events.
 
>  EQMP
> diff --git a/trace/Makefile.objs b/trace/Makefile.objs
> index 387f191..01b3718 100644
> --- a/trace/Makefile.objs
> +++ b/trace/Makefile.objs
> @@ -145,3 +145,4 @@ util-obj-$(CONFIG_TRACE_FTRACE) += ftrace.o
>  util-obj-$(CONFIG_TRACE_UST) += generated-ust.o
>  util-obj-y += control.o
>  util-obj-y += generated-tracers.o
> +util-obj-y += qmp.o
> diff --git a/trace/commands.json b/trace/commands.json
> new file mode 100644
> index 0000000..6e6313d
> --- /dev/null
> +++ b/trace/commands.json
> @@ -0,0 +1,44 @@
> +# -*- mode: python -*-
> +
> +##
> +# @TraceEventState:
> +#
> +# State of a tracing event.
> +#
> +# @name: Event name.
> +# @sstatic: Static tracing state.
> +# @sdynamic: Dynamic tracing state.
> +#
> +# Since 2.2
> +##
> +{ 'type': 'TraceEventState',
> +  'data': {'name': 'str', 'sstatic': 'bool', 'sdynamic': 'bool'} }
> +
> +##
> +# @trace-event-get-state:
> +#
> +# Query the state of events.
> +#
> +# @name: Event name pattern.
> +#
> +# Returns: @TraceEventState of the matched events
> +#
> +# Since 2.2
> +##
> +{ 'command': 'trace-event-get-state',
> +  'data': {'name': 'str'},
> +  'returns': ['TraceEventState'] }
> +
> +##
> +# @trace-event-set-state:
> +#
> +# Set the dynamic state of events.
> +#
> +# @name: Event name pattern.
> +# @state: Dynamic tracing state.
> +# @keepgoing: #optional Apply changes even if not all events can be changed.
> +#
> +# Since 2.2
> +##
> +{ 'command': 'trace-event-set-state',
> +  'data': {'name': 'str', 'state': 'bool', '*keepgoing': 'bool'} }
> diff --git a/trace/control.c b/trace/control.c
> index 9631a40..0d30801 100644
> --- a/trace/control.c
> +++ b/trace/control.c
> @@ -85,19 +85,6 @@ TraceEvent *trace_event_pattern(const char *pat, 
> TraceEvent *ev)
>      return NULL;
>  }
 
> -void trace_print_events(FILE *stream, fprintf_function stream_printf)
> -{
> -    TraceEventID i;
> -
> -    for (i = 0; i < trace_event_count(); i++) {
> -        TraceEvent *ev = trace_event_id(i);
> -        stream_printf(stream, "%s [Event ID %u] : state %u\n",
> -                      trace_event_get_name(ev), i,
> -                      trace_event_get_state_static(ev) &&
> -                      trace_event_get_state_dynamic(ev));
> -    }
> -}
> -
>  static void trace_init_events(const char *fname)
>  {
>      Location loc;
> diff --git a/trace/control.h b/trace/control.h
> index e1ec033..da9bb6b 100644
> --- a/trace/control.h
> +++ b/trace/control.h
> @@ -149,13 +149,6 @@ static void trace_event_set_state_dynamic(TraceEvent 
> *ev, bool state);
 
 
>  /**
> - * trace_print_events:
> - *
> - * Print the state of all events.
> - */
> -void trace_print_events(FILE *stream, fprintf_function stream_printf);
> -
> -/**
>   * trace_init_backends:
>   * @events: Name of file with events to be enabled at startup; may be NULL.
>   *          Corresponds to commandline option "-trace events=...".
> diff --git a/trace/qmp.c b/trace/qmp.c
> new file mode 100644
> index 0000000..b142c55
> --- /dev/null
> +++ b/trace/qmp.c
> @@ -0,0 +1,77 @@
> +/*
> + * QMP commands for tracing events.
> + *
> + * Copyright (C) 2014 Lluís Vilanova <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/typedefs.h"
> +#include "qmp-commands.h"
> +#include "trace/control.h"
> +
> +
> +TraceEventStateList *qmp_trace_event_get_state(const char *name, Error 
> **errp)
> +{
> +    TraceEventStateList dummy = {};
> +    TraceEventStateList *prev = &dummy;
> +
> +    bool found = false;
> +    TraceEvent *ev = NULL;
> +    while ((ev = trace_event_pattern(name, ev)) != NULL) {
> +        found = true;
> +        TraceEventStateList *elem = g_malloc0(sizeof(*elem));
> +        elem->value = g_malloc0(sizeof(*elem->value));
> +        elem->value->name = g_strdup(trace_event_get_name(ev));
> +        elem->value->sstatic = trace_event_get_state_static(ev);
> +        elem->value->sdynamic = trace_event_get_state_dynamic(ev);
> +        prev->next = elem;
> +        prev = elem;
> +    }
> +    if (!trace_event_is_pattern(name) && !found) {
> +        error_setg(errp, "unknown event name \"%s\"\n", name);
> +    }
> +
> +    return dummy.next;
> +}
> +
> +void qmp_trace_event_set_state(const char *name, bool state, bool 
> has_keepgoing,
> +                               bool keepgoing, Error **errp)
> +{
> +    bool error = false;
> +    bool found = false;
> +    TraceEvent *ev = NULL;
> +
> +    /* Check all selected events are dynamic */
> +    while ((ev = trace_event_pattern(name, ev)) != NULL) {
> +        found = true;
> +        if (!trace_event_get_state_static(ev)) {
> +            error_setg(errp, "cannot set dynamic tracing state for \"%s\"\n",
> +                       trace_event_get_name(ev));
> +            if (!(has_keepgoing && keepgoing)) {
> +                error = true;
> +            }
> +            break;
> +        }
> +    }
> +    if (error) {
> +        return;
> +    }
> +    if (!trace_event_is_pattern(name) && !found) {
> +        error_setg(errp, "unknown event name \"%s\"\n", name);
> +        return;
> +    }
> +
> +    if (error) {
> +        return;
> +    }
> +
> +    /* Apply changes */
> +    ev = NULL;
> +    while ((ev = trace_event_pattern(name, ev)) != NULL) {
> +        if (trace_event_get_state_static(ev)) {
> +            trace_event_set_state_dynamic(ev, state);
> +        }
> +    }
> +}
> diff --git a/trace/simple.c b/trace/simple.c
> index 11ad030..7a2e44d 100644
> --- a/trace/simple.c
> +++ b/trace/simple.c
> @@ -277,7 +277,7 @@ void trace_record_finish(TraceBufferRecord *rec)
>      }
>  }
 
> -void st_set_trace_file_enabled(bool enable)
> +static void st_set_trace_file_enabled(bool enable)
>  {
>      if (enable == !!trace_fp) {
>          return; /* no change */
> @@ -322,7 +322,7 @@ void st_set_trace_file_enabled(bool enable)
>   * @file        The trace file name or NULL for the default name-<pid> set at
>   *              config time
>   */
> -bool st_set_trace_file(const char *file)
> +static bool st_set_trace_file(const char *file)
>  {
>      st_set_trace_file_enabled(false);
 
> @@ -338,12 +338,6 @@ bool st_set_trace_file(const char *file)
>      return true;
>  }
 
> -void st_print_trace_file_status(FILE *stream, int (*stream_printf)(FILE 
> *stream, const char *fmt, ...))
> -{
> -    stream_printf(stream, "Trace file \"%s\" %s.\n",
> -                  trace_file_name, trace_fp ? "on" : "off");
> -}
> -
>  void st_flush_trace_buffer(void)
>  {
>      flush_trace_file(true);
> diff --git a/trace/simple.h b/trace/simple.h
> index 6997996..316a479 100644
> --- a/trace/simple.h
> +++ b/trace/simple.h
> @@ -18,9 +18,6 @@
>  #include "trace/generated-events.h"
 
 
> -void st_print_trace_file_status(FILE *stream, fprintf_function 
> stream_printf);
> -void st_set_trace_file_enabled(bool enable);
> -bool st_set_trace_file(const char *file);
>  bool st_init(const char *file);
>  void st_flush_trace_buffer(void);
 



-- 
 "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