| Message ID | 20101019115750.7ce4a0c7@zephyr |
|---|---|
| State | New |
| Headers | show |
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>
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 ?
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
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 >
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 > > > >
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
[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(-)