On 02/08/2018 03:58 PM, Marek Marczykowski-Górecki wrote:
Introduce global libxl option for enabling nested HVM feature, similar
to kvm module parameter. This will prevent enabling experimental feature
by mere presence of <cpu mode='host-passthrough'> element in domain
config, unless explicitly enabled. <cpu mode='host-passthrough'> element
may be used to configure other features, like NUMA, or CPUID.
I don't think we should mix this change with the next two.
Also, adjust xenconfig driver to appropriately translate to/from
nestedhvm=1.
While at it, adjust xenconfig driver to not override def->cpu if already
set elsewhere. This will help with adding cpuid support.
These two look fine, but are actually separate bug fixes IMO. (This series keeps
uncovering all sorts of goodies.)
---
As for xenconfig part, I'm not sure if missing "nestedhvm" option in xl
config shouldn't produce <cpu> element too, to disable nested HVM (as it
would be on plain xl). Any preference?
Given the global option to disable nesting, I don't think we need to worry about
missing "nestedhvm". Even if xl.cfg contains "vnuma" and/or "cpuid", and hence
we have a def->cpu, the global setting would ensure nestedhvm is disabled. If
one were paranoid, <feature policy='disable' name='vmx|svm'/> could be added to
an existing def->cpu.
Changes since v3:
- use config option nested_hvm, instead of requiring explicit <feature
...> entries
- title changed from "libxl: do not enable nested HVM by mere presence
of <cpu> element"
- xenconfig: don't add <feature policy='force' name='vmx'/> since it is
implied by presence of <cpu> element
- xenconfig: produce <cpu> element even when converting on host not
supporting vmx/svm, to not lose setting value
Changes since v2:
- new patch
---
src/libxl/libxl.conf | 6 ++++++-
src/libxl/libxl_conf.c | 7 ++++++-
src/libxl/libxl_conf.h | 2 ++-
src/xenconfig/xen_xl.c | 37 +++++++++++------------------------
tests/libxlxml2domconfigtest.c | 3 +++-
5 files changed, 29 insertions(+), 26 deletions(-)
diff --git a/src/libxl/libxl.conf b/src/libxl/libxl.conf
index 264af7c..0e842c9 100644
--- a/src/libxl/libxl.conf
+++ b/src/libxl/libxl.conf
@@ -41,3 +41,9 @@
#
#keepalive_interval = 5
#keepalive_count = 5
+
+# Nested HVM global control. In order to use nested HVM feature, this option
+# needs to be enabled, in addition to specifying <cpu mode='host-passthrough'>
+# in domain configuration.
+# By default it is disabled.
+#nested_hvm = 0
I think per-domain settings should override this one. Users would find it odd
that they don't have vmx in their hvm guest with
<cpu mode='host-passthrough'>
<feature policy='require' name='vmx'/>
</cpu>
Regards,
Jim
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 66956a7..417ce7c 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -366,7 +366,9 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
return -1;
}
- if (ARCH_IS_X86(def->os.arch)) {
+ /* consider host support for nested HVM only if global nested_hvm
+ * option enable it */
+ if (cfg->nested_hvm && ARCH_IS_X86(def->os.arch)) {
vmx = virCPUCheckFeature(caps->host.arch, caps->host.cpu, "vmx");
svm = virCPUCheckFeature(caps->host.arch, caps->host.cpu, "svm");
hasHwVirt = vmx | svm;
@@ -1699,6 +1701,9 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg,
if (virConfGetValueUInt(conf, "keepalive_count", &cfg->keepAliveCount) < 0)
goto cleanup;
+ if (virConfGetValueBool(conf, "nested_hvm", &cfg->nested_hvm) < 0)
+ goto cleanup;
+
ret = 0;
cleanup:
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 8eefe06..e32a5e7 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -88,6 +88,8 @@ struct _libxlDriverConfig {
int keepAliveInterval;
unsigned int keepAliveCount;
+ bool nested_hvm;
+
/* Once created, caps are immutable */
virCapsPtr caps;
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 6cda305..394cc0d 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -170,17 +170,8 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
if (xenConfigGetBool(conf, "nestedhvm", &val, -1) < 0)
return -1;
- if (val == 1) {
- virCPUDefPtr cpu;
-
- if (VIR_ALLOC(cpu) < 0)
- return -1;
-
- cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
- cpu->type = VIR_CPU_TYPE_GUEST;
- def->cpu = cpu;
- } else if (val == 0) {
- const char *vtfeature = NULL;
+ if (val != -1) {
+ const char *vtfeature = "vmx";
if (caps && caps->host.cpu && ARCH_IS_X86(def->os.arch)) {
if (virCPUCheckFeature(caps->host.arch, caps->host.cpu, "vmx"))
@@ -189,28 +180,24 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
vtfeature = "svm";
}
- if (vtfeature) {
+ if (!def->cpu) {
virCPUDefPtr cpu;
-
if (VIR_ALLOC(cpu) < 0)
return -1;
- if (VIR_ALLOC(cpu->features) < 0) {
- VIR_FREE(cpu);
- return -1;
- }
-
- if (VIR_STRDUP(cpu->features->name, vtfeature) < 0) {
- VIR_FREE(cpu->features);
- VIR_FREE(cpu);
- return -1;
- }
- cpu->features->policy = VIR_CPU_FEATURE_DISABLE;
- cpu->nfeatures = cpu->nfeatures_max = 1;
cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
cpu->type = VIR_CPU_TYPE_GUEST;
+ cpu->nfeatures = 0;
+ cpu->nfeatures_max = 0;
def->cpu = cpu;
}
+
+ if (val == 0) {
+ if (virCPUDefAddFeature(def->cpu,
+ vtfeature,
+ VIR_CPU_FEATURE_DISABLE) < 0)
+ return -1;
+ }
}
} else {
if (xenConfigCopyStringOpt(conf, "bootloader", &def->os.bootloader) < 0)
diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c
index 0105550..f2af286 100644
--- a/tests/libxlxml2domconfigtest.c
+++ b/tests/libxlxml2domconfigtest.c
@@ -74,6 +74,9 @@ testCompareXMLToDomConfig(const char *xmlfile,
if (!(log = (xentoollog_logger *)xtl_createlogger_stdiostream(stderr, XTL_DEBUG, 0)))
goto cleanup;
+ /* for testing nested HVM */
+ cfg->nested_hvm = true;
+
/* replace logger with stderr one */
libxl_ctx_free(cfg->ctx);
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
