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(a)invisiblethingslab.com&gt; --- 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(&regex); 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 Tue, Oct 09, 2018 at 04:45:01PM -0600, Jim Fehlig wrote: 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 Tue, Oct 09, 2018 at 04:45:01PM -0600, Jim Fehlig wrote: 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 Wed, Oct 10, 2018 at 11:06:38AM +0200, Michal Privoznik wrote: Sigh, yes, of course. 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 10/7/18 9:39 AM, Marek Marczykowski-Górecki wrote: Nice! I was unable to get this to work, probably due to some botched quoting. 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

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(a)invisiblethingslab.com&gt; --- "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