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&gt; >>>> --- >>>> 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&gt; 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?