1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH 0/2] Avoid issues due to qemu dropping osxsave and ospke
  4. View patch

[libvirt] [PATCH 1/2] qemu: do not define known no-op features

Christian Ehrhardt posted 2 patches 5 years, 7 months ago
[libvirt] [PATCH 1/2] qemu: do not define known no-op features
Posted by Christian Ehrhardt 5 years, 7 months ago 
Qemu dropped cpu features for osxsave and ospke [1][2].
The reason for the instant removal is that those features were never
configurable as discussed in [3].

Fortunately the use cases adding those flags in the past are rare, but
they exist. One that I identified are e.g. older virt-install when used
with --cpu=host-model and there always could be the case of a user
adding it to the guest xml.

This triggers an issue like:
  qemu-system-x86_64: can't apply global Broadwell-noTSX-x86_64-
  cpu.osxsave=on: Property '.osxsave' not found

Ensure that this does no more break spawning newer qemu versions by
not rendering those features into the qemu command line.

Fixes: https://bugs.launchpad.net/fedora/+source/qemu/+bug/1825195
Resolves: https://bugzilla.redhat.com/1644848

[1]: https://git.qemu.org/?p=qemu.git;a=commit;h=f1a2352
[2]: https://git.qemu.org/?p=qemu.git;a=commit;h=9ccb978
[3]: https://www.mail-archive.com/qemu-devel@nongnu.org/msg561877.html

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
---
 src/qemu/qemu_command.c                       | 23 +++++++++++++++++++
 .../qemuxml2argvdata/cpu-host-model-cmt.args  |  2 +-
 tests/qemuxml2argvdata/cpu-tsc-frequency.args |  4 ++--
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1425d97b1e..e0c8ae50a1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7076,6 +7076,27 @@ qemuBuildGlobalControllerCommandLine(virCommandPtr cmd,
     return 0;
 }
 
