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<pre...@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<pre...@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
--
Prerna Saxena
Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
