Re: [libvirt] [PATCH 1/8] Add qemuMonitorGetObjectListPaths() method for QMP qom-list command

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.