On 02/15/2018 02:47 PM, Marek Marczykowski-Górecki wrote:
On Tue, Feb 13, 2018 at 09:02:35AM -0700, Jim Fehlig wrote:
On 02/08/2018 03:58 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.
But nevertheless this commit break some configurations that were working
before.
---
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/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 +-
10 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 8cced29..66956a7 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;
+ }
It looks like we never answered my question from V3, i.e. should we change
the default mode in the post-parse callback if NUMA or CPU features are
defined within <cpu>?
Hmm, but this means changing the config behind user's back, no?
Well, sort of. But it is really just making the implicit explicit.
You have disabled nested HVM, upgrade, now you have enabled.
Global switch is some consolation here, but is it enough?
I _think_ so. With a global default that disables nesting, are there any
scenarios you envision where a previously disabled nesting becomes enabled after
upgrade?
BTW, I'm fine with this patch, just playing devil's advocate with handling such
things post-parse vs domain start. It's not the only place where default xen
config is not reflected in libvirt :-/.
Regards,
Jim
It sure feels like such configuration tweaks and error checks should be
handled in the parsing phase, similar to qemuDomainDefVcpusPostParse() and
qemuDomainDefCPUPostParse() in the qemu driver. I think danpb is vaguely
familiar with this series and can help answer that question. Daniel, do you
have an opinion? Or a general comment about guidelines for checking/tweaking
config in parse phase vs domain start phase?
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
