Re: [Qemu-devel] [PATCH v4 6/6] 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:
>> Markus Armbruster writes:
>> 
>>> Lluís Vilanova <address@hidden> writes:
>>>> Signed-off-by: Lluís Vilanova <address@hidden>
>>>> Reviewed-by: Stefan Hajnoczi <address@hidden>
>>>> ---
>>>> monitor.c       |    4 +-
>>>> qapi/trace.json |   20 ++++++--
>>>> qmp-commands.hx |   17 ++++++-
>>>> trace/qmp.c     |  143 
>>>> ++++++++++++++++++++++++++++++++++++++++++++-----------
>>>> 4 files changed, 147 insertions(+), 37 deletions(-)
>>>> 
>>>> diff --git a/monitor.c b/monitor.c
>>>> index a27e115..bb89877 100644
>>>> --- a/monitor.c
>>>> +++ b/monitor.c
>>>> @@ -910,7 +910,7 @@ static void hmp_trace_event(Monitor *mon, const QDict 
>>>> *qdict)
>>>> bool new_state = qdict_get_bool(qdict, "option");
>>>> Error *local_err = NULL;
>>>> 
>>>> -    qmp_trace_event_set_state(tp_name, new_state, true, true, &local_err);
>>>> +    qmp_trace_event_set_state(tp_name, new_state, true, true, false, 0, 
>>>> &local_err);
>>>> if (local_err) {
>>>> error_report_err(local_err);
>>>> }
>>>> @@ -1069,7 +1069,7 @@ static void hmp_info_cpustats(Monitor *mon, const 
>>>> QDict *qdict)
>>>> 
>>>> static void hmp_info_trace_events(Monitor *mon, const QDict *qdict)
>>>> {
>>>> -    TraceEventInfoList *events = qmp_trace_event_get_state("*", NULL);
>>>> +    TraceEventInfoList *events = qmp_trace_event_get_state("*", false, 0, 
>>>> NULL);
>>>> TraceEventInfoList *elem;
>>>> 
>>>> for (elem = events; elem != NULL; elem = elem->next) {
>> 
>>> The new feature remains inaccessible in HMP.  Any plans to extend HMP?
>>> Any reasons not to?
>> 
>> BTW, I was just looking at "info trace-events" and it only shows the state 
>> for
>> all events (there's no name pattern argument). Is it worth updating HMP 
>> knowing
>> it should be replaced in favour of the newer QAPI interface?

> QMP is not meant as replacement for HMP.  QMP is a stable interface for
> machines, HMP is a convenient interface for humans.  New functionality
> is usually added to both.  HMP sometimes gets additional convenience
> features.  In rare cases, a feature is deemed of no interest to humans,
> and left out of HMP.  Is filtering by vCPU such a case?

Oh, I thought there were plans to provide an HMP-like interface from the events
in the QAPI files. But maybe I just mixed this with the plans for dropping the
QMP commands file in favour of the QAPI command definitions.

I'll then add the proper HMP commands.


Thanks,
  Lluis