Patchwork [Tracing,v4,2/2] Add documentation for QMP interfaces

login
register
mail settings
Submitter Prerna Saxena
Date Oct. 19, 2010, 6:27 a.m.
Message ID <20101019115750.7ce4a0c7@zephyr>
Download mbox | patch
Permalink /patch/68273/
State New
Headers show

Comments

Prerna Saxena - Oct. 19, 2010, 6:27 a.m.
[PATCH 2/2] Add documentation for QMP commands:
 - query-trace
 - query-trace-events
 - query-trace-file.


Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
---
 qmp-commands.hx |   94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 94 insertions(+), 0 deletions(-)
Stefan Hajnoczi - Oct. 19, 2010, 9:12 a.m.
On Tue, Oct 19, 2010 at 11:57:50AM +0530, Prerna Saxena wrote:
> [PATCH 2/2] Add documentation for QMP commands:
>  - query-trace
>  - query-trace-events
>  - query-trace-file.
> 
> 
> Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
> ---
>  qmp-commands.hx |   94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 94 insertions(+), 0 deletions(-)

Acked-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Prerna Saxena - Oct. 19, 2010, 12:44 p.m.
On 10/19/2010 11:57 AM, Prerna Saxena wrote:
> [PATCH 2/2] Add documentation for QMP commands:
>   - query-trace
>   - query-trace-events
>   - query-trace-file.
>
>

I've been trying ways to avoid building this documentation for other 
trace backends ( since these commands are only available with the 
'simple' backend ). However, looks like hxtool blindly copies text 
between SQMP and EQMP.
I can only think of making hxtool a wee bit intelligent to be able to 
parse CONFIG_* options and build documentation accordingly. Is there a 
workaround I'm missing ?
Luiz Capitulino - Oct. 20, 2010, 7:17 p.m.
On Tue, 19 Oct 2010 11:57:50 +0530
Prerna Saxena <prerna@linux.vnet.ibm.com> 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 <prerna@linux.vnet.ibm.com>
> ---
>  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.

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.

> +
> +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?

> +
> +- "event": Event ID for the trace-event(json-int)

Maybe this should be called event_id or just id.

> +- "timestamp": trace timestamp (json-int)

Unit?

> +- "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.

> +
> +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.

> +
> +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.

> +
> +- "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).

> +- "state": State of trace-event [ '0': inactive; '1':active  ] (json-int)

This should be a json-bool.

> +
> +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.

> +
> +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.

> +
> +- "trace-file": Name of Trace-file (json-string)

Name or path?

> +- "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?

> +
> +Example:
> +
> +-> { "execute": "query-trace-file" }
> +<- {
> +      "return":{
> +         "trace-file": "trace-26609",
> +         "status": 1
> +      }
> +   }
> +
> +EQMP
Prerna Saxena - Oct. 21, 2010, 6:51 a.m.
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<prerna@linux.vnet.ibm.com>  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<prerna@linux.vnet.ibm.com>
>> ---
>>   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-<PID> ( 
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
>
Luiz Capitulino - Oct. 21, 2010, 12:12 p.m.
On Thu, 21 Oct 2010 12:21:46 +0530
Prerna Saxena <prerna@linux.vnet.ibm.com> wrote:

> 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<prerna@linux.vnet.ibm.com>  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<prerna@linux.vnet.ibm.com>
> >> ---
> >>   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 )

This equivalence is not always desirable, HMP interfaces are designed
for humans. We have to think what's the best machine interface.

> 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.)

Ok.

> 
> >> +
> >> +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 ?

Yes.

> 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-<PID> ( 
> 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
> >
> 
>

Patch

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
+-------------
+
+Show contents of trace buffer.
+
+Returns a set of json-objects containing the following data:
+
+- "event": Event ID for the trace-event(json-int)
+- "timestamp": trace timestamp (json-int)
+- "arg1 .. arg6": Arguments logged by the trace-event (json-int)
+
+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
+       },
+       ...
+   }
+
+EQMP
+
+SQMP
+query-trace-events
+------------------
+
+Show all available trace-events & their state.
+
+Returns a set of json-objects containing the following data:
+
+- "name": Name of Trace-event (json-string)
+- "event-id": Event ID of Trace-event (json-int)
+- "state": State of trace-event [ '0': inactive; '1':active  ] (json-int)
+
+Example:
+
+-> { "execute": "query-trace-events" }
+<- {
+      "return":{
+         "name": "qemu_malloc",
+         "event-id": 0
+         "state": 0,
+      },
+      {
+         "name": "qemu_realloc",
+         "event-id": 1,
+         "state": 0
+      },
+      ...
+   }
+
+EQMP
+
+SQMP
+query-trace-file
+----------------
+
+Display currently set trace file name and its status.
+
+Returns a set of json-objects containing the following data:
+
+- "trace-file": Name of Trace-file (json-string)
+- "status": State of trace-event [ '0': disabled; '1':enabled  ] (json-int)
+
+Example:
+
+-> { "execute": "query-trace-file" }
+<- {
+      "return":{
+         "trace-file": "trace-26609",
+         "status": 1
+      }
+   }
+
+EQMP