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

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(a)redhat.com&gt; >>> >>> 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(a)redhat.com&gt; >> >> 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(a)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.