Re: [libvirt] [PATCH 2/2] qemu: Add support for pseries machine's max-cpu-compat= parameter
On Thu, 2018-01-04 at 18:04 +0530, Shivaprasad G Bhat wrote:
When the -machine pseries,max-cpu-compat=X is supported use
machine parameter instead of -cpu host,compat=X parameter as
that is deprecated now with qemu > v2.10.
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1519146
Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com>
---
src/qemu/qemu_command.c | 14 ++++++++++--
.../pseries-machine-max-cpu-compat.args | 23 ++++++++++++++++++++
.../pseries-machine-max-cpu-compat.xml | 21 ++++++++++++++++++
tests/qemuxml2argvtest.c | 5 ++++
4 files changed, 60 insertions(+), 3 deletions(-)
create mode 100644 tests/qemuxml2argvdata/pseries-machine-max-cpu-compat.args
create mode 100644 tests/qemuxml2argvdata/pseries-machine-max-cpu-compat.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4d0c141..966ad75 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6922,9 +6922,12 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver,
case VIR_CPU_MODE_HOST_MODEL:
if (ARCH_IS_PPC64(def->os.arch)) {
- virBufferAddLit(buf, "host");
- if (cpu->model)
- virBufferAsprintf(buf, ",compat=%s", cpu->model);
+ if (!virQEMUCapsGet(qemuCaps,
+ QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT)) {
You don't need to use two lines here.
We probably want to check against the machine type as well, though:
at least formally, the 'compat' option is only specific to the CPU,
so you could in theory have non-pseries guests using it. I don't
think that would actually work, but it still makes sense that we
would only use 'max-cpu-compat' instead of 'compat' for pseries
guests.
+ virBufferAddLit(buf, "host");
+ if (cpu->model)
+ virBufferAsprintf(buf, ",compat=%s", cpu->model);
+ }
I think we still want to output '-cpu host' regardless of whether
we're going to use the CPU-level or machine-level compat option.
It just feels safer - even though I tried migrating a guest and it
doesn't seem to make a difference in practice.
[...]
@@ -7588,6 +7592,10 @@ qemuBuildMachineCommandLine(virCommandPtr
cmd,
virBufferAsprintf(&buf, ",resize-hpt=%s", str);
}
+ if (cpu && cpu->model && cpu->mode ==
VIR_CPU_MODE_HOST_MODEL &&
+ virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT))
+ virBufferAsprintf(&buf, ",max-cpu-compat=%s", cpu->model);
Our coding style requires braces around the body here.
As mentioned above, a check against the machine type should probably
be added here as well, to make sure we're not using this
pseries-specific option with any other machine type.
[...]
--- /dev/null
+++ b/tests/qemuxml2argvdata/pseries-machine-max-cpu-compat.xml
@@ -0,0 +1,21 @@
+<domain type='kvm'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219100</memory>
+ <currentMemory unit='KiB'>219100</currentMemory>
The <currentMemory> element can be omitted.
+ <vcpu placement='static'>4</vcpu>
+ <os>
+ <type arch='ppc64' machine='pseries'>hvm</type>
+ </os>
+ <cpu mode='host-model'>
+ <model>power7</model>
+ </cpu>
+ <clock offset='utc'/>
The <clock> element can be omitted.
+ <devices>
+ <emulator>/usr/bin/qemu-system-ppc64</emulator>
+ <console type='pty'>
+ <address type="spapr-vio"/>
+ </console>
The <console> element can be omitted; on the other hand, I'd add
<controller type='usb' model='none'/>
here.
[...]
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1816,6 +1816,11 @@ mymain(void)
DO_TEST("pseries-cpu-compat", QEMU_CAPS_KVM,
QEMU_CAPS_DEVICE_SPAPR_VTY,
QEMU_CAPS_NODEFCONFIG);
+ DO_TEST("pseries-machine-max-cpu-compat", QEMU_CAPS_KVM,
Move QEMU_CAPS_KVM to its own line, please.
+ QEMU_CAPS_MACHINE_OPT,
+ QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT,
+ QEMU_CAPS_DEVICE_SPAPR_VTY,
+ QEMU_CAPS_NODEFCONFIG);
QEMU_CAPS_DEVICE_SPAPR_VTY can be omitted once you've gotten
rid of the corresponding XML element in the input file.
--
Andrea Bolognani / Red Hat / Virtualization