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

[Daniel P. Berrange]

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 :|