[libvirt] [PATCH 15/24] qemu_command: Use canonical names of CPU features

Jiri Denemark posted 24 patches 6 years, 7 months ago
There is a newer version of this series
[libvirt] [PATCH 15/24] qemu_command: Use canonical names of CPU features
Posted by Jiri Denemark 6 years, 7 months ago
When building QEMU command line, we should use the preferred spelling of
each CPU feature without relying on compatibility aliases (which may be
removed at some point).

The "unavailable-features" CPU property is used as a witness for the
correct names of the features in our translation table.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 src/qemu/qemu_capabilities.c                              | 8 +++++++-
 src/qemu/qemu_capabilities.h                              | 1 +
 src/qemu/qemu_command.c                                   | 2 ++
 tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml          | 1 +
 tests/qemuxml2argvdata/cpu-translation.x86_64-latest.args | 6 +++---
 tests/qemuxml2argvdata/eoi-disabled.x86_64-latest.args    | 2 +-
 tests/qemuxml2argvdata/eoi-enabled.x86_64-latest.args     | 2 +-
 .../kvmclock+eoi-disabled.x86_64-latest.args              | 2 +-
 .../pv-spinlock-disabled.x86_64-latest.args               | 2 +-
 .../pv-spinlock-enabled.x86_64-latest.args                | 2 +-
 10 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 3af6923e6f..c742838383 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -532,6 +532,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
               "nbd-bitmap",
               "x86-max-cpu",
               "cpu-unavailable-features",
+              "canonical-cpu-features",
     );
 
 
@@ -2892,7 +2893,9 @@ virQEMUCapsCPUFeatureTranslate(virQEMUCapsPtr qemuCaps,
     if (ARCH_IS_X86(qemuCaps->arch))
         table = virQEMUCapsCPUFeaturesX86;
 
-    if (!table || !feature)
+    if (!table ||
+        !feature ||
+        !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CANONICAL_CPU_FEATURES))
         return feature;
 
     for (entry = table; entry->libvirt; entry++) {
@@ -4398,6 +4401,9 @@ virQEMUCapsInitProcessCaps(virQEMUCapsPtr qemuCaps)
      * we are able to pass the custom 'device_id' for SCSI disks and cdroms. */
     if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_DEVICE_ID))
         virQEMUCapsClear(qemuCaps, QEMU_CAPS_BLOCKDEV);
+
+    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_UNAVAILABLE_FEATURES))
+        virQEMUCapsSet(qemuCaps, QEMU_CAPS_CANONICAL_CPU_FEATURES);
 }
 
 
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index e1727a1a1c..29f13ae977 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -514,6 +514,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */
     QEMU_CAPS_NBD_BITMAP, /* nbd-server-add supports bitmap */
     QEMU_CAPS_X86_MAX_CPU,/* max-x86_64-cpu type exists */
     QEMU_CAPS_CPU_UNAVAILABLE_FEATURES, /* "unavailable-features" CPU property */
+    QEMU_CAPS_CANONICAL_CPU_FEATURES, /* avoid CPU feature aliases */
 
     QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 08da30d10c..15f2990189 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7045,6 +7045,8 @@ qemuBuildCpuFeature(virQEMUCapsPtr qemuCaps,
                     const char *name,
                     bool state)
 {
+    name = virQEMUCapsCPUFeatureToQEMU(qemuCaps, name);
+
     if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION))
         virBufferAsprintf(buf, ",%s=%s", name, state ? "on" : "off");
     else
diff --git a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
index 24be3a546e..4cc91e677e 100644
--- a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
@@ -206,6 +206,7 @@
   <flag name='nbd-bitmap'/>
   <flag name='x86-max-cpu'/>
   <flag name='cpu-unavailable-features'/>
+  <flag name='canonical-cpu-features'/>
   <version>4000050</version>
   <kvmVersion>0</kvmVersion>
   <microcodeVersion>43100759</microcodeVersion>
