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

[Jim Fehlig]

On 10/3/18 3:25 PM, Marek Marczykowski-Górecki wrote:
> On Wed, Oct 03, 2018 at 03:13:30PM -0600, Jim Fehlig wrote:
>> On 10/2/18 4:50 PM, 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.
>>>
>>>>    - 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.
>>
>> Actually it is not. LIBXL_DOMAIN_TYPE_PVH is a value in the enum
>> libxl_domain_type. Too bad PVH support isn't advertised in libxl.h with
>> something like LIBXL_HAVE_PVH. Looks like we are stuck with the version
>> check :-(.
> 
> Hmm, but that version check is a runtime check. How to fix compile error
> regarding missing LIBXL_DOMAIN_TYPE_PVH then? Let configure check that
> and define some HAVE_xxx?

AFAICT, PVH support actually came in Xen 4.10

commit 27f826844d3666f151e86d66198583757cccbbed
Author: Roger Pau Monne <roger.pau at citrix.com>
Date:   Fri Sep 22 16:25:07 2017 +0100

     libxl: introduce a PVH guest type

git describe --contains 27f826844d3
4.10.0-rc1~219

And the only thing in that commit to check is the addition of 
LIBXL_DOMAIN_TYPE_PVH to the libxl_domain_type enum. I think we'll have to check 
it with AC_COMPILE_IFELSE. I've been fiddling with the below patch, which at 
least works on a Xen >= 4.10. Can you check it on Xen < 4.10 too, and squash it 
into your series?

Regards,
Jim

diff --git a/m4/virt-driver-libxl.m4 b/m4/virt-driver-libxl.m4 
             index 479d9116a4..ae309abb80 100644 
                          --- a/m4/virt-driver-libxl.m4 
                                       +++ b/m4/virt-driver-libxl.m4 
                                                    @@ -73,6 +73,22 @@ 
AC_DEFUN([LIBVIRT_DRIVER_CHECK_LIBXL], [ 
],[ 
               LIBXL_LIBS="$LIBXL_LIBS -lxenstore -lxenctrl" 
                          ]) 
                                  + 
                                               +    dnl Check if Xen has support 
for PVH                                                    + 
AC_MSG_CHECKING([for Xen PVH support]) 
        +    AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <libxl.h>]], 
                     +                                       [[int dummy = 
LIBXL_DOMAIN_TYPE_PVH;]])              +                      ], 
                                                    +                      [ 
                                                                 + 
          AC_MSG_RESULT([yes])                                                + 
                       AC_DEFINE_UNQUOTED([HAVE_XEN_PVH], [1], [Define to 1 if 
Xen has PVH support.]) 
                         +                      ], 
                                      +                      [ 
                                                   + 
AC_MSG_RESULT([no])                                                 + 
             AC_DEFINE_UNQUOTED([HAVE_XEN_PVH], [0], [Define to 1 if Xen has PVH 
support.]) 
             +                      ] 
                          +                     ) 
                                       + 
                                                       fi 
 
 
AC_SUBST([LIBXL_CFLAGS])