[libvirt] [PATCH v2 0/8] 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 Marek Marczykowski-Górecki (8): docs: add documentation of arch element of capabilities.xml libxl: set shadow memory for any guest type, not only HVM libxl: prefer new location of nested_hvm in libxl_domain_build_info 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 | 22 ++++- docs/formatdomain.html.in | 11 +- docs/schemas/domaincommon.rng | 1 +- src/libxl/libxl_capabilities.c | 33 ++++++- src/libxl/libxl_conf.c | 81 ++++++++++++----- src/libxl/libxl_driver.c | 6 +- src/xenconfig/xen_common.c | 27 +++++- src/xenconfig/xen_xl.c | 5 +- tests/libxlxml2domconfigdata/basic-pv.json | 1 +- tests/libxlxml2domconfigdata/basic-pvh.json | 49 ++++++++++- tests/libxlxml2domconfigdata/basic-pvh.xml | 28 ++++++- tests/libxlxml2domconfigdata/fullvirt-cpuid.json | 2 +- tests/libxlxml2domconfigdata/multiple-ip.json | 1 +- tests/libxlxml2domconfigdata/vnuma-hvm.json | 2 +- tests/libxlxml2domconfigtest.c | 1 +- 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 +- 23 files changed, 359 insertions(+), 41 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: 16858439deaec0832de61c5ddb93d8e80adccf6c -- git-series 0.9.1

Specifically, list sub-elements and where they can be used. In addition, describe supported machine types for Xen. Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes in v2: - clarify type@domain description, add a link to domain xml there --- docs/formatcaps.html.in | 22 +++++++++++++++++++++- docs/formatdomain.html.in | 11 ++++++----- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in index 41a9a3a..0d9c53d 100644 --- a/docs/formatcaps.html.in +++ b/docs/formatcaps.html.in @@ -91,7 +91,27 @@ </dd> <dt><code>arch</code></dt> - <dd>This element brings some information on supported guest architecture.</dd> + <dd>This element brings some information on supported guest + architecture. Possible subelements are: + <dl> + <dt><code>wordsize</code></dt><dd>Size of CPU word in bits, for example 64.</dd> + <dt><code>emulator</code></dt><dd>Emulator (device model) path, for + use in <a href="formatdomain.html#elementEmulator">emulator</a> + element of domain XML.</dd> + <dt><code>loader</code></dt><dd>Loader path, for use in + <a href="formatdomain.html#elementLoader">loader</a> element of domain + XML.</dd> + <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> + <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> + attribute of the domain root element.</dd> + </dl> + </dd> <dt><code>features</code></dt> <dd>This optional element encases possible features that can be used diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 1f12ab5..8189959 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -19,7 +19,8 @@ <p> The root element required for all virtual machines is named <code>domain</code>. It has two attributes, the - <code>type</code> specifies the hypervisor used for running + <a id="attributeDomainType"><code>type</code></a> + specifies the hypervisor used for running the domain. The allowed values are driver specific, but include "xen", "kvm", "qemu", "lxc" and "kqemu". The second attribute is <code>id</code> which is a unique @@ -148,11 +149,11 @@ (badly named!) refers to an OS that supports the Xen 3 hypervisor guest ABI. There are also two optional attributes, <code>arch</code> specifying the CPU architecture to virtualization, - and <code>machine</code> referring to the machine - type. The <a href="formatcaps.html">Capabilities XML</a> + and <a id="attributeOSTypeMachine"><code>machine</code></a> referring + to the machine type. The <a href="formatcaps.html">Capabilities XML</a> provides details on allowed values for these. <span class="since">Since 0.0.1</span></dd> - <dt><code>loader</code></dt> + <dt><a id="elementLoader"><code>loader</code></a></dt> <dd>The optional <code>loader</code> tag refers to a firmware blob, which is specified by absolute path, used to assist the domain creation process. It is used by Xen @@ -2621,7 +2622,7 @@ ...</pre> <dl> - <dt><code>emulator</code></dt> + <dt><a id="elementEmulator"><code>emulator</code></a></dt> <dd> The contents of the <code>emulator</code> element specify the fully qualified path to the device model emulator binary. -- git-series 0.9.1

