9:26 p.m.
Add support for CPUID setting based on <cpu> element. Since libxl format
support only adjusting specific bits over host CPU, only
mode='host-passthrough' is supported - other values are rejected (including
default 'custom'). This will break some configurations working before (bare
<cpu> element with for example NUMA configuration), but libxl driver never
supported full 'custom' mode - it was silently ignored, which might lead to
some unexpected effects.
Since mode='host-passthrough' is now necessary to specify CPU options, do not
enable nested HVM feature by mere presence of this element, require also
enabling it in libxl.conf. Nested HVM is still in "preview" state, so better be
explicit here.
v2 of this patch series:
https://www.redhat.com/archives/libvir-list/2017-July/msg00050.html
v3 of this patch series:
https://www.redhat.com/archives/libvir-list/2017-December/msg00314.html
v4 of this patch series:
https://www.redhat.com/archives/libvir-list/2018-February/msg00504.html
Marek Marczykowski-Górecki (9):
libxl: fix libxlDriverConfigDispose for partially constructed object
libxl: pass driver config to libxlMakeDomBuildInfo
libxl: error out on not supported CPU mode, instead of silently ignoring
libxl: do not enable nested HVM unless global nested_hvm option enabled
xenconfig: do not override def->cpu if already set elsewhere
libxl: add support for CPUID features policy
tests: check CPU features handling in libxl driver
xenconfig: add CPUID handling to domXML <-> xl.cfg conversion
tests: add test case for CPUID in xenconfig driver
src/libxl/libvirtd_libxl.aug | 2 +-
src/libxl/libxl.conf | 8 +-
src/libxl/libxl_conf.c | 70 ++-
src/libxl/libxl_conf.h | 6 +-
src/libxl/libxl_domain.c | 7 +-
src/libxl/test_libvirtd_libxl.aug.in | 1 +-
src/xenconfig/xen_xl.c | 238 ++++++++-
src/xenconfig/xen_xl.h | 2 +-
tests/libxlxml2domconfigdata/fullvirt-cpuid.json | 64 ++-
tests/libxlxml2domconfigdata/fullvirt-cpuid.xml | 37 +-
tests/libxlxml2domconfigtest.c | 27 +-
tests/virmocklibxl.c | 25 +-
tests/xlconfigdata/test-fullvirt-cpuid.cfg | 25 +-
tests/xlconfigdata/test-fullvirt-cpuid.xml | 35 +-
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 +-
tests/xlconfigtest.c | 1 +-
23 files changed, 509 insertions(+), 51 deletions(-)
create mode 100644 tests/libxlxml2domconfigdata/fullvirt-cpuid.json
create mode 100644 tests/libxlxml2domconfigdata/fullvirt-cpuid.xml
create mode 100644 tests/xlconfigdata/test-fullvirt-cpuid.cfg
create mode 100644 tests/xlconfigdata/test-fullvirt-cpuid.xml
base-commit: 6ce3acc129bfdbe7fd02bcb8bbe8af6d13903684
--
git-series 0.9.1
9:26 p.m.
New subject: [libvirt] [PATCH v5 1/9] libxl: fix libxlDriverConfigDispose for partially constructed object
libxlDriverConfigNew() use libxlDriverConfigDispose() for cleanup in
case of errors. Do not call libxlLoggerFree() on not allocated logger
(NULL).
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
Reviewed-by: Jim Fehlig <jfehlig(a)suse.com>
---
Changes since v3:
- new patch, mostly unrelated, but found while adjusting tests
---
src/libxl/libxl_conf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 970cff2..2d2a707 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -80,7 +80,8 @@ libxlDriverConfigDispose(void *obj)
virObjectUnref(cfg->caps);
libxl_ctx_free(cfg->ctx);
- libxlLoggerFree(cfg->logger);
+ if (cfg->logger)
+ libxlLoggerFree(cfg->logger);
VIR_FREE(cfg->configDir);
VIR_FREE(cfg->autostartDir);
--
git-series 0.9.1
12:34 p.m.
New subject: [libvirt] [PATCH v5 1/9] libxl: fix libxlDriverConfigDispose for partially constructed object
On Wed, Mar 14, 2018 at 03:26:08AM +0100, Marek Marczykowski-Górecki wrote:
libxlDriverConfigNew() use libxlDriverConfigDispose() for cleanup in
case of errors. Do not call libxlLoggerFree() on not allocated logger
(NULL).
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
Reviewed-by: Jim Fehlig <jfehlig(a)suse.com>
---
Changes since v3:
- new patch, mostly unrelated, but found while adjusting tests
---
src/libxl/libxl_conf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 970cff2..2d2a707 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -80,7 +80,8 @@ libxlDriverConfigDispose(void *obj)
virObjectUnref(cfg->caps);
libxl_ctx_free(cfg->ctx);
- libxlLoggerFree(cfg->logger);
+ if (cfg->logger)
+ libxlLoggerFree(cfg->logger);
VIR_FREE(cfg->configDir);
VIR_FREE(cfg->autostartDir);
--
git-series 0.9.1
--
libvir-list mailing list
libvir-list(a)redhat.com
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 :|
9:26 p.m.
New subject: [libvirt] [PATCH v5 2/9] libxl: pass driver config to libxlMakeDomBuildInfo
Preparation for global nestedhvm configuration - libxlMakeDomBuildInfo
needs access to libxlDriverConfig.
No functional change.
Adjusting tests require slightly more mockup functions, because of
libxlDriverConfigNew() call.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
Changes since v4:
- drop now unneeded parameters
Changes since v3:
- new patch, preparation
---
src/libxl/libxl_conf.c | 13 +++++++------
src/libxl/libxl_conf.h | 4 +---
src/libxl/libxl_domain.c | 2 +-
tests/libxlxml2domconfigtest.c | 23 ++++++++++++++++-------
tests/virmocklibxl.c | 25 +++++++++++++++++++++++++
5 files changed, 50 insertions(+), 17 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 2d2a707..e7727a1 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -271,10 +271,11 @@ libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf)
static int
libxlMakeDomBuildInfo(virDomainDefPtr def,
- libxl_ctx *ctx,
+ libxlDriverConfigPtr cfg,
virCapsPtr caps,
libxl_domain_config *d_config)
{
+ libxl_ctx *ctx = cfg->ctx;
libxl_domain_build_info *b_info = &d_config->b_info;
int hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM;
size_t i;
@@ -2287,17 +2288,17 @@ libxlDriverNodeGetInfo(libxlDriverPrivatePtr driver,
virNodeInfoPtr info)
int
libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
virDomainDefPtr def,
- const char *channelDir LIBXL_ATTR_UNUSED,
- libxl_ctx *ctx,
- virCapsPtr caps,
+ libxlDriverConfigPtr cfg,
libxl_domain_config *d_config)
{
+ virCapsPtr caps = cfg->caps;
+ libxl_ctx *ctx = cfg->ctx;
libxl_domain_config_init(d_config);
if (libxlMakeDomCreateInfo(ctx, def, &d_config->c_info) < 0)
return -1;
- if (libxlMakeDomBuildInfo(def, ctx, caps, d_config) < 0)
+ if (libxlMakeDomBuildInfo(def, cfg, caps, d_config) < 0)
return -1;
#ifdef LIBXL_HAVE_VNUMA
@@ -2329,7 +2330,7 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
#endif
#ifdef LIBXL_HAVE_DEVICE_CHANNEL
- if (libxlMakeChannelList(channelDir, def, d_config) < 0)
+ if (libxlMakeChannelList(cfg->channelDir, def, d_config) < 0)
return -1;
#endif
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 264df11..ce9db26 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -215,9 +215,7 @@ libxlCreateXMLConf(void);
int
libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
virDomainDefPtr def,
- const char *channelDir LIBXL_ATTR_UNUSED,
- libxl_ctx *ctx,
- virCapsPtr caps,
+ libxlDriverConfigPtr cfg,
libxl_domain_config *d_config);
static inline void
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 395c8a9..8879481 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1253,7 +1253,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
goto cleanup_dom;
if (libxlBuildDomainConfig(driver->reservedGraphicsPorts, vm->def,
- cfg->channelDir, cfg->ctx, cfg->caps,
&d_config) < 0)
+ cfg, &d_config) < 0)
goto cleanup_dom;
if (cfg->autoballoon && libxlDomainFreeMem(cfg->ctx, &d_config)
< 0)
diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c
index bd4c3af..cfbc37c 100644
--- a/tests/libxlxml2domconfigtest.c
+++ b/tests/libxlxml2domconfigtest.c
@@ -56,8 +56,8 @@ testCompareXMLToDomConfig(const char *xmlfile,
int ret = -1;
libxl_domain_config actualconfig;
libxl_domain_config expectconfig;
+ libxlDriverConfigPtr cfg;
xentoollog_logger *log = NULL;
- libxl_ctx *ctx = NULL;
virPortAllocatorPtr gports = NULL;
virDomainXMLOptionPtr xmlopt = NULL;
virDomainDefPtr vmdef = NULL;
@@ -68,10 +68,18 @@ testCompareXMLToDomConfig(const char *xmlfile,
libxl_domain_config_init(&actualconfig);
libxl_domain_config_init(&expectconfig);
+ if (!(cfg = libxlDriverConfigNew()))
+ goto cleanup;
+
+ cfg->caps = caps;
+
if (!(log = (xentoollog_logger *)xtl_createlogger_stdiostream(stderr, XTL_DEBUG,
0)))
goto cleanup;
- if (libxl_ctx_alloc(&ctx, LIBXL_VERSION, 0, log) < 0)
+ /* replace logger with stderr one */
+ libxl_ctx_free(cfg->ctx);
+
+ if (libxl_ctx_alloc(&cfg->ctx, LIBXL_VERSION, 0, log) < 0)
goto cleanup;
if (!(gports = virPortAllocatorNew("vnc", 5900, 6000,
@@ -85,22 +93,22 @@ testCompareXMLToDomConfig(const char *xmlfile,
NULL, VIR_DOMAIN_XML_INACTIVE)))
goto cleanup;
- if (libxlBuildDomainConfig(gports, vmdef, NULL, ctx, caps, &actualconfig) <
0)
+ if (libxlBuildDomainConfig(gports, vmdef, cfg, &actualconfig) < 0)
goto cleanup;
- if (!(actualjson = libxl_domain_config_to_json(ctx, &actualconfig))) {
+ if (!(actualjson = libxl_domain_config_to_json(cfg->ctx, &actualconfig))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
"Failed to retrieve JSON doc for libxl_domain_config");
goto cleanup;
}
virTestLoadFile(jsonfile, &tempjson);
- if (libxl_domain_config_from_json(ctx, &expectconfig, tempjson) != 0) {
+ if (libxl_domain_config_from_json(cfg->ctx, &expectconfig, tempjson) != 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
"Failed to create libxl_domain_config from JSON doc");
goto cleanup;
}
- if (!(expectjson = libxl_domain_config_to_json(ctx, &expectconfig))) {
+ if (!(expectjson = libxl_domain_config_to_json(cfg->ctx, &expectconfig))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
"Failed to retrieve JSON doc for libxl_domain_config");
goto cleanup;
@@ -118,10 +126,11 @@ testCompareXMLToDomConfig(const char *xmlfile,
virDomainDefFree(vmdef);
virObjectUnref(gports);
virObjectUnref(xmlopt);
- libxl_ctx_free(ctx);
libxl_domain_config_dispose(&actualconfig);
libxl_domain_config_dispose(&expectconfig);
xtl_logger_destroy(log);
+ cfg->caps = NULL;
+ virObjectUnref(cfg);
return ret;
}
diff --git a/tests/virmocklibxl.c b/tests/virmocklibxl.c
index 747f9f8..133064b 100644
--- a/tests/virmocklibxl.c
+++ b/tests/virmocklibxl.c
@@ -27,6 +27,7 @@
# include <sys/stat.h>
# include <unistd.h>
# include <libxl.h>
+# include <util/virfile.h>
# include <xenstore.h>
# include <xenctrl.h>
@@ -48,6 +49,19 @@ VIR_MOCK_IMPL_RET_ARGS(xc_interface_open,
}
+VIR_MOCK_IMPL_RET_ARGS(libxl_get_version_info,
+ const libxl_version_info*,
+ libxl_ctx *, ctx)
+{
+ static libxl_version_info info;
+
+ memset(&info, 0, sizeof(info));
+
+ return &info;
+ /* silence gcc warning */
+ return real_libxl_get_version_info(ctx);
+}
+
VIR_MOCK_STUB_RET_ARGS(xc_interface_close,
int, 0,
xc_interface *, handle)
@@ -68,6 +82,17 @@ VIR_MOCK_STUB_RET_ARGS(xc_sharing_used_frames,
VIR_MOCK_STUB_VOID_ARGS(xs_daemon_close,
struct xs_handle *, handle)
+VIR_MOCK_IMPL_RET_ARGS(virFileMakePath, int,
+ const char *, path)
+{
+ /* replace log path with a writable directory */
+ if (strstr(path, "/log/")) {
+ snprintf((char*)path, strlen(path), ".");
+ return 0;
+ }
+ return real_virFileMakePath(path);
+}
+
VIR_MOCK_IMPL_RET_ARGS(__xstat, int,
int, ver,
const char *, path,
--
git-series 0.9.1
12:36 p.m.
New subject: [libvirt] [PATCH v5 2/9] libxl: pass driver config to libxlMakeDomBuildInfo
On Wed, Mar 14, 2018 at 03:26:09AM +0100, Marek Marczykowski-Górecki wrote:
Preparation for global nestedhvm configuration -
libxlMakeDomBuildInfo
needs access to libxlDriverConfig.
No functional change.
Adjusting tests require slightly more mockup functions, because of
libxlDriverConfigNew() call.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
Changes since v4:
- drop now unneeded parameters
Changes since v3:
- new patch, preparation
---
src/libxl/libxl_conf.c | 13 +++++++------
src/libxl/libxl_conf.h | 4 +---
src/libxl/libxl_domain.c | 2 +-
tests/libxlxml2domconfigtest.c | 23 ++++++++++++++++-------
tests/virmocklibxl.c | 25 +++++++++++++++++++++++++
5 files changed, 50 insertions(+), 17 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
Though one question...
@@ -48,6 +49,19 @@ VIR_MOCK_IMPL_RET_ARGS(xc_interface_open,
}
+VIR_MOCK_IMPL_RET_ARGS(libxl_get_version_info,
+ const libxl_version_info*,
+ libxl_ctx *, ctx)
+{
+ static libxl_version_info info;
+
+ memset(&info, 0, sizeof(info));
+
+ return &info;
+ /* silence gcc warning */
+ return real_libxl_get_version_info(ctx);
Why was gcc warning about that requires the second return
statement ? I would have though this would /cause/ a
warning by creating unreachable code ?
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 :|
2:38 p.m.
New subject: [libvirt] [PATCH v5 2/9] libxl: pass driver config to libxlMakeDomBuildInfo
On Fri, Mar 16, 2018 at 05:36:44PM +0000, Daniel P. Berrangé wrote:
On Wed, Mar 14, 2018 at 03:26:09AM +0100, Marek Marczykowski-Górecki
wrote:
> @@ -48,6 +49,19 @@ VIR_MOCK_IMPL_RET_ARGS(xc_interface_open,
> }
>
>
> +VIR_MOCK_IMPL_RET_ARGS(libxl_get_version_info,
> + const libxl_version_info*,
> + libxl_ctx *, ctx)
> +{
> + static libxl_version_info info;
> +
> + memset(&info, 0, sizeof(info));
> +
> + return &info;
> + /* silence gcc warning */
> + return real_libxl_get_version_info(ctx);
Why was gcc warning about that requires the second return
statement ? I would have though this would /cause/ a
warning by creating unreachable code ?
Because or static real_##name in (unused otherwise):
# define VIR_MOCK_IMPL_RET_ARGS(name, rettype, ...) \
rettype name(VIR_MOCK_ARGTYPENAMES(__VA_ARGS__)); \
static rettype (*real_##name)(VIR_MOCK_ARGTYPES(__VA_ARGS__)); \
rettype name(VIR_MOCK_ARGTYPENAMES_UNUSED(__VA_ARGS__))
--
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?
9:26 p.m.
New subject: [libvirt] [PATCH v5 3/9] libxl: error out on not supported CPU mode, instead of silently ignoring
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'.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
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
12:39 p.m.
New subject: [libvirt] [PATCH v5 3/9] libxl: error out on not supported CPU mode, instead of silently ignoring
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(a)invisiblethingslab.com>
---
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(a)redhat.com
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 :|
3:05 p.m.
New subject: [libvirt] [PATCH v5 3/9] libxl: error out on not supported CPU mode, instead of silently ignoring
On Fri, Mar 16, 2018 at 05:39:35PM +0000, Daniel P. Berrangé wrote:
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.
Mostly - yes.
I don't have a particular better idea though if we have alot of
pre-existing usage with mode=custom ?
Previously mode was mostly ignored (besides using it for enabling nested
HVM, which is now controlled additionally by global switch).
I'm not specifically attached to the PostParse, but I think it may be
nicer for users, to not throw an error on previously "valid"
configuration. If libvirt would have XML versioning, it could execute
such conversion only on upgrade...
Alternatively, instead of rejecting mode=custom, the whole cpuid
handling could be made conditional to mode=host-passthrough (and ignored
with mode=custom). I have an impression this would better fit into the
(libxl?) driver - I find it often that various settings are ignored
instead of throwing VIR_ERR_CONFIG_UNSUPPORTED... Admittedly, this
makes it easier to maintain a configuration compatible with wide range
of libvirt versions.
Has this been widely
done, or can we justify breaking it ?
> Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
> ---
> 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'
--
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?
9:26 p.m.
New subject: [libvirt] [PATCH v5 4/9] libxl: do not enable nested HVM unless global nested_hvm option enabled
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(a)invisiblethingslab.com>
---
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.
+# By default it is disabled.
+#nested_hvm = 0
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 9301731..09264ce 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)) {
+ /* 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;
@@ -386,6 +388,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);
--
git-series 0.9.1
12:40 p.m.
New subject: [libvirt] [PATCH v5 4/9] libxl: do not enable nested HVM unless global nested_hvm option enabled
On Wed, Mar 14, 2018 at 03:26:11AM +0100, 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(a)invisiblethingslab.com>
---
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(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
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 :|
9:26 p.m.
New subject: [libvirt] [PATCH v5 5/9] xenconfig: do not override def->cpu if already set elsewhere
This will help with adding cpuid support.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
Changes since v4:
- patch separated from "libxl: do not enable nested HVM unless global
nested_hvm option enabled"
---
src/xenconfig/xen_xl.c | 37 ++++++++++++-------------------------
1 file changed, 12 insertions(+), 25 deletions(-)
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)
--
git-series 0.9.1
12:40 p.m.
New subject: [libvirt] [PATCH v5 5/9] xenconfig: do not override def->cpu if already set elsewhere
On Wed, Mar 14, 2018 at 03:26:12AM +0100, Marek Marczykowski-Górecki wrote:
This will help with adding cpuid support.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
Changes since v4:
- patch separated from "libxl: do not enable nested HVM unless global
nested_hvm option enabled"
---
src/xenconfig/xen_xl.c | 37 ++++++++++++-------------------------
1 file changed, 12 insertions(+), 25 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
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 :|
9:26 p.m.
New subject: [libvirt] [PATCH v5 6/9] libxl: add support for CPUID features policy
Convert CPU features policy into libxl cpuid policy settings. Use new
("libxl") syntax, which allow to enable/disable specific bits, using
host CPU as a base. For this reason, only "host-passthrough" mode is
accepted.
Libxl do not have distinction between "force" and "required" policy
(there is only "force") and also between "forbid" and
"disable" (there
is only "disable"). So, merge them appropriately. If anything,
"require"
and "forbid" should be enforced outside of specific driver.
Nested HVM (vmx and svm features) is handled separately, so exclude it
from translation.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
Changes since v4:
- added spec-ctrl/ibrsb
Changes since v2:
- drop spurious changes
- move libxlTranslateCPUFeature function to xen_xl.c, to be reused by
xenconfig driver
Changes since v1:
- use new ("libxl") syntax to set only bits explicitly mentioned in
domain XML
---
src/libxl/libxl_conf.c | 36 +++++++++++++++++++++++++++++++++---
src/xenconfig/xen_xl.c | 35 +++++++++++++++++++++++++++++++++++
src/xenconfig/xen_xl.h | 2 ++
3 files changed, 70 insertions(+), 3 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 09264ce..c3ed7c2 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -50,6 +50,7 @@
#include "secret_util.h"
#include "cpu/cpu.h"
#include "xen_common.h"
+#include "xen_xl.h"
#define VIR_FROM_THIS VIR_FROM_LIBXL
@@ -358,6 +359,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
if (caps && def->cpu) {
bool hasHwVirt = false;
bool svm = false, vmx = false;
+ char xlCPU[32];
if (def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -382,17 +384,45 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
case VIR_CPU_FEATURE_DISABLE:
case VIR_CPU_FEATURE_FORBID:
if ((vmx && STREQ(def->cpu->features[i].name,
"vmx")) ||
- (svm && STREQ(def->cpu->features[i].name,
"svm")))
+ (svm && STREQ(def->cpu->features[i].name,
"svm"))) {
hasHwVirt = false;
+ continue;
+ }
+
+ snprintf(xlCPU,
+ sizeof(xlCPU),
+ "%s=0",
+ xenTranslateCPUFeature(
+ def->cpu->features[i].name,
+ false));
+ if (libxl_cpuid_parse_config(&b_info->cpuid, xlCPU))
{
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported cpu feature
'%s'"),
+ def->cpu->features[i].name);
+ return -1;
+ }
break;
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")))
+ (svm && STREQ(def->cpu->features[i].name,
"svm"))) {
hasHwVirt = true;
+ continue;
+ }
+
+ snprintf(xlCPU,
+ sizeof(xlCPU),
+ "%s=1",
+ xenTranslateCPUFeature(
+ def->cpu->features[i].name, false));
+ if (libxl_cpuid_parse_config(&b_info->cpuid, xlCPU))
{
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported cpu feature
'%s'"),
+ def->cpu->features[i].name);
+ return -1;
+ }
break;
-
case VIR_CPU_FEATURE_OPTIONAL:
case VIR_CPU_FEATURE_LAST:
break;
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 394cc0d..3bbc5d8 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -218,6 +218,41 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
return 0;
}
+/*
+ * Translate CPU feature name from libvirt to libxl (from_libxl=false) or from
+ * libxl to libvirt (from_libxl=true).
+ */
+const char *
+xenTranslateCPUFeature(const char *feature_name, bool from_libxl)
+{
+ static const char *translation_table[][2] = {
+ /* libvirt name, libxl name */
+ { "cx16", "cmpxchg16" },
+ { "cid", "cntxid" },
+ { "ds_cpl", "dscpl" },
+ { "pclmuldq", "pclmulqdq" },
+ { "pni", "sse3" },
+ { "ht", "htt" },
+ { "pn", "psn" },
+ { "clflush", "clfsh" },
+ { "sep", "sysenter" },
+ { "cx8", "cmpxchg8" },
+ { "nodeid_msr", "nodeid" },
+ { "cr8legacy", "altmovcr8" },
+ { "lahf_lm", "lahfsahf" },
+ { "cmp_legacy", "cmplegacy" },
+ { "fxsr_opt", "ffxsr" },
+ { "pdpe1gb", "page1gb" },
+ { "spec-ctrl", "ibrsb" },
+ };
+ size_t i;
+
+ for (i = 0; i < ARRAY_CARDINALITY(translation_table); i++)
+ if (STREQ(translation_table[i][from_libxl], feature_name))
+ return translation_table[i][!from_libxl];
+ return feature_name;
+}
+
static int
xenParseXLSpice(virConfPtr conf, virDomainDefPtr def)
diff --git a/src/xenconfig/xen_xl.h b/src/xenconfig/xen_xl.h
index dd96326..68f332a 100644
--- a/src/xenconfig/xen_xl.h
+++ b/src/xenconfig/xen_xl.h
@@ -33,4 +33,6 @@ virDomainDefPtr xenParseXL(virConfPtr conn,
virConfPtr xenFormatXL(virDomainDefPtr def, virConnectPtr);
+const char *xenTranslateCPUFeature(const char *feature_name, bool from_libxl);
+
#endif /* __VIR_XEN_XL_H__ */
--
git-series 0.9.1
12:42 p.m.
New subject: [libvirt] [PATCH v5 6/9] libxl: add support for CPUID features policy
On Wed, Mar 14, 2018 at 03:26:13AM +0100, Marek Marczykowski-Górecki wrote:
Convert CPU features policy into libxl cpuid policy settings. Use
new
("libxl") syntax, which allow to enable/disable specific bits, using
host CPU as a base. For this reason, only "host-passthrough" mode is
accepted.
Libxl do not have distinction between "force" and "required" policy
(there is only "force") and also between "forbid" and
"disable" (there
is only "disable"). So, merge them appropriately. If anything,
"require"
and "forbid" should be enforced outside of specific driver.
Nested HVM (vmx and svm features) is handled separately, so exclude it
from translation.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
Changes since v4:
- added spec-ctrl/ibrsb
Changes since v2:
- drop spurious changes
- move libxlTranslateCPUFeature function to xen_xl.c, to be reused by
xenconfig driver
Changes since v1:
- use new ("libxl") syntax to set only bits explicitly mentioned in
domain XML
---
src/libxl/libxl_conf.c | 36 +++++++++++++++++++++++++++++++++---
src/xenconfig/xen_xl.c | 35 +++++++++++++++++++++++++++++++++++
src/xenconfig/xen_xl.h | 2 ++
3 files changed, 70 insertions(+), 3 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
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 :|
9:26 p.m.
New subject: [libvirt] [PATCH v5 7/9] tests: check CPU features handling in libxl driver
Test enabling/disabling individual CPU features and also setting
nested HVM support, which is also controlled by CPU features node.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
Reviewed-by: Jim Fehlig <jfehlig(a)suse.com>
---
Changes since v3:
- adjust for modified nested HVM handling
Changes since v1:
- rewritten to Jim's test suite for libxl_domain_config generator
---
tests/libxlxml2domconfigdata/fullvirt-cpuid.json | 64 +++++++++++++++++-
tests/libxlxml2domconfigdata/fullvirt-cpuid.xml | 37 ++++++++++-
tests/libxlxml2domconfigtest.c | 1 +-
3 files changed, 102 insertions(+)
create mode 100644 tests/libxlxml2domconfigdata/fullvirt-cpuid.json
create mode 100644 tests/libxlxml2domconfigdata/fullvirt-cpuid.xml
diff --git a/tests/libxlxml2domconfigdata/fullvirt-cpuid.json
b/tests/libxlxml2domconfigdata/fullvirt-cpuid.json
new file mode 100644
index 0000000..28037be
--- /dev/null
+++ b/tests/libxlxml2domconfigdata/fullvirt-cpuid.json
@@ -0,0 +1,64 @@
+{
+ "c_info": {
+ "type": "hvm",
+ "name": "XenGuest2",
+ "uuid": "c7a5fdb2-cdaf-9455-926a-d65c16db1809"
+ },
+ "b_info": {
+ "max_vcpus": 1,
+ "avail_vcpus": [
+ 0
+ ],
+ "max_memkb": 592896,
+ "target_memkb": 403456,
+ "video_memkb": 8192,
+ "shadow_memkb": 5656,
+ "cpuid": [
+ {
+ "leaf": 1,
+ "ecx": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx0",
+ "edx": "xxxxxxxxxxxxxxxxxxxxxxxxxxx1xxxx"
+ }
+ ],
+ "sched_params": {
+ "weight": 1000
+ },
+ "type.hvm": {
+ "pae": "True",
+ "apic": "True",
+ "acpi": "True",
+ "vga": {
+
+ },
+ "nested_hvm": "False",
+ "vnc": {
+ "enable": "False"
+ },
+ "sdl": {
+ "enable": "False"
+ },
+ "spice": {
+
+ },
+ "boot": "c",
+ "rdm": {
+
+ }
+ },
+ "arch_arm": {
+
+ }
+ },
+ "disks": [
+ {
+ "pdev_path": "/dev/HostVG/XenGuest2",
+ "vdev": "hda",
+ "backend": "phy",
+ "format": "raw",
+ "removable": 1,
+ "readwrite": 1
+ }
+ ],
+ "on_reboot": "restart",
+ "on_crash": "restart"
+}
diff --git a/tests/libxlxml2domconfigdata/fullvirt-cpuid.xml
b/tests/libxlxml2domconfigdata/fullvirt-cpuid.xml
new file mode 100644
index 0000000..e9eba2e
--- /dev/null
+++ b/tests/libxlxml2domconfigdata/fullvirt-cpuid.xml
@@ -0,0 +1,37 @@
+<domain type='xen'>
+ <name>XenGuest2</name>
+ <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>592896</memory>
+ <currentMemory unit='KiB'>403456</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='xenfv'>hvm</type>
+ </os>
+ <features>
+ <acpi/>
+ <apic/>
+ <pae/>
+ </features>
+ <cpu mode='host-passthrough'>
+ <feature policy='forbid' name='pni'/>
+ <feature policy='forbid' name='vmx'/>
+ <feature policy='require' name='tsc'/>
+ </cpu>
+ <clock offset='variable' adjustment='0' basis='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>restart</on_crash>
+ <devices>
+ <disk type='block' device='disk'>
+ <driver name='phy' type='raw'/>
+ <source dev='/dev/HostVG/XenGuest2'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <video>
+ <model type='cirrus' vram='8192' heads='1'
primary='yes'/>
+ </video>
+ </devices>
+</domain>
diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c
index 8819032..10e1bb2 100644
--- a/tests/libxlxml2domconfigtest.c
+++ b/tests/libxlxml2domconfigtest.c
@@ -203,6 +203,7 @@ mymain(void)
DO_TEST("moredevs-hvm");
DO_TEST("vnuma-hvm");
DO_TEST("multiple-ip");
+ DO_TEST("fullvirt-cpuid");
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
git-series 0.9.1
12:44 p.m.
New subject: [libvirt] [PATCH v5 7/9] tests: check CPU features handling in libxl driver
On Wed, Mar 14, 2018 at 03:26:14AM +0100, Marek Marczykowski-Górecki wrote:
Test enabling/disabling individual CPU features and also setting
nested HVM support, which is also controlled by CPU features node.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
Reviewed-by: Jim Fehlig <jfehlig(a)suse.com>
---
Changes since v3:
- adjust for modified nested HVM handling
Changes since v1:
- rewritten to Jim's test suite for libxl_domain_config generator
---
tests/libxlxml2domconfigdata/fullvirt-cpuid.json | 64 +++++++++++++++++-
tests/libxlxml2domconfigdata/fullvirt-cpuid.xml | 37 ++++++++++-
tests/libxlxml2domconfigtest.c | 1 +-
3 files changed, 102 insertions(+)
create mode 100644 tests/libxlxml2domconfigdata/fullvirt-cpuid.json
create mode 100644 tests/libxlxml2domconfigdata/fullvirt-cpuid.xml
diff --git a/tests/libxlxml2domconfigdata/fullvirt-cpuid.json
b/tests/libxlxml2domconfigdata/fullvirt-cpuid.json
new file mode 100644
index 0000000..28037be
--- /dev/null
+++ b/tests/libxlxml2domconfigdata/fullvirt-cpuid.json
@@ -0,0 +1,64 @@
+{
+ "c_info": {
+ "type": "hvm",
+ "name": "XenGuest2",
+ "uuid": "c7a5fdb2-cdaf-9455-926a-d65c16db1809"
+ },
+ "b_info": {
+ "max_vcpus": 1,
+ "avail_vcpus": [
+ 0
+ ],
+ "max_memkb": 592896,
+ "target_memkb": 403456,
+ "video_memkb": 8192,
+ "shadow_memkb": 5656,
+ "cpuid": [
+ {
+ "leaf": 1,
+ "ecx": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx0",
+ "edx": "xxxxxxxxxxxxxxxxxxxxxxxxxxx1xxxx"
+ }
Since Xen lets you specify raw "cpuid" register values here, surely
this is flexible enough to allow us to support the mode=custom CPU
models ?
We would just need to make sure every bit poisition used either
0 or 1, and not 'x', so that we are fully overriding whatever
defaults are presented by the hypervisor "host" CPU model. Or is
life more complicated than that ?
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 :|
2:54 p.m.
New subject: [libvirt] [PATCH v5 7/9] tests: check CPU features handling in libxl driver
On Fri, Mar 16, 2018 at 05:44:28PM +0000, Daniel P. Berrangé wrote:
Since Xen lets you specify raw "cpuid" register values
here, surely
this is flexible enough to allow us to support the mode=custom CPU
models ?
We would just need to make sure every bit poisition used either
0 or 1, and not 'x', so that we are fully overriding whatever
defaults are presented by the hypervisor "host" CPU model. Or is
life more complicated than that ?
This is what v1 of this series had... See discussion there, especially
message from Jiri:
https://www.redhat.com/archives/libvir-list/2017-June/msg01304.html
And this, from... you:
https://www.redhat.com/archives/libvir-list/2017-June/msg01308.html
This is not only about 'x'. But also about setting '1' where hardware
does not really support given feature. This will also result in "broken"
CPU.
--
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?
9:11 a.m.
New subject: [libvirt] [PATCH v5 7/9] tests: check CPU features handling in libxl driver
On Fri, Mar 16, 2018 at 08:54:42PM +0100, Marek Marczykowski-Górecki wrote:
On Fri, Mar 16, 2018 at 05:44:28PM +0000, Daniel P. Berrangé wrote:
> Since Xen lets you specify raw "cpuid" register values here, surely
> this is flexible enough to allow us to support the mode=custom CPU
> models ?
>
> We would just need to make sure every bit poisition used either
> 0 or 1, and not 'x', so that we are fully overriding whatever
> defaults are presented by the hypervisor "host" CPU model. Or is
> life more complicated than that ?
This is what v1 of this series had... See discussion there, especially
message from Jiri:
https://www.redhat.com/archives/libvir-list/2017-June/msg01304.html
And this, from... you:
https://www.redhat.com/archives/libvir-list/2017-June/msg01308.html
Ok, yeah, I remember these discussions now. What I didn't realize at the
time though, was that the revised patches would end up silently changing
any XML that says mode="custom" + model="Foobar", into
mode="host-passthrough"
The core problem we faced in QEMU was that if you take a base named CPU
model and then add/remove 20+ features, some guest OS can get confused.
This is because the combination of features may be unusual, or the
level & xlevel values reported alongside CPUID are unsual for the given
set of features enabled. The Xen config doesn't provide a way to configure
the level & xlevel values for the CPU, so will potentially hit this same
problem. This is *already* possible though, even with host-passthrough,
since the user can ask for host-passthrough and also disable a whole
bunch of features.
IOW, Jiri is right, but I don't think that neccesarily implies we should
not implement support for mode="custom". I see we have 3 options here
currently
1. Support mode="custom", using the cpuid config option in libxl
2. Raise an error when starting a guest with mode="custom"
3. Translate mode="custom" into mode="host-passthrough" and ignore
requested named model
So currently situation is that users of libxl libvirt driver may have
deployed guests using arbitrary <cpu> model setup, and that would have
been ignored, giving that a host-passthrough setup.
If we choose option (1), users will have changed semantics for their
existing guests, as instead of getting full host-passthrough, they'll
get a CPU that actually matches the mode=custom their XML has beeen
requesting all along.
If we choose option (2), we'll break existing deployed guests with
mode=custom.
If we choose option (3), users will have preserved semantics for their
existing guests, but we'll never be honouring the actual XML config
the user requested. If we did ever want to properly support mode=custom
in future we're still going doom existing users.
Ideally we would have choosen option (2) from day 1, but that ship
has sailed.
I really dislike, even hate, option (3) because it is explicitly
ignoring a valid XML configuration request and turning it into
something completely different.
So despite the problems / limitations of mode=custom that were previously
raised, I none the less think we should implement mode=custom for the
libxl driver. Then document that its usage is discouraged - for the same
reason that we discourage users from arbitrarily blanking out features
for mode=host-passthrough.
Perhaps in future, we could make mode=custom work more reliably if the
libxl driver provided a way to set the level & xlevel values.
This is not only about 'x'. But also about setting
'1' where hardware
does not really support given feature. This will also result in "broken"
CPU.
If we host hardware does not support a given feature bit, then the code
has to make a decision based on the <feature policy=...> setting. ie
"force" would present the feature to the guest, regardless of whether
the host supports ir, while "require" would refuse to start the guest,
and "optional" would silently disable the feature.
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 :|
10:14 a.m.
New subject: [libvirt] [PATCH v5 7/9] tests: check CPU features handling in libxl driver
On Mon, Mar 19, 2018 at 02:11:02PM +0000, Daniel P. Berrangé wrote:
On Fri, Mar 16, 2018 at 08:54:42PM +0100, Marek Marczykowski-Górecki
wrote:
> On Fri, Mar 16, 2018 at 05:44:28PM +0000, Daniel P. Berrangé wrote:
> > Since Xen lets you specify raw "cpuid" register values here, surely
> > this is flexible enough to allow us to support the mode=custom CPU
> > models ?
> >
> > We would just need to make sure every bit poisition used either
> > 0 or 1, and not 'x', so that we are fully overriding whatever
> > defaults are presented by the hypervisor "host" CPU model. Or is
> > life more complicated than that ?
>
> This is what v1 of this series had... See discussion there, especially
> message from Jiri:
> https://www.redhat.com/archives/libvir-list/2017-June/msg01304.html
>
> And this, from... you:
> https://www.redhat.com/archives/libvir-list/2017-June/msg01308.html
Ok, yeah, I remember these discussions now. What I didn't realize at the
time though, was that the revised patches would end up silently changing
any XML that says mode="custom" + model="Foobar", into
mode="host-passthrough"
The core problem we faced in QEMU was that if you take a base named CPU
model and then add/remove 20+ features, some guest OS can get confused.
This is because the combination of features may be unusual, or the
level & xlevel values reported alongside CPUID are unsual for the given
set of features enabled. The Xen config doesn't provide a way to configure
the level & xlevel values for the CPU, so will potentially hit this same
problem. This is *already* possible though, even with host-passthrough,
since the user can ask for host-passthrough and also disable a whole
bunch of features.
IOW, Jiri is right, but I don't think that neccesarily implies we should
not implement support for mode="custom". I see we have 3 options here
currently
1. Support mode="custom", using the cpuid config option in libxl
2. Raise an error when starting a guest with mode="custom"
3. Translate mode="custom" into mode="host-passthrough" and ignore
requested named model
So currently situation is that users of libxl libvirt driver may have
deployed guests using arbitrary <cpu> model setup, and that would have
been ignored, giving that a host-passthrough setup.
If we choose option (1), users will have changed semantics for their
existing guests, as instead of getting full host-passthrough, they'll
get a CPU that actually matches the mode=custom their XML has beeen
requesting all along.
If we choose option (2), we'll break existing deployed guests with
mode=custom.
If we choose option (3), users will have preserved semantics for their
existing guests, but we'll never be honouring the actual XML config
the user requested. If we did ever want to properly support mode=custom
in future we're still going doom existing users.
There is a variant of this option: do not translate mode="custom" into
mode="host-passthrough" explicitly (keep it mode="custom" in the
config), but ignore both model and features in mode="custom" - as it is
currently.
In other words - add CPUID support only to mode="host-passthrough" and
keep other modes untouched.
Implementing proper mode=custom support (either now or later) will
change guest view of the system anyway, it doesn't matter when we do it.
And I'm leaning to doing it later.
Ideally we would have choosen option (2) from day 1, but that ship
has sailed.
I really dislike, even hate, option (3) because it is explicitly
ignoring a valid XML configuration request and turning it into
something completely different.
Well, libvirt does it all the time - by ignoring not supported domain
config elements instead of raising some error. For example most
<feature policy=...> are in this category, regardless of the mode, until
this patch series. Same for <cputune>, <memtune>, and other <*tune>,
various hypervisor features (libxl supports apic, pae, and acpi, others
are silently ignored). I can go on with the list...
And I guess it isn't only libxl driver.
I'm not saying it's fundamentally wrong - it make it easier to write
universal domain XML configuration that will work on any libvirt
version. One just need to check if given feature is supported by
particular libvirt version.
What I propose here is to keep this behavior for mode=custom.
I don't want to implement both mode=custom and mode=host-passthrough
in this patch series, mostly for non-technical reasons. It already took
very long time and I'd like to finally have at least some of it in. And
also it looks like mode=custom will be mostly separate code (maybe even
using old xend syntax, to have control over all cpuid bits, not only
those listed in libxl_cpuid.c).
So despite the problems / limitations of mode=custom that were
previously
raised, I none the less think we should implement mode=custom for the
libxl driver. Then document that its usage is discouraged - for the same
reason that we discourage users from arbitrarily blanking out features
for mode=host-passthrough.
Perhaps in future, we could make mode=custom work more reliably if the
libxl driver provided a way to set the level & xlevel values.
What exactly do you mean by level & xlevel values?
libxl cpuid syntax supports maxleaf and maxhvleaf - is it the same?
See here for details:
http://xenbits.xen.org/docs/unstable/man/xl.cfg.5.html#cpuid-LIBXL_STRING...
> This is not only about 'x'. But also about setting
'1' where hardware
> does not really support given feature. This will also result in "broken"
> CPU.
If we host hardware does not support a given feature bit, then the code
has to make a decision based on the <feature policy=...> setting. ie
"force" would present the feature to the guest, regardless of whether
the host supports ir, while "require" would refuse to start the guest,
and "optional" would silently disable the feature.
Hmm, but <feature policy=...> applies only to bits you explicitly
specify, not all those implied by given model, right? This means that
innocent model setting may also result in weird configuration. This is
nothing libxl specific, just about mode=custom on hardware assisted
virtualization (as opposed to pure software one, that could provide some
features regardless of host support).
--
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?
10:31 a.m.
New subject: [libvirt] [PATCH v5 7/9] tests: check CPU features handling in libxl driver
On Mon, Mar 19, 2018 at 04:14:20PM +0100, Marek Marczykowski-Górecki wrote:
On Mon, Mar 19, 2018 at 02:11:02PM +0000, Daniel P. Berrangé wrote:
> On Fri, Mar 16, 2018 at 08:54:42PM +0100, Marek Marczykowski-Górecki wrote:
> > On Fri, Mar 16, 2018 at 05:44:28PM +0000, Daniel P. Berrangé wrote:
> > > Since Xen lets you specify raw "cpuid" register values here,
surely
> > > this is flexible enough to allow us to support the mode=custom CPU
> > > models ?
> > >
> > > We would just need to make sure every bit poisition used either
> > > 0 or 1, and not 'x', so that we are fully overriding whatever
> > > defaults are presented by the hypervisor "host" CPU model. Or
is
> > > life more complicated than that ?
> >
> > This is what v1 of this series had... See discussion there, especially
> > message from Jiri:
> > https://www.redhat.com/archives/libvir-list/2017-June/msg01304.html
> >
> > And this, from... you:
> > https://www.redhat.com/archives/libvir-list/2017-June/msg01308.html
>
> Ok, yeah, I remember these discussions now. What I didn't realize at the
> time though, was that the revised patches would end up silently changing
> any XML that says mode="custom" + model="Foobar", into
mode="host-passthrough"
>
> The core problem we faced in QEMU was that if you take a base named CPU
> model and then add/remove 20+ features, some guest OS can get confused.
> This is because the combination of features may be unusual, or the
> level & xlevel values reported alongside CPUID are unsual for the given
> set of features enabled. The Xen config doesn't provide a way to configure
> the level & xlevel values for the CPU, so will potentially hit this same
> problem. This is *already* possible though, even with host-passthrough,
> since the user can ask for host-passthrough and also disable a whole
> bunch of features.
>
> IOW, Jiri is right, but I don't think that neccesarily implies we should
> not implement support for mode="custom". I see we have 3 options here
> currently
>
> 1. Support mode="custom", using the cpuid config option in libxl
> 2. Raise an error when starting a guest with mode="custom"
> 3. Translate mode="custom" into mode="host-passthrough" and
ignore
> requested named model
>
> So currently situation is that users of libxl libvirt driver may have
> deployed guests using arbitrary <cpu> model setup, and that would have
> been ignored, giving that a host-passthrough setup.
>
> If we choose option (1), users will have changed semantics for their
> existing guests, as instead of getting full host-passthrough, they'll
> get a CPU that actually matches the mode=custom their XML has beeen
> requesting all along.
>
> If we choose option (2), we'll break existing deployed guests with
> mode=custom.
>
> If we choose option (3), users will have preserved semantics for their
> existing guests, but we'll never be honouring the actual XML config
> the user requested. If we did ever want to properly support mode=custom
> in future we're still going doom existing users.
There is a variant of this option: do not translate mode="custom" into
mode="host-passthrough" explicitly (keep it mode="custom" in the
config), but ignore both model and features in mode="custom" - as it is
currently.
In other words - add CPUID support only to mode="host-passthrough" and
keep other modes untouched.
Implementing proper mode=custom support (either now or later) will
change guest view of the system anyway, it doesn't matter when we do it.
And I'm leaning to doing it later.
Yes, just continuing to ignore mode=custom entirely would be acceptable,
as that's no worse than what we do today.
Perhaps you could actually emit a warning message
"Ignoring CPU with mode=custom, update your config to mode=host-passthrough
to avoid risk of changed guest semantics when mode=custom is supported in
the future".
so that we reduce level of surprise in future if we implement it.
> Ideally we would have choosen option (2) from day 1, but that
ship
> has sailed.
>
> I really dislike, even hate, option (3) because it is explicitly
> ignoring a valid XML configuration request and turning it into
> something completely different.
Well, libvirt does it all the time - by ignoring not supported domain
config elements instead of raising some error. For example most
<feature policy=...> are in this category, regardless of the mode, until
this patch series. Same for <cputune>, <memtune>, and other <*tune>,
various hypervisor features (libxl supports apic, pae, and acpi, others
are silently ignored). I can go on with the list...
And I guess it isn't only libxl driver.
Yes, we do unfortunately ignore unsupported XML elements in general, but
once we do support something, we try to report errors if we can't support
every enum choice it offers.
I don't want to implement both mode=custom and
mode=host-passthrough
in this patch series, mostly for non-technical reasons. It already took
very long time and I'd like to finally have at least some of it in. And
also it looks like mode=custom will be mostly separate code (maybe even
using old xend syntax, to have control over all cpuid bits, not only
those listed in libxl_cpuid.c).
Yes, understandable - if you drop the code that silently re-writes the
XML for mode=custom & add a warning message, that would be acceptable
to merge from my pov.
> So despite the problems / limitations of mode=custom that were
previously
> raised, I none the less think we should implement mode=custom for the
> libxl driver. Then document that its usage is discouraged - for the same
> reason that we discourage users from arbitrarily blanking out features
> for mode=host-passthrough.
>
> Perhaps in future, we could make mode=custom work more reliably if the
> libxl driver provided a way to set the level & xlevel values.
What exactly do you mean by level & xlevel values?
libxl cpuid syntax supports maxleaf and maxhvleaf - is it the same?
See here for details:
http://xenbits.xen.org/docs/unstable/man/xl.cfg.5.html#cpuid-LIBXL_STRING...
Ok, yeah, I guess I should have avoided QEMU specific terminology.
'level', and 'xlevel' are the values reported in eax, when
the user issues CPU ID index '0' or '0x800000000' respectively.
IIUC, essentially they value of 'level' is saying what is the
maximum leaf supported, and xlevel is the maximum hypevisor
leaf value. So this does sound like it probably matches to the Xen
maxleaf/maxhvleaf values.
The other thing I forgot were the family, model & stepping values.
I think those were actually the more critical thing from POV of
avoiding guest bugs, as certain OS / apps have (broken) assumption
about existance of features implying a certain min stepping level
or vica-verca.
> > This is not only about 'x'. But also about setting
'1' where hardware
> > does not really support given feature. This will also result in
"broken"
> > CPU.
>
> If we host hardware does not support a given feature bit, then the code
> has to make a decision based on the <feature policy=...> setting. ie
> "force" would present the feature to the guest, regardless of whether
> the host supports ir, while "require" would refuse to start the guest,
> and "optional" would silently disable the feature.
Hmm, but <feature policy=...> applies only to bits you explicitly
specify, not all those implied by given model, right? This means that
innocent model setting may also result in weird configuration. This is
nothing libxl specific, just about mode=custom on hardware assisted
virtualization (as opposed to pure software one, that could provide some
features regardless of host support).
For any CPU ID bits implied by the model, I believe we just decided they
would be assumed to have policy=require. Though in QEMU driver we actually
end up having policy=optional in many cases since historically we've no
way to identify if KVM silently blocks certain features, even if the host
CPU supports them. Even if a feature is implied by the <model>, the user
could still explicitly list if with <feature> to set a contrary policy.
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 :|
9:26 p.m.
New subject: [libvirt] [PATCH v5 8/9] xenconfig: add CPUID handling to domXML <-> xl.cfg conversion
Only "libxl" format supported for now. Special care needed around
vmx/svm, because those two are translated into "nestedhvm" setting.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
Reviewed-by: Jim Fehlig <jfehlig(a)suse.com>
---
Changes since v3:
- improve error reporting (VIR_ERR_CONF_SYNTAX)
- ignore empty cpuid option - same as libxl
- fix cleanup on error
Changes since v2:
- new patch
---
src/xenconfig/xen_xl.c | 165 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 165 insertions(+)
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 3bbc5d8..3e3a5d7 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -253,6 +253,91 @@ xenTranslateCPUFeature(const char *feature_name, bool from_libxl)
return feature_name;
}
+static int
+xenParseXLCPUID(virConfPtr conf, virDomainDefPtr def)
+{
+ const char *cpuid_str = NULL;
+ char **cpuid_pairs = NULL;
+ char **name_and_value = NULL;
+ size_t i;
+ int ret = -1;
+ int policy;
+
+ if (xenConfigGetString(conf, "cpuid", &cpuid_str, NULL) < 0)
+ return -1;
+
+ if (!cpuid_str)
+ return 0;
+
+ if (!def->cpu) {
+ if (VIR_ALLOC(def->cpu) < 0)
+ goto cleanup;
+ def->cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
+ def->cpu->type = VIR_CPU_TYPE_GUEST;
+ def->cpu->nfeatures = 0;
+ def->cpu->nfeatures_max = 0;
+ }
+
+ cpuid_pairs = virStringSplit(cpuid_str, ",", 0);
+ if (!cpuid_pairs)
+ goto cleanup;
+
+ if (!cpuid_pairs[0]) {
+ ret = 0;
+ goto cleanup;
+ }
+
+ if (STRNEQ(cpuid_pairs[0], "host")) {
+ virReportError(VIR_ERR_CONF_SYNTAX,
+ _("cpuid starting with %s is not supported, only libxl format
is"),
+ cpuid_pairs[0]);
+ goto cleanup;
+ }
+
+ for (i = 1; cpuid_pairs[i]; i++) {
+ name_and_value = virStringSplit(cpuid_pairs[i], "=", 2);
+ if (!name_and_value)
+ goto cleanup;
+ if (!name_and_value[0] || !name_and_value[1]) {
+ virReportError(VIR_ERR_CONF_SYNTAX,
+ _("Invalid libxl cpuid key=value element: %s"),
+ cpuid_pairs[i]);
+ goto cleanup;
+ }
+ if (STREQ(name_and_value[1], "1")) {
+ policy = VIR_CPU_FEATURE_FORCE;
+ } else if (STREQ(name_and_value[1], "0")) {
+ policy = VIR_CPU_FEATURE_DISABLE;
+ } else if (STREQ(name_and_value[1], "x")) {
+ policy = VIR_CPU_FEATURE_OPTIONAL;
+ } else if (STREQ(name_and_value[1], "k")) {
+ policy = VIR_CPU_FEATURE_OPTIONAL;
+ } else if (STREQ(name_and_value[1], "s")) {
+ policy = VIR_CPU_FEATURE_OPTIONAL;
+ } else {
+ virReportError(VIR_ERR_CONF_SYNTAX,
+ _("Invalid libxl cpuid value: %s"),
+ cpuid_pairs[i]);
+ goto cleanup;
+ }
+
+ if (virCPUDefAddFeature(def->cpu,
+ xenTranslateCPUFeature(name_and_value[0], true),
+ policy) < 0)
+ goto cleanup;
+
+ virStringListFree(name_and_value);
+ name_and_value = NULL;
+ }
+
+ ret = 0;
+
+ cleanup:
+ virStringListFree(name_and_value);
+ virStringListFree(cpuid_pairs);
+ return ret;
+}
+
static int
xenParseXLSpice(virConfPtr conf, virDomainDefPtr def)
@@ -1090,6 +1175,9 @@ xenParseXL(virConfPtr conf,
goto cleanup;
#endif
+ if (xenParseXLCPUID(conf, def) < 0)
+ goto cleanup;
+
if (xenParseXLDisk(conf, def) < 0)
goto cleanup;
@@ -1232,6 +1320,80 @@ xenFormatXLOS(virConfPtr conf, virDomainDefPtr def)
return 0;
}
+static int
+xenFormatXLCPUID(virConfPtr conf, virDomainDefPtr def)
+{
+ char **cpuid_pairs = NULL;
+ char *cpuid_string = NULL;
+ size_t i, j;
+ int ret = -1;
+
+ if (!def->cpu)
+ return 0;
+
+ if (def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("'%s' CPU mode not supported, only host-passthrough
is"),
+ virCPUModeTypeToString(def->cpu->mode));
+ return -1;
+ }
+
+ /* "host" + all features + NULL */
+ if (VIR_ALLOC_N(cpuid_pairs, def->cpu->nfeatures + 2) < 0)
+ return -1;
+
+ if (VIR_STRDUP(cpuid_pairs[0], "host") < 0)
+ goto cleanup;
+
+ j = 1;
+ for (i = 0; i < def->cpu->nfeatures; i++) {
+ const char *feature_name = xenTranslateCPUFeature(
+ def->cpu->features[i].name,
+ false);
+ const char *policy = NULL;
+
+ if (STREQ(feature_name, "vmx") || STREQ(feature_name,
"svm"))
+ /* ignore vmx/svm in cpuid option, translated into nestedhvm
+ * elsewhere */
+ continue;
+
+ switch (def->cpu->features[i].policy) {
+ case VIR_CPU_FEATURE_FORCE:
+ case VIR_CPU_FEATURE_REQUIRE:
+ policy = "1";
+ break;
+ case VIR_CPU_FEATURE_OPTIONAL:
+ policy = "x";
+ break;
+ case VIR_CPU_FEATURE_DISABLE:
+ case VIR_CPU_FEATURE_FORBID:
+ policy = "0";
+ break;
+ }
+ if (virAsprintf(&cpuid_pairs[j++], "%s=%s",
+ feature_name,
+ policy) < 0)
+ goto cleanup;
+ }
+ cpuid_pairs[j] = NULL;
+
+ if (j > 1) {
+ cpuid_string = virStringListJoin((const char **)cpuid_pairs, ",");
+ if (!cpuid_string)
+ goto cleanup;
+
+ if (xenConfigSetString(conf, "cpuid", cpuid_string) < 0)
+ goto cleanup;
+ }
+
+ ret = 0;
+
+ cleanup:
+ virStringListFree(cpuid_pairs);
+ VIR_FREE(cpuid_string);
+ return ret;
+}
+
#ifdef LIBXL_HAVE_VNUMA
static int
xenFormatXLVnode(virConfValuePtr list,
@@ -1997,6 +2159,9 @@ xenFormatXL(virDomainDefPtr def, virConnectPtr conn)
if (xenFormatXLOS(conf, def) < 0)
goto cleanup;
+ if (xenFormatXLCPUID(conf, def) < 0)
+ goto cleanup;
+
#ifdef LIBXL_HAVE_VNUMA
if (xenFormatXLDomainVnuma(conf, def) < 0)
goto cleanup;
--
git-series 0.9.1
9:26 p.m.
New subject: [libvirt] [PATCH v5 9/9] tests: add test case for CPUID in xenconfig driver
Check conversion of "cpuid" setting, check all supported policy settings
("1", "0", "x"). Also, check interaction with
"nestedhvm" - should not
be included as "vmx=1" in "cpuid" setting.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
Reviewed-by: Jim Fehlig <jfehlig(a)suse.com>
---
Changes since v3:
- adjust for nested HVM enabled by just <cpu> element
Changes since v2:
- new patch
---
tests/xlconfigdata/test-fullvirt-cpuid.cfg | 25 ++++++++++++++++-
tests/xlconfigdata/test-fullvirt-cpuid.xml | 35 +++++++++++++++++++++++-
tests/xlconfigtest.c | 1 +-
3 files changed, 61 insertions(+)
create mode 100644 tests/xlconfigdata/test-fullvirt-cpuid.cfg
create mode 100644 tests/xlconfigdata/test-fullvirt-cpuid.xml
diff --git a/tests/xlconfigdata/test-fullvirt-cpuid.cfg
b/tests/xlconfigdata/test-fullvirt-cpuid.cfg
new file mode 100644
index 0000000..bb7b9c7
--- /dev/null
+++ b/tests/xlconfigdata/test-fullvirt-cpuid.cfg
@@ -0,0 +1,25 @@
+name = "XenGuest2"
+uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809"
+maxmem = 579
+memory = 394
+vcpus = 1
+pae = 1
+acpi = 0
+apic = 0
+viridian = 0
+rtc_timeoffset = 0
+localtime = 0
+on_poweroff = "destroy"
+on_reboot = "restart"
+on_crash = "restart"
+device_model = "/usr/lib/xen/bin/qemu-system-i386"
+sdl = 0
+vnc = 1
+vncunused = 1
+vnclisten = "127.0.0.1"
+parallel = "none"
+serial = "none"
+builder = "hvm"
+boot = "d"
+nestedhvm = 1
+cpuid = "host,tm=0,sse3=1,page1gb=x"
diff --git a/tests/xlconfigdata/test-fullvirt-cpuid.xml
b/tests/xlconfigdata/test-fullvirt-cpuid.xml
new file mode 100644
index 0000000..adc55d2
--- /dev/null
+++ b/tests/xlconfigdata/test-fullvirt-cpuid.xml
@@ -0,0 +1,35 @@
+<domain type='xen'>
+ <name>XenGuest2</name>
+ <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>592896</memory>
+ <currentMemory unit='KiB'>403456</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='xenfv'>hvm</type>
+ <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader>
+ <boot dev='cdrom'/>
+ </os>
+ <features>
+ <pae/>
+ </features>
+ <cpu mode='host-passthrough'>
+ <feature policy='disable' name='tm'/>
+ <feature policy='force' name='pni'/>
+ <feature policy='optional' name='pdpe1gb'/>
+ </cpu>
+ <clock offset='variable' adjustment='0' basis='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>restart</on_crash>
+ <devices>
+ <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <graphics type='vnc' port='-1' autoport='yes'
listen='127.0.0.1'>
+ <listen type='address' address='127.0.0.1'/>
+ </graphics>
+ <video>
+ <model type='cirrus' vram='8192' heads='1'
primary='yes'/>
+ </video>
+ </devices>
+</domain>
diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c
index 57a0a67..39f51e2 100644
--- a/tests/xlconfigtest.c
+++ b/tests/xlconfigtest.c
@@ -270,6 +270,7 @@ mymain(void)
DO_TEST("fullvirt-multi-timer");
DO_TEST("fullvirt-nestedhvm");
DO_TEST("fullvirt-nestedhvm-disabled");
+ DO_TEST("fullvirt-cpuid");
#ifdef LIBXL_HAVE_VNUMA
DO_TEST("fullvirt-vnuma");
DO_TEST_PARSE("fullvirt-vnuma-autocomplete", false);
--
git-series 0.9.1
2440
days inactive
2445
days old
22 comments
2 participants
participants (2)
-
Daniel P. Berrangé -
Marek Marczykowski-Górecki