+/**
+ * qemuFeatureNoEffect:
+ * @feature: CPU Feature
+ *
+ * Returns true, if the feature is known to have (never had) an effect on QEMU.
+ * Those features might be dropped in qemu without a longer deprecation cycle
+ * and must therefore be known e.g. to no more define them on command line.
+ */
+static bool
+qemuFeatureNoEffect(virCPUFeatureDefPtr feature)
+{
+    if (!feature->name)
+        return false;
+
+    if (STREQ(feature->name, "osxsave"))
+        return true;
+    if (STREQ(feature->name, "ospke"))
+        return true;
+
+    return false;
+}
 
 static int
 qemuBuildCpuModelArgStr(virQEMUDriverPtr driver,
@@ -7144,6 +7165,8 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver,
         virBufferAsprintf(buf, ",vendor=%s", cpu->vendor_id);
 
     for (i = 0; i < cpu->nfeatures; i++) {
+        if (qemuFeatureNoEffect(&(cpu->features[i])))
+            continue;
         switch ((virCPUFeaturePolicy) cpu->features[i].policy) {
         case VIR_CPU_FEATURE_FORCE:
         case VIR_CPU_FEATURE_REQUIRE:
diff --git a/tests/qemuxml2argvdata/cpu-host-model-cmt.args b/tests/qemuxml2argvdata/cpu-host-model-cmt.args
index 36f706b836..42f969fd62 100644
--- a/tests/qemuxml2argvdata/cpu-host-model-cmt.args
+++ b/tests/qemuxml2argvdata/cpu-host-model-cmt.args
@@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \
 -S \
 -machine pc,accel=tcg,usb=off,dump-guest-core=off \
 -cpu Haswell,+vme,+ds,+acpi,+ss,+ht,+tm,+pbe,+dtes64,+monitor,+ds_cpl,+vmx,\
-+smx,+est,+tm2,+xtpr,+pdcm,+osxsave,+f16c,+rdrand,+pdpe1gb,+abm,+lahf_lm \
++smx,+est,+tm2,+xtpr,+pdcm,+f16c,+rdrand,+pdpe1gb,+abm,+lahf_lm \
 -m 214 \
 -realtime mlock=off \
 -smp 6,sockets=6,cores=1,threads=1 \
diff --git a/tests/qemuxml2argvdata/cpu-tsc-frequency.args b/tests/qemuxml2argvdata/cpu-tsc-frequency.args
index 55bcf89617..55b72b4404 100644
--- a/tests/qemuxml2argvdata/cpu-tsc-frequency.args
+++ b/tests/qemuxml2argvdata/cpu-tsc-frequency.args
@@ -12,8 +12,8 @@ QEMU_AUDIO_DRV=none \
 -S \
 -machine pc,accel=kvm,usb=off,dump-guest-core=off \
 -cpu Haswell,+vme,+ds,+acpi,+ss,+ht,+tm,+pbe,+dtes64,+monitor,+ds_cpl,+vmx,\
-+smx,+est,+tm2,+xtpr,+pdcm,+osxsave,+f16c,+rdrand,+pdpe1gb,+abm,+lahf_lm,\
-+invtsc,tsc-frequency=3504000000 \
++smx,+est,+tm2,+xtpr,+pdcm,+f16c,+rdrand,+pdpe1gb,+abm,+lahf_lm,+invtsc,\
+tsc-frequency=3504000000 \
 -m 214 \
 -realtime mlock=off \
 -smp 1,sockets=1,cores=1,threads=1 \
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: do not define known no-op features
Posted by Daniel Henrique Barboza 5 years, 6 months ago 

On 4/25/19 9:50 AM, Christian Ehrhardt wrote:
> Qemu dropped cpu features for osxsave and ospke [1][2].
> The reason for the instant removal is that those features were never
> configurable as discussed in [3].
>
> Fortunately the use cases adding those flags in the past are rare, but
> they exist. One that I identified are e.g. older virt-install when used
> with --cpu=host-model and there always could be the case of a user
> adding it to the guest xml.
>
> This triggers an issue like:
>    qemu-system-x86_64: can't apply global Broadwell-noTSX-x86_64-
>    cpu.osxsave=on: Property '.osxsave' not found
>
> Ensure that this does no more break spawning newer qemu versions by
> not rendering those features into the qemu command line.
>
> Fixes: https://bugs.launchpad.net/fedora/+source/qemu/+bug/1825195
> Resolves: https://bugzilla.redhat.com/1644848

I am not sure if I understood why this is the solution you opted for. I'll
start a discussion in the cover-letter, which seems more suitable.

As far as this patch goes:

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>




>
> [1]: https://git.qemu.org/?p=qemu.git;a=commit;h=f1a2352
> [2]: https://git.qemu.org/?p=qemu.git;a=commit;h=9ccb978
> [3]: https://www.mail-archive.com/qemu-devel@nongnu.org/msg561877.html
>
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> ---
>   src/qemu/qemu_command.c                       | 23 +++++++++++++++++++
>   .../qemuxml2argvdata/cpu-host-model-cmt.args  |  2 +-
>   tests/qemuxml2argvdata/cpu-tsc-frequency.args |  4 ++--
>   3 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 1425d97b1e..e0c8ae50a1 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7076,6 +7076,27 @@ qemuBuildGlobalControllerCommandLine(virCommandPtr cmd,
>       return 0;
>   }
>   
> +/**
> + * qemuFeatureNoEffect:
> + * @feature: CPU Feature
> + *
> + * Returns true, if the feature is known to have (never had) an effect on QEMU.
> + * Those features might be dropped in qemu without a longer deprecation cycle
> + * and must therefore be known e.g. to no more define them on command line.
> + */
> +static bool
> +qemuFeatureNoEffect(virCPUFeatureDefPtr feature)
> +{
> +    if (!feature->name)
> +        return false;
> +
> +    if (STREQ(feature->name, "osxsave"))
> +        return true;
> +    if (STREQ(feature->name, "ospke"))
> +        return true;
> +
> +    return false;
> +}
>   
>   static int
>   qemuBuildCpuModelArgStr(virQEMUDriverPtr driver,
> @@ -7144,6 +7165,8 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver,
>           virBufferAsprintf(buf, ",vendor=%s", cpu->vendor_id);
>   
>       for (i = 0; i < cpu->nfeatures; i++) {
> +        if (qemuFeatureNoEffect(&(cpu->features[i])))
> +            continue;
>           switch ((virCPUFeaturePolicy) cpu->features[i].policy) {
>           case VIR_CPU_FEATURE_FORCE:
>           case VIR_CPU_FEATURE_REQUIRE:
> diff --git a/tests/qemuxml2argvdata/cpu-host-model-cmt.args b/tests/qemuxml2argvdata/cpu-host-model-cmt.args
> index 36f706b836..42f969fd62 100644
> --- a/tests/qemuxml2argvdata/cpu-host-model-cmt.args
> +++ b/tests/qemuxml2argvdata/cpu-host-model-cmt.args
> @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \
>   -S \
>   -machine pc,accel=tcg,usb=off,dump-guest-core=off \
>   -cpu Haswell,+vme,+ds,+acpi,+ss,+ht,+tm,+pbe,+dtes64,+monitor,+ds_cpl,+vmx,\
> -+smx,+est,+tm2,+xtpr,+pdcm,+osxsave,+f16c,+rdrand,+pdpe1gb,+abm,+lahf_lm \
> ++smx,+est,+tm2,+xtpr,+pdcm,+f16c,+rdrand,+pdpe1gb,+abm,+lahf_lm \
>   -m 214 \
>   -realtime mlock=off \
>   -smp 6,sockets=6,cores=1,threads=1 \
> diff --git a/tests/qemuxml2argvdata/cpu-tsc-frequency.args b/tests/qemuxml2argvdata/cpu-tsc-frequency.args
> index 55bcf89617..55b72b4404 100644
> --- a/tests/qemuxml2argvdata/cpu-tsc-frequency.args
> +++ b/tests/qemuxml2argvdata/cpu-tsc-frequency.args
> @@ -12,8 +12,8 @@ QEMU_AUDIO_DRV=none \
>   -S \
>   -machine pc,accel=kvm,usb=off,dump-guest-core=off \
>   -cpu Haswell,+vme,+ds,+acpi,+ss,+ht,+tm,+pbe,+dtes64,+monitor,+ds_cpl,+vmx,\
> -+smx,+est,+tm2,+xtpr,+pdcm,+osxsave,+f16c,+rdrand,+pdpe1gb,+abm,+lahf_lm,\
> -+invtsc,tsc-frequency=3504000000 \
> ++smx,+est,+tm2,+xtpr,+pdcm,+f16c,+rdrand,+pdpe1gb,+abm,+lahf_lm,+invtsc,\
> +tsc-frequency=3504000000 \
>   -m 214 \
>   -realtime mlock=off \
>   -smp 1,sockets=1,cores=1,threads=1 \

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

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.