diff --git a/tests/qemuxml2argvdata/cpu-translation.x86_64-latest.args b/tests/qemuxml2argvdata/cpu-translation.x86_64-latest.args
index 7a1546fb36..9322b826f4 100644
--- a/tests/qemuxml2argvdata/cpu-translation.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/cpu-translation.x86_64-latest.args
@@ -13,9 +13,9 @@ QEMU_AUDIO_DRV=none \
 -object secret,id=masterKey0,format=raw,\
 file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
 -machine pc,accel=tcg,usb=off,dump-guest-core=off \
--cpu Haswell,pclmuldq=on,ds_cpl=on,tsc_adjust=on,fxsr_opt=on,lahf_lm=on,\
-cmp_legacy=on,nodeid_msr=on,perfctr_core=on,perfctr_nb=on,kvm_pv_eoi=on,\
-kvm_pv_unhalt=on \
+-cpu Haswell,pclmulqdq=on,ds-cpl=on,tsc-adjust=on,fxsr-opt=on,lahf-lm=on,\
+cmp-legacy=on,nodeid-msr=on,perfctr-core=on,perfctr-nb=on,kvm-pv-eoi=on,\
+kvm-pv-unhalt=on \
 -m 214 \
 -overcommit mem-lock=off \
 -smp 1,sockets=1,cores=1,threads=1 \
diff --git a/tests/qemuxml2argvdata/eoi-disabled.x86_64-latest.args b/tests/qemuxml2argvdata/eoi-disabled.x86_64-latest.args
index caae868abf..f811931759 100644
--- a/tests/qemuxml2argvdata/eoi-disabled.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/eoi-disabled.x86_64-latest.args
@@ -13,7 +13,7 @@ QEMU_AUDIO_DRV=none \
 -object secret,id=masterKey0,format=raw,\
 file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
 -machine pc,accel=tcg,usb=off,dump-guest-core=off \
--cpu qemu32,kvm_pv_eoi=off \
+-cpu qemu32,kvm-pv-eoi=off \
 -m 214 \
 -overcommit mem-lock=off \
 -smp 6,sockets=6,cores=1,threads=1 \
diff --git a/tests/qemuxml2argvdata/eoi-enabled.x86_64-latest.args b/tests/qemuxml2argvdata/eoi-enabled.x86_64-latest.args
index abafafa411..25c03ae4d2 100644
--- a/tests/qemuxml2argvdata/eoi-enabled.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/eoi-enabled.x86_64-latest.args
@@ -13,7 +13,7 @@ QEMU_AUDIO_DRV=none \
 -object secret,id=masterKey0,format=raw,\
 file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
 -machine pc,accel=tcg,usb=off,dump-guest-core=off \
--cpu qemu32,kvm_pv_eoi=on \
+-cpu qemu32,kvm-pv-eoi=on \
 -m 214 \
 -overcommit mem-lock=off \
 -smp 6,sockets=6,cores=1,threads=1 \
diff --git a/tests/qemuxml2argvdata/kvmclock+eoi-disabled.x86_64-latest.args b/tests/qemuxml2argvdata/kvmclock+eoi-disabled.x86_64-latest.args
index 23d2bcb87e..f13573df8a 100644
--- a/tests/qemuxml2argvdata/kvmclock+eoi-disabled.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/kvmclock+eoi-disabled.x86_64-latest.args
@@ -13,7 +13,7 @@ QEMU_AUDIO_DRV=none \
 -object secret,id=masterKey0,format=raw,\
 file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
 -machine pc,accel=tcg,usb=off,dump-guest-core=off \
--cpu qemu32,kvmclock=off,kvm_pv_eoi=off \
+-cpu qemu32,kvmclock=off,kvm-pv-eoi=off \
 -m 214 \
 -overcommit mem-lock=off \
 -smp 6,sockets=6,cores=1,threads=1 \
