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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list