Hey Chris, Your patches are looking better with each iteration. Awesome progress thus far! I'd to make one suggestion for you that will keep your patches a bit cleaner and also makes the review process smoother: introduce code in the same patches that it gets used. Take for example: 6/11 qemu_monitor: Introduce qemuMonitorCPUModelInfoRemovePropByBoolValue The code introduced by this patch is not used until: 11/11 qemu_driver: BaselineHypervisorCPU supports S390 using QEMU/QMP Which makes it tricky when looking at patch 11, then trying to remember which patch introduced the new function. Having everything in one patch allows a reviewer to see all of your new code and when it gets used in once glance (or in one email / git show). Last thing: make sure to commit with --signoff so your name is present at the bottom of each commit :) On 07/09/2018 11:56 PM, Chris Venteicher wrote:
Some architectures (S390) depend on QEMU to compute baseline CPU model and expand a models feature set.
Interacting with QEMU requires starting the QEMU process and completing one or more query-cpu-model-baseline QMP exchanges with QEMU in addition to a query-cpu-model-expansion QMP exchange to expand all features in the model.
See "s390x CPU models: exposing features" patch set on Qemu-devel for discussion of QEMU aspects.
This is part of resolution of: https://bugzilla.redhat.com/show_bug.cgi?id=1511999
Version 2 address all issues raised by Collin as well as all other issues identified with additional testing.
Chris Venteicher (11): qemu_monitor_json: Introduce qemuMonitorCPUModelInfo / JSON conversion qemu_monitor: Introduce qemuMonitorGetCPUModelBaseline (query-cpu-model-baseline) qemu_monitor: Indicate when CPUModelInfo props report migratablity qemu_monitor: Introduce qemuMonitorCPUModelInfoInit and qemuMonitorCPUModelInfoFreeContents qemu_monitor: CPUModelExpansion on both model name and properties qemu_monitor: Introduce qemuMonitorCPUModelInfoRemovePropByBoolValue qemu_capabilities: Introduce virCPUDef / qemuMonitorCPUModelInfo conversions qemu_capabilities: QMPCommandPtr without maintaining shadow qmperr qemu_capabilities: Persist QEMU instance over multiple QMP Cmds qemu_capabilities: Introduce virQEMUCapsQMPBaselineCPUModel (baseline using QEMU) qemu_driver: BaselineHypervisorCPU supports S390 using QEMU/QMP
src/qemu/qemu_capabilities.c | 349 +++++++++++++++++++++++++++++------ src/qemu/qemu_capabilities.h | 33 ++++ src/qemu/qemu_driver.c | 74 +++++++- src/qemu/qemu_monitor.c | 117 +++++++++++- src/qemu/qemu_monitor.h | 21 ++- src/qemu/qemu_monitor_json.c | 232 +++++++++++++++++++---- src/qemu/qemu_monitor_json.h | 14 +- tests/cputest.c | 7 +- 8 files changed, 724 insertions(+), 123 deletions(-)
-- Respectfully, - Collin Walling