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

[Lluís Vilanova]

Markus Armbruster writes:

> 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) {
[...]

I was trying to differentiate between the default value (-1) and other
user-provided negative values. I'll change it to use qdict_haskey() (I wasn't
aware of it).

Thanks,
  Lluis