Lluís Vilanova <vilan...@ac.upc.edu> writes:
> Markus Armbruster writes:
>
>> Lluís Vilanova <vilan...@ac.upc.edu> writes:
>>> Signed-off-by: Lluís Vilanova <vilan...@ac.upc.edu>
>>> Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com>
>>> ---
>>> 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?
>
> My bad, I forgot about it. I'll see to adding it.
>
>
>>> diff --git a/qapi/trace.json b/qapi/trace.json
>>> index 01b0a52..25d8095 100644
>>> --- a/qapi/trace.json
>>> +++ b/qapi/trace.json
>>> @@ -1,6 +1,6 @@
>>> # -*- mode: python -*-
>>> #
>>> -# Copyright (C) 2011-2014 Lluís Vilanova <vilan...@ac.upc.edu>
>>> +# Copyright (C) 2011-2016 Lluís Vilanova <vilan...@ac.upc.edu>
>>> #
>>> # This work is licensed under the terms of the GNU GPL, version 2 or later.
>>> # See the COPYING file in the top-level directory.
>>> @@ -29,11 +29,12 @@
>>> #
>>> # @name: Event name.
>>> # @state: Tracing state.
>>> +# @vcpu: Whether this is a per-vCPU event (since 2.7).
>>> #
>>> # Since 2.2
>>> ##
>>> { 'struct': 'TraceEventInfo',
>>> - 'data': {'name': 'str', 'state': 'TraceEventState'} }
>>> + 'data': {'name': 'str', 'state': 'TraceEventState', 'vcpu': 'bool'} }
>>>
>>> ##
>>> # @trace-event-get-state:
>>> @@ -41,13 +42,18 @@
>>> # Query the state of events.
>>> #
>>> # @name: Event name pattern (case-sensitive glob).
>>> +# @vcpu: #optional The vCPU to check (any by default; since 2.7).
>>> #
>>> # Returns: a list of @TraceEventInfo for the matching events
>>> #
>>> +# For any event without the "vcpu" property:
>
>> All events have the vcpu property, but for some of them the property's
>> value is false.
>
> Hmmm, do you mean in the generated event array? Here, I was referring to
> wether
> an event has the "vcpu" property in "trace-events". I can clarify that.
What is "trace-events"? The file in the source tree?
>>> +# - If @name is a pattern and @vcpu is set, events are ignored.
>
>> I figure "ignored" means they're not included in the return value.
>> Correct?
>
> Exactly.
>
>
>>> +# - If @name is not a pattern and @vcpu is set, an error is raised.
>
>> Perhaps we could clarify as follows:
>
>> # Returns: a list of @TraceEventInfo for the matching events
>> #
>> # An event matches if
>> # - its name matches the @name pattern, and
>> # - if @vcpu is given, its vCPU equals @vcpu.
>> # It follows that with @vcpu given, the query can match only per-vCPU
>> # events. Special case: if the name is an exact match, you get an error
>> # instead of an empty list.
>
> Doesn't sound entirely right to me:
>
> # Returns: a list of @TraceEventInfo for the matching events
> #
> # An event matches if
> # - its name matches the @name pattern, and
> # - if @vcpu is given, the event is a per-vCPU one (has the "vcpu" property
> in
> # "trace-events").
Fails to mention that @vcpu selects the TraceEventInfo for that
particular VCPU. That's what I tried to express by "its vCPU equals
@vcpu".
> #
> # Therefore, if @vcpu is given, the query will only match per-vCPU events.
> # Special case: if @name is an exact match and @vcpu is given, you will get
> an
> # error if the event does not have the "vcpu" property.
>
>
>>> +#
>>> # Since 2.2
>>> ##
>>> { 'command': 'trace-event-get-state',
>>> - 'data': {'name': 'str'},
>>> + 'data': {'name': 'str', '*vcpu': 'int'},
>>> 'returns': ['TraceEventInfo'] }
>>>
>>> ##
>>> @@ -58,8 +64,14 @@
>>> # @name: Event name pattern (case-sensitive glob).
>>> # @enable: Whether to enable tracing.
>>> # @ignore-unavailable: #optional Do not match unavailable events with @name.
>>> +# @vcpu: #optional The vCPU to act upon (all by default; since 2.7).
>>> +#
>>> +# For any event without the "vcpu" property:
>>> +# - If @name is a pattern and @vcpu is set, events are ignored.
>>> +# - If @name is not a pattern and @vcpu is set, an error is raised.
>
>> Likewise.
>
> Ok.
>
>
>>> #
>>> # Since 2.2
>>> ##
>>> { 'command': 'trace-event-set-state',
>>> - 'data': {'name': 'str', 'enable': 'bool', '*ignore-unavailable': 'bool'}
>>> }
>>> + 'data': {'name': 'str', 'enable': 'bool', '*ignore-unavailable': 'bool',
>>> + '*vcpu': 'int'} }
>>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>>> index 28801a2..c9eb25c 100644
>>> --- a/qmp-commands.hx
>>> +++ b/qmp-commands.hx
>>> @@ -4676,7 +4676,7 @@ EQMP
>>>
>>> {
>>> .name = "trace-event-get-state",
>>> - .args_type = "name:s",
>>> + .args_type = "name:s,vcpu:i?",
>>> .mhandler.cmd_new = qmp_marshal_trace_event_get_state,
>>> },
>>>
>>> @@ -4686,6 +4686,11 @@ trace-event-get-state
>>>
>>> Query the state of events.
>>>
>>> +Arguments:
>>> +
>>> +- "name": Event name pattern (json-string).
>>> +- "vcpu": Specific vCPU to query, any vCPU by default (json-int, optional).
>>> +
>
>> Less information than you give in the schema. Easy enough to fix.
>
> I guess you mean extending the docs here to cover the same info given in the
> JSON file.
Yes.
I hope we can get rid of the duplication some day. Marc-André has
patches.
[...]
