Re: [Qemu-devel] [PATCH v7 7/7] trace: Add QAPI/QMP interfaces to query and control per-vCPU tracing state

[Markus Armbruster]

Lluís Vilanova <address@hidden> writes:

> Signed-off-by: Lluís Vilanova <address@hidden>
> Reviewed-by: Stefan Hajnoczi <address@hidden>
> ---
>  hmp-commands-info.hx |    6 +-
>  hmp-commands.hx      |    7 +-
>  monitor.c            |   17 +++++-
>  qapi/trace.json      |   32 +++++++++--
>  qmp-commands.hx      |   35 +++++++++++-
>  trace/qmp.c          |  148 
> ++++++++++++++++++++++++++++++++++++++++----------
>  6 files changed, 202 insertions(+), 43 deletions(-)
>
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 3d07ca6..74446c6 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -646,10 +646,10 @@ ETEXI
>  
>      {
>          .name       = "trace-events",
> -        .args_type  = "name:s?",
> -        .params     = "[name]",
> +        .args_type  = "name:s?,vcpu:i?",
> +        .params     = "[name] [vcpu]",
>          .help       = "show available trace-events & their state "
> -                      "(name: event name pattern)",
> +                      "(name: event name pattern; vcpu: vCPU to query, 
> default is any)",
>          .mhandler.cmd = hmp_info_trace_events,
>          .command_completion = info_trace_events_completion,
>      },
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 98b4b1a..848efee 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -281,9 +281,10 @@ ETEXI
>  
>      {
>          .name       = "trace-event",
> -        .args_type  = "name:s,option:b",
> -        .params     = "name on|off",
> -        .help       = "changes status of a specific trace event",
> +        .args_type  = "name:s,option:b,vcpu:i?",
> +        .params     = "name on|off [vcpu]",
> +        .help       = "changes status of a specific trace event "
> +                      "(vcpu: vCPU to set, default is all)",
>          .mhandler.cmd = hmp_trace_event,
>          .command_completion = trace_event_completion,
>      },
> diff --git a/monitor.c b/monitor.c
> index 7bd0f32..91a377b 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -908,9 +908,16 @@ static void hmp_trace_event(Monitor *mon, const QDict 
> *qdict)
>  {
>      const char *tp_name = qdict_get_str(qdict, "name");
>      bool new_state = qdict_get_bool(qdict, "option");
> +    int vcpu = qdict_get_try_int(qdict, "vcpu", -1);
>      Error *local_err = NULL;
>  
> -    qmp_trace_event_set_state(tp_name, new_state, true, true, &local_err);
> +    if (vcpu < -1) {
> +        /* some user-provided negative number */
> +        monitor_printf(mon, "argument vcpu must be positive");
> +        return;

If -2 is not okay, why is -1 okay?

What about:

    bool has_vcpu = qdict_haskey(qdict, "vcpu");
    int vcpu = qdict_get_try_int(qdict, "vcpu", 0);

    if (has_vcpu && vcpu < 0) {

> +    }
> +
> +    qmp_trace_event_set_state(tp_name, new_state, true, true, vcpu != -1, 
> vcpu, &local_err);
>      if (local_err) {
>          error_report_err(local_err);
>      }
> @@ -1070,6 +1077,7 @@ static void hmp_info_cpustats(Monitor *mon, const QDict 
> *qdict)
>  static void hmp_info_trace_events(Monitor *mon, const QDict *qdict)
>  {
>      const char *name = qdict_get_try_str(qdict, "name");
> +    int vcpu = qdict_get_try_int(qdict, "vcpu", -1);
>      TraceEventInfoList *events;
>      TraceEventInfoList *elem;
>      Error *local_err = NULL;
> @@ -1077,8 +1085,13 @@ static void hmp_info_trace_events(Monitor *mon, const 
> QDict *qdict)
>      if (name == NULL) {
>          name = "*";
>      }
> +    if (vcpu < -1) {
> +        /* some user-provided negative number */
> +        monitor_printf(mon, "argument vcpu must be positive");
> +        return;

Likewise.

> +    }
>  
> -    events = qmp_trace_event_get_state(name, &local_err);
> +    events = qmp_trace_event_get_state(name, vcpu != -1, vcpu, &local_err);
>      if (local_err) {
>          error_report_err(local_err);
>          return;
[...]