diff --git a/tests/qemuxml2argvdata/pv-spinlock-disabled.x86_64-latest.args b/tests/qemuxml2argvdata/pv-spinlock-disabled.x86_64-latest.args
index b78533cf39..13fd7253a9 100644
--- a/tests/qemuxml2argvdata/pv-spinlock-disabled.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/pv-spinlock-disabled.x86_64-latest.args
@@ -13,7 +13,7 @@ QEMU_AUDIO_DRV=none \
 -object secret,id=masterKey0,format=raw,\
 file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
 -machine pc,accel=tcg,usb=off,dump-guest-core=off \
--cpu qemu32,kvm_pv_unhalt=off \
+-cpu qemu32,kvm-pv-unhalt=off \
 -m 214 \
 -overcommit mem-lock=off \
 -smp 6,sockets=6,cores=1,threads=1 \
diff --git a/tests/qemuxml2argvdata/pv-spinlock-enabled.x86_64-latest.args b/tests/qemuxml2argvdata/pv-spinlock-enabled.x86_64-latest.args
index a016e80016..919bbd037e 100644
--- a/tests/qemuxml2argvdata/pv-spinlock-enabled.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/pv-spinlock-enabled.x86_64-latest.args
@@ -13,7 +13,7 @@ QEMU_AUDIO_DRV=none \
 -object secret,id=masterKey0,format=raw,\
 file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
 -machine pc,accel=tcg,usb=off,dump-guest-core=off \
--cpu qemu32,kvm_pv_unhalt=on \
+-cpu qemu32,kvm-pv-unhalt=on \
 -m 214 \
 -overcommit mem-lock=off \
 -smp 6,sockets=6,cores=1,threads=1 \
-- 
2.22.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 15/24] qemu_command: Use canonical names of CPU features
Posted by Ján Tomko 6 years, 7 months ago
On Wed, Jun 19, 2019 at 11:38:12AM +0200, Jiri Denemark wrote:
>When building QEMU command line, we should use the preferred spelling of
>each CPU feature without relying on compatibility aliases (which may be
>removed at some point).
>
>The "unavailable-features" CPU property is used as a witness for the
>correct names of the features in our translation table.
>
>Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
>---
> src/qemu/qemu_capabilities.c                              | 8 +++++++-
> src/qemu/qemu_capabilities.h                              | 1 +
> src/qemu/qemu_command.c                                   | 2 ++
> tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml          | 1 +
> tests/qemuxml2argvdata/cpu-translation.x86_64-latest.args | 6 +++---
> tests/qemuxml2argvdata/eoi-disabled.x86_64-latest.args    | 2 +-
> tests/qemuxml2argvdata/eoi-enabled.x86_64-latest.args     | 2 +-
> .../kvmclock+eoi-disabled.x86_64-latest.args              | 2 +-
> .../pv-spinlock-disabled.x86_64-latest.args               | 2 +-
> .../pv-spinlock-enabled.x86_64-latest.args                | 2 +-
> 10 files changed, 19 insertions(+), 9 deletions(-)
>
>diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>index 3af6923e6f..c742838383 100644
>--- a/src/qemu/qemu_capabilities.c
>+++ b/src/qemu/qemu_capabilities.c
>@@ -532,6 +532,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
>               "nbd-bitmap",
>               "x86-max-cpu",
>               "cpu-unavailable-features",
>+              "canonical-cpu-features",
>     );
>
>
>@@ -2892,7 +2893,9 @@ virQEMUCapsCPUFeatureTranslate(virQEMUCapsPtr qemuCaps,
>     if (ARCH_IS_X86(qemuCaps->arch))
>         table = virQEMUCapsCPUFeaturesX86;
>
>-    if (!table || !feature)
>+    if (!table ||
>+        !feature ||
>+        !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CANONICAL_CPU_FEATURES))
>         return feature;
>
>     for (entry = table; entry->libvirt; entry++) {
>@@ -4398,6 +4401,9 @@ virQEMUCapsInitProcessCaps(virQEMUCapsPtr qemuCaps)
>      * we are able to pass the custom 'device_id' for SCSI disks and cdroms. */
>     if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_DEVICE_ID))
>         virQEMUCapsClear(qemuCaps, QEMU_CAPS_BLOCKDEV);
>+
>+    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_UNAVAILABLE_FEATURES))
>+        virQEMUCapsSet(qemuCaps, QEMU_CAPS_CANONICAL_CPU_FEATURES);
> }

Do we need an alias for the QEMU_CAPS_CPU_UNAVAILABLE_FEATURES
capability?

I think virQEMUCapsCPUFeatureTranslate can use QEMU_CAPS_CPU_UNAVAILABLE_FEATURES directly

>
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 15/24] qemu_command: Use canonical names of CPU features
Posted by Jiri Denemark 6 years, 7 months ago
On Wed, Jun 19, 2019 at 14:08:58 +0200, Ján Tomko wrote:
> On Wed, Jun 19, 2019 at 11:38:12AM +0200, Jiri Denemark wrote:
> >When building QEMU command line, we should use the preferred spelling of
> >each CPU feature without relying on compatibility aliases (which may be
> >removed at some point).
> >
> >The "unavailable-features" CPU property is used as a witness for the
> >correct names of the features in our translation table.
> >
> >Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> >---
> > src/qemu/qemu_capabilities.c                              | 8 +++++++-
> > src/qemu/qemu_capabilities.h                              | 1 +
> > src/qemu/qemu_command.c                                   | 2 ++
> > tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml          | 1 +
> > tests/qemuxml2argvdata/cpu-translation.x86_64-latest.args | 6 +++---
> > tests/qemuxml2argvdata/eoi-disabled.x86_64-latest.args    | 2 +-
> > tests/qemuxml2argvdata/eoi-enabled.x86_64-latest.args     | 2 +-
> > .../kvmclock+eoi-disabled.x86_64-latest.args              | 2 +-
> > .../pv-spinlock-disabled.x86_64-latest.args               | 2 +-
> > .../pv-spinlock-enabled.x86_64-latest.args                | 2 +-
> > 10 files changed, 19 insertions(+), 9 deletions(-)
> >
> >diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> >index 3af6923e6f..c742838383 100644
> >--- a/src/qemu/qemu_capabilities.c
> >+++ b/src/qemu/qemu_capabilities.c
> >@@ -532,6 +532,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
> >               "nbd-bitmap",
> >               "x86-max-cpu",
> >               "cpu-unavailable-features",
> >+              "canonical-cpu-features",
> >     );
> >
> >
> >@@ -2892,7 +2893,9 @@ virQEMUCapsCPUFeatureTranslate(virQEMUCapsPtr qemuCaps,
> >     if (ARCH_IS_X86(qemuCaps->arch))
> >         table = virQEMUCapsCPUFeaturesX86;
> >
> >-    if (!table || !feature)
> >+    if (!table ||
> >+        !feature ||
> >+        !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CANONICAL_CPU_FEATURES))
> >         return feature;
> >
> >     for (entry = table; entry->libvirt; entry++) {
> >@@ -4398,6 +4401,9 @@ virQEMUCapsInitProcessCaps(virQEMUCapsPtr qemuCaps)
> >      * we are able to pass the custom 'device_id' for SCSI disks and cdroms. */
> >     if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_DEVICE_ID))
> >         virQEMUCapsClear(qemuCaps, QEMU_CAPS_BLOCKDEV);
> >+
> >+    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_UNAVAILABLE_FEATURES))
> >+        virQEMUCapsSet(qemuCaps, QEMU_CAPS_CANONICAL_CPU_FEATURES);
> > }
> 
> Do we need an alias for the QEMU_CAPS_CPU_UNAVAILABLE_FEATURES
> capability?
> 
> I think virQEMUCapsCPUFeatureTranslate can use QEMU_CAPS_CPU_UNAVAILABLE_FEATURES directly

Both virQEMUCapsProbeQMPHostCPU and virQEMUCapsCPUFeatureTranslate could
use QEMU_CAPS_CPU_UNAVAILABLE_FEATURES directly, but I felt the alias
will make it clear what's going on in there since "unavailable-features"
property serves just as a witness and we'd likely need to add a comment
about this to every usage which is not doing anything with the new
property.

Jirka

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