[libvirt] [PATCH 07/10] libxl: add support for PVH
Jim Fehlig
jfehlig at suse.com
Fri Sep 14 21:21:50 UTC 2018
On 9/10/18 7:10 PM, Marek Marczykowski-Górecki wrote:
> On Mon, Sep 10, 2018 at 04:45:50PM -0600, Jim Fehlig wrote:
>> On 09/10/2018 04:02 PM, Marek Marczykowski-Górecki wrote:
>>> On Mon, Sep 10, 2018 at 03:44:33PM -0600, Jim Fehlig wrote:
>>>> On 08/05/2018 03:48 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>
>>>>> ---
>>>>> docs/formatcaps.html.in | 4 +--
>>>>> docs/schemas/domaincommon.rng | 1 +-
>>>>> src/libxl/libxl_capabilities.c | 23 ++++++++++++++++++--
>>>>> src/libxl/libxl_conf.c | 41 ++++++++++++++++++++++++++++++-----
>>>>> src/libxl/libxl_driver.c | 6 +++--
>>>>> 5 files changed, 64 insertions(+), 11 deletions(-)
>>>>>
>>>>> diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in
>>>>> index 34a74a9..1f17aa9 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>Supported domain type, named by the
>>>>> <code>type</code> attribute.</dd>
>>>>> </dl>
>>>>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>>>>> index eded1ca..d32b0cb 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..e7b26f1 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,29 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
>>>>> guest_archs[i].nonpae = nonpae;
>>>>> if (ia64_be)
>>>>> guest_archs[i].ia64_be = ia64_be;
>>>>> +
>>>>> + /* On Xen >= 4.9 add PVH for each HVM guest, and do it only once */
>>>>
>>>> I'm having problems understanding this. Do you mean add a PVH for each
>>>> supported HVM arch, but exclude PAE? E.g. standard xen_caps on x86_64
>>>> contains
>>>>
>>>> xen-3.0-x86_64 xen-3.0-x86_32p hvm-3.0-x86_32 hvm-3.0-x86_32p hvm-3.0-x86_64
>>>>
>>>> Given these caps, should a PVH be added that corresponds to the
>>>> hvm-3.0-x86_32 cap and another for the hvm-3.0-x86_64 cap, but the
>>>> hvm-3.0-x86_32p cap excluded?
>>>
>>> Yes, exactly. Setting PAE (or not) is possible only for HVM, but not
>>> PVH.
>>>
>>> It would be much better if Xen would report support for PVH
>>> explicitly...
>>>
>>>>> + if ((ver_info->xen_version_major > 4 ||
>>>>> + (ver_info->xen_version_major == 4 &&
>>>>> + ver_info->xen_version_minor >= 9)) &&
>>>>> + hvm && i == nr_guest_archs-1) {
>>
>> How about checking for hvm && !pae instead of i == nr_guest_archs-1?
>
> At this time it should be ok. The i == nr_guest_archs-1 is based
> directly on
>
> if (i == nr_guest_archs)
> nr_guest_archs++
>
> earlier up. So, it is indeed added only when given arch was added to
> guest_archs, regardless of what conditions were used for that.
Right. This function is a little intense. I had to look at it a third time to
see i == nr_guest_archs-1 also means we are adding a new entry in guest_archs :-).
> Maybe, use something like this instead:
>
>
> bool new_arch_added;
> if ((new_arch_added = (i == nr_guest_archs)))
> nr_guest_archs++
> ...
> if (... hvm && new_arch_added)
Along with an improved comment, I think this will help me remember what's going
on in this code in the event I ever find time to refactor it. What do you think
of the below diff?
Regards,
Jim
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index e7b26f12d8..446fa4ccf9 100644
--- a/src/libxl/libxl_capabilities.c
+++ b/src/libxl/libxl_capabilities.c
@@ -440,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;
@@ -476,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;
@@ -493,18 +496,23 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
if (ia64_be)
guest_archs[i].ia64_be = ia64_be;
- /* On Xen >= 4.9 add PVH for each HVM guest, and do it only once */
+ /*
+ * 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 && i == nr_guest_archs-1) {
+ hvm && new_arch_added) {
i = nr_guest_archs;
- /* Too many arch flavours - highly unlikely ! */
- if (i >= ARRAY_CARDINALITY(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++;
- guest_archs[i].arch = arch;
- guest_archs[i].pvh = 1;
}
}
}
More information about the libvir-list
mailing list