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 <sb...@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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
