Re: [libvirt] [PATCHv2 00/11] BaselineHypervisorCPU using QEMU QMP exchanges
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