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@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)) {
+                virBufferAddLit(buf, "host");
+                if (cpu->model)
+                    virBufferAsprintf(buf, ",compat=%s", cpu->model);
+            }
         } else {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("unexpected host-model CPU for %s architecture"),
@@ -7414,6 +7417,7 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
     } else {
         virTristateSwitch vmport = def->features[VIR_DOMAIN_FEATURE_VMPORT];
         virTristateSwitch smm = def->features[VIR_DOMAIN_FEATURE_SMM];
+        virCPUDefPtr cpu = def->cpu;
 
         virCommandAddArg(cmd, "-machine");
         virBufferAdd(&buf, def->os.machine, -1);
@@ -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);
+
         if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX) &&
             virQEMUCapsGet(qemuCaps, QEMU_CAPS_LOADPARM))
             qemuAppendLoadparmMachineParm(&buf, def);
diff --git a/tests/qemuxml2argvdata/pseries-machine-max-cpu-compat.args b/tests/qemuxml2argvdata/pseries-machine-max-cpu-compat.args
new file mode 100644
index 0000000..f53db85
--- /dev/null
+++ b/tests/qemuxml2argvdata/pseries-machine-max-cpu-compat.args
@@ -0,0 +1,23 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-ppc64 \
+-name QEMUGuest1 \
+-S \
+-machine pseries,accel=kvm,max-cpu-compat=power7 \
+-m 256 \
+-smp 4,sockets=4,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefconfig \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-boot c \
+-usb \
+-chardev pty,id=charserial0 \
+-device spapr-vty,chardev=charserial0,id=serial0,reg=0x30000000
diff --git a/tests/qemuxml2argvdata/pseries-machine-max-cpu-compat.xml b/tests/qemuxml2argvdata/pseries-machine-max-cpu-compat.xml
new file mode 100644
index 0000000..f6c9516
--- /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>
+  <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'/>
+  <devices>
+      <emulator>/usr/bin/qemu-system-ppc64</emulator>
+      <console type='pty'>
+        <address type="spapr-vio"/>
+      </console>
+      <memballoon model="none"/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index fe15072..e5699ea 100644
--- 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,
+            QEMU_CAPS_MACHINE_OPT,
+            QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT,
+            QEMU_CAPS_DEVICE_SPAPR_VTY,
+            QEMU_CAPS_NODEFCONFIG);
     DO_TEST("pseries-cpu-le", QEMU_CAPS_KVM,
             QEMU_CAPS_DEVICE_SPAPR_VTY,
             QEMU_CAPS_NODEFCONFIG);

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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@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