[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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.

I cannot think of an issue with the change, but I'm just making sure to
avoid continual rework.

That also seems to mean modifying the test to include
'qemu_monitor_json.h'...

John


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]