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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxx
> 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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux