1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH v3 0/6] Add setting CPU features (CPUID) with libxenlight driver.
  4. View patch

[libvirt] [PATCH v3 1/6] libxl: error out on not supported CPU mode, instead of silently ignoring

Marek Marczykowski-Górecki posted 6 patches 7 years, 5 months ago
There is a newer version of this series
[libvirt] [PATCH v3 1/6] libxl: error out on not supported CPU mode, instead of silently ignoring
Posted by Marek Marczykowski-Górecki 7 years, 5 months ago 
This change make libvirt XML with plain <cpu> element invalid for libxl,
which affect not only upcoming CPUID support, but also NUMA. In fact,
default mode 'custom' does not match what the driver actually does, so
it was a bug. Adjust xenconfig driver accordingly.

But nevertheless this commit break some configurations that were working
before.

---
Changes since v2:
 - change separated from 'libxl: add support for CPUID features policy'
---
 src/libxl/libxl_conf.c                                  | 10 ++++++++--
 src/xenconfig/xen_xl.c                                  |  1 +-
 tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml |  2 +-
 tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml  |  2 +-
 tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml  |  2 +-
 tests/xlconfigdata/test-fullvirt-vnuma.xml              |  2 +-
 6 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 970cff2..f39e783 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -353,11 +353,17 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
                           def->features[VIR_DOMAIN_FEATURE_ACPI] ==
                           VIR_TRISTATE_SWITCH_ON);
 
