Re: [libvirt] [PATCH v3 2/5] libxl: add support for PVH
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(a)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(a)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])