[libvirt] [PATCH v4 2/5] libxl: add support for PVH

Daniel P. Berrangé berrange at redhat.com
Wed Oct 10 08:45:50 UTC 2018


On Tue, Oct 09, 2018 at 04:45:01PM -0600, Jim Fehlig wrote:
> On 10/7/18 9:39 AM, 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
> >   - use #ifdef LIBXL_DOMAIN_TYPE_PVH instead of hypervisor version to
> >   check for PVH support
> >   - compile fix for Xen < 4.9
> > 
> > Changes in v4:
> >   - revert to "i == nr_guest_archs-1" check
> >   - let configure check for LIBXL_DOMAIN_TYPE_PVH and use #ifdef
> >   HAVE_XEN_PVH then
> >   - fix comment about Xen version
> > ---
> >   docs/formatcaps.html.in        |  4 +--
> >   docs/schemas/domaincommon.rng  |  1 +-
> >   m4/virt-driver-libxl.m4        |  3 ++-
> >   src/conf/domain_conf.c         |  6 ++--
> >   src/libxl/libxl_capabilities.c | 38 ++++++++++++++++++++++-----
> >   src/libxl/libxl_conf.c         | 50 ++++++++++++++++++++++++++++++-----
> >   src/libxl/libxl_driver.c       |  6 ++--
> >   7 files changed, 90 insertions(+), 18 deletions(-)
> > 
> > diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in
> > index 0d9c53d..fe113bc 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 PVH.</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/m4/virt-driver-libxl.m4 b/m4/virt-driver-libxl.m4
> > index 479d911..2cd97cc 100644
> > --- a/m4/virt-driver-libxl.m4
> > +++ b/m4/virt-driver-libxl.m4
> > @@ -75,6 +75,9 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_LIBXL], [
> >       ])
> >     fi
> > +  dnl Check if Xen has support for PVH
> > +  AC_CHECK_DECL(LIBXL_DOMAIN_TYPE_PVH, [AC_DEFINE([HAVE_XEN_PVH], [1], [Define to 1 if Xen has PVH support.])], [], [#include <libxl.h>])
> > +
> >     AC_SUBST([LIBXL_CFLAGS])
> >     AC_SUBST([LIBXL_LIBS])
> >   ])
> > 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
> 
> I'd still like to hear what others think of extending the hack. mprivozn?
> You often have an interesting opinion :-).

IIUC, this means that we get this behaviour:

    Input                            Output

 type=linux, machine=NULL        type=linux, machine=NULL
 type=linux, machine=xenpv       type=linux, machine=xenpvh
 type=xen, machine=NULL          type=linux, machine=NULL
 type=xen, machine=xenpv         type=linux, machine=xenpv
 type=xen, machine=xenpvh        type=xen, machine=xenpvh

And one combo not allowed

 type=linux, machine=xenpvh

I can understand the motivation behind this change, but I wonder if it is
worth it or not to have the difference in behaviour.  I'm quite torn.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|




More information about the libvir-list mailing list