From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=50344 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1P8p0F-0004Fu-45 for qemu-devel@nongnu.org; Thu, 21 Oct 2010 02:51:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1P8p0D-00047i-S3 for qemu-devel@nongnu.org; Thu, 21 Oct 2010 02:51:55 -0400 Received: from e28smtp07.in.ibm.com ([122.248.162.7]:51650) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1P8p0C-00047U-SA for qemu-devel@nongnu.org; Thu, 21 Oct 2010 02:51:53 -0400 Received: from d28relay03.in.ibm.com (d28relay03.in.ibm.com [9.184.220.60]) by e28smtp07.in.ibm.com (8.14.4/8.13.1) with ESMTP id o9L6pmnS027678 for ; Thu, 21 Oct 2010 12:21:48 +0530 Received: from d28av05.in.ibm.com (d28av05.in.ibm.com [9.184.220.67]) by d28relay03.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id o9L6plwe4112526 for ; Thu, 21 Oct 2010 12:21:47 +0530 Received: from d28av05.in.ibm.com (loopback [127.0.0.1]) by d28av05.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id o9L6pkXt030557 for ; Thu, 21 Oct 2010 17:51:46 +1100 Message-ID: <4CBFE302.1020504@linux.vnet.ibm.com> Date: Thu, 21 Oct 2010 12:21:46 +0530 From: Prerna Saxena MIME-Version: 1.0 References: <20101019114922.66d7c31d@zephyr> <20101019115750.7ce4a0c7@zephyr> <20101020171749.35fa56dd@doriath> In-Reply-To: <20101020171749.35fa56dd@doriath> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [Tracing][v4 PATCH 2/2] Add documentation for QMP interfaces List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: Mahesh , Ananth Narayan , qemu-devel , Stefan Hajnoczi Hi Luiz, Thanks for your feedback. On 10/21/2010 12:47 AM, Luiz Capitulino wrote: > On Tue, 19 Oct 2010 11:57:50 +0530 > Prerna Saxena wrote: > >> [PATCH 2/2] Add documentation for QMP commands: >> - query-trace >> - query-trace-events >> - query-trace-file. > > Please, split this. Each command should be in a separate patch. > >> >> >> Signed-off-by: Prerna Saxena >> --- >> qmp-commands.hx | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 files changed, 94 insertions(+), 0 deletions(-) >> >> diff --git a/qmp-commands.hx b/qmp-commands.hx >> index 793cf1c..bc79b55 100644 >> --- a/qmp-commands.hx >> +++ b/qmp-commands.hx >> @@ -1539,3 +1539,97 @@ Example: >> >> EQMP >> >> +SQMP >> +query-trace >> +------------- > > It's recommended to first send documentation patches when adding new QMP > commands, it can be catastrophic to do both at the same time. > Right. I'm posting a set of separate set of documentation patches, where we can discuss the interfaces individually. > So, I'll ignore the code for now and discuss the interface only. > > My main question is: What are the expected use cases for this interface in > the perspective of a QMP client? > > I can think of two: > > 1. Enabling/Disabling trace points (eg. from a GUI) > 2. Get trace data to generate trace output or do some kind of analysis > > If we're only interested in 1, then we don't need query-trace and if we > do need query-trace then we'll have to rethink some things as it can be > automatically flushed. > At present, qemu has all trace-events disabled at build-time, by default. The trace-events of interest are dynamically enabled using the human monitor at run time. '1' is useful to have, so that a QMP client can do the same. The 'query-trace' interface simply displays the current contents of trace-buffer before they are flushed to disk.(equivalent to the 'info trace' HMP command ) To enabled logged traces to be forcibly flushed, HMP has a command : (qemu)trace-file flush I'm still working on the QMP interface for the same. ( I'll cover the proposed QMP interface in my documentation patchset that I'll be sending out shortly.) >> + >> +Show contents of trace buffer. >> + >> +Returns a set of json-objects containing the following data: > > Looks like you return a json-array of json-objects, is that correct? > Yes. >> + >> +- "event": Event ID for the trace-event(json-int) > > Maybe this should be called event_id or just id. > I've renamed it to event_id for next patch. >> +- "timestamp": trace timestamp (json-int) > > Unit? ns. I've corrected the description for the upcoming patchset. > >> +- "arg1 .. arg6": Arguments logged by the trace-event (json-int) > > Are they positional or named arguments? > > If they are positional, you should use a json-array, if we have the > argument name, then we could be nicer and have a json-object of arguments. > These are keyword arguments. I have changed the definition to have these enclosed in a QMP object for the next patchset version. >> + >> +Example: >> + >> +-> { "execute": "query-trace" } >> +<- { >> + "return":{ >> + "event": 22, >> + "timestamp": 129456235912365, >> + "arg1": 886 >> + "arg2": 80, >> + "arg3": 0, >> + "arg4": 0, >> + "arg5": 0, >> + "arg6": 0, >> + }, >> + { >> + "event": 22, >> + "timestamp": 129456235973407, >> + "arg1": 886, >> + "arg2": 80, >> + "arg3": 0, >> + "arg4": 0, >> + "arg5": 0, >> + "arg6": 0 >> + }, >> + ... >> + } > > The example above is invalid json. I'm guessing this is invalid because it could be denoted as an array of objects ? Corrected in upcoming patchset. > >> + >> +EQMP >> + >> +SQMP >> +query-trace-events >> +------------------ >> + >> +Show all available trace-events& their state. >> + >> +Returns a set of json-objects containing the following data: > > Again, I believe you want to return a json-array of json-objects. Agree, corrected this for the documentation patchset. > >> + >> +- "name": Name of Trace-event (json-string) >> +- "event-id": Event ID of Trace-event (json-int) > > query-trace's key is called event, we should use either event_id or just id > (I think I prefer the former). > Renamed to event_id. >> +- "state": State of trace-event [ '0': inactive; '1':active ] (json-int) > > This should be a json-bool. > Done. >> + >> +Example: >> + >> +-> { "execute": "query-trace-events" } >> +<- { >> + "return":{ >> + "name": "qemu_malloc", >> + "event-id": 0 >> + "state": 0, >> + }, >> + { >> + "name": "qemu_realloc", >> + "event-id": 1, >> + "state": 0 >> + }, >> + ... >> + } > > This also invalid json. > Again, I'm guessing the reason it is invalid that it ought to be an array. Changed for next patches. >> + >> +EQMP >> + >> +SQMP >> +query-trace-file >> +---------------- >> + >> +Display currently set trace file name and its status. >> + >> +Returns a set of json-objects containing the following data: > > This is actually just one json-object. > Oops, typo. Changed ! >> + >> +- "trace-file": Name of Trace-file (json-string) > > Name or path? > This ought to display full path -- changed for next patch set. >> +- "status": State of trace-event [ '0': disabled; '1':enabled ] (json-int) > > This should be a json bool called 'enabled' or 'disabled', but what happens > when a file is not defined? > Changed type to json bool. The trace infrastructure sets the trace-output file to trace- ( created in current dir) if no explicit trace-file is specified at startup. (Users can also change the default trace-file at runtime using the hmp command 'trace-file set FILE' I'll be covering QMP interface for the same in the upcoming patchset. ) >> + >> +Example: >> + >> +-> { "execute": "query-trace-file" } >> +<- { >> + "return":{ >> + "trace-file": "trace-26609", >> + "status": 1 >> + } >> + } >> + >> +EQMP > -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India