[libvirt] [PATCH v3 2/5] libxl: add support for PVH
Jim Fehlig
jfehlig at suse.com
Tue Oct 2 22:50:02 UTC 2018
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 at 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 {
>
More information about the libvir-list
mailing list