[libvirt] [PATCH v4 0/5] libxl: PVHv2 support
This is a respin of my old PVHv1 patch[1], converted to PVHv2. The actual code use "PVH" name. The guest ostype reuse VIR_DOMAIN_OSTYPE_XEN, but to request PVH machine, machine="xenpvh" attribute is used. Since PVHv2 relies on features in newer Xen versions, I needed to convert also some older code. For example b_info->u.hvm.nested_hvm was deprecated in favor of b_info->nested_hvm. While the code do handle both old and new versions (obviously refusing PVHv2 if Xen is too old), this isn't the case for tests. How it should be handled, if at all? To test this with all supported Xen versions (4.6, 4.7, 4.8, 4.9, 4.10, 4.11, unstable), I've extended travis configuration for that[2]. It isn't exactly trivial to build Xen 4.6-4.8 on recent system, mostly thanks to -Werror and new warnings, but also other toolchain changes. Because of this, the configuration is rather hacky. But it may be a good idea to include multiple pre-built Xen versions in the base docker image and choose one using ./configure options (maybe together with some symlink or environment variable like PKG_CONFIG_PATH). [1] https://www.redhat.com/archives/libvir-list/2016-August/msg00376.html [2] https://github.com/marmarek/libvirt/tree/travis-xen-pvh Changes in v2: - drop "docs: don't refer to deprecated 'linux' ostype in example" patch - migrating further away from "linux" os type is offtopic to this series and apparently is a controversial thing - drop "docs: update domain schema for machine attribute" patch - already applied - apply review comments from Jim - rebase on master Changes in v3: - rebase on master, drop already applied patches - use #ifdef LIBXL_DOMAIN_TYPE_PVH to detect PVH support, fix compilation failure on older Xen - exclude PVH from VIR_DOMAIN_OSTYPE_XEN <-> VIR_DOMAIN_OSTYPE_LINUX conversion - fix reported capabilities for PVH Changes in v4: - change PVH support detection method Marek Marczykowski-Górecki (5): libxl: reorder libxlMakeDomBuildInfo for upcoming PVH support libxl: add support for PVH tests: add basic Xen PVH test xenconfig: add support for parsing type= xl config entry xenconfig: add support for type="pvh" docs/formatcaps.html.in | 4 +- docs/schemas/domaincommon.rng | 1 +- m4/virt-driver-libxl.m4 | 3 +- src/conf/domain_conf.c | 6 +- src/libxl/libxl_capabilities.c | 38 +++++++++-- src/libxl/libxl_conf.c | 76 ++++++++++++++++------ src/libxl/libxl_driver.c | 6 +- src/xenconfig/xen_common.c | 27 ++++++-- src/xenconfig/xen_xl.c | 5 +- tests/libxlxml2domconfigdata/basic-pvh.json | 49 ++++++++++++++- tests/libxlxml2domconfigdata/basic-pvh.xml | 28 ++++++++- tests/libxlxml2domconfigtest.c | 3 +- tests/testutilsxen.c | 3 +- tests/xlconfigdata/test-fullvirt-type.cfg | 21 ++++++- tests/xlconfigdata/test-fullvirt-type.xml | 27 ++++++++- tests/xlconfigdata/test-paravirt-type.cfg | 13 ++++- tests/xlconfigdata/test-paravirt-type.xml | 25 +++++++- tests/xlconfigdata/test-pvh-type.cfg | 13 ++++- tests/xlconfigdata/test-pvh-type.xml | 25 +++++++- tests/xlconfigtest.c | 3 +- 20 files changed, 341 insertions(+), 35 deletions(-) create mode 100644 tests/libxlxml2domconfigdata/basic-pvh.json create mode 100644 tests/libxlxml2domconfigdata/basic-pvh.xml create mode 100644 tests/xlconfigdata/test-fullvirt-type.cfg create mode 100644 tests/xlconfigdata/test-fullvirt-type.xml create mode 100644 tests/xlconfigdata/test-paravirt-type.cfg create mode 100644 tests/xlconfigdata/test-paravirt-type.xml create mode 100644 tests/xlconfigdata/test-pvh-type.cfg create mode 100644 tests/xlconfigdata/test-pvh-type.xml base-commit: 199eee6aae7af3d813fbe98660c7e0fa1a8ae7b7 -- git-series 0.9.1
Make it easier to share HVM and PVH code where relevant. No functional change. Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- src/libxl/libxl_conf.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index e2bfa2f..f3da0ed 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -376,18 +376,6 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, b_info->max_memkb = virDomainDefGetMemoryInitial(def); b_info->target_memkb = def->mem.cur_balloon; if (hvm) { - char bootorder[VIR_DOMAIN_BOOT_LAST + 1]; - - libxl_defbool_set(&b_info->u.hvm.pae, - def->features[VIR_DOMAIN_FEATURE_PAE] == - VIR_TRISTATE_SWITCH_ON); - libxl_defbool_set(&b_info->u.hvm.apic, - def->features[VIR_DOMAIN_FEATURE_APIC] == - VIR_TRISTATE_SWITCH_ON); - libxl_defbool_set(&b_info->u.hvm.acpi, - def->features[VIR_DOMAIN_FEATURE_ACPI] == - VIR_TRISTATE_SWITCH_ON); - if (caps && def->cpu && def->cpu->mode == (VIR_CPU_MODE_HOST_PASSTHROUGH)) { bool hasHwVirt = false; @@ -474,6 +462,20 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, "mode=host-passthrough to avoid risk of changed guest " "semantics when mode=custom is supported in the future"); } + } + + if (hvm) { + char bootorder[VIR_DOMAIN_BOOT_LAST + 1]; + + libxl_defbool_set(&b_info->u.hvm.pae, + def->features[VIR_DOMAIN_FEATURE_PAE] == + VIR_TRISTATE_SWITCH_ON); + libxl_defbool_set(&b_info->u.hvm.apic, + def->features[VIR_DOMAIN_FEATURE_APIC] == + VIR_TRISTATE_SWITCH_ON); + libxl_defbool_set(&b_info->u.hvm.acpi, + def->features[VIR_DOMAIN_FEATURE_ACPI] == + VIR_TRISTATE_SWITCH_ON); if (def->nsounds > 0) { /* -- git-series 0.9.1
Since this is something between PV and HVM, it makes sense to put the setting in place where domain type is specified. To enable it, use <os><type machine="xenpvh">...</type></os>. It is also included in capabilities.xml, for every supported HVM guest type - it doesn't seems to be any other requirement (besides new enough Xen). Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes in v2 proposed by Jim: - use new_arch_added var instead of i == nr_guest_archs for clarity - improve comment - adjust for now required Xen >= 4.6 (remove part for Xen < 4.5) Changes in v3: - limit VIR_DOMAIN_OSTYPE_XEN -> VIR_DOMAIN_OSTYPE_LINUX conversion to Xen PV only - do not accept VIR_DOMAIN_OSTYPE_LINUX for PVH - fix reported capabilities for PVH - remove hostdev passthrough and video/graphics - use #ifdef LIBXL_DOMAIN_TYPE_PVH instead of hypervisor version to check for PVH support - compile fix for Xen < 4.9 Changes in v4: - revert to "i == nr_guest_archs-1" check - let configure check for LIBXL_DOMAIN_TYPE_PVH and use #ifdef HAVE_XEN_PVH then - fix comment about Xen version --- docs/formatcaps.html.in | 4 +-- docs/schemas/domaincommon.rng | 1 +- m4/virt-driver-libxl.m4 | 3 ++- src/conf/domain_conf.c | 6 ++-- src/libxl/libxl_capabilities.c | 38 ++++++++++++++++++++++----- src/libxl/libxl_conf.c | 50 ++++++++++++++++++++++++++++++----- src/libxl/libxl_driver.c | 6 ++-- 7 files changed, 90 insertions(+), 18 deletions(-) diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in index 0d9c53d..fe113bc 100644 --- a/docs/formatcaps.html.in +++ b/docs/formatcaps.html.in @@ -104,8 +104,8 @@ <dt><code>machine</code></dt><dd>Machine type, for use in <a href="formatdomain.html#attributeOSTypeMachine">machine</a> attribute of os/type element in domain XML. For example Xen - supports <code>xenfv</code> for HVM or <code>xenpv</code> for - PV.</dd> + supports <code>xenfv</code> for HVM, <code>xenpv</code> for + PV, or <code>xenpvh</code> for PVH.</dd> <dt><code>domain</code></dt><dd>The <code>type</code> attribute of this element specifies the type of hypervisor required to run the domain. Use in <a href="formatdomain.html#attributeDomainType">type</a> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 099a949..820a7c6 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -341,6 +341,7 @@ <choice> <value>xenpv</value> <value>xenfv</value> + <value>xenpvh</value> </choice> </attribute> </optional> diff --git a/m4/virt-driver-libxl.m4 b/m4/virt-driver-libxl.m4 index 479d911..2cd97cc 100644 --- a/m4/virt-driver-libxl.m4 +++ b/m4/virt-driver-libxl.m4 @@ -75,6 +75,9 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_LIBXL], [ ]) fi + dnl Check if Xen has support for PVH + AC_CHECK_DECL(LIBXL_DOMAIN_TYPE_PVH, [AC_DEFINE([HAVE_XEN_PVH], [1], [Define to 1 if Xen has PVH support.])], [], [#include <libxl.h>]) + AC_SUBST([LIBXL_CFLAGS]) AC_SUBST([LIBXL_LIBS]) ]) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9911d56..a945ad4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19094,7 +19094,8 @@ virDomainDefParseCaps(virDomainDefPtr def, * be 'xen'. So we accept the former and convert */ if (def->os.type == VIR_DOMAIN_OSTYPE_LINUX && - def->virtType == VIR_DOMAIN_VIRT_XEN) { + def->virtType == VIR_DOMAIN_VIRT_XEN && + (!def->os.machine || STREQ(def->os.machine, "xenpv"))) { def->os.type = VIR_DOMAIN_OSTYPE_XEN; } @@ -27705,7 +27706,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, * be 'xen'. So we convert to the former for backcompat */ if (def->virtType == VIR_DOMAIN_VIRT_XEN && - def->os.type == VIR_DOMAIN_OSTYPE_XEN) + def->os.type == VIR_DOMAIN_OSTYPE_XEN && + (!def->os.machine || STREQ(def->os.machine, "xenpv"))) virBufferAsprintf(buf, ">%s</type>\n", virDomainOSTypeToString(VIR_DOMAIN_OSTYPE_LINUX)); else diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c index 18596c7..eecd5e9 100644 --- a/src/libxl/libxl_capabilities.c +++ b/src/libxl/libxl_capabilities.c @@ -57,6 +57,7 @@ struct guest_arch { virArch arch; int bits; int hvm; + int pvh; int pae; int nonpae; int ia64_be; @@ -491,13 +492,33 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) guest_archs[i].nonpae = nonpae; if (ia64_be) guest_archs[i].ia64_be = ia64_be; + + /* + * Xen 4.10 introduced support for the PVH guest type, which + * requires hardware virtualization support similar to the + * HVM guest type. Add a PVH guest type for each new HVM + * guest type. + */ +#ifdef HAVE_XEN_PVH + if (hvm && i == nr_guest_archs-1) { + i = nr_guest_archs; + /* Ensure we have not exhausted the guest_archs array */ + if (nr_guest_archs >= ARRAY_CARDINALITY(guest_archs)) + continue; + guest_archs[nr_guest_archs].arch = arch; + guest_archs[nr_guest_archs].pvh = 1; + nr_guest_archs++; + } +#endif } } regfree(®ex); for (i = 0; i < nr_guest_archs; ++i) { virCapsGuestPtr guest; - char const *const xen_machines[] = {guest_archs[i].hvm ? "xenfv" : "xenpv"}; + char const *const xen_machines[] = { + guest_archs[i].hvm ? "xenfv" : + (guest_archs[i].pvh ? "xenpvh" : "xenpv")}; virCapsGuestMachinePtr *machines; if ((machines = virCapabilitiesAllocMachines(xen_machines, 1)) == NULL) @@ -557,7 +578,9 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) 1, 0) == NULL) return -1; + } + if (guest_archs[i].hvm || guest_archs[i].pvh) { if (virCapabilitiesAddGuestFeature(guest, "hap", 1, @@ -580,7 +603,7 @@ libxlMakeDomainOSCaps(const char *machine, os->supported = true; - if (STREQ(machine, "xenpv")) + if (STREQ(machine, "xenpv") || STREQ(machine, "xenpvh")) return 0; capsLoader->supported = true; @@ -732,11 +755,14 @@ libxlMakeDomainCapabilities(virDomainCapsPtr domCaps, domCaps->maxvcpus = PV_MAX_VCPUS; if (libxlMakeDomainOSCaps(domCaps->machine, os, firmwares, nfirmwares) < 0 || - libxlMakeDomainDeviceDiskCaps(disk) < 0 || - libxlMakeDomainDeviceGraphicsCaps(graphics) < 0 || - libxlMakeDomainDeviceVideoCaps(video) < 0 || - libxlMakeDomainDeviceHostdevCaps(hostdev) < 0) + libxlMakeDomainDeviceDiskCaps(disk) < 0) + return -1; + if (STRNEQ(domCaps->machine, "xenpvh") && + (libxlMakeDomainDeviceGraphicsCaps(graphics) < 0 || + libxlMakeDomainDeviceVideoCaps(video) < 0 || + libxlMakeDomainDeviceHostdevCaps(hostdev) < 0)) return -1; + return 0; } diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index f3da0ed..db50a73 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -133,8 +133,20 @@ libxlMakeDomCreateInfo(libxl_ctx *ctx, libxl_domain_create_info_init(c_info); - if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM || + (def->os.type == VIR_DOMAIN_OSTYPE_XEN && + STREQ(def->os.machine, "xenpvh"))) { +#ifdef HAVE_XEN_PVH + c_info->type = def->os.type == VIR_DOMAIN_OSTYPE_HVM ? + LIBXL_DOMAIN_TYPE_HVM : LIBXL_DOMAIN_TYPE_PVH; +#else + if (STREQ(def->os.machine, "xenpvh")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("PVH guest type not supported")); + return -1; + } c_info->type = LIBXL_DOMAIN_TYPE_HVM; +#endif switch ((virTristateSwitch) def->features[VIR_DOMAIN_FEATURE_HAP]) { case VIR_TRISTATE_SWITCH_OFF: libxl_defbool_set(&c_info->hap, false); @@ -276,16 +288,26 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, virDomainClockDef clock = def->clock; libxl_ctx *ctx = cfg->ctx; libxl_domain_build_info *b_info = &d_config->b_info; - int hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM; + bool hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM; + bool pvh = STREQ(def->os.machine, "xenpvh"); size_t i; size_t nusbdevice = 0; libxl_domain_build_info_init(b_info); - if (hvm) + if (hvm) { libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_HVM); - else + } else if (pvh) { +#ifdef HAVE_XEN_PVH + libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PVH); +#else + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("PVH guest type not supported")); + return -1; +#endif + } else { libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PV); + } b_info->max_vcpus = virDomainDefGetVcpusMax(def); if (libxl_cpu_bitmap_alloc(ctx, &b_info->avail_vcpus, b_info->max_vcpus)) @@ -375,7 +397,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, def->mem.cur_balloon = VIR_ROUND_UP(def->mem.cur_balloon, 1024); b_info->max_memkb = virDomainDefGetMemoryInitial(def); b_info->target_memkb = def->mem.cur_balloon; - if (hvm) { + if (hvm || pvh) { if (caps && def->cpu && def->cpu->mode == (VIR_CPU_MODE_HOST_PASSTHROUGH)) { bool hasHwVirt = false; @@ -647,6 +669,22 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, return -1; } #endif + } else if (pvh) { + if (VIR_STRDUP(b_info->cmdline, def->os.cmdline) < 0) + return -1; + if (VIR_STRDUP(b_info->kernel, def->os.kernel) < 0) + return -1; + if (VIR_STRDUP(b_info->ramdisk, def->os.initrd) < 0) + return -1; +#ifdef LIBXL_HAVE_BUILDINFO_BOOTLOADER + if (VIR_STRDUP(b_info->bootloader, def->os.bootloader) < 0) + return -1; + if (def->os.bootloaderArgs) { + if (!(b_info->bootloader_args = + virStringSplit(def->os.bootloaderArgs, " \t\n", 0))) + return -1; + } +#endif } else { /* * For compatibility with the legacy xen toolstack, default to pygrub @@ -1230,7 +1268,7 @@ libxlMakeNic(virDomainDefPtr def, STRNEQ(l_nic->model, "netfront")) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("only model 'netfront' is supported for " - "Xen PV domains")); + "Xen PV(H) domains")); return -1; } if (VIR_STRDUP(x_nic->model, l_nic->model) < 0) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index efd47a3..6287cef 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -6398,9 +6398,11 @@ libxlConnectGetDomainCapabilities(virConnectPtr conn, emulatorbin = "/usr/bin/qemu-system-x86_64"; if (machine) { - if (STRNEQ(machine, "xenpv") && STRNEQ(machine, "xenfv")) { + if (STRNEQ(machine, "xenpv") && + STRNEQ(machine, "xenpvh") && + STRNEQ(machine, "xenfv")) { virReportError(VIR_ERR_INVALID_ARG, "%s", - _("Xen only supports 'xenpv' and 'xenfv' machines")); + _("Xen only supports 'xenpv', 'xenpvh' and 'xenfv' machines")); goto cleanup; } } else { -- git-series 0.9.1
On 10/7/18 9:39 AM, Marek Marczykowski-Górecki wrote:
Since this is something between PV and HVM, it makes sense to put the setting in place where domain type is specified. To enable it, use <os><type machine="xenpvh">...</type></os>. It is also included in capabilities.xml, for every supported HVM guest type - it doesn't seems to be any other requirement (besides new enough Xen).
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes in v2 proposed by Jim: - use new_arch_added var instead of i == nr_guest_archs for clarity - improve comment - adjust for now required Xen >= 4.6 (remove part for Xen < 4.5)
Changes in v3: - limit VIR_DOMAIN_OSTYPE_XEN -> VIR_DOMAIN_OSTYPE_LINUX conversion to Xen PV only - do not accept VIR_DOMAIN_OSTYPE_LINUX for PVH - fix reported capabilities for PVH - remove hostdev passthrough and video/graphics - use #ifdef LIBXL_DOMAIN_TYPE_PVH instead of hypervisor version to check for PVH support - compile fix for Xen < 4.9
Changes in v4: - revert to "i == nr_guest_archs-1" check - let configure check for LIBXL_DOMAIN_TYPE_PVH and use #ifdef HAVE_XEN_PVH then - fix comment about Xen version --- docs/formatcaps.html.in | 4 +-- docs/schemas/domaincommon.rng | 1 +- m4/virt-driver-libxl.m4 | 3 ++- src/conf/domain_conf.c | 6 ++-- src/libxl/libxl_capabilities.c | 38 ++++++++++++++++++++++----- src/libxl/libxl_conf.c | 50 ++++++++++++++++++++++++++++++----- src/libxl/libxl_driver.c | 6 ++-- 7 files changed, 90 insertions(+), 18 deletions(-)
diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in index 0d9c53d..fe113bc 100644 --- a/docs/formatcaps.html.in +++ b/docs/formatcaps.html.in @@ -104,8 +104,8 @@ <dt><code>machine</code></dt><dd>Machine type, for use in <a href="formatdomain.html#attributeOSTypeMachine">machine</a> attribute of os/type element in domain XML. For example Xen - supports <code>xenfv</code> for HVM or <code>xenpv</code> for - PV.</dd> + supports <code>xenfv</code> for HVM, <code>xenpv</code> for + PV, or <code>xenpvh</code> for PVH.</dd> <dt><code>domain</code></dt><dd>The <code>type</code> attribute of this element specifies the type of hypervisor required to run the domain. Use in <a href="formatdomain.html#attributeDomainType">type</a> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 099a949..820a7c6 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -341,6 +341,7 @@ <choice> <value>xenpv</value> <value>xenfv</value> + <value>xenpvh</value> </choice> </attribute> </optional> diff --git a/m4/virt-driver-libxl.m4 b/m4/virt-driver-libxl.m4 index 479d911..2cd97cc 100644 --- a/m4/virt-driver-libxl.m4 +++ b/m4/virt-driver-libxl.m4 @@ -75,6 +75,9 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_LIBXL], [ ]) fi
+ dnl Check if Xen has support for PVH + AC_CHECK_DECL(LIBXL_DOMAIN_TYPE_PVH, [AC_DEFINE([HAVE_XEN_PVH], [1], [Define to 1 if Xen has PVH support.])], [], [#include <libxl.h>]) + AC_SUBST([LIBXL_CFLAGS]) AC_SUBST([LIBXL_LIBS]) ]) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9911d56..a945ad4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19094,7 +19094,8 @@ virDomainDefParseCaps(virDomainDefPtr def, * be 'xen'. So we accept the former and convert */ if (def->os.type == VIR_DOMAIN_OSTYPE_LINUX && - def->virtType == VIR_DOMAIN_VIRT_XEN) { + def->virtType == VIR_DOMAIN_VIRT_XEN && + (!def->os.machine || STREQ(def->os.machine, "xenpv"))) { def->os.type = VIR_DOMAIN_OSTYPE_XEN; }
@@ -27705,7 +27706,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, * be 'xen'. So we convert to the former for backcompat */ if (def->virtType == VIR_DOMAIN_VIRT_XEN && - def->os.type == VIR_DOMAIN_OSTYPE_XEN) + def->os.type == VIR_DOMAIN_OSTYPE_XEN && + (!def->os.machine || STREQ(def->os.machine, "xenpv"))) virBufferAsprintf(buf, ">%s</type>\n", virDomainOSTypeToString(VIR_DOMAIN_OSTYPE_LINUX)); else
I'd still like to hear what others think of extending the hack. mprivozn? You often have an interesting opinion :-).
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c index 18596c7..eecd5e9 100644 --- a/src/libxl/libxl_capabilities.c +++ b/src/libxl/libxl_capabilities.c @@ -57,6 +57,7 @@ struct guest_arch { virArch arch; int bits; int hvm; + int pvh; int pae; int nonpae; int ia64_be; @@ -491,13 +492,33 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) guest_archs[i].nonpae = nonpae; if (ia64_be) guest_archs[i].ia64_be = ia64_be; + + /* + * Xen 4.10 introduced support for the PVH guest type, which + * requires hardware virtualization support similar to the + * HVM guest type. Add a PVH guest type for each new HVM + * guest type. + */ +#ifdef HAVE_XEN_PVH + if (hvm && i == nr_guest_archs-1) { + i = nr_guest_archs; + /* Ensure we have not exhausted the guest_archs array */ + if (nr_guest_archs >= ARRAY_CARDINALITY(guest_archs)) + continue; + guest_archs[nr_guest_archs].arch = arch; + guest_archs[nr_guest_archs].pvh = 1; + nr_guest_archs++; + } +#endif } } regfree(®ex);
for (i = 0; i < nr_guest_archs; ++i) { virCapsGuestPtr guest; - char const *const xen_machines[] = {guest_archs[i].hvm ? "xenfv" : "xenpv"}; + char const *const xen_machines[] = { + guest_archs[i].hvm ? "xenfv" : + (guest_archs[i].pvh ? "xenpvh" : "xenpv")}; virCapsGuestMachinePtr *machines;
if ((machines = virCapabilitiesAllocMachines(xen_machines, 1)) == NULL) @@ -557,7 +578,9 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) 1, 0) == NULL) return -1; + }
+ if (guest_archs[i].hvm || guest_archs[i].pvh) { if (virCapabilitiesAddGuestFeature(guest, "hap", 1, @@ -580,7 +603,7 @@ libxlMakeDomainOSCaps(const char *machine,
os->supported = true;
- if (STREQ(machine, "xenpv")) + if (STREQ(machine, "xenpv") || STREQ(machine, "xenpvh")) return 0;
capsLoader->supported = true; @@ -732,11 +755,14 @@ libxlMakeDomainCapabilities(virDomainCapsPtr domCaps, domCaps->maxvcpus = PV_MAX_VCPUS;
if (libxlMakeDomainOSCaps(domCaps->machine, os, firmwares, nfirmwares) < 0 || - libxlMakeDomainDeviceDiskCaps(disk) < 0 || - libxlMakeDomainDeviceGraphicsCaps(graphics) < 0 || - libxlMakeDomainDeviceVideoCaps(video) < 0 || - libxlMakeDomainDeviceHostdevCaps(hostdev) < 0) + libxlMakeDomainDeviceDiskCaps(disk) < 0) + return -1; + if (STRNEQ(domCaps->machine, "xenpvh") && + (libxlMakeDomainDeviceGraphicsCaps(graphics) < 0 || + libxlMakeDomainDeviceVideoCaps(video) < 0 || + libxlMakeDomainDeviceHostdevCaps(hostdev) < 0)) return -1; + return 0; }
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index f3da0ed..db50a73 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -133,8 +133,20 @@ libxlMakeDomCreateInfo(libxl_ctx *ctx,
libxl_domain_create_info_init(c_info);
- if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM || + (def->os.type == VIR_DOMAIN_OSTYPE_XEN && + STREQ(def->os.machine, "xenpvh"))) { +#ifdef HAVE_XEN_PVH + c_info->type = def->os.type == VIR_DOMAIN_OSTYPE_HVM ? + LIBXL_DOMAIN_TYPE_HVM : LIBXL_DOMAIN_TYPE_PVH; +#else + if (STREQ(def->os.machine, "xenpvh")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("PVH guest type not supported")); + return -1; + } c_info->type = LIBXL_DOMAIN_TYPE_HVM; +#endif switch ((virTristateSwitch) def->features[VIR_DOMAIN_FEATURE_HAP]) { case VIR_TRISTATE_SWITCH_OFF: libxl_defbool_set(&c_info->hap, false); @@ -276,16 +288,26 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, virDomainClockDef clock = def->clock; libxl_ctx *ctx = cfg->ctx; libxl_domain_build_info *b_info = &d_config->b_info; - int hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM; + bool hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM; + bool pvh = STREQ(def->os.machine, "xenpvh"); size_t i; size_t nusbdevice = 0;
libxl_domain_build_info_init(b_info);
- if (hvm) + if (hvm) { libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_HVM); - else + } else if (pvh) { +#ifdef HAVE_XEN_PVH + libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PVH); +#else + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("PVH guest type not supported")); + return -1; +#endif + } else { libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PV); + }
b_info->max_vcpus = virDomainDefGetVcpusMax(def); if (libxl_cpu_bitmap_alloc(ctx, &b_info->avail_vcpus, b_info->max_vcpus)) @@ -375,7 +397,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, def->mem.cur_balloon = VIR_ROUND_UP(def->mem.cur_balloon, 1024); b_info->max_memkb = virDomainDefGetMemoryInitial(def); b_info->target_memkb = def->mem.cur_balloon; - if (hvm) { + if (hvm || pvh) { if (caps && def->cpu && def->cpu->mode == (VIR_CPU_MODE_HOST_PASSTHROUGH)) { bool hasHwVirt = false; @@ -647,6 +669,22 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, return -1; } #endif + } else if (pvh) { + if (VIR_STRDUP(b_info->cmdline, def->os.cmdline) < 0) + return -1; + if (VIR_STRDUP(b_info->kernel, def->os.kernel) < 0) + return -1; + if (VIR_STRDUP(b_info->ramdisk, def->os.initrd) < 0) + return -1; +#ifdef LIBXL_HAVE_BUILDINFO_BOOTLOADER + if (VIR_STRDUP(b_info->bootloader, def->os.bootloader) < 0) + return -1; + if (def->os.bootloaderArgs) { + if (!(b_info->bootloader_args = + virStringSplit(def->os.bootloaderArgs, " \t\n", 0))) + return -1; + } +#endif
This is probably fine, but AFAIK no bootloaders currently support pvh. Juergen is working on support in grub2 but that seems to be a little ways off. Regards, Jim
} else { /* * For compatibility with the legacy xen toolstack, default to pygrub @@ -1230,7 +1268,7 @@ libxlMakeNic(virDomainDefPtr def, STRNEQ(l_nic->model, "netfront")) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("only model 'netfront' is supported for " - "Xen PV domains")); + "Xen PV(H) domains")); return -1; } if (VIR_STRDUP(x_nic->model, l_nic->model) < 0) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index efd47a3..6287cef 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -6398,9 +6398,11 @@ libxlConnectGetDomainCapabilities(virConnectPtr conn, emulatorbin = "/usr/bin/qemu-system-x86_64";
if (machine) { - if (STRNEQ(machine, "xenpv") && STRNEQ(machine, "xenfv")) { + if (STRNEQ(machine, "xenpv") && + STRNEQ(machine, "xenpvh") && + STRNEQ(machine, "xenfv")) { virReportError(VIR_ERR_INVALID_ARG, "%s", - _("Xen only supports 'xenpv' and 'xenfv' machines")); + _("Xen only supports 'xenpv', 'xenpvh' and 'xenfv' machines")); goto cleanup; } } else {
On Tue, Oct 09, 2018 at 04:45:01PM -0600, Jim Fehlig wrote:
On 10/7/18 9:39 AM, Marek Marczykowski-Górecki wrote:
@@ -647,6 +669,22 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, return -1; } #endif + } else if (pvh) { + if (VIR_STRDUP(b_info->cmdline, def->os.cmdline) < 0) + return -1; + if (VIR_STRDUP(b_info->kernel, def->os.kernel) < 0) + return -1; + if (VIR_STRDUP(b_info->ramdisk, def->os.initrd) < 0) + return -1; +#ifdef LIBXL_HAVE_BUILDINFO_BOOTLOADER + if (VIR_STRDUP(b_info->bootloader, def->os.bootloader) < 0) + return -1; + if (def->os.bootloaderArgs) { + if (!(b_info->bootloader_args = + virStringSplit(def->os.bootloaderArgs, " \t\n", 0))) + return -1; + } +#endif
This is probably fine, but AFAIK no bootloaders currently support pvh. Juergen is working on support in grub2 but that seems to be a little ways off.
This is independent of grub2 (which could be set as "kernel"), the "bootloader" option here is about a program run _in dom0_ to find the kernel to load, instead of providing a path directly (see xl.cfg(5)). Like pygrub. And while I haven't tested it with PVH, I see no reason why it shouldn't work. -- 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?
On 10/9/18 5:04 PM, Marek Marczykowski-Górecki wrote:
On Tue, Oct 09, 2018 at 04:45:01PM -0600, Jim Fehlig wrote:
On 10/7/18 9:39 AM, Marek Marczykowski-Górecki wrote:
@@ -647,6 +669,22 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, return -1; } #endif + } else if (pvh) { + if (VIR_STRDUP(b_info->cmdline, def->os.cmdline) < 0) + return -1; + if (VIR_STRDUP(b_info->kernel, def->os.kernel) < 0) + return -1; + if (VIR_STRDUP(b_info->ramdisk, def->os.initrd) < 0) + return -1; +#ifdef LIBXL_HAVE_BUILDINFO_BOOTLOADER + if (VIR_STRDUP(b_info->bootloader, def->os.bootloader) < 0) + return -1; + if (def->os.bootloaderArgs) { + if (!(b_info->bootloader_args = + virStringSplit(def->os.bootloaderArgs, " \t\n", 0))) + return -1; + } +#endif
This is probably fine, but AFAIK no bootloaders currently support pvh. Juergen is working on support in grub2 but that seems to be a little ways off.
This is independent of grub2 (which could be set as "kernel"), the "bootloader" option here is about a program run _in dom0_ to find the kernel to load, instead of providing a path directly (see xl.cfg(5)).
Yes of course. I had grub2 on the mind since I had just talked to Juergen about the status.
Like pygrub. And while I haven't tested it with PVH, I see no reason why it shouldn't work.
Yep, agreed. As long as you don't mind poking at VM images in dom0 is should just work. Regards, Jim
On Tue, Oct 09, 2018 at 04:45:01PM -0600, Jim Fehlig wrote:
On 10/7/18 9:39 AM, Marek Marczykowski-Górecki wrote:
Since this is something between PV and HVM, it makes sense to put the setting in place where domain type is specified. To enable it, use <os><type machine="xenpvh">...</type></os>. It is also included in capabilities.xml, for every supported HVM guest type - it doesn't seems to be any other requirement (besides new enough Xen).
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes in v2 proposed by Jim: - use new_arch_added var instead of i == nr_guest_archs for clarity - improve comment - adjust for now required Xen >= 4.6 (remove part for Xen < 4.5)
Changes in v3: - limit VIR_DOMAIN_OSTYPE_XEN -> VIR_DOMAIN_OSTYPE_LINUX conversion to Xen PV only - do not accept VIR_DOMAIN_OSTYPE_LINUX for PVH - fix reported capabilities for PVH - remove hostdev passthrough and video/graphics - use #ifdef LIBXL_DOMAIN_TYPE_PVH instead of hypervisor version to check for PVH support - compile fix for Xen < 4.9
Changes in v4: - revert to "i == nr_guest_archs-1" check - let configure check for LIBXL_DOMAIN_TYPE_PVH and use #ifdef HAVE_XEN_PVH then - fix comment about Xen version --- docs/formatcaps.html.in | 4 +-- docs/schemas/domaincommon.rng | 1 +- m4/virt-driver-libxl.m4 | 3 ++- src/conf/domain_conf.c | 6 ++-- src/libxl/libxl_capabilities.c | 38 ++++++++++++++++++++++----- src/libxl/libxl_conf.c | 50 ++++++++++++++++++++++++++++++----- src/libxl/libxl_driver.c | 6 ++-- 7 files changed, 90 insertions(+), 18 deletions(-)
diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in index 0d9c53d..fe113bc 100644 --- a/docs/formatcaps.html.in +++ b/docs/formatcaps.html.in @@ -104,8 +104,8 @@ <dt><code>machine</code></dt><dd>Machine type, for use in <a href="formatdomain.html#attributeOSTypeMachine">machine</a> attribute of os/type element in domain XML. For example Xen - supports <code>xenfv</code> for HVM or <code>xenpv</code> for - PV.</dd> + supports <code>xenfv</code> for HVM, <code>xenpv</code> for + PV, or <code>xenpvh</code> for PVH.</dd> <dt><code>domain</code></dt><dd>The <code>type</code> attribute of this element specifies the type of hypervisor required to run the domain. Use in <a href="formatdomain.html#attributeDomainType">type</a> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 099a949..820a7c6 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -341,6 +341,7 @@ <choice> <value>xenpv</value> <value>xenfv</value> + <value>xenpvh</value> </choice> </attribute> </optional> diff --git a/m4/virt-driver-libxl.m4 b/m4/virt-driver-libxl.m4 index 479d911..2cd97cc 100644 --- a/m4/virt-driver-libxl.m4 +++ b/m4/virt-driver-libxl.m4 @@ -75,6 +75,9 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_LIBXL], [ ]) fi + dnl Check if Xen has support for PVH + AC_CHECK_DECL(LIBXL_DOMAIN_TYPE_PVH, [AC_DEFINE([HAVE_XEN_PVH], [1], [Define to 1 if Xen has PVH support.])], [], [#include <libxl.h>]) + AC_SUBST([LIBXL_CFLAGS]) AC_SUBST([LIBXL_LIBS]) ]) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9911d56..a945ad4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19094,7 +19094,8 @@ virDomainDefParseCaps(virDomainDefPtr def, * be 'xen'. So we accept the former and convert */ if (def->os.type == VIR_DOMAIN_OSTYPE_LINUX && - def->virtType == VIR_DOMAIN_VIRT_XEN) { + def->virtType == VIR_DOMAIN_VIRT_XEN && + (!def->os.machine || STREQ(def->os.machine, "xenpv"))) { def->os.type = VIR_DOMAIN_OSTYPE_XEN; } @@ -27705,7 +27706,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, * be 'xen'. So we convert to the former for backcompat */ if (def->virtType == VIR_DOMAIN_VIRT_XEN && - def->os.type == VIR_DOMAIN_OSTYPE_XEN) + def->os.type == VIR_DOMAIN_OSTYPE_XEN && + (!def->os.machine || STREQ(def->os.machine, "xenpv"))) virBufferAsprintf(buf, ">%s</type>\n", virDomainOSTypeToString(VIR_DOMAIN_OSTYPE_LINUX)); else
I'd still like to hear what others think of extending the hack. mprivozn? You often have an interesting opinion :-).
IIUC, this means that we get this behaviour: Input Output type=linux, machine=NULL type=linux, machine=NULL type=linux, machine=xenpv type=linux, machine=xenpvh type=xen, machine=NULL type=linux, machine=NULL type=xen, machine=xenpv type=linux, machine=xenpv type=xen, machine=xenpvh type=xen, machine=xenpvh And one combo not allowed type=linux, machine=xenpvh I can understand the motivation behind this change, but I wonder if it is worth it or not to have the difference in behaviour. I'm quite torn. 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 :|
On 10/10/2018 10:45 AM, Daniel P. Berrangé wrote:
On Tue, Oct 09, 2018 at 04:45:01PM -0600, Jim Fehlig wrote:
On 10/7/18 9:39 AM, Marek Marczykowski-Górecki wrote:
Since this is something between PV and HVM, it makes sense to put the setting in place where domain type is specified. To enable it, use <os><type machine="xenpvh">...</type></os>. It is also included in capabilities.xml, for every supported HVM guest type - it doesn't seems to be any other requirement (besides new enough Xen).
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes in v2 proposed by Jim: - use new_arch_added var instead of i == nr_guest_archs for clarity - improve comment - adjust for now required Xen >= 4.6 (remove part for Xen < 4.5)
Changes in v3: - limit VIR_DOMAIN_OSTYPE_XEN -> VIR_DOMAIN_OSTYPE_LINUX conversion to Xen PV only - do not accept VIR_DOMAIN_OSTYPE_LINUX for PVH - fix reported capabilities for PVH - remove hostdev passthrough and video/graphics - use #ifdef LIBXL_DOMAIN_TYPE_PVH instead of hypervisor version to check for PVH support - compile fix for Xen < 4.9
Changes in v4: - revert to "i == nr_guest_archs-1" check - let configure check for LIBXL_DOMAIN_TYPE_PVH and use #ifdef HAVE_XEN_PVH then - fix comment about Xen version --- docs/formatcaps.html.in | 4 +-- docs/schemas/domaincommon.rng | 1 +- m4/virt-driver-libxl.m4 | 3 ++- src/conf/domain_conf.c | 6 ++-- src/libxl/libxl_capabilities.c | 38 ++++++++++++++++++++++----- src/libxl/libxl_conf.c | 50 ++++++++++++++++++++++++++++++----- src/libxl/libxl_driver.c | 6 ++-- 7 files changed, 90 insertions(+), 18 deletions(-)
diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in index 0d9c53d..fe113bc 100644 --- a/docs/formatcaps.html.in +++ b/docs/formatcaps.html.in @@ -104,8 +104,8 @@ <dt><code>machine</code></dt><dd>Machine type, for use in <a href="formatdomain.html#attributeOSTypeMachine">machine</a> attribute of os/type element in domain XML. For example Xen - supports <code>xenfv</code> for HVM or <code>xenpv</code> for - PV.</dd> + supports <code>xenfv</code> for HVM, <code>xenpv</code> for + PV, or <code>xenpvh</code> for PVH.</dd> <dt><code>domain</code></dt><dd>The <code>type</code> attribute of this element specifies the type of hypervisor required to run the domain. Use in <a href="formatdomain.html#attributeDomainType">type</a> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 099a949..820a7c6 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -341,6 +341,7 @@ <choice> <value>xenpv</value> <value>xenfv</value> + <value>xenpvh</value> </choice> </attribute> </optional> diff --git a/m4/virt-driver-libxl.m4 b/m4/virt-driver-libxl.m4 index 479d911..2cd97cc 100644 --- a/m4/virt-driver-libxl.m4 +++ b/m4/virt-driver-libxl.m4 @@ -75,6 +75,9 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_LIBXL], [ ]) fi + dnl Check if Xen has support for PVH + AC_CHECK_DECL(LIBXL_DOMAIN_TYPE_PVH, [AC_DEFINE([HAVE_XEN_PVH], [1], [Define to 1 if Xen has PVH support.])], [], [#include <libxl.h>]) + AC_SUBST([LIBXL_CFLAGS]) AC_SUBST([LIBXL_LIBS]) ]) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9911d56..a945ad4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19094,7 +19094,8 @@ virDomainDefParseCaps(virDomainDefPtr def, * be 'xen'. So we accept the former and convert */ if (def->os.type == VIR_DOMAIN_OSTYPE_LINUX && - def->virtType == VIR_DOMAIN_VIRT_XEN) { + def->virtType == VIR_DOMAIN_VIRT_XEN && + (!def->os.machine || STREQ(def->os.machine, "xenpv"))) { def->os.type = VIR_DOMAIN_OSTYPE_XEN; } @@ -27705,7 +27706,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, * be 'xen'. So we convert to the former for backcompat */ if (def->virtType == VIR_DOMAIN_VIRT_XEN && - def->os.type == VIR_DOMAIN_OSTYPE_XEN) + def->os.type == VIR_DOMAIN_OSTYPE_XEN && + (!def->os.machine || STREQ(def->os.machine, "xenpv"))) virBufferAsprintf(buf, ">%s</type>\n", virDomainOSTypeToString(VIR_DOMAIN_OSTYPE_LINUX)); else
I'd still like to hear what others think of extending the hack. mprivozn? You often have an interesting opinion :-).
IIUC, this means that we get this behaviour:
Input Output
type=linux, machine=NULL type=linux, machine=NULL type=linux, machine=xenpv type=linux, machine=xenpvh
I think we would get type=xen, machine=xenpv, wouldn't we?
type=xen, machine=NULL type=linux, machine=NULL type=xen, machine=xenpv type=linux, machine=xenpv type=xen, machine=xenpvh type=xen, machine=xenpvh
And one combo not allowed
type=linux, machine=xenpvh
I can understand the motivation behind this change, but I wonder if it is worth it or not to have the difference in behaviour. I'm quite torn.
I think we can do this. xenpvh falls into xen world and not Linux. IIRC the question in previous versions was if it is safe to make decisions based on machine type in post parse callbacks. Short answer is yes. The longer answer is yes, but in driver specific callbacks. I view code under src/conf/ to be hypervisor agnostic (at least it should be) and thus any hypervisor specific decisions/changes to user XML should be made from src/hypervisor/. Anyway, maybe that was a bit off topic. Michal
On Wed, Oct 10, 2018 at 11:06:38AM +0200, Michal Privoznik wrote:
On 10/10/2018 10:45 AM, Daniel P. Berrangé wrote:
On Tue, Oct 09, 2018 at 04:45:01PM -0600, Jim Fehlig wrote:
On 10/7/18 9:39 AM, Marek Marczykowski-Górecki wrote:
Since this is something between PV and HVM, it makes sense to put the setting in place where domain type is specified. To enable it, use <os><type machine="xenpvh">...</type></os>. It is also included in capabilities.xml, for every supported HVM guest type - it doesn't seems to be any other requirement (besides new enough Xen).
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes in v2 proposed by Jim: - use new_arch_added var instead of i == nr_guest_archs for clarity - improve comment - adjust for now required Xen >= 4.6 (remove part for Xen < 4.5)
Changes in v3: - limit VIR_DOMAIN_OSTYPE_XEN -> VIR_DOMAIN_OSTYPE_LINUX conversion to Xen PV only - do not accept VIR_DOMAIN_OSTYPE_LINUX for PVH - fix reported capabilities for PVH - remove hostdev passthrough and video/graphics - use #ifdef LIBXL_DOMAIN_TYPE_PVH instead of hypervisor version to check for PVH support - compile fix for Xen < 4.9
Changes in v4: - revert to "i == nr_guest_archs-1" check - let configure check for LIBXL_DOMAIN_TYPE_PVH and use #ifdef HAVE_XEN_PVH then - fix comment about Xen version --- docs/formatcaps.html.in | 4 +-- docs/schemas/domaincommon.rng | 1 +- m4/virt-driver-libxl.m4 | 3 ++- src/conf/domain_conf.c | 6 ++-- src/libxl/libxl_capabilities.c | 38 ++++++++++++++++++++++----- src/libxl/libxl_conf.c | 50 ++++++++++++++++++++++++++++++----- src/libxl/libxl_driver.c | 6 ++-- 7 files changed, 90 insertions(+), 18 deletions(-)
diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in index 0d9c53d..fe113bc 100644 --- a/docs/formatcaps.html.in +++ b/docs/formatcaps.html.in @@ -104,8 +104,8 @@ <dt><code>machine</code></dt><dd>Machine type, for use in <a href="formatdomain.html#attributeOSTypeMachine">machine</a> attribute of os/type element in domain XML. For example Xen - supports <code>xenfv</code> for HVM or <code>xenpv</code> for - PV.</dd> + supports <code>xenfv</code> for HVM, <code>xenpv</code> for + PV, or <code>xenpvh</code> for PVH.</dd> <dt><code>domain</code></dt><dd>The <code>type</code> attribute of this element specifies the type of hypervisor required to run the domain. Use in <a href="formatdomain.html#attributeDomainType">type</a> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 099a949..820a7c6 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -341,6 +341,7 @@ <choice> <value>xenpv</value> <value>xenfv</value> + <value>xenpvh</value> </choice> </attribute> </optional> diff --git a/m4/virt-driver-libxl.m4 b/m4/virt-driver-libxl.m4 index 479d911..2cd97cc 100644 --- a/m4/virt-driver-libxl.m4 +++ b/m4/virt-driver-libxl.m4 @@ -75,6 +75,9 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_LIBXL], [ ]) fi + dnl Check if Xen has support for PVH + AC_CHECK_DECL(LIBXL_DOMAIN_TYPE_PVH, [AC_DEFINE([HAVE_XEN_PVH], [1], [Define to 1 if Xen has PVH support.])], [], [#include <libxl.h>]) + AC_SUBST([LIBXL_CFLAGS]) AC_SUBST([LIBXL_LIBS]) ]) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9911d56..a945ad4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19094,7 +19094,8 @@ virDomainDefParseCaps(virDomainDefPtr def, * be 'xen'. So we accept the former and convert */ if (def->os.type == VIR_DOMAIN_OSTYPE_LINUX && - def->virtType == VIR_DOMAIN_VIRT_XEN) { + def->virtType == VIR_DOMAIN_VIRT_XEN && + (!def->os.machine || STREQ(def->os.machine, "xenpv"))) { def->os.type = VIR_DOMAIN_OSTYPE_XEN; } @@ -27705,7 +27706,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, * be 'xen'. So we convert to the former for backcompat */ if (def->virtType == VIR_DOMAIN_VIRT_XEN && - def->os.type == VIR_DOMAIN_OSTYPE_XEN) + def->os.type == VIR_DOMAIN_OSTYPE_XEN && + (!def->os.machine || STREQ(def->os.machine, "xenpv"))) virBufferAsprintf(buf, ">%s</type>\n", virDomainOSTypeToString(VIR_DOMAIN_OSTYPE_LINUX)); else
I'd still like to hear what others think of extending the hack. mprivozn? You often have an interesting opinion :-).
IIUC, this means that we get this behaviour:
Input Output
type=linux, machine=NULL type=linux, machine=NULL type=linux, machine=xenpv type=linux, machine=xenpvh
I think we would get type=xen, machine=xenpv, wouldn't we?
Sigh, yes, of course.
type=xen, machine=NULL type=linux, machine=NULL type=xen, machine=xenpv type=linux, machine=xenpv type=xen, machine=xenpvh type=xen, machine=xenpvh
And one combo not allowed
type=linux, machine=xenpvh
I can understand the motivation behind this change, but I wonder if it is worth it or not to have the difference in behaviour. I'm quite torn.
I think we can do this. xenpvh falls into xen world and not Linux. IIRC the question in previous versions was if it is safe to make decisions based on machine type in post parse callbacks. Short answer is yes. The longer answer is yes, but in driver specific callbacks.
I view code under src/conf/ to be hypervisor agnostic (at least it should be) and thus any hypervisor specific decisions/changes to user XML should be made from src/hypervisor/.
So we should move the hack to the post-parse callbacks in the xen driver - but we don't have an equivalent pre-format callback for the reverse hack, do we ? 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 :|
On Wed, Oct 10, 2018 at 10:14:44 +0100, Daniel Berrange wrote:
On Wed, Oct 10, 2018 at 11:06:38AM +0200, Michal Privoznik wrote:
On 10/10/2018 10:45 AM, Daniel P. Berrangé wrote:
[...]
I view code under src/conf/ to be hypervisor agnostic (at least it should be) and thus any hypervisor specific decisions/changes to user XML should be made from src/hypervisor/.
So we should move the hack to the post-parse callbacks in the xen driver - but we don't have an equivalent pre-format callback for the reverse hack, do we ?
No we don't. Any hacks necessary are built into the formatter so that they operate on temporary copies of the data. Having anything that would modify the def on formatting would seem like a bad idea since that can happen at random points. Doing a copy of the def to do that would be expensive and also in current situation pointless since copy is done via format->parse, so a post-parse gets applied anyways there.
On 10/7/18 9:39 AM, Marek Marczykowski-Górecki wrote:
Since this is something between PV and HVM, it makes sense to put the setting in place where domain type is specified. To enable it, use <os><type machine="xenpvh">...</type></os>. It is also included in capabilities.xml, for every supported HVM guest type - it doesn't seems to be any other requirement (besides new enough Xen).
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes in v2 proposed by Jim: - use new_arch_added var instead of i == nr_guest_archs for clarity - improve comment - adjust for now required Xen >= 4.6 (remove part for Xen < 4.5)
Changes in v3: - limit VIR_DOMAIN_OSTYPE_XEN -> VIR_DOMAIN_OSTYPE_LINUX conversion to Xen PV only - do not accept VIR_DOMAIN_OSTYPE_LINUX for PVH - fix reported capabilities for PVH - remove hostdev passthrough and video/graphics - use #ifdef LIBXL_DOMAIN_TYPE_PVH instead of hypervisor version to check for PVH support - compile fix for Xen < 4.9
Changes in v4: - revert to "i == nr_guest_archs-1" check - let configure check for LIBXL_DOMAIN_TYPE_PVH and use #ifdef HAVE_XEN_PVH then - fix comment about Xen version --- docs/formatcaps.html.in | 4 +-- docs/schemas/domaincommon.rng | 1 +- m4/virt-driver-libxl.m4 | 3 ++- src/conf/domain_conf.c | 6 ++-- src/libxl/libxl_capabilities.c | 38 ++++++++++++++++++++++----- src/libxl/libxl_conf.c | 50 ++++++++++++++++++++++++++++++----- src/libxl/libxl_driver.c | 6 ++-- 7 files changed, 90 insertions(+), 18 deletions(-)
diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in index 0d9c53d..fe113bc 100644 --- a/docs/formatcaps.html.in +++ b/docs/formatcaps.html.in @@ -104,8 +104,8 @@ <dt><code>machine</code></dt><dd>Machine type, for use in <a href="formatdomain.html#attributeOSTypeMachine">machine</a> attribute of os/type element in domain XML. For example Xen - supports <code>xenfv</code> for HVM or <code>xenpv</code> for - PV.</dd> + supports <code>xenfv</code> for HVM, <code>xenpv</code> for + PV, or <code>xenpvh</code> for PVH.</dd> <dt><code>domain</code></dt><dd>The <code>type</code> attribute of this element specifies the type of hypervisor required to run the domain. Use in <a href="formatdomain.html#attributeDomainType">type</a> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 099a949..820a7c6 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -341,6 +341,7 @@ <choice> <value>xenpv</value> <value>xenfv</value> + <value>xenpvh</value> </choice> </attribute> </optional> diff --git a/m4/virt-driver-libxl.m4 b/m4/virt-driver-libxl.m4 index 479d911..2cd97cc 100644 --- a/m4/virt-driver-libxl.m4 +++ b/m4/virt-driver-libxl.m4 @@ -75,6 +75,9 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_LIBXL], [ ]) fi
+ dnl Check if Xen has support for PVH + AC_CHECK_DECL(LIBXL_DOMAIN_TYPE_PVH, [AC_DEFINE([HAVE_XEN_PVH], [1], [Define to 1 if Xen has PVH support.])], [], [#include <libxl.h>])
Nice! I was unable to get this to work, probably due to some botched quoting.
+ AC_SUBST([LIBXL_CFLAGS]) AC_SUBST([LIBXL_LIBS]) ]) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9911d56..a945ad4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19094,7 +19094,8 @@ virDomainDefParseCaps(virDomainDefPtr def, * be 'xen'. So we accept the former and convert */ if (def->os.type == VIR_DOMAIN_OSTYPE_LINUX && - def->virtType == VIR_DOMAIN_VIRT_XEN) { + def->virtType == VIR_DOMAIN_VIRT_XEN && + (!def->os.machine || STREQ(def->os.machine, "xenpv"))) { def->os.type = VIR_DOMAIN_OSTYPE_XEN; }
@@ -27705,7 +27706,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, * be 'xen'. So we convert to the former for backcompat */ if (def->virtType == VIR_DOMAIN_VIRT_XEN && - def->os.type == VIR_DOMAIN_OSTYPE_XEN) + def->os.type == VIR_DOMAIN_OSTYPE_XEN && + (!def->os.machine || STREQ(def->os.machine, "xenpv"))) virBufferAsprintf(buf, ">%s</type>\n", virDomainOSTypeToString(VIR_DOMAIN_OSTYPE_LINUX)); else diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c index 18596c7..eecd5e9 100644 --- a/src/libxl/libxl_capabilities.c +++ b/src/libxl/libxl_capabilities.c @@ -57,6 +57,7 @@ struct guest_arch { virArch arch; int bits; int hvm; + int pvh; int pae; int nonpae; int ia64_be; @@ -491,13 +492,33 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) guest_archs[i].nonpae = nonpae; if (ia64_be) guest_archs[i].ia64_be = ia64_be; + + /* + * Xen 4.10 introduced support for the PVH guest type, which + * requires hardware virtualization support similar to the + * HVM guest type. Add a PVH guest type for each new HVM + * guest type. + */ +#ifdef HAVE_XEN_PVH + if (hvm && i == nr_guest_archs-1) { + i = nr_guest_archs; + /* Ensure we have not exhausted the guest_archs array */ + if (nr_guest_archs >= ARRAY_CARDINALITY(guest_archs)) + continue; + guest_archs[nr_guest_archs].arch = arch; + guest_archs[nr_guest_archs].pvh = 1; + nr_guest_archs++; + } +#endif } } regfree(®ex);
for (i = 0; i < nr_guest_archs; ++i) { virCapsGuestPtr guest; - char const *const xen_machines[] = {guest_archs[i].hvm ? "xenfv" : "xenpv"}; + char const *const xen_machines[] = { + guest_archs[i].hvm ? "xenfv" : + (guest_archs[i].pvh ? "xenpvh" : "xenpv")};
I had some couch time at the start of the weekend and was finally able to try using this series with virt-install. As it turns out, reporting duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we will want to report the additional <machine> under the existing 'xen' <guest> blocks. E.g. instead of adding a duplicate <guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator> <machine>xenpvh</machine> <domain type='xen'/> </arch> ... </guest> we'll want to include the xenpvh machine in the existing <guest> config for xen <guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator> <machine>xenpvh</machine> <machine>xenpv</machine> <domain type='xen'/> </arch> </guest> With this change to the capabilities reporting, virt-install works without modification using virt-install --paravirt --machine xenpv ... I didn't think too hard about the best way to handle this, but the attached diff is a POC hack that works with unmodified virt-install. Regards, Jim
On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote:
I had some couch time at the start of the weekend and was finally able to try using this series with virt-install. As it turns out, reporting duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we will want to report the additional <machine> under the existing 'xen' <guest> blocks. E.g. instead of adding a duplicate
<guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator> <machine>xenpvh</machine> <domain type='xen'/> </arch> ... </guest>
we'll want to include the xenpvh machine in the existing <guest> config for xen
<guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator> <machine>xenpvh</machine> <machine>xenpv</machine> <domain type='xen'/> </arch> </guest>
With this change to the capabilities reporting, virt-install works without modification using
virt-install --paravirt --machine xenpv ...
I didn't think too hard about the best way to handle this, but the attached diff is a POC hack that works with unmodified virt-install.
I can get it reported this way, but it will be wrong, because <features> will be incorrectly reported. For example hap should be reported for xenpvh, but not for xenpv, so bundling them together makes it impossible. Similar for acpi and probably others too. If this really needs to be reported together, I'd go with reporting superset of features, so os_type xen entry will have all features of PV and PVH. But then, it should probably reject PV features for PVH machine somewhere - in post parse hook? Or maybe it should forcibly remove them - for example I see PAE is forcibly enabled for 64bit HVM guests. -- 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?
On 10/16/18 7:23 PM, Marek Marczykowski-Górecki wrote:
On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote:
I had some couch time at the start of the weekend and was finally able to try using this series with virt-install. As it turns out, reporting duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we will want to report the additional <machine> under the existing 'xen' <guest> blocks. E.g. instead of adding a duplicate
<guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator> <machine>xenpvh</machine> <domain type='xen'/> </arch> ... </guest>
we'll want to include the xenpvh machine in the existing <guest> config for xen
<guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator> <machine>xenpvh</machine> <machine>xenpv</machine> <domain type='xen'/> </arch> </guest>
With this change to the capabilities reporting, virt-install works without modification using
virt-install --paravirt --machine xenpv ...
I didn't think too hard about the best way to handle this, but the attached diff is a POC hack that works with unmodified virt-install.
I can get it reported this way, but it will be wrong, because <features> will be incorrectly reported. For example hap should be reported for xenpvh, but not for xenpv, so bundling them together makes it impossible. Similar for acpi and probably others too.
Yes, you are correct :-(. Modeling PVH has been more of a PITA than I expected.
If this really needs to be reported together, I'd go with reporting superset of features, so os_type xen entry will have all features of PV and PVH.
Reporting features that cannot possibly be supported doesn't see right. Let's summarize what we've learned thus far and see if that helps with modeling PVH. PVH is a new xen machine type that is a hybrid of PV and HVM. Like HVM, PVH requires hardware virtualization support. Like PV, PVH requires a modified guest kernel, and one that is modified differently than PV (e.g. CONFIG_XEN_PVH vs CONFIG_XEN_PV in the linux kernel). PV and PVH have different feature sets. E.g. PVH supports HAP, ACPI, etc., but PV does not. (Perhaps another useful data point: the long term goal of the xen community is to remove CONFIG_XEN_PV, essentially making PVH the new PV from xen perspective, but that is obviously a long ways out.) Currently in libvirt, PV is modeled as VIR_DOMAIN_OSTYPE_XEN and machine "xenpv". HVM is modeled as VIR_DOMAIN_OSTYPE_HVM and machine "xenfv". For PVH we've considered VIR_DOMAIN_OSTYPE_XENPVH with machine "xenpvh", or simply adding PVH as machine "xenpvh" under existing VIR_DOMAIN_OSTYPE_XEN. I've pushed for adding a new machine "xenpvh" under existing VIR_DOMAIN_OSTYPE_XEN with primary reason that both are OS types modified to run on xen. Also existing tools like virt-{install,manager} know how to handle OSTYPE_{HVM,XEN} and machines, allowing them to support PVH without (or with minimal) modification. Have I been narrow-minded with my "both are OS types modified to run on xen" reasoning for using VIR_DOMAIN_OSTYPE_XEN? Should we really consider PVH a new OSTYPE? Your reminder that PV and PVH have a different feature set hints to modeling PVH as VIR_DOMAIN_OSTYPE_XENPVH. It is unfortunate that tools above libvirt would have to be taught about this new OSTYPE, but that shouldn't prevent using VIR_DOMAIN_OSTYPE_XENPVH if in fact it is the best way to model PVH. Sadly I haven't strongly convinced myself one way or the other. I still like the idea of reusing VIR_DOMAIN_OSTYPE_XEN and adding machine "xenpvh" since it seems easier from an app perspective, but maybe I just need slapped and admit that PVH is a new OSTYPE. (I'm sure you'd like to do the slapping at this point...) Regards, Jim
On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote:
I had some couch time at the start of the weekend and was finally able to try using this series with virt-install. As it turns out, reporting duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we will want to report the additional <machine> under the existing 'xen' <guest> blocks.
Is that virt-install limitation? In that case, IMO virt-install should be fixed, instead of changing capabilities xml to match its limitations. -- 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?
On 10/17/18 12:59 PM, Marek Marczykowski-Górecki wrote:
On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote:
I had some couch time at the start of the weekend and was finally able to try using this series with virt-install. As it turns out, reporting duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we will want to report the additional <machine> under the existing 'xen' <guest> blocks.
Is that virt-install limitation? In that case, IMO virt-install should be fixed, instead of changing capabilities xml to match its limitations.
Perhaps it is a virt-install limitation, but my suggestion was based more on how the qemu driver reports the different machines <guest> <os_type>hvm</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine maxCpus='255'>pc-i440fx-3.0</machine> <machine maxCpus='288'>pc-q35-3.0</machine> ... </arch> </guest> Compare that with reporting PV and PVH in different <guest> blocks, where the <os_type> and <arch> are the same. It seems confusing from a consumers POV <guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>xenpv</machine> </arch> </guest> <guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>xenpvh</machine> </arch> </guest> How should a consumer interpret that? Is the machine for os_type=xen, arch=x86_64 a xenpv or a xenpvh? Regards, Jim
On Thu, Oct 18, 2018 at 11:08:34AM -0600, Jim Fehlig wrote:
On 10/17/18 12:59 PM, Marek Marczykowski-Górecki wrote:
On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote:
I had some couch time at the start of the weekend and was finally able to try using this series with virt-install. As it turns out, reporting duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we will want to report the additional <machine> under the existing 'xen' <guest> blocks.
Is that virt-install limitation? In that case, IMO virt-install should be fixed, instead of changing capabilities xml to match its limitations.
Perhaps it is a virt-install limitation, but my suggestion was based more on how the qemu driver reports the different machines
<guest> <os_type>hvm</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine maxCpus='255'>pc-i440fx-3.0</machine> <machine maxCpus='288'>pc-q35-3.0</machine> ... </arch> </guest>
Compare that with reporting PV and PVH in different <guest> blocks, where the <os_type> and <arch> are the same. It seems confusing from a consumers POV
<guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>xenpv</machine> </arch> </guest>
<guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>xenpvh</machine> </arch> </guest>
How should a consumer interpret that? Is the machine for os_type=xen, arch=x86_64 a xenpv or a xenpvh?
I don't see a problem - each guest block represent set of possible configurations. Given the current structure, you could also ask "is the os_type for arch=x86_64 a xen or a hvm?". Both are valid, with possibly different set of features available. And the same goes for xenpv and xenpvh machines. Actually, I see qemu had similar problem as we have now with some features being specific to some machine value - maxCpus. And as solution, it was put in machine's attributes. But I think this approach is short-sighted. -- 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?
On 10/18/18 11:35 AM, Marek Marczykowski-Górecki wrote:
On Thu, Oct 18, 2018 at 11:08:34AM -0600, Jim Fehlig wrote:
On 10/17/18 12:59 PM, Marek Marczykowski-Górecki wrote:
On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote:
I had some couch time at the start of the weekend and was finally able to try using this series with virt-install. As it turns out, reporting duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we will want to report the additional <machine> under the existing 'xen' <guest> blocks.
Is that virt-install limitation? In that case, IMO virt-install should be fixed, instead of changing capabilities xml to match its limitations.
Perhaps it is a virt-install limitation, but my suggestion was based more on how the qemu driver reports the different machines
<guest> <os_type>hvm</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine maxCpus='255'>pc-i440fx-3.0</machine> <machine maxCpus='288'>pc-q35-3.0</machine> ... </arch> </guest>
Compare that with reporting PV and PVH in different <guest> blocks, where the <os_type> and <arch> are the same. It seems confusing from a consumers POV
<guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>xenpv</machine> </arch> </guest>
<guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>xenpvh</machine> </arch> </guest>
How should a consumer interpret that? Is the machine for os_type=xen, arch=x86_64 a xenpv or a xenpvh?
I don't see a problem - each guest block represent set of possible configurations. Given the current structure, you could also ask "is the os_type for arch=x86_64 a xen or a hvm?". Both are valid, with possibly different set of features available. And the same goes for xenpv and xenpvh machines.
Right, it is not a problem. I've not been super confident in our modeling choice and keep coming up with lame reasons while VIR_DOMAIN_OSTYPE_XENPVH might be a better approach. But it is time for me to stop talking in circles and commit this series. VIR_DOMAIN_OSTYPE_XENPV and machine xenpvh still feels like the best approach and no one has flat out objected to that. We can always adjust the capabilities reporting later if we feel there is a better way to do it.
Actually, I see qemu had similar problem as we have now with some features being specific to some machine value - maxCpus. And as solution, it was put in machine's attributes. But I think this approach is short-sighted.
Agreed, we can't just keep adding attributes. Seems a better approach would be <features> for each <machine>, but that is beyond the scope of this series. Regards, Jim
On Thu, Oct 18, 2018 at 11:08:34AM -0600, Jim Fehlig wrote:
On 10/17/18 12:59 PM, Marek Marczykowski-Górecki wrote:
On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote:
I had some couch time at the start of the weekend and was finally able to try using this series with virt-install. As it turns out, reporting duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we will want to report the additional <machine> under the existing 'xen' <guest> blocks.
Is that virt-install limitation? In that case, IMO virt-install should be fixed, instead of changing capabilities xml to match its limitations.
Perhaps it is a virt-install limitation, but my suggestion was based more on how the qemu driver reports the different machines
<guest> <os_type>hvm</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine maxCpus='255'>pc-i440fx-3.0</machine> <machine maxCpus='288'>pc-q35-3.0</machine> ... </arch> </guest>
Compare that with reporting PV and PVH in different <guest> blocks, where the <os_type> and <arch> are the same. It seems confusing from a consumers POV
<guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>xenpv</machine> </arch> </guest>
<guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>xenpvh</machine> </arch> </guest>
How should a consumer interpret that? Is the machine for os_type=xen, arch=x86_64 a xenpv or a xenpvh?
Yes, you are right - any pair of (os_type, arch) should be unique in the capabilities XML. So all machines should be reported in the same block. 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 :|
On 10/19/18 3:11 AM, Daniel P. Berrangé wrote:
On Thu, Oct 18, 2018 at 11:08:34AM -0600, Jim Fehlig wrote:
On 10/17/18 12:59 PM, Marek Marczykowski-Górecki wrote:
On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote:
I had some couch time at the start of the weekend and was finally able to try using this series with virt-install. As it turns out, reporting duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we will want to report the additional <machine> under the existing 'xen' <guest> blocks.
Is that virt-install limitation? In that case, IMO virt-install should be fixed, instead of changing capabilities xml to match its limitations.
Perhaps it is a virt-install limitation, but my suggestion was based more on how the qemu driver reports the different machines
<guest> <os_type>hvm</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine maxCpus='255'>pc-i440fx-3.0</machine> <machine maxCpus='288'>pc-q35-3.0</machine> ... </arch> </guest>
Compare that with reporting PV and PVH in different <guest> blocks, where the <os_type> and <arch> are the same. It seems confusing from a consumers POV
<guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>xenpv</machine> </arch> </guest>
<guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>xenpvh</machine> </arch> </guest>
How should a consumer interpret that? Is the machine for os_type=xen, arch=x86_64 a xenpv or a xenpvh?
Yes, you are right - any pair of (os_type, arch) should be unique in the capabilities XML. So all machines should be reported in the same block.
Our difficulty with that is xenpv and xenpvh machines have different features. Marek pointed out that the qemu driver reports the "feature" maxCpus as an attribute on the machine element, but we're hesitant to keep adding attributes for each feature that is unique to a machine. Another option we discussed was reporting a superset of all features so that e.g. (xen, x86_64) block would contain features supported by both PV and PVH and then rejecting unsupported features when parsing domXML or starting the VM. This option is rather distasteful. And we also have the option of adding VIR_DOMAIN_OSTYPE_XENPVH, which I've shied away from but may be a better way to go in the end. Do you have any suggestions we may have overlooked? Regards, Jim
On Fri, Oct 19, 2018 at 08:06:18AM -0600, Jim Fehlig wrote:
On 10/19/18 3:11 AM, Daniel P. Berrangé wrote:
On Thu, Oct 18, 2018 at 11:08:34AM -0600, Jim Fehlig wrote:
On 10/17/18 12:59 PM, Marek Marczykowski-Górecki wrote:
On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote:
I had some couch time at the start of the weekend and was finally able to try using this series with virt-install. As it turns out, reporting duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we will want to report the additional <machine> under the existing 'xen' <guest> blocks.
Is that virt-install limitation? In that case, IMO virt-install should be fixed, instead of changing capabilities xml to match its limitations.
Perhaps it is a virt-install limitation, but my suggestion was based more on how the qemu driver reports the different machines
<guest> <os_type>hvm</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine maxCpus='255'>pc-i440fx-3.0</machine> <machine maxCpus='288'>pc-q35-3.0</machine> ... </arch> </guest>
Compare that with reporting PV and PVH in different <guest> blocks, where the <os_type> and <arch> are the same. It seems confusing from a consumers POV
<guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>xenpv</machine> </arch> </guest>
<guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>xenpvh</machine> </arch> </guest>
How should a consumer interpret that? Is the machine for os_type=xen, arch=x86_64 a xenpv or a xenpvh?
Yes, you are right - any pair of (os_type, arch) should be unique in the capabilities XML. So all machines should be reported in the same block.
Our difficulty with that is xenpv and xenpvh machines have different features. Marek pointed out that the qemu driver reports the "feature" maxCpus as an attribute on the machine element, but we're hesitant to keep adding attributes for each feature that is unique to a machine.
Another option we discussed was reporting a superset of all features so that e.g. (xen, x86_64) block would contain features supported by both PV and PVH and then rejecting unsupported features when parsing domXML or starting the VM. This option is rather distasteful.
And we also have the option of adding VIR_DOMAIN_OSTYPE_XENPVH, which I've shied away from but may be a better way to go in the end. Do you have any suggestions we may have overlooked?
Oooh, it looks like i've been mis-understanding PVH in all my previous reviews. I thought it was simply a "normal" Xen paravirtualized guest kernel. ie any 'pv' guest is also a valid 'pvh' guest. Looking at the docs https://wiki.xen.org/wiki/Xen_Project_Software_Overview#Guest_Types It appears I was wrong. It says a pvh guest kernel relies on hardware virt extensions for part of its work and paravirt for other parts. So really is a hybrid between pv and hvm. With that in mind, we should indeed have a distinct OS type constant to express this. 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 :|
On 10/19/18 8:14 AM, Daniel P. Berrangé wrote:
On Fri, Oct 19, 2018 at 08:06:18AM -0600, Jim Fehlig wrote:
On 10/19/18 3:11 AM, Daniel P. Berrangé wrote:
On Thu, Oct 18, 2018 at 11:08:34AM -0600, Jim Fehlig wrote:
On 10/17/18 12:59 PM, Marek Marczykowski-Górecki wrote:
On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote:
I had some couch time at the start of the weekend and was finally able to try using this series with virt-install. As it turns out, reporting duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we will want to report the additional <machine> under the existing 'xen' <guest> blocks.
Is that virt-install limitation? In that case, IMO virt-install should be fixed, instead of changing capabilities xml to match its limitations.
Perhaps it is a virt-install limitation, but my suggestion was based more on how the qemu driver reports the different machines
<guest> <os_type>hvm</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine maxCpus='255'>pc-i440fx-3.0</machine> <machine maxCpus='288'>pc-q35-3.0</machine> ... </arch> </guest>
Compare that with reporting PV and PVH in different <guest> blocks, where the <os_type> and <arch> are the same. It seems confusing from a consumers POV
<guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>xenpv</machine> </arch> </guest>
<guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>xenpvh</machine> </arch> </guest>
How should a consumer interpret that? Is the machine for os_type=xen, arch=x86_64 a xenpv or a xenpvh?
Yes, you are right - any pair of (os_type, arch) should be unique in the capabilities XML. So all machines should be reported in the same block.
Our difficulty with that is xenpv and xenpvh machines have different features. Marek pointed out that the qemu driver reports the "feature" maxCpus as an attribute on the machine element, but we're hesitant to keep adding attributes for each feature that is unique to a machine.
Another option we discussed was reporting a superset of all features so that e.g. (xen, x86_64) block would contain features supported by both PV and PVH and then rejecting unsupported features when parsing domXML or starting the VM. This option is rather distasteful.
And we also have the option of adding VIR_DOMAIN_OSTYPE_XENPVH, which I've shied away from but may be a better way to go in the end. Do you have any suggestions we may have overlooked?
Oooh, it looks like i've been mis-understanding PVH in all my previous reviews.
I thought it was simply a "normal" Xen paravirtualized guest kernel. ie any 'pv' guest is also a valid 'pvh' guest. Looking at the docs
https://wiki.xen.org/wiki/Xen_Project_Software_Overview#Guest_Types
It appears I was wrong. It says a pvh guest kernel relies on hardware virt extensions for part of its work and paravirt for other parts. So really is a hybrid between pv and hvm.
Right. The Xen wiki also has a good writeup about the various guest types https://wiki.xenproject.org/wiki/Understanding_the_Virtualization_Spectrum
With that in mind, we should indeed have a distinct OS type constant to express this.
There have been some long threads in the various versions of this series with a lot of waffling :-). I made a few attempts at summarizing what we learned about PV vs PVH but could never build a strong case (at least in my own head) for either of the two modeling approaches https://www.redhat.com/archives/libvir-list/2018-October/msg00214.html https://www.redhat.com/archives/libvir-list/2018-October/msg00891.html Regards, Jim
On Fri, Oct 19, 2018 at 08:53:15AM -0600, Jim Fehlig wrote:
On 10/19/18 8:14 AM, Daniel P. Berrangé wrote:
On Fri, Oct 19, 2018 at 08:06:18AM -0600, Jim Fehlig wrote:
On 10/19/18 3:11 AM, Daniel P. Berrangé wrote:
On Thu, Oct 18, 2018 at 11:08:34AM -0600, Jim Fehlig wrote:
On 10/17/18 12:59 PM, Marek Marczykowski-Górecki wrote:
On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote: > I had some couch time at the start of the weekend and was finally able to > try using this series with virt-install. As it turns out, reporting > duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we > will want to report the additional <machine> under the existing 'xen' > <guest> blocks.
Is that virt-install limitation? In that case, IMO virt-install should be fixed, instead of changing capabilities xml to match its limitations.
Perhaps it is a virt-install limitation, but my suggestion was based more on how the qemu driver reports the different machines
<guest> <os_type>hvm</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine maxCpus='255'>pc-i440fx-3.0</machine> <machine maxCpus='288'>pc-q35-3.0</machine> ... </arch> </guest>
Compare that with reporting PV and PVH in different <guest> blocks, where the <os_type> and <arch> are the same. It seems confusing from a consumers POV
<guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>xenpv</machine> </arch> </guest>
<guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>xenpvh</machine> </arch> </guest>
How should a consumer interpret that? Is the machine for os_type=xen, arch=x86_64 a xenpv or a xenpvh?
Yes, you are right - any pair of (os_type, arch) should be unique in the capabilities XML. So all machines should be reported in the same block.
Our difficulty with that is xenpv and xenpvh machines have different features. Marek pointed out that the qemu driver reports the "feature" maxCpus as an attribute on the machine element, but we're hesitant to keep adding attributes for each feature that is unique to a machine.
Another option we discussed was reporting a superset of all features so that e.g. (xen, x86_64) block would contain features supported by both PV and PVH and then rejecting unsupported features when parsing domXML or starting the VM. This option is rather distasteful.
And we also have the option of adding VIR_DOMAIN_OSTYPE_XENPVH, which I've shied away from but may be a better way to go in the end. Do you have any suggestions we may have overlooked?
Oooh, it looks like i've been mis-understanding PVH in all my previous reviews.
I thought it was simply a "normal" Xen paravirtualized guest kernel. ie any 'pv' guest is also a valid 'pvh' guest. Looking at the docs
https://wiki.xen.org/wiki/Xen_Project_Software_Overview#Guest_Types
It appears I was wrong. It says a pvh guest kernel relies on hardware virt extensions for part of its work and paravirt for other parts. So really is a hybrid between pv and hvm.
Right. The Xen wiki also has a good writeup about the various guest types
https://wiki.xenproject.org/wiki/Understanding_the_Virtualization_Spectrum
With that in mind, we should indeed have a distinct OS type constant to express this.
There have been some long threads in the various versions of this series with a lot of waffling :-). I made a few attempts at summarizing what we learned about PV vs PVH but could never build a strong case (at least in my own head) for either of the two modeling approaches
https://www.redhat.com/archives/libvir-list/2018-October/msg00214.html https://www.redhat.com/archives/libvir-list/2018-October/msg00891.html
It has a bad name, but essentially you should consider "ostype" to refer to the Hypervisor <-> Guest hardware ABI. Based on what I read, and your 2 links here, PV is clearly a different hardware ABI from PVH. Guest kernels needs different modifications for PV vs PVH. Sorry I didn't spot this sooner, and let this go off down the blind alley of switching based on machine type, when we should have used the ostype :-( 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 :|
On Fri, Oct 19, 2018 at 03:59:03PM +0100, Daniel P. Berrangé wrote:
On Fri, Oct 19, 2018 at 08:53:15AM -0600, Jim Fehlig wrote:
On 10/19/18 8:14 AM, Daniel P. Berrangé wrote:
On Fri, Oct 19, 2018 at 08:06:18AM -0600, Jim Fehlig wrote:
On 10/19/18 3:11 AM, Daniel P. Berrangé wrote:
On Thu, Oct 18, 2018 at 11:08:34AM -0600, Jim Fehlig wrote:
On 10/17/18 12:59 PM, Marek Marczykowski-Górecki wrote: > On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote: > > I had some couch time at the start of the weekend and was finally able to > > try using this series with virt-install. As it turns out, reporting > > duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we > > will want to report the additional <machine> under the existing 'xen' > > <guest> blocks. > > Is that virt-install limitation? In that case, IMO virt-install should > be fixed, instead of changing capabilities xml to match its limitations.
Perhaps it is a virt-install limitation, but my suggestion was based more on how the qemu driver reports the different machines
<guest> <os_type>hvm</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine maxCpus='255'>pc-i440fx-3.0</machine> <machine maxCpus='288'>pc-q35-3.0</machine> ... </arch> </guest>
Compare that with reporting PV and PVH in different <guest> blocks, where the <os_type> and <arch> are the same. It seems confusing from a consumers POV
<guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>xenpv</machine> </arch> </guest>
<guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>xenpvh</machine> </arch> </guest>
How should a consumer interpret that? Is the machine for os_type=xen, arch=x86_64 a xenpv or a xenpvh?
Yes, you are right - any pair of (os_type, arch) should be unique in the capabilities XML. So all machines should be reported in the same block.
Our difficulty with that is xenpv and xenpvh machines have different features. Marek pointed out that the qemu driver reports the "feature" maxCpus as an attribute on the machine element, but we're hesitant to keep adding attributes for each feature that is unique to a machine.
Another option we discussed was reporting a superset of all features so that e.g. (xen, x86_64) block would contain features supported by both PV and PVH and then rejecting unsupported features when parsing domXML or starting the VM. This option is rather distasteful.
And we also have the option of adding VIR_DOMAIN_OSTYPE_XENPVH, which I've shied away from but may be a better way to go in the end. Do you have any suggestions we may have overlooked?
Oooh, it looks like i've been mis-understanding PVH in all my previous reviews.
I thought it was simply a "normal" Xen paravirtualized guest kernel. ie any 'pv' guest is also a valid 'pvh' guest. Looking at the docs
https://wiki.xen.org/wiki/Xen_Project_Software_Overview#Guest_Types
It appears I was wrong. It says a pvh guest kernel relies on hardware virt extensions for part of its work and paravirt for other parts. So really is a hybrid between pv and hvm.
Right. The Xen wiki also has a good writeup about the various guest types
https://wiki.xenproject.org/wiki/Understanding_the_Virtualization_Spectrum
With that in mind, we should indeed have a distinct OS type constant to express this.
There have been some long threads in the various versions of this series with a lot of waffling :-). I made a few attempts at summarizing what we learned about PV vs PVH but could never build a strong case (at least in my own head) for either of the two modeling approaches
https://www.redhat.com/archives/libvir-list/2018-October/msg00214.html https://www.redhat.com/archives/libvir-list/2018-October/msg00891.html
It has a bad name, but essentially you should consider "ostype" to refer to the Hypervisor <-> Guest hardware ABI.
Oh, if that's the case, then indeed separate ostype makes sense. Maybe it worth expanding ostype description somewhere in documentation?
Based on what I read, and your 2 links here, PV is clearly a different hardware ABI from PVH. Guest kernels needs different modifications for PV vs PVH.
Sorry I didn't spot this sooner, and let this go off down the blind alley of switching based on machine type, when we should have used the ostype :-(
What machine type should it use then? Still xenpvh, but make all the decisions based on ostype? -- 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?
On Fri, Oct 19, 2018 at 05:10:30PM +0200, Marek Marczykowski-Górecki wrote:
On Fri, Oct 19, 2018 at 03:59:03PM +0100, Daniel P. Berrangé wrote:
On Fri, Oct 19, 2018 at 08:53:15AM -0600, Jim Fehlig wrote:
own head) for either of the two modeling approaches
https://www.redhat.com/archives/libvir-list/2018-October/msg00214.html https://www.redhat.com/archives/libvir-list/2018-October/msg00891.html
It has a bad name, but essentially you should consider "ostype" to refer to the Hypervisor <-> Guest hardware ABI.
Oh, if that's the case, then indeed separate ostype makes sense. Maybe it worth expanding ostype description somewhere in documentation?
Based on what I read, and your 2 links here, PV is clearly a different hardware ABI from PVH. Guest kernels needs different modifications for PV vs PVH.
Sorry I didn't spot this sooner, and let this go off down the blind alley of switching based on machine type, when we should have used the ostype :-(
What machine type should it use then? Still xenpvh, but make all the decisions based on ostype?
If Xen/QEMU calls the machine type 'xenpvh' then yeah we can still use it. By having a distinct ostype, when the XML says "xenpvh" for OS type, the XML parser can automatically find the correct machine type name from the capabilities data. So mgmt apps using libvirt won't need to set the machine type themselves, can just rely on the default, after they've set the ostype. 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 :|
On Fri, Oct 19, 2018 at 04:19:21PM +0100, Daniel P. Berrangé wrote:
On Fri, Oct 19, 2018 at 05:10:30PM +0200, Marek Marczykowski-Górecki wrote:
On Fri, Oct 19, 2018 at 03:59:03PM +0100, Daniel P. Berrangé wrote:
On Fri, Oct 19, 2018 at 08:53:15AM -0600, Jim Fehlig wrote:
own head) for either of the two modeling approaches
https://www.redhat.com/archives/libvir-list/2018-October/msg00214.html https://www.redhat.com/archives/libvir-list/2018-October/msg00891.html
It has a bad name, but essentially you should consider "ostype" to refer to the Hypervisor <-> Guest hardware ABI.
Oh, if that's the case, then indeed separate ostype makes sense. Maybe it worth expanding ostype description somewhere in documentation?
Also, such definition of os type, make "linux" os type for Xen PV even weirder...
Based on what I read, and your 2 links here, PV is clearly a different hardware ABI from PVH. Guest kernels needs different modifications for PV vs PVH.
Sorry I didn't spot this sooner, and let this go off down the blind alley of switching based on machine type, when we should have used the ostype :-(
What machine type should it use then? Still xenpvh, but make all the decisions based on ostype?
If Xen/QEMU calls the machine type 'xenpvh' then yeah we can still use it.
There is no qemu in the picture, and Xen (libxl) have just one thing: type, not separate os type and machine type. So, it's only libvirt specific bit here and we can choose it arbitrarily. As you wrote below, libvirt can fill it based on os type, so I'll make it "xenpvh".
By having a distinct ostype, when the XML says "xenpvh" for OS type, the XML parser can automatically find the correct machine type name from the capabilities data. So mgmt apps using libvirt won't need to set the machine type themselves, can just rely on the default, after they've set the ostype.
-- 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?
On 10/19/18 9:25 AM, Marek Marczykowski-Górecki wrote:
On Fri, Oct 19, 2018 at 04:19:21PM +0100, Daniel P. Berrangé wrote:
On Fri, Oct 19, 2018 at 05:10:30PM +0200, Marek Marczykowski-Górecki wrote:
On Fri, Oct 19, 2018 at 03:59:03PM +0100, Daniel P. Berrangé wrote:
On Fri, Oct 19, 2018 at 08:53:15AM -0600, Jim Fehlig wrote:
own head) for either of the two modeling approaches
https://www.redhat.com/archives/libvir-list/2018-October/msg00214.html https://www.redhat.com/archives/libvir-list/2018-October/msg00891.html
It has a bad name, but essentially you should consider "ostype" to refer to the Hypervisor <-> Guest hardware ABI.
Oh, if that's the case, then indeed separate ostype makes sense. Maybe it worth expanding ostype description somewhere in documentation?
Also, such definition of os type, make "linux" os type for Xen PV even weirder...
Yeah, I wish we could ditch it. BTW, please include a patch for docs/news.xml in V5. Thanks! Regards, Jim
On 10/19/18 8:59 AM, Daniel P. Berrangé wrote:
On Fri, Oct 19, 2018 at 08:53:15AM -0600, Jim Fehlig wrote:
On 10/19/18 8:14 AM, Daniel P. Berrangé wrote:
On Fri, Oct 19, 2018 at 08:06:18AM -0600, Jim Fehlig wrote:
On 10/19/18 3:11 AM, Daniel P. Berrangé wrote:
On Thu, Oct 18, 2018 at 11:08:34AM -0600, Jim Fehlig wrote:
On 10/17/18 12:59 PM, Marek Marczykowski-Górecki wrote: > On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote: >> I had some couch time at the start of the weekend and was finally able to >> try using this series with virt-install. As it turns out, reporting >> duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we >> will want to report the additional <machine> under the existing 'xen' >> <guest> blocks. > > Is that virt-install limitation? In that case, IMO virt-install should > be fixed, instead of changing capabilities xml to match its limitations.
Perhaps it is a virt-install limitation, but my suggestion was based more on how the qemu driver reports the different machines
<guest> <os_type>hvm</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine maxCpus='255'>pc-i440fx-3.0</machine> <machine maxCpus='288'>pc-q35-3.0</machine> ... </arch> </guest>
Compare that with reporting PV and PVH in different <guest> blocks, where the <os_type> and <arch> are the same. It seems confusing from a consumers POV
<guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>xenpv</machine> </arch> </guest>
<guest> <os_type>xen</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>xenpvh</machine> </arch> </guest>
How should a consumer interpret that? Is the machine for os_type=xen, arch=x86_64 a xenpv or a xenpvh?
Yes, you are right - any pair of (os_type, arch) should be unique in the capabilities XML. So all machines should be reported in the same block.
Our difficulty with that is xenpv and xenpvh machines have different features. Marek pointed out that the qemu driver reports the "feature" maxCpus as an attribute on the machine element, but we're hesitant to keep adding attributes for each feature that is unique to a machine.
Another option we discussed was reporting a superset of all features so that e.g. (xen, x86_64) block would contain features supported by both PV and PVH and then rejecting unsupported features when parsing domXML or starting the VM. This option is rather distasteful.
And we also have the option of adding VIR_DOMAIN_OSTYPE_XENPVH, which I've shied away from but may be a better way to go in the end. Do you have any suggestions we may have overlooked?
Oooh, it looks like i've been mis-understanding PVH in all my previous reviews.
I thought it was simply a "normal" Xen paravirtualized guest kernel. ie any 'pv' guest is also a valid 'pvh' guest. Looking at the docs
https://wiki.xen.org/wiki/Xen_Project_Software_Overview#Guest_Types
It appears I was wrong. It says a pvh guest kernel relies on hardware virt extensions for part of its work and paravirt for other parts. So really is a hybrid between pv and hvm.
Right. The Xen wiki also has a good writeup about the various guest types
https://wiki.xenproject.org/wiki/Understanding_the_Virtualization_Spectrum
With that in mind, we should indeed have a distinct OS type constant to express this.
There have been some long threads in the various versions of this series with a lot of waffling :-). I made a few attempts at summarizing what we learned about PV vs PVH but could never build a strong case (at least in my own head) for either of the two modeling approaches
https://www.redhat.com/archives/libvir-list/2018-October/msg00214.html https://www.redhat.com/archives/libvir-list/2018-October/msg00891.html
It has a bad name, but essentially you should consider "ostype" to refer to the Hypervisor <-> Guest hardware ABI.
This is the key point I didn't consider :-(.
Based on what I read, and your 2 links here, PV is clearly a different hardware ABI from PVH. Guest kernels needs different modifications for PV vs PVH.
Right.
Sorry I didn't spot this sooner, and let this go off down the blind alley of switching based on machine type, when we should have used the ostype :-(
I've been around here long enough that I should have realized your key point above. Marek, I don't know what else to say but I'm sorry and will owe you many drinks of your choice should our paths cross... Regards, Jim
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes in v3: - update for modified "libxl: add support for PVH" - skip PVH test on too old Xen --- tests/libxlxml2domconfigdata/basic-pvh.json | 49 ++++++++++++++++++++++- tests/libxlxml2domconfigdata/basic-pvh.xml | 28 +++++++++++++- tests/libxlxml2domconfigtest.c | 3 +- 3 files changed, 80 insertions(+) create mode 100644 tests/libxlxml2domconfigdata/basic-pvh.json create mode 100644 tests/libxlxml2domconfigdata/basic-pvh.xml diff --git a/tests/libxlxml2domconfigdata/basic-pvh.json b/tests/libxlxml2domconfigdata/basic-pvh.json new file mode 100644 index 0000000..48365c9 --- /dev/null +++ b/tests/libxlxml2domconfigdata/basic-pvh.json @@ -0,0 +1,49 @@ +{ + "c_info": { + "type": "pvh", + "name": "test-pvh", + "uuid": "039e9ee6-4a84-3055-4c81-8ba426ae2656" + }, + "b_info": { + "max_vcpus": 4, + "avail_vcpus": [ + 0, + 1, + 2, + 3 + ], + "max_memkb": 524288, + "target_memkb": 524288, + "shadow_memkb": 8192, + "sched_params": { + + }, + "kernel": "/boot/vmlinuz", + "ramdisk": "/boot/initrd.img", + "type.pvh": { + }, + "arch_arm": { + + } + }, + "disks": [ + { + "pdev_path": "/var/lib/xen/images/test-pv.img", + "vdev": "xvda", + "backend": "qdisk", + "format": "raw", + "removable": 1, + "readwrite": 1 + } + ], + "nics": [ + { + "devid": 0, + "mac": "00:16:3e:3e:86:60", + "bridge": "br0", + "script": "/etc/xen/scripts/vif-bridge", + "nictype": "vif" + } + ], + "on_reboot": "restart" +} diff --git a/tests/libxlxml2domconfigdata/basic-pvh.xml b/tests/libxlxml2domconfigdata/basic-pvh.xml new file mode 100644 index 0000000..7bcf67f --- /dev/null +++ b/tests/libxlxml2domconfigdata/basic-pvh.xml @@ -0,0 +1,28 @@ +<domain type='xen'> + <name>test-pvh</name> + <uuid>039e9ee6-4a84-3055-4c81-8ba426ae2656</uuid> + <memory>524288</memory> + <currentMemory>524288</currentMemory> + <vcpu>4</vcpu> + <os> + <type arch='x86_64' machine='xenpvh'>xen</type> + <kernel>/boot/vmlinuz</kernel> + <initrd>/boot/initrd.img</initrd> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <disk type='file' device='disk'> + <driver name='qemu'/> + <source file='/var/lib/xen/images/test-pv.img'/> + <target dev='xvda'/> + </disk> + <interface type='bridge'> + <source bridge='br0'/> + <mac address='00:16:3e:3e:86:60'/> + <script path='/etc/xen/scripts/vif-bridge'/> + </interface> + </devices> +</domain> diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c index 22f9c2c..a2b2f81 100644 --- a/tests/libxlxml2domconfigtest.c +++ b/tests/libxlxml2domconfigtest.c @@ -204,6 +204,9 @@ mymain(void) DO_TEST("basic-pv"); DO_TEST("basic-hvm"); +# ifdef HAVE_XEN_PVH + DO_TEST("basic-pvh"); +# endif DO_TEST("cpu-shares-hvm"); DO_TEST("variable-clock-hvm"); DO_TEST("moredevs-hvm"); -- git-series 0.9.1
builder="hvm" is deprecated since Xen 4.10, new syntax is type="hvm" (or type="pv", which is default). Since the old one is still supported, still use it when writing native config, so the config will work on older Xen too (and will also not complicate tests). Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- "type" option is the only syntax for specifying PVH guest, coming in next patch. Changes in v2: - fix code style --- src/xenconfig/xen_common.c | 18 +++++++++++++--- tests/xlconfigdata/test-fullvirt-type.cfg | 21 +++++++++++++++++++- tests/xlconfigdata/test-fullvirt-type.xml | 27 ++++++++++++++++++++++++- tests/xlconfigdata/test-paravirt-type.cfg | 13 ++++++++++++- tests/xlconfigdata/test-paravirt-type.xml | 25 ++++++++++++++++++++++- tests/xlconfigtest.c | 2 ++- 6 files changed, 103 insertions(+), 3 deletions(-) create mode 100644 tests/xlconfigdata/test-fullvirt-type.cfg create mode 100644 tests/xlconfigdata/test-fullvirt-type.xml create mode 100644 tests/xlconfigdata/test-paravirt-type.cfg create mode 100644 tests/xlconfigdata/test-paravirt-type.xml diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 0a99587..6c27936 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -1098,9 +1098,21 @@ xenParseGeneralMeta(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps) if (xenConfigGetUUID(conf, "uuid", def->uuid) < 0) goto out; - if ((xenConfigGetString(conf, "builder", &str, "linux") == 0) && - STREQ(str, "hvm")) - hvm = 1; + if (xenConfigGetString(conf, "type", &str, NULL) == 0 && str) { + if (STREQ(str, "pv")) { + hvm = 0; + } else if (STREQ(str, "hvm")) { + hvm = 1; + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("type %s is not supported"), str); + return -1; + } + } else { + if ((xenConfigGetString(conf, "builder", &str, "linux") == 0) && + STREQ(str, "hvm")) + hvm = 1; + } def->os.type = (hvm ? VIR_DOMAIN_OSTYPE_HVM : VIR_DOMAIN_OSTYPE_XEN); diff --git a/tests/xlconfigdata/test-fullvirt-type.cfg b/tests/xlconfigdata/test-fullvirt-type.cfg new file mode 100644 index 0000000..f8ecc2e --- /dev/null +++ b/tests/xlconfigdata/test-fullvirt-type.cfg @@ -0,0 +1,21 @@ +name = "XenGuest2" +uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809" +maxmem = 579 +memory = 394 +vcpus = 1 +pae = 1 +acpi = 1 +apic = 1 +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 = 0 +parallel = "none" +serial = "none" +type = "hvm" +boot = "d" diff --git a/tests/xlconfigdata/test-fullvirt-type.xml b/tests/xlconfigdata/test-fullvirt-type.xml new file mode 100644 index 0000000..da8e360 --- /dev/null +++ b/tests/xlconfigdata/test-fullvirt-type.xml @@ -0,0 +1,27 @@ +<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> + <acpi/> + <apic/> + <pae/> + </features> + <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'/> + <memballoon model='xen'/> + </devices> +</domain> diff --git a/tests/xlconfigdata/test-paravirt-type.cfg b/tests/xlconfigdata/test-paravirt-type.cfg new file mode 100644 index 0000000..078db99 --- /dev/null +++ b/tests/xlconfigdata/test-paravirt-type.cfg @@ -0,0 +1,13 @@ +name = "XenGuest2" +uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809" +type = "pv" +maxmem = 579 +memory = 394 +vcpus = 1 +localtime = 0 +on_poweroff = "destroy" +on_reboot = "restart" +on_crash = "restart" +kernel = "/tmp/vmlinuz" +ramdisk = "/tmp/initrd" +cmdline = "root=/dev/xvda1 console=hvc0" diff --git a/tests/xlconfigdata/test-paravirt-type.xml b/tests/xlconfigdata/test-paravirt-type.xml new file mode 100644 index 0000000..4357640 --- /dev/null +++ b/tests/xlconfigdata/test-paravirt-type.xml @@ -0,0 +1,25 @@ +<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='xenpv'>linux</type> + <kernel>/tmp/vmlinuz</kernel> + <initrd>/tmp/initrd</initrd> + <cmdline>root=/dev/xvda1 console=hvc0</cmdline> + </os> + <clock offset='utc' adjustment='reset'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <console type='pty'> + <target type='xen' port='0'/> + </console> + <input type='mouse' bus='xen'/> + <input type='keyboard' bus='xen'/> + <memballoon model='xen'/> + </devices> +</domain> diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c index b2e4591..6e3267e 100644 --- a/tests/xlconfigtest.c +++ b/tests/xlconfigtest.c @@ -279,6 +279,8 @@ mymain(void) DO_TEST_FORMAT("paravirt-cmdline-extra-root", false); DO_TEST_FORMAT("paravirt-cmdline-bogus-extra-root", false); DO_TEST("rbd-multihost-noauth"); + DO_TEST_FORMAT("paravirt-type", false); + DO_TEST_FORMAT("fullvirt-type", false); #ifdef LIBXL_HAVE_DEVICE_CHANNEL DO_TEST("channel-pty"); -- git-series 0.9.1
Handle PVH domain type in both directions (xen-xl->xml, xml->xen-xl). And add a test for it. Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes in v3: - update for modified "libxl: add support for PVH" --- src/xenconfig/xen_common.c | 11 +++++++++-- src/xenconfig/xen_xl.c | 5 +++++ tests/testutilsxen.c | 3 ++- tests/xlconfigdata/test-pvh-type.cfg | 13 +++++++++++++ tests/xlconfigdata/test-pvh-type.xml | 25 +++++++++++++++++++++++++ tests/xlconfigtest.c | 1 + 6 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 tests/xlconfigdata/test-pvh-type.cfg create mode 100644 tests/xlconfigdata/test-pvh-type.xml diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 6c27936..30b1c9f 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -1091,6 +1091,7 @@ xenParseGeneralMeta(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps) virCapsDomainDataPtr capsdata = NULL; VIR_AUTOFREE(char *) str = NULL; int hvm = 0, ret = -1; + const char *machine = "xenpv"; if (xenConfigCopyString(conf, "name", &def->name) < 0) goto out; @@ -1101,8 +1102,12 @@ xenParseGeneralMeta(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps) if (xenConfigGetString(conf, "type", &str, NULL) == 0 && str) { if (STREQ(str, "pv")) { hvm = 0; + } else if (STREQ(str, "pvh")) { + hvm = 0; + machine = "xenpvh"; } else if (STREQ(str, "hvm")) { hvm = 1; + machine = "xenfv"; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("type %s is not supported"), str); @@ -1110,14 +1115,16 @@ xenParseGeneralMeta(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps) } } else { if ((xenConfigGetString(conf, "builder", &str, "linux") == 0) && - STREQ(str, "hvm")) + STREQ(str, "hvm")) { hvm = 1; + machine = "xenfv"; + } } def->os.type = (hvm ? VIR_DOMAIN_OSTYPE_HVM : VIR_DOMAIN_OSTYPE_XEN); if (!(capsdata = virCapabilitiesDomainDataLookup(caps, def->os.type, - VIR_ARCH_NONE, def->virtType, NULL, NULL))) + VIR_ARCH_NONE, def->virtType, NULL, machine))) goto out; def->os.arch = capsdata->arch; diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c index 7250e57..0df8f35 100644 --- a/src/xenconfig/xen_xl.c +++ b/src/xenconfig/xen_xl.c @@ -1287,6 +1287,11 @@ xenFormatXLOS(virConfPtr conf, virDomainDefPtr def) /* XXX floppy disks */ } else { + if (STREQ(def->os.machine, "xenpvh")) { + if (xenConfigSetString(conf, "type", "pvh") < 0) + return -1; + } + if (def->os.bootloader && xenConfigSetString(conf, "bootloader", def->os.bootloader) < 0) return -1; diff --git a/tests/testutilsxen.c b/tests/testutilsxen.c index d34be72..c112912 100644 --- a/tests/testutilsxen.c +++ b/tests/testutilsxen.c @@ -17,7 +17,8 @@ testXLInitCaps(void) "xenfv" }; static const char *const xen_machines[] = { - "xenpv" + "xenpv", + "xenpvh" }; if ((caps = virCapabilitiesNew(virArchFromHost(), diff --git a/tests/xlconfigdata/test-pvh-type.cfg b/tests/xlconfigdata/test-pvh-type.cfg new file mode 100644 index 0000000..2493535 --- /dev/null +++ b/tests/xlconfigdata/test-pvh-type.cfg @@ -0,0 +1,13 @@ +name = "XenGuest2" +uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809" +maxmem = 579 +memory = 394 +vcpus = 1 +localtime = 0 +on_poweroff = "destroy" +on_reboot = "restart" +on_crash = "restart" +type = "pvh" +kernel = "/tmp/vmlinuz" +ramdisk = "/tmp/initrd" +cmdline = "root=/dev/xvda1 console=hvc0" diff --git a/tests/xlconfigdata/test-pvh-type.xml b/tests/xlconfigdata/test-pvh-type.xml new file mode 100644 index 0000000..7e3f182 --- /dev/null +++ b/tests/xlconfigdata/test-pvh-type.xml @@ -0,0 +1,25 @@ +<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='xenpvh'>xen</type> + <kernel>/tmp/vmlinuz</kernel> + <initrd>/tmp/initrd</initrd> + <cmdline>root=/dev/xvda1 console=hvc0</cmdline> + </os> + <clock offset='utc' adjustment='reset'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <console type='pty'> + <target type='xen' port='0'/> + </console> + <input type='mouse' bus='xen'/> + <input type='keyboard' bus='xen'/> + <memballoon model='xen'/> + </devices> +</domain> diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c index 6e3267e..5159182 100644 --- a/tests/xlconfigtest.c +++ b/tests/xlconfigtest.c @@ -281,6 +281,7 @@ mymain(void) DO_TEST("rbd-multihost-noauth"); DO_TEST_FORMAT("paravirt-type", false); DO_TEST_FORMAT("fullvirt-type", false); + DO_TEST("pvh-type"); #ifdef LIBXL_HAVE_DEVICE_CHANNEL DO_TEST("channel-pty"); -- git-series 0.9.1
participants (5)
-
Daniel P. Berrangé -
Jim Fehlig -
Marek Marczykowski-Górecki -
Michal Privoznik -
Peter Krempa