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