[libvirt] [PATCH v3 2/5] libxl: add support for PVH
Marek Marczykowski-Górecki
marmarek at invisiblethingslab.com
Tue Oct 2 23:02:07 UTC 2018
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.
> 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.
--
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?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20181003/662eddc5/attachment-0001.sig>
More information about the libvir-list
mailing list