On 9/18/18 6:50 PM, Marek Marczykowski-Górecki wrote:
Specifically, list sub-elements and where they can be used. In addition, describe supported machine types for Xen.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
--- Changes in v2: - clarify type@domain description, add a link to domain xml there --- docs/formatcaps.html.in | 22 +++++++++++++++++++++- docs/formatdomain.html.in | 11 ++++++----- 2 files changed, 27 insertions(+), 6 deletions(-)
Reviewed-by: Jim Fehlig <jfehlig@suse.com> I've pushed patches 1-3. Regards, Jim
diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in index 41a9a3a..0d9c53d 100644 --- a/docs/formatcaps.html.in +++ b/docs/formatcaps.html.in @@ -91,7 +91,27 @@ </dd>
<dt><code>arch</code></dt> - <dd>This element brings some information on supported guest architecture.</dd> + <dd>This element brings some information on supported guest + architecture. Possible subelements are: + <dl> + <dt><code>wordsize</code></dt><dd>Size of CPU word in bits, for example 64.</dd> + <dt><code>emulator</code></dt><dd>Emulator (device model) path, for + use in <a href="formatdomain.html#elementEmulator">emulator</a> + element of domain XML.</dd> + <dt><code>loader</code></dt><dd>Loader path, for use in + <a href="formatdomain.html#elementLoader">loader</a> element of domain + XML.</dd> + <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> + <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> + attribute of the domain root element.</dd> + </dl> + </dd>
<dt><code>features</code></dt> <dd>This optional element encases possible features that can be used diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 1f12ab5..8189959 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -19,7 +19,8 @@ <p> The root element required for all virtual machines is named <code>domain</code>. It has two attributes, the - <code>type</code> specifies the hypervisor used for running + <a id="attributeDomainType"><code>type</code></a> + specifies the hypervisor used for running the domain. The allowed values are driver specific, but include "xen", "kvm", "qemu", "lxc" and "kqemu". The second attribute is <code>id</code> which is a unique @@ -148,11 +149,11 @@ (badly named!) refers to an OS that supports the Xen 3 hypervisor guest ABI. There are also two optional attributes, <code>arch</code> specifying the CPU architecture to virtualization, - and <code>machine</code> referring to the machine - type. The <a href="formatcaps.html">Capabilities XML</a> + and <a id="attributeOSTypeMachine"><code>machine</code></a> referring + to the machine type. The <a href="formatcaps.html">Capabilities XML</a> provides details on allowed values for these. <span class="since">Since 0.0.1</span></dd> - <dt><code>loader</code></dt> + <dt><a id="elementLoader"><code>loader</code></a></dt> <dd>The optional <code>loader</code> tag refers to a firmware blob, which is specified by absolute path, used to assist the domain creation process. It is used by Xen @@ -2621,7 +2622,7 @@ ...</pre>
<dl> - <dt><code>emulator</code></dt> + <dt><a id="elementEmulator"><code>emulator</code></a></dt> <dd> The contents of the <code>emulator</code> element specify the fully qualified path to the device model emulator binary.

Otherwise starting PVH guest will result in "arch_setup_bootlate: mapping shared_info failed (pfn=..., rc=-1, errno: 12): Internal error". After this change the behavior is the same as in `xl`. Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Reviewed-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_conf.c | 10 +++++----- tests/libxlxml2domconfigdata/basic-pv.json | 1 + tests/libxlxml2domconfigdata/multiple-ip.json | 1 + 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 998029d..476bcbe 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -634,11 +634,6 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, return -1; } #endif - - /* Allow libxl to calculate shadow memory requirements */ - b_info->shadow_memkb = - libxl_get_required_shadow_memory(b_info->max_memkb, - b_info->max_vcpus); } else { /* * For compatibility with the legacy xen toolstack, default to pygrub @@ -692,6 +687,11 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, } } + /* Allow libxl to calculate shadow memory requirements */ + b_info->shadow_memkb = + libxl_get_required_shadow_memory(b_info->max_memkb, + b_info->max_vcpus); + return 0; } diff --git a/tests/libxlxml2domconfigdata/basic-pv.json b/tests/libxlxml2domconfigdata/basic-pv.json index 0f846da..b71c3b0 100644 --- a/tests/libxlxml2domconfigdata/basic-pv.json +++ b/tests/libxlxml2domconfigdata/basic-pv.json @@ -14,6 +14,7 @@ ], "max_memkb": 524288, "target_memkb": 524288, + "shadow_memkb": 8192, "sched_params": { }, diff --git a/tests/libxlxml2domconfigdata/multiple-ip.json b/tests/libxlxml2domconfigdata/multiple-ip.json index 80dca82..2db98b8 100644 --- a/tests/libxlxml2domconfigdata/multiple-ip.json +++ b/tests/libxlxml2domconfigdata/multiple-ip.json @@ -14,6 +14,7 @@ ], "max_memkb": 524288, "target_memkb": 524288, + "shadow_memkb": 8192, "sched_params": { }, -- git-series 0.9.1

If available, use b_info->nested_hvm instead of b_info->u.hvm.nested_hvm. This will make nested HVM config available also for PVH domains. Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Reviewed-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_conf.c | 13 ++++++++++++- tests/libxlxml2domconfigdata/fullvirt-cpuid.json | 2 +- tests/libxlxml2domconfigdata/vnuma-hvm.json | 2 +- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 476bcbe..e2bfa2f 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -455,7 +455,18 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, } } } - libxl_defbool_set(&b_info->u.hvm.nested_hvm, hasHwVirt); +#ifdef LIBXL_HAVE_BUILDINFO_NESTED_HVM + libxl_defbool_set(&b_info->nested_hvm, hasHwVirt); +#else + if (hvm) { + libxl_defbool_set(&b_info->u.hvm.nested_hvm, hasHwVirt); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported nested HVM setting for %s machine on this Xen version"), + def->os.machine); + return -1; + } +#endif } if (def->cpu && def->cpu->mode == VIR_CPU_MODE_CUSTOM) { diff --git a/tests/libxlxml2domconfigdata/fullvirt-cpuid.json b/tests/libxlxml2domconfigdata/fullvirt-cpuid.json index cdc8b98..d46b464 100644 --- a/tests/libxlxml2domconfigdata/fullvirt-cpuid.json +++ b/tests/libxlxml2domconfigdata/fullvirt-cpuid.json @@ -21,11 +21,11 @@ ], "sched_params": { }, + "nested_hvm": "False", "type.hvm": { "pae": "True", "apic": "True", "acpi": "True", - "nested_hvm": "False", "nographic": "True", "vnc": { "enable": "False" diff --git a/tests/libxlxml2domconfigdata/vnuma-hvm.json b/tests/libxlxml2domconfigdata/vnuma-hvm.json index 3b2fc5f..02c10a9 100644 --- a/tests/libxlxml2domconfigdata/vnuma-hvm.json +++ b/tests/libxlxml2domconfigdata/vnuma-hvm.json @@ -109,11 +109,11 @@ "sched_params": { }, + "nested_hvm": "True", "type.hvm": { "pae": "True", "apic": "True", "acpi": "True", - "nested_hvm": "True", "vga": { "kind": "cirrus" }, -- 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) --- docs/formatcaps.html.in | 4 ++-- docs/schemas/domaincommon.rng | 1 + src/libxl/libxl_capabilities.c | 33 ++++++++++++++++++++++++++++++--- src/libxl/libxl_conf.c | 32 +++++++++++++++++++++++++++----- src/libxl/libxl_driver.c | 6 ++++-- 5 files changed, 64 insertions(+), 12 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/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c index 18596c7..446fa4c 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,7 @@ 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; + bool new_arch_added = false; if (STRPREFIX(&token[subs[2].rm_so], "x86_32")) { arch = VIR_ARCH_I686; @@ -475,8 +477,10 @@ 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++; + new_arch_added = true; + } guest_archs[i].arch = arch; guest_archs[i].hvm = hvm; @@ -491,13 +495,34 @@ 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. + */ + if ((ver_info->xen_version_major > 4 || + (ver_info->xen_version_major == 4 && + ver_info->xen_version_minor >= 9)) && + 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++; + } } } 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 +582,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 +607,7 @@ libxlMakeDomainOSCaps(const char *machine, os->supported = true; - if (STREQ(machine, "xenpv")) + if (STREQ(machine, "xenpv") || STREQ(machine, "xenpvh")) return 0; capsLoader->supported = true; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index f3da0ed..5541a72 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -133,8 +133,11 @@ libxlMakeDomCreateInfo(libxl_ctx *ctx, libxl_domain_create_info_init(c_info); - if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { - c_info->type = LIBXL_DOMAIN_TYPE_HVM; + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM || + (def->os.type == VIR_DOMAIN_OSTYPE_XEN && + STREQ(def->os.machine, "xenpvh"))) { + c_info->type = def->os.type == VIR_DOMAIN_OSTYPE_HVM ? + LIBXL_DOMAIN_TYPE_HVM : LIBXL_DOMAIN_TYPE_PVH; switch ((virTristateSwitch) def->features[VIR_DOMAIN_FEATURE_HAP]) { case VIR_TRISTATE_SWITCH_OFF: libxl_defbool_set(&c_info->hap, false); @@ -276,7 +279,8 @@ 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; @@ -284,6 +288,8 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, if (hvm) libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_HVM); + else if (pvh) + libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PVH); else libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PV); @@ -375,7 +381,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 +653,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 +1252,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/18/18 6:50 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) --- docs/formatcaps.html.in | 4 ++-- docs/schemas/domaincommon.rng | 1 + src/libxl/libxl_capabilities.c | 33 ++++++++++++++++++++++++++++++--- src/libxl/libxl_conf.c | 32 +++++++++++++++++++++++++++----- src/libxl/libxl_driver.c | 6 ++++-- 5 files changed, 64 insertions(+), 12 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/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c index 18596c7..446fa4c 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,7 @@ 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; + bool new_arch_added = false;
if (STRPREFIX(&token[subs[2].rm_so], "x86_32")) { arch = VIR_ARCH_I686; @@ -475,8 +477,10 @@ 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++; + new_arch_added = true; + }
guest_archs[i].arch = arch; guest_archs[i].hvm = hvm; @@ -491,13 +495,34 @@ 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. + */ + if ((ver_info->xen_version_major > 4 || + (ver_info->xen_version_major == 4 && + ver_info->xen_version_minor >= 9)) && + hvm && new_arch_added) {
In v1 I forgot to mention that libvirt generally frowns upon version checks, but I don't think we have any other options since pvh is not advertised in Xen's meager capabilities reporting. Also, I doubt anyone cares. No one is going to backport pvh enablement to xen < 4.9 and expect to also use libvirt with that.
+ 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++; + } } } 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 +582,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 +607,7 @@ libxlMakeDomainOSCaps(const char *machine,
os->supported = true;
- if (STREQ(machine, "xenpv")) + if (STREQ(machine, "xenpv") || STREQ(machine, "xenpvh")) return 0;
capsLoader->supported = true;
AFAIK, pvh does not support hostdev passthrough, correct? If that is the case, libxlMakeDomainDeviceHostdevCaps() will also need adjusted. Can you double check that all of the capabilities are reported correctly for pvh? FYI on my xen 4.10 setup this patch produces the following domcapabilties # virsh domcapabilities --machine xenpvh <domainCapabilities> <path>/usr/bin/qemu-system-x86_64</path> <domain>xen</domain> <machine>xenpvh</machine> <arch>x86_64</arch> <vcpu max='512'/> <iothreads supported='no'/> <os supported='yes'> <loader supported='no'/> </os> <cpu> <mode name='host-passthrough' supported='no'/> <mode name='host-model' supported='no'/> <mode name='custom' supported='no'/> </cpu> <devices> <disk supported='yes'> <enum name='diskDevice'> <value>disk</value> <value>cdrom</value> </enum> <enum name='bus'> <value>ide</value> <value>scsi</value> <value>xen</value> </enum> </disk> <graphics supported='yes'> <enum name='type'> <value>sdl</value> <value>vnc</value> <value>spice</value> </enum> </graphics> <video supported='yes'> <enum name='modelType'> <value>vga</value> <value>cirrus</value> <value>xen</value> </enum> </video> <hostdev supported='yes'> <enum name='mode'> <value>subsystem</value> </enum> <enum name='startupPolicy'> <value>default</value> <value>mandatory</value> <value>requisite</value> <value>optional</value> </enum> <enum name='subsysType'> <value>usb</value> <value>pci</value> </enum> <enum name='capsType'/> <enum name='pciBackend'> <value>xen</value> </enum> </hostdev> </devices> <features> <gic supported='no'/> <vmcoreinfo supported='no'/> <genid supported='no'/> <sev supported='no'/> </features> </domainCapabilities> Related observation: The <cpu> caps for machine type xenfv needs to be updated. It reports host-passthrough mode as unsupported.
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index f3da0ed..5541a72 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -133,8 +133,11 @@ libxlMakeDomCreateInfo(libxl_ctx *ctx,
libxl_domain_create_info_init(c_info);
- if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { - c_info->type = LIBXL_DOMAIN_TYPE_HVM; + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM || + (def->os.type == VIR_DOMAIN_OSTYPE_XEN && + STREQ(def->os.machine, "xenpvh"))) { + c_info->type = def->os.type == VIR_DOMAIN_OSTYPE_HVM ? + LIBXL_DOMAIN_TYPE_HVM : LIBXL_DOMAIN_TYPE_PVH;
Fails to compile on Xen without pvh support libxl/libxl_conf.c: In function 'libxlMakeDomCreateInfo': libxl/libxl_conf.c:140:37: error: 'LIBXL_DOMAIN_TYPE_PVH' undeclared (first use in this function)
switch ((virTristateSwitch) def->features[VIR_DOMAIN_FEATURE_HAP]) { case VIR_TRISTATE_SWITCH_OFF: libxl_defbool_set(&c_info->hap, false); @@ -276,7 +279,8 @@ 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;
@@ -284,6 +288,8 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
if (hvm) libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_HVM); + else if (pvh) + libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PVH);
Another instance here. Looks good otherwise. Regards, Jim
else libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PV);
@@ -375,7 +381,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 +653,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 +1252,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 {

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- tests/libxlxml2domconfigdata/basic-pvh.json | 49 ++++++++++++++++++++++- tests/libxlxml2domconfigdata/basic-pvh.xml | 28 +++++++++++++- tests/libxlxml2domconfigtest.c | 1 +- 3 files changed, 78 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..7576596 --- /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'>linux</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 b814461..7ee1063 100644 --- a/tests/libxlxml2domconfigtest.c +++ b/tests/libxlxml2domconfigtest.c @@ -207,6 +207,7 @@ mymain(void) DO_TEST("basic-pv"); DO_TEST("basic-hvm"); + DO_TEST("basic-pvh"); 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 fdca984..4a7625e 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -1088,9 +1088,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 36f7699..ad6a964 100644 --- a/tests/xlconfigtest.c +++ b/tests/xlconfigtest.c @@ -281,6 +281,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> --- 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 4a7625e..3c120a0 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -1081,6 +1081,7 @@ xenParseGeneralMeta(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps) virCapsDomainDataPtr capsdata = NULL; const char *str; int hvm = 0, ret = -1; + const char *machine = "xenpv"; if (xenConfigCopyString(conf, "name", &def->name) < 0) goto out; @@ -1091,8 +1092,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); @@ -1100,14 +1105,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 19b6604..f1853bd 100644 --- a/src/xenconfig/xen_xl.c +++ b/src/xenconfig/xen_xl.c @@ -1285,6 +1285,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 c08ec4a..04f8920 100644 --- a/tests/testutilsxen.c +++ b/tests/testutilsxen.c @@ -18,7 +18,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..96b0e7a --- /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'>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 ad6a964..b9cf939 100644 --- a/tests/xlconfigtest.c +++ b/tests/xlconfigtest.c @@ -283,6 +283,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/18/18 6:50 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.
From libvirt's perspective it should be "PVH". If anything, the Xen documentation should change. AFAIK the various PVH attempts/names (PVHv1, hvmlite, PVHv2, ...) have merged to simply be known as "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. 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 should have read your v1 cover letter closer and agreed on the approach before spending time looking at code :-). But in the end I still think the OS type is VIR_DOMAIN_OSTYPE_XEN with the machine attribute specifying PV vs PVH. I use the fact that both OS types are modified to run on Xen to rationalize using VIR_DOMAIN_OSTYPE_XEN for both. Perhaps it would be best for a third opinion and Daniel (cc'd) can chime in.
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?
Due to the compilation error in patch 5, I haven't checked patches 6-8 on xen < 4.9, which may help me understand the problem you describe. Sorry it is not obvious to me from the reading. Regards, Jim

On Mon, Sep 24, 2018 at 05:09:44PM -0600, Jim Fehlig wrote:
On 9/18/18 6:50 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.
From libvirt's perspective it should be "PVH". If anything, the Xen documentation should change. AFAIK the various PVH attempts/names (PVHv1, hvmlite, PVHv2, ...) have merged to simply be known as "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. 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 should have read your v1 cover letter closer and agreed on the approach before spending time looking at code :-). But in the end I still think the OS type is VIR_DOMAIN_OSTYPE_XEN with the machine attribute specifying PV vs PVH. I use the fact that both OS types are modified to run on Xen to rationalize using VIR_DOMAIN_OSTYPE_XEN for both. Perhaps it would be best for a third opinion and Daniel (cc'd) can chime in.
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?
Due to the compilation error in patch 5, I haven't checked patches 6-8 on xen < 4.9, which may help me understand the problem you describe. Sorry it is not obvious to me from the reading.
You can already observe the problem after applying "libxl: prefer new location of nested_hvm in libxl_domain_build_info". -- 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?
participants (2)
-
Jim Fehlig
-
Marek Marczykowski-Górecki