From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43438) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bEy5U-0007y2-LJ for qemu-devel@nongnu.org; Mon, 20 Jun 2016 08:13:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bEy5P-0007bb-Dx for qemu-devel@nongnu.org; Mon, 20 Jun 2016 08:13:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60511) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bEy5P-0007bR-5k for qemu-devel@nongnu.org; Mon, 20 Jun 2016 08:13:51 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AF3E163175 for ; Mon, 20 Jun 2016 12:13:50 +0000 (UTC) From: Markus Armbruster References: <146590985033.16561.16324808287190305042.stgit@fimbulvetr.bsc.es> <146590988276.16561.7454935023049676591.stgit@fimbulvetr.bsc.es> <87y460coi6.fsf@dusky.pond.sub.org> <8760t4uoey.fsf@fimbulvetr.bsc.es> Date: Mon, 20 Jun 2016 14:13:48 +0200 In-Reply-To: <8760t4uoey.fsf@fimbulvetr.bsc.es> (=?utf-8?Q?=22Llu=C3=ADs?= Vilanova"'s message of "Mon, 20 Jun 2016 12:48:05 +0200") Message-ID: <878ty06osj.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 6/6] trace: Add QAPI/QMP interfaces to query and control per-vCPU tracing state List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Luiz Capitulino , qemu-devel@nongnu.org, Stefan Hajnoczi , Eduardo Habkost Llu=C3=ADs Vilanova writes: > Markus Armbruster writes: > >> Llu=C3=ADs Vilanova writes: >>> Signed-off-by: Llu=C3=ADs Vilanova >>> Reviewed-by: Stefan Hajnoczi >>> --- >>> monitor.c | 4 +- >>> qapi/trace.json | 20 ++++++-- >>> qmp-commands.hx | 17 ++++++- >>> trace/qmp.c | 143 ++++++++++++++++++++++++++++++++++++++++++++----= ------- >>> 4 files changed, 147 insertions(+), 37 deletions(-) >>>=20 >>> 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 QDi= ct *qdict) >>> bool new_state =3D qdict_get_bool(qdict, "option"); >>> Error *local_err =3D NULL; >>>=20 >>> - qmp_trace_event_set_state(tp_name, new_state, true, true, &local_e= rr); >>> + 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) >>>=20 >>> static void hmp_info_trace_events(Monitor *mon, const QDict *qdict) >>> { >>> - TraceEventInfoList *events =3D qmp_trace_event_get_state("*", NULL= ); >>> + TraceEventInfoList *events =3D qmp_trace_event_get_state("*", fals= e, 0, NULL); >>> TraceEventInfoList *elem; >>>=20 >>> for (elem =3D events; elem !=3D NULL; elem =3D 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=C3=ADs Vilanova >>> +# Copyright (C) 2011-2016 Llu=C3=ADs Vilanova >>> # >>> # This work is licensed under the terms of the GNU GPL, version 2 or la= ter. >>> # 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'} } >>>=20 >>> ## >>> # @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" prop= erty 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 even= ts. > # 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'] } >>>=20 >>> ## >>> @@ -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': 'bo= ol'} } >>> + 'data': {'name': 'str', 'enable': 'bool', '*ignore-unavailable': 'bo= ol', >>> + '*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 >>>=20 >>> { >>> .name =3D "trace-event-get-state", >>> - .args_type =3D "name:s", >>> + .args_type =3D "name:s,vcpu:i?", >>> .mhandler.cmd_new =3D qmp_marshal_trace_event_get_state, >>> }, >>>=20 >>> @@ -4686,6 +4686,11 @@ trace-event-get-state >>>=20 >>> Query the state of events. >>>=20 >>> +Arguments: >>> + >>> +- "name": Event name pattern (json-string). >>> +- "vcpu": Specific vCPU to query, any vCPU by default (json-int, optio= nal). >>> + > >> 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=C3=A9 has patches. [...]