[libvirt] [PATCH v3 0/5] libxl: PVHv2 support
This is a respin of my old PVHv1 patch[1], converted to PVHv2. Should the code use "PVH" name (as libxl does internally), or "PVHv2" as in many places in Xen documentation? I've chosen the former, but want to confirm it. Also, not sure about VIR_DOMAIN_OSTYPE_XENPVH (as discussed on PVHv1 patch) - while it will be messy in many cases, there is libxl_domain_build_info.u.{hvm,pv,pvh} which would be good to not mess up. Also, PVHv2 needs different kernel features than PV (CONFIG_XEN_PVH vs CONFIG_XEN_PV), so keeping the same VIR_DOMAIN_OSTYPE_XEN could be confusing. On the other hand, libxl_domain_build_info.u.pv is used in very few places (one section of libxlMakeDomBuildInfo), so guarding u.hvm access with VIR_DOMAIN_OSTYPE_HVM may be enough. For now I've reused VIR_DOMAIN_OSTYPE_XEN - in the driver itself, most of the code is the same as for PV. 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? First few preparatory patches can be applied independently. [1] https://www.redhat.com/archives/libvir-list/2016-August/msg00376.html 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 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 +- src/conf/domain_conf.c | 6 +- src/libxl/libxl_capabilities.c | 47 +++++++++++--- 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 +- 19 files changed, 346 insertions(+), 36 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 --- docs/formatcaps.html.in | 4 +-- docs/schemas/domaincommon.rng | 1 +- src/conf/domain_conf.c | 6 ++-- src/libxl/libxl_capabilities.c | 47 ++++++++++++++++++++++++++++----- src/libxl/libxl_conf.c | 50 ++++++++++++++++++++++++++++++----- src/libxl/libxl_driver.c | 6 ++-- 6 files changed, 95 insertions(+), 19 deletions(-) diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in index 0d9c53d..b8102fd 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 PVHv2.</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/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..be51a4d 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; @@ -439,6 +440,9 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) int hvm = STRPREFIX(&token[subs[1].rm_so], "hvm"); virArch arch; int pae = 0, nonpae = 0, ia64_be = 0; +#ifdef LIBXL_DOMAIN_TYPE_PVH + bool new_arch_added = false; +#endif if (STRPREFIX(&token[subs[2].rm_so], "x86_32")) { arch = VIR_ARCH_I686; @@ -475,8 +479,12 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) if (i >= ARRAY_CARDINALITY(guest_archs)) continue; /* Didn't find a match, so create a new one */ - if (i == nr_guest_archs) + if (i == nr_guest_archs) { nr_guest_archs++; +#ifdef LIBXL_DOMAIN_TYPE_PVH + new_arch_added = true; +#endif + } guest_archs[i].arch = arch; guest_archs[i].hvm = hvm; @@ -491,13 +499,33 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) guest_archs[i].nonpae = nonpae; if (ia64_be) guest_archs[i].ia64_be = ia64_be; + + /* + * Xen 4.9 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 LIBXL_DOMAIN_TYPE_PVH + if (hvm && new_arch_added) { + 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 +585,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 +610,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 +762,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..0007906 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 LIBXL_DOMAIN_TYPE_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 LIBXL_DOMAIN_TYPE_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 9/30/18 8:15 PM, 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
No video, graphics or hostdev passthrough - bummer. Begs the question: what to do with PVH XML config containing these devices? Reject it? Silently ignore? I'll also need to remember to enable these as PVH gains support for the devices.
- use #ifdef LIBXL_DOMAIN_TYPE_PVH instead of hypervisor version to check for PVH support
This is a much better approach than the version check. I should have thought of that earlier, sorry.
- compile fix for Xen < 4.9 --- docs/formatcaps.html.in | 4 +-- docs/schemas/domaincommon.rng | 1 +- src/conf/domain_conf.c | 6 ++-- src/libxl/libxl_capabilities.c | 47 ++++++++++++++++++++++++++++----- src/libxl/libxl_conf.c | 50 ++++++++++++++++++++++++++++++----- src/libxl/libxl_driver.c | 6 ++-- 6 files changed, 95 insertions(+), 19 deletions(-)
diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in index 0d9c53d..b8102fd 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 PVHv2.</dd>
s/PVHv2/PVH/
<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/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..be51a4d 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; @@ -439,6 +440,9 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) int hvm = STRPREFIX(&token[subs[1].rm_so], "hvm"); virArch arch; int pae = 0, nonpae = 0, ia64_be = 0; +#ifdef LIBXL_DOMAIN_TYPE_PVH + bool new_arch_added = false; +#endif
if (STRPREFIX(&token[subs[2].rm_so], "x86_32")) { arch = VIR_ARCH_I686; @@ -475,8 +479,12 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) if (i >= ARRAY_CARDINALITY(guest_archs)) continue; /* Didn't find a match, so create a new one */ - if (i == nr_guest_archs) + if (i == nr_guest_archs) { nr_guest_archs++; +#ifdef LIBXL_DOMAIN_TYPE_PVH + new_arch_added = true; +#endif + }
My idea to add this variable for clarity now seems diluted by the need for the extra ifdef's :-/. Your original approach would avoid them. The rest of the patch looks good, but before going further let's get a resolution to the VIR_DOMAIN_OSTYPE_XENPVH vs os.machine approach to modeling PVH. I can do some poking on IRC if there are no responses to this thread. Regards, Jim
guest_archs[i].arch = arch; guest_archs[i].hvm = hvm; @@ -491,13 +499,33 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) guest_archs[i].nonpae = nonpae; if (ia64_be) guest_archs[i].ia64_be = ia64_be; + + /* + * Xen 4.9 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 LIBXL_DOMAIN_TYPE_PVH + if (hvm && new_arch_added) { + 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 +585,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 +610,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 +762,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..0007906 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 LIBXL_DOMAIN_TYPE_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 LIBXL_DOMAIN_TYPE_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 {
On Tue, Oct 02, 2018 at 04:50:02PM -0600, Jim Fehlig wrote:
On 9/30/18 8:15 PM, 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
No video, graphics or hostdev passthrough - bummer. Begs the question: what to do with PVH XML config containing these devices? Reject it? Silently ignore? I'll also need to remember to enable these as PVH gains support for the devices.
This is based on assumption that by design there is no QEMU, so there is nothing that could emulate VGA. But actually I'm not so sure about it right now - because from libxl side it is the same as for PV, and actually I see some code to start qemu for PV domains. I'm not sure if it's capable of emulating VGA, but probably can do vfb+vkb backend and expose it as VNC. Hostdev passthrough will be there one day...
- use #ifdef LIBXL_DOMAIN_TYPE_PVH instead of hypervisor version to check for PVH support
This is a much better approach than the version check. I should have thought of that earlier, sorry.
- compile fix for Xen < 4.9 --- docs/formatcaps.html.in | 4 +-- docs/schemas/domaincommon.rng | 1 +- src/conf/domain_conf.c | 6 ++-- src/libxl/libxl_capabilities.c | 47 ++++++++++++++++++++++++++++----- src/libxl/libxl_conf.c | 50 ++++++++++++++++++++++++++++++----- src/libxl/libxl_driver.c | 6 ++-- 6 files changed, 95 insertions(+), 19 deletions(-)
diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in index 0d9c53d..b8102fd 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 PVHv2.</dd>
s/PVHv2/PVH/
<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/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..be51a4d 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; @@ -439,6 +440,9 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) int hvm = STRPREFIX(&token[subs[1].rm_so], "hvm"); virArch arch; int pae = 0, nonpae = 0, ia64_be = 0; +#ifdef LIBXL_DOMAIN_TYPE_PVH + bool new_arch_added = false; +#endif if (STRPREFIX(&token[subs[2].rm_so], "x86_32")) { arch = VIR_ARCH_I686; @@ -475,8 +479,12 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) if (i >= ARRAY_CARDINALITY(guest_archs)) continue; /* Didn't find a match, so create a new one */ - if (i == nr_guest_archs) + if (i == nr_guest_archs) { nr_guest_archs++; +#ifdef LIBXL_DOMAIN_TYPE_PVH + new_arch_added = true; +#endif + }
My idea to add this variable for clarity now seems diluted by the need for the extra ifdef's :-/. Your original approach would avoid them.
This #ifdef is only to mute compiler warning (variable set but unused). If you have a better idea for that, I'd very much like to drop #ifdef here.
The rest of the patch looks good, but before going further let's get a resolution to the VIR_DOMAIN_OSTYPE_XENPVH vs os.machine approach to modeling PVH. I can do some poking on IRC if there are no responses to this thread.
-- 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/2/18 5:02 PM, Marek Marczykowski-Górecki wrote:
On Tue, Oct 02, 2018 at 04:50:02PM -0600, Jim Fehlig wrote:
On 9/30/18 8:15 PM, 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
No video, graphics or hostdev passthrough - bummer. Begs the question: what to do with PVH XML config containing these devices? Reject it? Silently ignore? I'll also need to remember to enable these as PVH gains support for the devices.
This is based on assumption that by design there is no QEMU, so there is nothing that could emulate VGA. But actually I'm not so sure about it right now - because from libxl side it is the same as for PV, and actually I see some code to start qemu for PV domains. I'm not sure if it's capable of emulating VGA, but probably can do vfb+vkb backend and expose it as VNC.
Hostdev passthrough will be there one day...
- use #ifdef LIBXL_DOMAIN_TYPE_PVH instead of hypervisor version to check for PVH support
This is a much better approach than the version check. I should have thought of that earlier, sorry.
- compile fix for Xen < 4.9 --- docs/formatcaps.html.in | 4 +-- docs/schemas/domaincommon.rng | 1 +- src/conf/domain_conf.c | 6 ++-- src/libxl/libxl_capabilities.c | 47 ++++++++++++++++++++++++++++----- src/libxl/libxl_conf.c | 50 ++++++++++++++++++++++++++++++----- src/libxl/libxl_driver.c | 6 ++-- 6 files changed, 95 insertions(+), 19 deletions(-)
diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in index 0d9c53d..b8102fd 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 PVHv2.</dd>
s/PVHv2/PVH/
<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/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..be51a4d 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; @@ -439,6 +440,9 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) int hvm = STRPREFIX(&token[subs[1].rm_so], "hvm"); virArch arch; int pae = 0, nonpae = 0, ia64_be = 0; +#ifdef LIBXL_DOMAIN_TYPE_PVH + bool new_arch_added = false; +#endif if (STRPREFIX(&token[subs[2].rm_so], "x86_32")) { arch = VIR_ARCH_I686; @@ -475,8 +479,12 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) if (i >= ARRAY_CARDINALITY(guest_archs)) continue; /* Didn't find a match, so create a new one */ - if (i == nr_guest_archs) + if (i == nr_guest_archs) { nr_guest_archs++; +#ifdef LIBXL_DOMAIN_TYPE_PVH + new_arch_added = true; +#endif + }
My idea to add this variable for clarity now seems diluted by the need for the extra ifdef's :-/. Your original approach would avoid them.
This #ifdef is only to mute compiler warning (variable set but unused). If you have a better idea for that, I'd very much like to drop #ifdef here.
Given the ifdef trickery needed with the extra variable, I think the better idea is your original idea :-) i == nr_guest_archs - 1 Regards, Jim
On Tue, Oct 02, 2018 at 06:54:01PM -0600, Jim Fehlig wrote:
On 10/2/18 5:02 PM, Marek Marczykowski-Górecki wrote:
On Tue, Oct 02, 2018 at 04:50:02PM -0600, Jim Fehlig wrote:
My idea to add this variable for clarity now seems diluted by the need for the extra ifdef's :-/. Your original approach would avoid them.
This #ifdef is only to mute compiler warning (variable set but unused). If you have a better idea for that, I'd very much like to drop #ifdef here.
Given the ifdef trickery needed with the extra variable, I think the better idea is your original idea :-)
i == nr_guest_archs - 1
Ok, will rollback to that. But I'll wait for os.machine/os.type opinion. -- 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/2/18 4:50 PM, Jim Fehlig wrote:
On 9/30/18 8:15 PM, 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
No video, graphics or hostdev passthrough - bummer. Begs the question: what to do with PVH XML config containing these devices? Reject it? Silently ignore? I'll also need to remember to enable these as PVH gains support for the devices.
- use #ifdef LIBXL_DOMAIN_TYPE_PVH instead of hypervisor version to check for PVH support
This is a much better approach than the version check. I should have thought of that earlier, sorry.
Actually it is not. LIBXL_DOMAIN_TYPE_PVH is a value in the enum libxl_domain_type. Too bad PVH support isn't advertised in libxl.h with something like LIBXL_HAVE_PVH. Looks like we are stuck with the version check :-(. Regards, Jim
On Wed, Oct 03, 2018 at 03:13:30PM -0600, Jim Fehlig wrote:
On 10/2/18 4:50 PM, Jim Fehlig wrote:
On 9/30/18 8:15 PM, 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
No video, graphics or hostdev passthrough - bummer. Begs the question: what to do with PVH XML config containing these devices? Reject it? Silently ignore? I'll also need to remember to enable these as PVH gains support for the devices.
- use #ifdef LIBXL_DOMAIN_TYPE_PVH instead of hypervisor version to check for PVH support
This is a much better approach than the version check. I should have thought of that earlier, sorry.
Actually it is not. LIBXL_DOMAIN_TYPE_PVH is a value in the enum libxl_domain_type. Too bad PVH support isn't advertised in libxl.h with something like LIBXL_HAVE_PVH. Looks like we are stuck with the version check :-(.
Hmm, but that version check is a runtime check. How to fix compile error regarding missing LIBXL_DOMAIN_TYPE_PVH then? Let configure check that and define some HAVE_xxx? -- 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/3/18 3:25 PM, Marek Marczykowski-Górecki wrote:
On Wed, Oct 03, 2018 at 03:13:30PM -0600, Jim Fehlig wrote:
On 10/2/18 4:50 PM, Jim Fehlig wrote:
On 9/30/18 8:15 PM, 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
No video, graphics or hostdev passthrough - bummer. Begs the question: what to do with PVH XML config containing these devices? Reject it? Silently ignore? I'll also need to remember to enable these as PVH gains support for the devices.
- use #ifdef LIBXL_DOMAIN_TYPE_PVH instead of hypervisor version to check for PVH support
This is a much better approach than the version check. I should have thought of that earlier, sorry.
Actually it is not. LIBXL_DOMAIN_TYPE_PVH is a value in the enum libxl_domain_type. Too bad PVH support isn't advertised in libxl.h with something like LIBXL_HAVE_PVH. Looks like we are stuck with the version check :-(.
Hmm, but that version check is a runtime check. How to fix compile error regarding missing LIBXL_DOMAIN_TYPE_PVH then? Let configure check that and define some HAVE_xxx?
AFAICT, PVH support actually came in Xen 4.10 commit 27f826844d3666f151e86d66198583757cccbbed Author: Roger Pau Monne <roger.pau@citrix.com> Date: Fri Sep 22 16:25:07 2017 +0100 libxl: introduce a PVH guest type git describe --contains 27f826844d3 4.10.0-rc1~219 And the only thing in that commit to check is the addition of LIBXL_DOMAIN_TYPE_PVH to the libxl_domain_type enum. I think we'll have to check it with AC_COMPILE_IFELSE. I've been fiddling with the below patch, which at least works on a Xen >= 4.10. Can you check it on Xen < 4.10 too, and squash it into your series? Regards, Jim diff --git a/m4/virt-driver-libxl.m4 b/m4/virt-driver-libxl.m4 index 479d9116a4..ae309abb80 100644 --- a/m4/virt-driver-libxl.m4 +++ b/m4/virt-driver-libxl.m4 @@ -73,6 +73,22 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_LIBXL], [ ],[ LIBXL_LIBS="$LIBXL_LIBS -lxenstore -lxenctrl" ]) + + dnl Check if Xen has support for PVH + AC_MSG_CHECKING([for Xen PVH support]) + AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <libxl.h>]], + [[int dummy = LIBXL_DOMAIN_TYPE_PVH;]]) + ], + [ + AC_MSG_RESULT([yes]) + AC_DEFINE_UNQUOTED([HAVE_XEN_PVH], [1], [Define to 1 if Xen has PVH support.]) + ], + [ + AC_MSG_RESULT([no]) + AC_DEFINE_UNQUOTED([HAVE_XEN_PVH], [0], [Define to 1 if Xen has PVH support.]) + ] + ) + fi AC_SUBST([LIBXL_CFLAGS])
On 10/3/18 5:59 PM, Jim Fehlig wrote:
On 10/3/18 3:25 PM, Marek Marczykowski-Górecki wrote:
On Wed, Oct 03, 2018 at 03:13:30PM -0600, Jim Fehlig wrote:
On 10/2/18 4:50 PM, Jim Fehlig wrote:
On 9/30/18 8:15 PM, 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
No video, graphics or hostdev passthrough - bummer. Begs the question: what to do with PVH XML config containing these devices? Reject it? Silently ignore? I'll also need to remember to enable these as PVH gains support for the devices.
- use #ifdef LIBXL_DOMAIN_TYPE_PVH instead of hypervisor version to check for PVH support
This is a much better approach than the version check. I should have thought of that earlier, sorry.
Actually it is not. LIBXL_DOMAIN_TYPE_PVH is a value in the enum libxl_domain_type. Too bad PVH support isn't advertised in libxl.h with something like LIBXL_HAVE_PVH. Looks like we are stuck with the version check :-(.
Hmm, but that version check is a runtime check. How to fix compile error regarding missing LIBXL_DOMAIN_TYPE_PVH then? Let configure check that and define some HAVE_xxx?
AFAICT, PVH support actually came in Xen 4.10
commit 27f826844d3666f151e86d66198583757cccbbed Author: Roger Pau Monne <roger.pau@citrix.com> Date: Fri Sep 22 16:25:07 2017 +0100
libxl: introduce a PVH guest type
git describe --contains 27f826844d3 4.10.0-rc1~219
And the only thing in that commit to check is the addition of LIBXL_DOMAIN_TYPE_PVH to the libxl_domain_type enum. I think we'll have to check it with AC_COMPILE_IFELSE. I've been fiddling with the below patch, which at least works on a Xen >= 4.10. Can you check it on Xen < 4.10 too, and squash it into your series?
Formatting is jacked due to crazy whitespace. I'll attach it...
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..a11e525 100644 --- a/tests/libxlxml2domconfigtest.c +++ b/tests/libxlxml2domconfigtest.c @@ -204,6 +204,9 @@ mymain(void) DO_TEST("basic-pv"); DO_TEST("basic-hvm"); +# ifdef LIBXL_DOMAIN_TYPE_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
On 9/30/18 8:15 PM, Marek Marczykowski-Górecki wrote:
This is a respin of my old PVHv1 patch[1], converted to PVHv2. Should the code use "PVH" name (as libxl does internally), or "PVHv2" as in many places in Xen documentation? I've chosen the former, but want to confirm it.
"PVH".
Also, not sure about VIR_DOMAIN_OSTYPE_XENPVH (as discussed on PVHv1 patch) - while it will be messy in many cases, there is libxl_domain_build_info.u.{hvm,pv,pvh} which would be good to not mess up. Also, PVHv2 needs different kernel features than PV (CONFIG_XEN_PVH vs CONFIG_XEN_PV), so keeping the same VIR_DOMAIN_OSTYPE_XEN could be confusing.
These are good reasons for going with VIR_DOMAIN_OSTYPE_XENPVH. Another is it avoids the STREQ(def->os.machine, "xenpvh"), which I think others will find appealing.
On the other hand, libxl_domain_build_info.u.pv is used in very few places (one section of libxlMakeDomBuildInfo), so guarding u.hvm access with VIR_DOMAIN_OSTYPE_HVM may be enough. For now I've reused VIR_DOMAIN_OSTYPE_XEN - in the driver itself, most of the code is the same as for PV.
I'll reiterate my rationalization for using VIR_DOMAIN_OSTYPE_XEN for both and differentiating them with os.machine: both PV and PVH are OS types that have been modified to run on Xen. I'd still like to get some opinions from other maintainers on this. Anyone care to share their thoughts on which approach best models PVH? (Perhaps trying to add PVH support in virt-manager on top of this series will help flesh out the better approach. I'll give it a try...) Regards, Jim
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?
First few preparatory patches can be applied independently.
[1] https://www.redhat.com/archives/libvir-list/2016-August/msg00376.html
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
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 +- src/conf/domain_conf.c | 6 +- src/libxl/libxl_capabilities.c | 47 +++++++++++--- 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 +- 19 files changed, 346 insertions(+), 36 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
On 10/2/18 3:38 PM, Jim Fehlig wrote:
On 9/30/18 8:15 PM, Marek Marczykowski-Górecki wrote:
This is a respin of my old PVHv1 patch[1], converted to PVHv2. Should the code use "PVH" name (as libxl does internally), or "PVHv2" as in many places in Xen documentation? I've chosen the former, but want to confirm it.
"PVH".
Also, not sure about VIR_DOMAIN_OSTYPE_XENPVH (as discussed on PVHv1 patch) - while it will be messy in many cases, there is libxl_domain_build_info.u.{hvm,pv,pvh} which would be good to not mess up. Also, PVHv2 needs different kernel features than PV (CONFIG_XEN_PVH vs CONFIG_XEN_PV), so keeping the same VIR_DOMAIN_OSTYPE_XEN could be confusing.
These are good reasons for going with VIR_DOMAIN_OSTYPE_XENPVH. Another is it avoids the STREQ(def->os.machine, "xenpvh"), which I think others will find appealing.
On the other hand, libxl_domain_build_info.u.pv is used in very few places (one section of libxlMakeDomBuildInfo), so guarding u.hvm access with VIR_DOMAIN_OSTYPE_HVM may be enough. For now I've reused VIR_DOMAIN_OSTYPE_XEN - in the driver itself, most of the code is the same as for PV.
I'll reiterate my rationalization for using VIR_DOMAIN_OSTYPE_XEN for both and differentiating them with os.machine: both PV and PVH are OS types that have been modified to run on Xen.
I'd still like to get some opinions from other maintainers on this. Anyone care to share their thoughts on which approach best models PVH?
I've been thinking about this more and AFAIK on the qemu side the machine attribute models a chipset, e.g. <type arch='x86_64' machine='pc-i440fx'>hvm</type> <type arch='x86_64' machine='pc-q35'>hvm</type> Is it a stretch to refer to PV and PVH as chipsets? If so, my position on the fence in now leaning towards VIR_DOMAIN_OSTYPE_XENPVH :-/ Regards, Jim
On 10/2/18 7:05 PM, Jim Fehlig wrote:
On 10/2/18 3:38 PM, Jim Fehlig wrote:
On 9/30/18 8:15 PM, Marek Marczykowski-Górecki wrote:
This is a respin of my old PVHv1 patch[1], converted to PVHv2. Should the code use "PVH" name (as libxl does internally), or "PVHv2" as in many places in Xen documentation? I've chosen the former, but want to confirm it.
"PVH".
Also, not sure about VIR_DOMAIN_OSTYPE_XENPVH (as discussed on PVHv1 patch) - while it will be messy in many cases, there is libxl_domain_build_info.u.{hvm,pv,pvh} which would be good to not mess up. Also, PVHv2 needs different kernel features than PV (CONFIG_XEN_PVH vs CONFIG_XEN_PV), so keeping the same VIR_DOMAIN_OSTYPE_XEN could be confusing.
These are good reasons for going with VIR_DOMAIN_OSTYPE_XENPVH. Another is it avoids the STREQ(def->os.machine, "xenpvh"), which I think others will find appealing.
On the other hand, libxl_domain_build_info.u.pv is used in very few places (one section of libxlMakeDomBuildInfo), so guarding u.hvm access with VIR_DOMAIN_OSTYPE_HVM may be enough. For now I've reused VIR_DOMAIN_OSTYPE_XEN - in the driver itself, most of the code is the same as for PV.
I'll reiterate my rationalization for using VIR_DOMAIN_OSTYPE_XEN for both and differentiating them with os.machine: both PV and PVH are OS types that have been modified to run on Xen.
I'd still like to get some opinions from other maintainers on this. Anyone care to share their thoughts on which approach best models PVH?
I've been thinking about this more and AFAIK on the qemu side the machine attribute models a chipset, e.g.
<type arch='x86_64' machine='pc-i440fx'>hvm</type> <type arch='x86_64' machine='pc-q35'>hvm</type>
Is it a stretch to refer to PV and PVH as chipsets? If so, my position on the fence in now leaning towards VIR_DOMAIN_OSTYPE_XENPVH :-/
We had a bit more discussion on IRC today and generally concluded VIR_DOMAIN_OSTYPE_XEN and os.machine="xenpvh" is the better approach to model PVH. Some of the rational for the conclusion (in addition to items already mentioned above) was based on info in the related Xen wiki [0] which includes - PVH is described under the "Enhancements to PV:" section - PVH is described as "fully PV kernel mode, running with paravirtualized disk and network, paravirtualized interrupts and timers, no emulated devices of any kind (and thus no qemu), no BIOS or legacy boot - but instead of requiring PV MMU, it uses the HVM hardware extensions to virtualize the pagetables, as well as system calls and other privileged operations". So PVH is the result of the evolution of PV, a PV++ or PVImproved if you will. - Once PVH is mature and well established, the community plans to remove non-PVH support from the Linux kernel. IOW the long-term plan is for PVH to become the new PV. Regards, Jim [0] https://wiki.xenproject.org/wiki/Understanding_the_Virtualization_Spectrum
participants (2)
-
Jim Fehlig -
Marek Marczykowski-Górecki