Re: [Xen-devel] [PATCH QEMU-XEN v3 6/8] xen: Use stable library interfaces when they are available.

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 7 Oct 2015, Ian Campbell wrote:
> In Xen 4.7 we are refactoring parts libxenctrl into a number of
> separate libraries which will provide backward and forward API and ABI
> compatiblity.
> 
> Specifically libxenevtchn, libxengnttab and libxenforeignmemory.
> 
> Previous patches have already laid the groundwork for using these by
> switching the existing compatibility shims to reflect the intefaces to
> these libraries.
> 
> So all which remains is to update configure to detect the libraries
> and enable their use. Although they are notionally independent we take
> an all or nothing approach to the three libraries since they were
> added at the same time.
> 
> The only non-obvious bit is that we now open a proper xenforeignmemory
> handle for xen_fmem instead of reusing the xen_xc handle.
> 
> Build tested with 4.0, 4.5 and the patches targetting 4.7 which adds
> these libraries.
> 
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> ---
>  configure                   | 98 
> ++++++++++++++++++++++++++++++++++++++++++++-
>  include/hw/xen/xen_common.h | 42 ++++++++++++++++++-
>  xen-common.c                |  9 +++++
>  3 files changed, 145 insertions(+), 4 deletions(-)
> 
> diff --git a/configure b/configure
> index 779623a..508b157 100755
> --- a/configure
> +++ b/configure
> @@ -1839,6 +1839,52 @@ fi
>  # xen probe
>  
>  if test "$xen" != "no" ; then
> +  xenevtchn_libs="-lxenevtchn"
> +
> +  cat > $TMPC <<EOF
> +#include <xenevtchn.h>
> +int main(void) { return 0; }
> +EOF
> +  if compile_prog "" "$xenevtchn_libs" ; then
> +      xenevtchn=yes
> +  else
> +      xenevtchn=no
> +  fi
> +
> +  xengnttab_libs="-lxengnttab"
> +
> +  cat > $TMPC <<EOF
> +#include <xengnttab.h>
> +int main(void) { return 0; }
> +EOF
> +  if compile_prog "" "$xengnttab_libs" ; then
> +      xengnttab=yes
> +  else
> +      xengnttab=no
> +  fi
> +
> +  xenforeignmemory_libs="-lxenforeignmemory"
> +
> +  cat > $TMPC <<EOF
> +#include <xenforeignmemory.h>
> +int main(void) { return 0; }
> +EOF
> +  if compile_prog "" "$xenforeignmemory_libs" ; then
> +      xenforeignmemory=yes
> +  else
> +      xenforeignmemory=no
> +  fi
> +
> +  # These libraries were all introduced in the same release, to
> +  # simplify things we therefore assume its either all or nothing and
> +  # treat any other mix as an error
> +  case $xenevtchn$xengnttab$xenforeignmemory in
> +      yesyesyes) xenstablelibs=yes;;
> +      nonono)    xenstablelibs=no;;
> +      *)         error_exit "Inconsistent set of xen libraries found"
> +       ;;
> +  esac

I would remove all this and introduce a xen_stable_libs for the Xen 4.7
and beyond case:

xen_stable_libs="-lxenforeignmemory -lxengnttab -lxenevtchn"


>    xen_libs="-lxenstore -lxenctrl -lxenguest"
>  
>    # First we test whether Xen headers and libraries are available.
> @@ -1859,7 +1905,46 @@ EOF
>      fi
>      xen=no
>  
> -  # Xen unstable
> +  # Xen unstable (with stable libs)
> +  elif
> +      # If we have stable libs the we don't want the libxc compat
> +      # layers, regardless of what CFLAGS we may have been given.
> +      test "$xenstablelibs" = "yes" &&
> +      cat > $TMPC <<EOF &&
> +#undef XC_WANT_COMPAT_EVTCHN_API
> +#undef XC_WANT_COMPAT_GNTTAB_API
> +#undef XC_WANT_COMPAT_MAP_FOREIGN_API
> +#include <xenctrl.h>
> +#include <xenstore.h>
> +#include <stdint.h>
> +#include <xen/hvm/hvm_info_table.h>
> +#if !defined(HVM_MAX_VCPUS)
> +# error HVM_MAX_VCPUS not defined
> +#endif
> +int main(void) {
> +  xc_interface *xc;
> +  xs_daemon_open();
> +  xc = xc_interface_open(0, 0, 0);
> +  xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0);
> +  xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0);
> +  xc_hvm_inject_msi(xc, 0, 0xf0000000, 0x00000000);
> +  xc_hvm_create_ioreq_server(xc, 0, HVM_IOREQSRV_BUFIOREQ_ATOMIC, NULL);

I would add some of the new function calls here.


> +  return 0;
> +}
> +EOF
> +      compile_prog "" "$xen_libs"

                          ^ xen_stable_libs

> +  then
> +    xen_ctrl_version=470
> +    xen=yes
> +
> +  # Xen unstable (without stable libs)

I would ignore this case completely.


