On Wed, Jul 03, 2013 at 11:30:46AM -0400, John Ferlan wrote:
On 07/03/2013 06:56 AM, Daniel P. Berrange wrote:
On Tue, Jul 02, 2013 at 09:39:19AM -0400, John Ferlan wrote:
Add a new qemuMonitorGetObjectListPaths() method to support invocation of the 'qom-list' JSON monitor command with a provided path.
The returned list of paired data fields of "name" and "type" that can be used to peruse QOM configuration data and eventually utilize for the balloon statistics.
The test does a "{"execute":"qom-list", "arguments": { "path": "/"}}" which returns "{"return": [{"name": "machine", "type": "child<container>"}, {"name": "type", "type": "string"}]}" resulting in a return of an array of 2 elements with [0].name="machine", [0].type="child<container>". The [1] entry appears to be a header that could be used some day via a command such as "virsh qemuobject --list" to format output.
Top marks for adding a test !
--- src/qemu/qemu_monitor.c | 33 ++++++++++++++++ src/qemu/qemu_monitor.h | 15 +++++++ src/qemu/qemu_monitor_json.c | 93 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 6 +++ tests/qemumonitorjsontest.c | 77 ++++++++++++++++++++++++++++++++++++ 5 files changed, 224 insertions(+)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 86ef635..2e92f8c 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -679,6 +679,21 @@ int qemuMonitorGetKVMState(qemuMonitorPtr mon,
int qemuMonitorGetObjectTypes(qemuMonitorPtr mon, char ***types); + +typedef struct _qemuMonitorListPath qemuMonitorListPath; +typedef qemuMonitorListPath *qemuMonitorListPathPtr; + +struct _qemuMonitorListPath { + char *name; + char *type; +}; + +int qemuMonitorGetObjectListPaths(qemuMonitorPtr mon, + const char *path, + qemuMonitorListPathPtr **paths); + +void qemuMonitorListPathFree(qemuMonitorListPathPtr paths);
The qom related monitor calls are super generic, which also means they are a super PITA to deal with in code. To mitigate this, I think we should try to keep code that uses the qom commands isolated in the qemu_monitor* files, and then expose higher level APIs to the rest of the QEMU driver code.
So I think it is ok to expose these APIs in qemu_monitor_json.h, but we should not expose it in qemu_monitor.h
Daniel
The model I followed was the same model other entry points used through qemu_monitor and into qemu_monitor_json (and for some into qemu_monitor_text).
If I read this correctly, for the first 3 patches it seems you are advocating removing the middle man - that is later rather than calling qemuMonitorGetObjectListPaths call qemuMonitorJSONGetObjectListPaths directly. Similarly for the GetObject & PutObject calls.
No, I'm not recommending that qemu_driver/qemu_process call into qemu_monitor_json.h I'm saying that instead of directly exposing the ugly interface of the QOM commands to callers of qemu_monitor.h, we should have a higher level API exposed. eg, qemu_monitor.h should just expose qemuMonitorGetMemoryStats() (already exists in fact) qemuMonitorSetMemoryStatsRefresPeriod() and then the qemu_monitor.c impls of these methods can call qemuMonitorJSON{GetObjectListPaths,GetObject,PutObject}. No other code outside of the qemu_monitor.c file then has to know about qemuMonitorJSON{GetObjectListPaths,GetObject,PutObject} Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|