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>