-        if (caps &&
-            def->cpu && def->cpu->mode == (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
+        if (caps && def->cpu) {
             bool hasHwVirt = false;
             bool svm = false, vmx = false;
 
+            if (def->cpu->mode != (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("unsupported cpu mode '%s'"),
+                               virCPUModeTypeToString(def->cpu->mode));
+                return -1;
+            }
+
             if (ARCH_IS_X86(def->os.arch)) {
                 vmx = virCPUCheckFeature(caps->host.arch, caps->host.cpu, "vmx");
                 svm = virCPUCheckFeature(caps->host.arch, caps->host.cpu, "svm");
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 532d667..9e239a7 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -492,6 +492,7 @@ xenParseXLVnuma(virConfPtr conf,
         goto cleanup;
     }
 
+    cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
     cpu->type = VIR_CPU_TYPE_GUEST;
     def->cpu = cpu;
 
diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml
index e3639eb..3c486ad 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml
+++ b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml
@@ -14,7 +14,7 @@
     <apic/>
     <pae/>
   </features>
-  <cpu>
+  <cpu mode='host-passthrough'>
     <numa>
       <cell id='0' cpus='0,11' memory='2097152' unit='KiB'>
         <distances>
diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml
index 9cab3ca..17c9ca5 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml
+++ b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml
@@ -14,7 +14,7 @@
     <apic/>
     <pae/>
   </features>
-  <cpu>
+  <cpu mode='host-passthrough'>
     <numa>
       <cell id='0' cpus='0-1' memory='2097152' unit='KiB'/>
       <cell id='1' cpus='2-3' memory='2097152' unit='KiB'/>
diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml
index 084b889..291fc37 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml
+++ b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml
@@ -14,7 +14,7 @@
     <apic/>
     <pae/>
   </features>
-  <cpu>
+  <cpu mode='host-passthrough'>
     <numa>
       <cell id='0' cpus='0-1' memory='2097152' unit='KiB'>
         <distances>
diff --git a/tests/xlconfigdata/test-fullvirt-vnuma.xml b/tests/xlconfigdata/test-fullvirt-vnuma.xml
index 5368b0d..9a9f495 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma.xml
+++ b/tests/xlconfigdata/test-fullvirt-vnuma.xml
@@ -14,7 +14,7 @@
     <apic/>
     <pae/>
   </features>
-  <cpu>
+  <cpu mode='host-passthrough'>
     <numa>
       <cell id='0' cpus='0-1' memory='2097152' unit='KiB'>
         <distances>
-- 
git-series 0.9.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 1/6] libxl: error out on not supported CPU mode, instead of silently ignoring
Posted by Jim Fehlig 7 years, 4 months ago 
On 12/09/2017 07:10 PM, Marek Marczykowski-Górecki wrote:
> This change make libvirt XML with plain <cpu> element invalid for libxl,
> which affect not only upcoming CPUID support, but also NUMA. In fact,
> default mode 'custom' does not match what the driver actually does, so
> it was a bug. Adjust xenconfig driver accordingly.

Should we change the default mode in the post-parse callback if NUMA or CPU 
features are defined within <cpu>? That would allow existing configs to continue 
working, although I doubt there are many since all of this is new to 3.10.0.

> But nevertheless this commit break some configurations that were working
> before.

I guess that is fine if we explicitly require mode='host-passthrough'.

> ---
> Changes since v2:
>   - change separated from 'libxl: add support for CPUID features policy'
> ---
>   src/libxl/libxl_conf.c                                  | 10 ++++++++--
>   src/xenconfig/xen_xl.c                                  |  1 +-
>   tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml |  2 +-
>   tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml  |  2 +-
>   tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml  |  2 +-
>   tests/xlconfigdata/test-fullvirt-vnuma.xml              |  2 +-
>   6 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 970cff2..f39e783 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -353,11 +353,17 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
>                             def->features[VIR_DOMAIN_FEATURE_ACPI] ==
>                             VIR_TRISTATE_SWITCH_ON);
>   
> -        if (caps &&
> -            def->cpu && def->cpu->mode == (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
> +        if (caps && def->cpu) {
>               bool hasHwVirt = false;
>               bool svm = false, vmx = false;
>   
> +            if (def->cpu->mode != (VIR_CPU_MODE_HOST_PASSTHROUGH)) {

While changing this the unneeded parens around VIR_CPU_MODE_HOST_PASSTHROUGH can 
be dropped.

Regards,
Jim

> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                               _("unsupported cpu mode '%s'"),
> +                               virCPUModeTypeToString(def->cpu->mode));
> +                return -1;
> +            }
> +
>               if (ARCH_IS_X86(def->os.arch)) {
>                   vmx = virCPUCheckFeature(caps->host.arch, caps->host.cpu, "vmx");
>                   svm = virCPUCheckFeature(caps->host.arch, caps->host.cpu, "svm");
> diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
> index 532d667..9e239a7 100644
> --- a/src/xenconfig/xen_xl.c
> +++ b/src/xenconfig/xen_xl.c
> @@ -492,6 +492,7 @@ xenParseXLVnuma(virConfPtr conf,
>           goto cleanup;
>       }
>   
> +    cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
>       cpu->type = VIR_CPU_TYPE_GUEST;
>       def->cpu = cpu;
>   
> diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml
> index e3639eb..3c486ad 100644
> --- a/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml
> +++ b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml
> @@ -14,7 +14,7 @@
>       <apic/>
>       <pae/>
>     </features>
> -  <cpu>
> +  <cpu mode='host-passthrough'>
>       <numa>
>         <cell id='0' cpus='0,11' memory='2097152' unit='KiB'>
>           <distances>
> diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml
> index 9cab3ca..17c9ca5 100644
> --- a/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml
> +++ b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml
> @@ -14,7 +14,7 @@
>       <apic/>
>       <pae/>
>     </features>
> -  <cpu>
> +  <cpu mode='host-passthrough'>
>       <numa>
>         <cell id='0' cpus='0-1' memory='2097152' unit='KiB'/>
>         <cell id='1' cpus='2-3' memory='2097152' unit='KiB'/>
> diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml
> index 084b889..291fc37 100644
> --- a/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml
> +++ b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml
> @@ -14,7 +14,7 @@
>       <apic/>
>       <pae/>
>     </features>
> -  <cpu>
> +  <cpu mode='host-passthrough'>
>       <numa>
>         <cell id='0' cpus='0-1' memory='2097152' unit='KiB'>
>           <distances>
> diff --git a/tests/xlconfigdata/test-fullvirt-vnuma.xml b/tests/xlconfigdata/test-fullvirt-vnuma.xml
> index 5368b0d..9a9f495 100644
> --- a/tests/xlconfigdata/test-fullvirt-vnuma.xml
> +++ b/tests/xlconfigdata/test-fullvirt-vnuma.xml
> @@ -14,7 +14,7 @@
>       <apic/>
>       <pae/>
>     </features>
> -  <cpu>
> +  <cpu mode='host-passthrough'>
>       <numa>
>         <cell id='0' cpus='0-1' memory='2097152' unit='KiB'>
>           <distances>
> 

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

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.