On Wed, Mar 21, 2018 at 05:55:28PM -0600, Jim Fehlig wrote:
> On 03/21/2018 10:32 AM, 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.
> >
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> > Reviewed-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> > ---
> > Changes since v4:
> > - add nested_hvm option to test_libvirtd_libxl.aug.in and libvirtd_libxl.aug
> > - make it possible to override nested_hvm=0 with explicit <feature
> > policy='require' name='vmx'/>
> > - split xenconfig changes into separate commits
> > 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/libvirtd_libxl.aug | 2 ++
> > src/libxl/libxl.conf | 8 ++++++++
> > src/libxl/libxl_conf.c | 12 +++++++++++-
> > src/libxl/libxl_conf.h | 2 ++
> > src/libxl/test_libvirtd_libxl.aug.in | 1 +
> > tests/libxlxml2domconfigtest.c | 3 +++
> > 6 files changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/libxl/libvirtd_libxl.aug b/src/libxl/libvirtd_libxl.aug
> > index b31cc07..58b9af3 100644
> > --- a/src/libxl/libvirtd_libxl.aug
> > +++ b/src/libxl/libvirtd_libxl.aug
> > @@ -28,12 +28,14 @@ module Libvirtd_libxl =
> > let lock_entry = str_entry "lock_manager"
> > let keepalive_interval_entry = int_entry "keepalive_interval"
> > let keepalive_count_entry = int_entry "keepalive_count"
> > + let nested_hvm_entry = bool_entry "nested_hvm"
> > (* Each entry in the config is one of the following ... *)
> > let entry = autoballoon_entry
> > | lock_entry
> > | keepalive_interval_entry
> > | keepalive_count_entry
> > + | nested_hvm_entry
> > let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ]
> > let empty = [ label "#empty" . eol ]
> > diff --git a/src/libxl/libxl.conf b/src/libxl/libxl.conf
> > index 264af7c..72825a7 100644
> > --- a/src/libxl/libxl.conf
> > +++ b/src/libxl/libxl.conf
> > @@ -41,3 +41,11 @@
> > #
> > #keepalive_interval = 5
> > #keepalive_count = 5
> > +
> > +# Nested HVM default 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. This can be overridden in domain configuration by
> > +# explicitly setting <feature policy='require' name='vmx'/> inside <cpu/>
> > +# element.
>
> Cool, the setting can be overridden by per-domain config.
>
> > +# By default it is disabled.
> > +#nested_hvm = 0
> > diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> > index dcfdd67..3b9e828 100644
> > --- a/src/libxl/libxl_conf.c
> > +++ b/src/libxl/libxl_conf.c
> > @@ -360,7 +360,9 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
> > bool hasHwVirt = false;
> > bool svm = false, vmx = false;
> > - if (ARCH_IS_X86(def->os.arch)) {
> > + /* enable nested HVM only if global nested_hvm option enable it and
> > + * host support 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;
>
> But IIUC this change will not allow per-domain config to override the global
> setting. If cfg->nested_hvm is false, svm and vmx are both false and
> FEATURE_REQUIRE is not honored.
Ough, conflict resolution went wrong after changing 3/9 :/
Fixed patch will follow.
> Regards,
> Jim
>
> > @@ -380,6 +382,11 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
> > case VIR_CPU_FEATURE_FORCE:
> > case VIR_CPU_FEATURE_REQUIRE:
> > + if ((vmx && STREQ(def->cpu->features[i].name, "vmx")) ||
> > + (svm && STREQ(def->cpu->features[i].name, "svm")))
> > + hasHwVirt = true;
> > + break;
> > +
> > case VIR_CPU_FEATURE_OPTIONAL:
> > case VIR_CPU_FEATURE_LAST:
> > break;
> > @@ -1699,6 +1706,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 ce9db26..27cfc1a 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/libxl/test_libvirtd_libxl.aug.in b/src/libxl/test_libvirtd_libxl.aug.in
> > index 63558e5..9106abe 100644
> > --- a/src/libxl/test_libvirtd_libxl.aug.in
> > +++ b/src/libxl/test_libvirtd_libxl.aug.in
> > @@ -6,3 +6,4 @@ module Test_libvirtd_libxl =
> > { "lock_manager" = "lockd" }
> > { "keepalive_interval" = "5" }
> > { "keepalive_count" = "5" }
> > +{ "nested_hvm" = "1" }
> > diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c
> > index cfbc37c..8819032 100644
> > --- a/tests/libxlxml2domconfigtest.c
> > +++ b/tests/libxlxml2domconfigtest.c
> > @@ -76,6 +76,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);
> >
>
--
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?
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
