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

[libvirt] [PATCH v5 3/9] libxl: error out on not supported CPU mode, instead of silently ignoring

Marek Marczykowski-Górecki posted 9 patches 7 years, 2 months ago
There is a newer version of this series
[libvirt] [PATCH v5 3/9] libxl: error out on not supported CPU mode, instead of silently ignoring
Posted by Marek Marczykowski-Górecki 7 years, 2 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.
To not break existing configuration, add PostParse hook to update
(previously ignored) default mode 'custom' to 'host-passthrough'.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
Changes since v4:
 - add PostParse hook to automatically set host-passthrough mode, if
   was the default one before (custom)
Changes since v3:
 - fix vnuma tests (nested HVM implicitly enabled there)
Changes since v2:
 - change separated from 'libxl: add support for CPUID features policy'
---
 src/libxl/libxl_conf.c                                  | 10 ++++++++--
 src/libxl/libxl_domain.c                                |  5 +++++-
 src/xenconfig/xen_xl.c                                  |  1 +-
 tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg |  1 +-
 tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml |  2 +-
 tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg  |  1 +-
 tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml  |  2 +-
 tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg  |  1 +-
 tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml  |  2 +-
 tests/xlconfigdata/test-fullvirt-vnuma.cfg              |  1 +-
 tests/xlconfigdata/test-fullvirt-vnuma.xml              |  2 +-
 11 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index e7727a1..9301731 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -355,11 +355,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/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 8879481..98da68d 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -414,6 +414,11 @@ libxlDomainDefPostParse(virDomainDefPtr def,
             def->features[VIR_DOMAIN_FEATURE_ACPI] = VIR_TRISTATE_SWITCH_ON;
     }
 
+    /* set default CPU mode to host-passthrough, as the only one supported by
+     * the driver */
+    if (def->cpu && def->cpu->mode == VIR_CPU_MODE_CUSTOM)
+        def->cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
+
     return 0;
 }
 
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 2ef80eb..6cda305 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -494,6 +494,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.cfg b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg
index edba69a..2c9de44 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg
+++ b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg
@@ -22,5 +22,6 @@ parallel = "none"
 serial = "none"
 builder = "hvm"
 boot = "d"
+nestedhvm = 1
 vnuma = [ [ "pnode=0", "size=2048", "vcpus=0,11", "vdistances=10,21,31,41,51,61" ], [ "pnode=1", "size=2048", "vcpus=1,10", "vdistances=21,10,21,31,41,51" ], [ "pnode=2", "size=2048", "vcpus=2,9", "vdistances=31,21,10,21,31,41" ], [ "pnode=3", "size=2048", "vcpus=3,8", "vdistances=41,31,21,10,21,31" ], [ "pnode=4", "size=2048", "vcpus=4,7", "vdistances=51,41,31,21,10,21" ], [ "pnode=5", "size=2048", "vcpus=5-6", "vdistances=61,51,41,31,21,10" ] ]
 disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ]
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.cfg b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg
index 8186edf..ca8e5b0 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg
+++ b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg
@@ -22,5 +22,6 @@ parallel = "none"
 serial = "none"
 builder = "hvm"
 boot = "d"
+nestedhvm = 1
 vnuma = [ [ "pnode=0", "size=2048", "vcpus=0-1", "vdistances=10,20,20,20" ], [ "pnode=1", "size=2048", "vcpus=2-3", "vdistances=20,10,20,20" ], [ "pnode=2", "size=2048", "vcpus=4-5", "vdistances=20,20,10,20" ], [ "pnode=3", "size=2048", "vcpus=6-7", "vdistances=20,20,20,10" ] ]
 disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ]
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.cfg b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg
index 861a50e..46d5f90 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg
+++ b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg
@@ -22,5 +22,6 @@ parallel = "none"
 serial = "none"
 builder = "hvm"
 boot = "d"
+nestedhvm = 1
 vnuma = [ [ "pnode=0", "size=2048", "vcpus=0-1", "vdistances=10,21,31,41" ], [ "pnode=1", "size=2048", "vcpus=2-3", "vdistances=21,10,20,20" ], [ "pnode=2", "size=2048", "vcpus=4-5", "vdistances=31,20,10,20" ], [ "pnode=3", "size=2048", "vcpus=6-7", "vdistances=41,20,20,10" ] ]
 disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ]
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.cfg b/tests/xlconfigdata/test-fullvirt-vnuma.cfg
index 91e233a..813ad7f 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma.cfg
+++ b/tests/xlconfigdata/test-fullvirt-vnuma.cfg
@@ -22,5 +22,6 @@ parallel = "none"
 serial = "none"
 builder = "hvm"
 boot = "d"
