[libvirt] [PATCHv3 0/3] query-cpu-model-baseline QMP command

[Collin Walling]

On 05/02/2018 03:43 AM, Jiri Denemark wrote:
> On Tue, May 01, 2018 at 09:57:08 -0400, John Ferlan wrote:
>>
>>
>> On 04/30/2018 10:55 PM, Chris Venteicher wrote:
>>> This is part of resolution of: https://bugzilla.redhat.com/show_bug.cgi?id=1511999
>>>
>>> Signed-off-by: Chris Venteicher <cventeic at redhat.com>
>>>
>>> diff to v1:
>>> - Replaced c++ style comments with c style
>>> - qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON} use To/From form
>>> - qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON} return pointers not integers
>>> - VIR_STEAL_PTR form used
>>> - switch statement uses virReportEnumRangeError
>>> - qemuMonitorJSONHasError(reply, "GenericError") treated as no info, not fatal error
>>> - virJSONValueFree(cpu_props) during error case
>>>
>>> diff to v2:
>>> - qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON}
>>> works on {"name": "IvyBridge", "props": {}} 
>>> rather than  {"model": {"name": "IvyBridge", "props": {}}}
>>> - additional cleanups and fixes
>>>
>>> Chris Venteicher (3):
>>>   qemu_monitor_json: Populate CPUModelInfo struct from json
>>>   qemu_monitor_json: Build Json CPU Model Info
>>>   qemu_monitor: query-cpu-model-baseline QMP command
>>>
>>>  src/qemu/qemu_monitor.c      |  13 ++++
>>>  src/qemu/qemu_monitor.h      |   5 ++
>>>  src/qemu/qemu_monitor_json.c | 179 +++++++++++++++++++++++++++++++++++++------
>>>  src/qemu/qemu_monitor_json.h |   7 ++
>>>  4 files changed, 182 insertions(+), 22 deletions(-)
>>>
>>
>> Changes look good to me,
>>
>> Reviewed-by: John Ferlan <jferlan at redhat.com>
>>
>> I have to wait for 4.4.0 to open before pushing, but it would also be
>> good to know what the plans are for code that will use the new
>> functions. Do you plan on posting something for 4.4.0?
> 
> I don't think pushing this makes a lot of sense without any users.
> Without seeing the complete picture from a public APIs down to the
> monitor code it's a bit hard to see if the code fits together nicely. We
> have two approaches now... Chris makes the monitor APIs work on
> CPUModelInfo while Collin's APIs use virCPUDef. We need to see how this
> all fits into the public APIs first.
> 
> Jirka
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 

I agree with Jiri. In their current state, I think your patches look fine but
I'll hold off on giving my r-b's until I see how they get used.

Great progress! Looking forward to seeing how it all hooks up.


-- 
Respectfully,
- Collin Walling