> +  elif
> +      test "$xenstablelibs" = "yes"
> +  then
> +      # Inconsistent result, split libs but an older libxenctrl interface?
> +      xen=no
> +
> +  # Xen 4.6
>    elif
>        cat > $TMPC <<EOF &&
>  #include <xenctrl.h>
> @@ -2037,6 +2122,11 @@ EOF
>    fi
>  
>    if test "$xen" = yes; then
> +    if test "$xenstablelibs" = yes; then
> +      libs_softmmu="$xenevtchn_libs $libs_softmmu"
> +      libs_softmmu="$xengnttab_libs $libs_softmmu"
> +      libs_softmmu="$xenforeignmemory_libs $libs_softmmu"
> +    fi
>      libs_softmmu="$xen_libs $libs_softmmu"
>    fi
>  fi
> @@ -4337,7 +4427,8 @@ if test -n "$sparc_cpu"; then
>  fi
>  echo "xen support       $xen"
>  if test "$xen" = "yes" ; then
> -  echo "xen ctrl version  $xen_ctrl_version"
> +  echo "  stable libs     $xenstablelibs"
> +  echo "  ctrl version    $xen_ctrl_version"

I don't think is necessary to print something here


>  fi
>  echo "brlapi support    $brlapi"
>  echo "bluez  support    $bluez"
> @@ -4670,6 +4761,9 @@ fi
>  if test "$xen" = "yes" ; then
>    echo "CONFIG_XEN_BACKEND=y" >> $config_host_mak
>    echo "CONFIG_XEN_CTRL_INTERFACE_VERSION=$xen_ctrl_version" >> 
> $config_host_mak
> +  if test "$xenstablelibs" = "yes" ; then
> +    echo "CONFIG_XEN_STABLE_LIBS=y" >> $config_host_mak
> +  fi

I would avoid this and just use a simple Xen version check in the code


>  fi
>  if test "$linux_aio" = "yes" ; then
>    echo "CONFIG_LINUX_AIO=y" >> $config_host_mak
> diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
> index 28f3fb1..1b824aa 100644
> --- a/include/hw/xen/xen_common.h
> +++ b/include/hw/xen/xen_common.h
> @@ -6,6 +6,17 @@
>  #include <stddef.h>
>  #include <inttypes.h>
>  
> +#ifdef CONFIG_XEN_STABLE_LIBS

I would just do 

#if CONFIG_XEN_CTRL_INTERFACE_VERSION >= 470


> +/*
> + * If we have new enough libxenctrl then we do not want/need these compat
> + * interfaces, despite what the user supplied cflags might say. They
> + * must be undefined before including xenctrl.h
> + */
> +#undef XC_WANT_COMPAT_EVTCHN_API
> +#undef XC_WANT_COMPAT_GNTTAB_API
> +#undef XC_WANT_COMPAT_MAP_FOREIGN_API
> +#endif
> +
>  #include <xenctrl.h>
>  #if CONFIG_XEN_CTRL_INTERFACE_VERSION < 420
>  #  include <xs.h>
> @@ -146,8 +157,8 @@ static inline void xs_close(struct xs_handle *xsh)
>  }
>  
>  
> -/* Xen 4.1 */
> -#else
> +/* Xen 4.1 thru 4.6 */
> +#elif CONFIG_XEN_CTRL_INTERFACE_VERSION < 470
>  
>  typedef xc_interface *XenXC;
>  typedef xc_interface *xenforeignmemory_handle;
> @@ -187,6 +198,33 @@ static inline int xc_fd(xc_interface *xen_xc)
>  {
>      return -1;
>  }
> +#else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 470 */
> +
> +#ifndef CONFIG_XEN_STABLE_LIBS
> +#error XEN_CTRL_INTERFACE_VERSION >= 470, but no stable libs?
> +#endif

let's ignore this case


> +typedef xc_interface *XenXC;
> +
> +#  define XC_INTERFACE_FMT "%p"
> +#  define XC_HANDLER_INITIAL_VALUE    NULL
> +
> +#include <xenevtchn.h>
> +#include <xengnttab.h>
> +#include <xenforeignmemory.h>
> +
> +static inline XenXC xen_xc_interface_open(void *logger, void 
> *dombuild_logger,
> +                                          unsigned int open_flags)
> +{
> +    return xc_interface_open(logger, dombuild_logger, open_flags);
> +}
> +
> +/* FIXME There is now way to have the xen fd */
> +static inline int xc_fd(xc_interface *xen_xc)
> +{
> +    return -1;
> +}
> +
>  #endif
>  
>  /* Xen before 4.2 */
> diff --git a/xen-common.c b/xen-common.c
> index d319dcb..f689758 100644
> --- a/xen-common.c
> +++ b/xen-common.c
> @@ -117,7 +117,16 @@ static int xen_init(MachineState *ms)
>          xen_be_printf(NULL, 0, "can't open xen interface\n");
>          return -1;
>      }
> +#ifdef CONFIG_XEN_STABLE_LIBS /* We have libxenforeignmemory */
> +    xen_fmem = xenforeignmemory_open(0, 0);
> +    if (xen_fmem == NULL) {
> +        xen_be_printf(NULL, 0, "can't open xen fmem interface\n");
> +     xc_interface_close(xen_xc);
> +     return -1;
> +    }
> +#else
>      xen_fmem = &xen_xc;
> +#endif

Please introduce a wrapper for this in xen_common.h, avoid the ifdef
here.


>      qemu_add_vm_change_state_handler(xen_change_state_handler, NULL);
>  
>      return 0;
> -- 
> 2.1.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel