[libvirt] [PATCH v3 2/5] libxl: add support for PVH
Jim Fehlig
jfehlig at suse.com
Wed Oct 3 00:54:01 UTC 2018
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 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.
>
> 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
More information about the libvir-list
mailing list