+nestedhvm = 1
 vnuma = [ [ "pnode=0", "size=2048", "vcpus=0-1", "vdistances=10,21,31,41" ], [ "pnode=1", "size=2048", "vcpus=2-3", "vdistances=21,10,21,31" ], [ "pnode=2", "size=2048", "vcpus=4-5", "vdistances=31,21,10,21" ], [ "pnode=3", "size=2048", "vcpus=6-7", "vdistances=41,31,21,10" ] ]
 disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ]
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 v5 3/9] libxl: error out on not supported CPU mode, instead of silently ignoring
Posted by Daniel P. Berrangé 7 years, 1 month ago 
On Wed, Mar 14, 2018 at 03:26:10AM +0100, 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.
> To not break existing configuration, add PostParse hook to update
> (previously ignored) default mode 'custom' to 'host-passthrough'.

Maybe I'm missing something, but by doing this silent conversion
from custom -> host-passthrough, don't you make it such that the
error about unsupported CPU mode is largely unreachable ? IOW seems
to end up with the same functional result as the original code,
except for a error when 'host-model' is used.

I don't have a particular better idea though if we have alot of
pre-existing usage with mode=custom ?  Has this been widely
done, or can we justify breaking it ?

> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---
> Changes since v4:
>  - add PostParse hook to automatically set host-passthrough mode, if
>    was the default one before (custom)
> Changes since v3:
>  - fix vnuma tests (nested HVM implicitly enabled there)
> Changes since v2:
>  - change separated from 'libxl: add support for CPUID features policy'
> ---
>  src/libxl/libxl_conf.c                                  | 10 ++++++++--
>  src/libxl/libxl_domain.c                                |  5 +++++-
>  src/xenconfig/xen_xl.c                                  |  1 +-
>  tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg |  1 +-
>  tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml |  2 +-
>  tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg  |  1 +-
>  tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml  |  2 +-
>  tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg  |  1 +-
>  tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml  |  2 +-
>  tests/xlconfigdata/test-fullvirt-vnuma.cfg              |  1 +-
>  tests/xlconfigdata/test-fullvirt-vnuma.xml              |  2 +-
>  11 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index e7727a1..9301731 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -355,11 +355,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/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index 8879481..98da68d 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -414,6 +414,11 @@ libxlDomainDefPostParse(virDomainDefPtr def,
>              def->features[VIR_DOMAIN_FEATURE_ACPI] = VIR_TRISTATE_SWITCH_ON;
>      }
>  
> +    /* set default CPU mode to host-passthrough, as the only one supported by
> +     * the driver */
> +    if (def->cpu && def->cpu->mode == VIR_CPU_MODE_CUSTOM)
> +        def->cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
> +
>      return 0;
>  }
>  
> diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
> index 2ef80eb..6cda305 100644
> --- a/src/xenconfig/xen_xl.c
> +++ b/src/xenconfig/xen_xl.c
> @@ -494,6 +494,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.cfg b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg
> index edba69a..2c9de44 100644
> --- a/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg
> +++ b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg
> @@ -22,5 +22,6 @@ parallel = "none"
>  serial = "none"
>  builder = "hvm"
>  boot = "d"
> +nestedhvm = 1
>  vnuma = [ [ "pnode=0", "size=2048", "vcpus=0,11", "vdistances=10,21,31,41,51,61" ], [ "pnode=1", "size=2048", "vcpus=1,10", "vdistances=21,10,21,31,41,51" ], [ "pnode=2", "size=2048", "vcpus=2,9", "vdistances=31,21,10,21,31,41" ], [ "pnode=3", "size=2048", "vcpus=3,8", "vdistances=41,31,21,10,21,31" ], [ "pnode=4", "size=2048", "vcpus=4,7", "vdistances=51,41,31,21,10,21" ], [ "pnode=5", "size=2048", "vcpus=5-6", "vdistances=61,51,41,31,21,10" ] ]
>  disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ]
> 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.cfg b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg
> index 8186edf..ca8e5b0 100644
> --- a/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg
> +++ b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg
> @@ -22,5 +22,6 @@ parallel = "none"
>  serial = "none"
>  builder = "hvm"
>  boot = "d"
> +nestedhvm = 1
>  vnuma = [ [ "pnode=0", "size=2048", "vcpus=0-1", "vdistances=10,20,20,20" ], [ "pnode=1", "size=2048", "vcpus=2-3", "vdistances=20,10,20,20" ], [ "pnode=2", "size=2048", "vcpus=4-5", "vdistances=20,20,10,20" ], [ "pnode=3", "size=2048", "vcpus=6-7", "vdistances=20,20,20,10" ] ]
>  disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ]
> 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.cfg b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg
> index 861a50e..46d5f90 100644
> --- a/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg
> +++ b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg
> @@ -22,5 +22,6 @@ parallel = "none"
>  serial = "none"
>  builder = "hvm"
>  boot = "d"
> +nestedhvm = 1
>  vnuma = [ [ "pnode=0", "size=2048", "vcpus=0-1", "vdistances=10,21,31,41" ], [ "pnode=1", "size=2048", "vcpus=2-3", "vdistances=21,10,20,20" ], [ "pnode=2", "size=2048", "vcpus=4-5", "vdistances=31,20,10,20" ], [ "pnode=3", "size=2048", "vcpus=6-7", "vdistances=41,20,20,10" ] ]
>  disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ]
> 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.cfg b/tests/xlconfigdata/test-fullvirt-vnuma.cfg
> index 91e233a..813ad7f 100644
> --- a/tests/xlconfigdata/test-fullvirt-vnuma.cfg
> +++ b/tests/xlconfigdata/test-fullvirt-vnuma.cfg
> @@ -22,5 +22,6 @@ parallel = "none"
>  serial = "none"
>  builder = "hvm"
>  boot = "d"
> +nestedhvm = 1
>  vnuma = [ [ "pnode=0", "size=2048", "vcpus=0-1", "vdistances=10,21,31,41" ], [ "pnode=1", "size=2048", "vcpus=2-3", "vdistances=21,10,21,31" ], [ "pnode=2", "size=2048", "vcpus=4-5", "vdistances=31,21,10,21" ], [ "pnode=3", "size=2048", "vcpus=6-7", "vdistances=41,31,21,10" ] ]
>  disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ]
> 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

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 3/9] libxl: error out on not supported CPU mode, instead of silently ignoring
Posted by Marek Marczykowski-Górecki 7 years, 1 month ago 
On Fri, Mar 16, 2018 at 05:39:35PM +0000, Daniel P. Berrangé wrote:
> On Wed, Mar 14, 2018 at 03:26:10AM +0100, 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.
> > To not break existing configuration, add PostParse hook to update
> > (previously ignored) default mode 'custom' to 'host-passthrough'.
> 
> Maybe I'm missing something, but by doing this silent conversion
> from custom -> host-passthrough, don't you make it such that the
> error about unsupported CPU mode is largely unreachable ? IOW seems
> to end up with the same functional result as the original code,
> except for a error when 'host-model' is used.

Mostly - yes.

> I don't have a particular better idea though if we have alot of
> pre-existing usage with mode=custom ? 

Previously mode was mostly ignored (besides using it for enabling nested
HVM, which is now controlled additionally by global switch).

I'm not specifically attached to the PostParse, but I think it may be
nicer for users, to not throw an error on previously "valid"
configuration. If libvirt would have XML versioning, it could execute
such conversion only on upgrade...

Alternatively, instead of rejecting mode=custom, the whole cpuid
handling could be made conditional to mode=host-passthrough (and ignored
with mode=custom). I have an impression this would better fit into the
(libxl?) driver - I find it often that various settings are ignored
instead of throwing VIR_ERR_CONFIG_UNSUPPORTED... Admittedly, this
makes it easier to maintain a configuration compatible with wide range
of libvirt versions.

> Has this been widely
> done, or can we justify breaking it ?
>
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> > ---
> > Changes since v4:
> >  - add PostParse hook to automatically set host-passthrough mode, if
> >    was the default one before (custom)
> > Changes since v3:
> >  - fix vnuma tests (nested HVM implicitly enabled there)
> > Changes since v2:
> >  - change separated from 'libxl: add support for CPUID features policy'

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
--
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.