Re: [Qemu-devel] [PATCH V11 03/15] xen: Support new libxc calls from xen unstable.

[Alexander Graf]

On 01.03.2011, at 19:35, address@hidden wrote:

> From: Anthony PERARD <address@hidden>
> 
> This patch updates the libxenctrl calls in Qemu to use the new interface,
> otherwise Qemu wouldn't be able to build against new versions of the
> library.
> 
> We check libxenctrl version in configure, from Xen 3.3.0 to Xen
> unstable.
> 
> Signed-off-by: Anthony PERARD <address@hidden>
> Signed-off-by: Stefano Stabellini <address@hidden>
> Acked-by: Alexander Graf <address@hidden>
> ---
> configure            |   67 ++++++++++++++++++++++++++++++++++++++++++++++++-
> hw/xen_backend.c     |   21 ++++++++-------
> hw/xen_backend.h     |    6 ++--
> hw/xen_common.h      |   64 +++++++++++++++++++++++++++++++++++++----------
> hw/xen_disk.c        |    4 +-
> hw/xen_domainbuild.c |    3 +-
> 6 files changed, 133 insertions(+), 32 deletions(-)
> 
> diff --git a/configure b/configure
> index 3036faf..a84d974 100755
> --- a/configure
> +++ b/configure
> @@ -126,6 +126,7 @@ vnc_jpeg=""
> vnc_png=""
> vnc_thread="no"
> xen=""
> +xen_ctrl_version=""
> linux_aio=""
> attr=""
> vhost_net=""
> @@ -1147,20 +1148,81 @@ fi
> 
> if test "$xen" != "no" ; then
>   xen_libs="-lxenstore -lxenctrl -lxenguest"
> +
> +  # Xen unstable
>   cat > $TMPC <<EOF
> #include <xenctrl.h>
> #include <xs.h>
> -int main(void) { xs_daemon_open(); xc_interface_open(); return 0; }
> +#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_gnttab_open(NULL, 0);
> +  return 0;
> +}
> EOF
>   if compile_prog "" "$xen_libs" ; then
> +    xen_ctrl_version=410
>     xen=yes
> -    libs_softmmu="$xen_libs $libs_softmmu"
> +
> +  # Xen 4.0.0
> +  elif (
> +      cat > $TMPC <<EOF
> +#include <xenctrl.h>
> +#include <xs.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) {
> +  xs_daemon_open();
> +  xc_interface_open();
> +  xc_gnttab_open();
> +  xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0);
> +  return 0;
> +}
> +EOF
> +      compile_prog "" "$xen_libs"
> +    ) ; then
> +    xen_ctrl_version=400
> +    xen=yes
> +
> +  # Xen 3.3.0, 3.4.0
> +  elif (
> +      cat > $TMPC <<EOF
> +#include <xenctrl.h>
> +#include <xs.h>
> +int main(void) {
> +  xs_daemon_open();
> +  xc_interface_open();
> +  xc_gnttab_open();
> +  xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0);
> +  return 0;
> +}
> +EOF
> +      compile_prog "" "$xen_libs"
> +    ) ; then
> +    xen_ctrl_version=330
> +    xen=yes
> +
> +  # Xen not found or unsupported
>   else
>     if test "$xen" = "yes" ; then
>       feature_not_found "xen"
>     fi
>     xen=no
>   fi
> +
> +  if test "$xen" = yes; then
> +    libs_softmmu="$xen_libs $libs_softmmu"
> +  fi
> fi
> 
> ##########################################
> @@ -2755,6 +2817,7 @@ if test "$bluez" = "yes" ; then
> fi
> if test "$xen" = "yes" ; then
>   echo "CONFIG_XEN=y" >> $config_host_mak
> +  echo "CONFIG_XEN_CTRL_INTERFACE_VERSION=$xen_ctrl_version" >> 
> $config_host_mak
> fi
> if test "$io_thread" = "yes" ; then
>   echo "CONFIG_IOTHREAD=y" >> $config_host_mak
> diff --git a/hw/xen_backend.c b/hw/xen_backend.c
> index 9f4ec4b..3907b83 100644
> --- a/hw/xen_backend.c
> +++ b/hw/xen_backend.c
> @@ -43,7 +43,8 @@
> /* ------------------------------------------------------------- */
> 
> /* public */
> -int xen_xc;
> +XenXC xen_xc = XC_HANDLER_INITIAL_VALUE;
> +XenGnttab xen_xcg = XC_HANDLER_INITIAL_VALUE;
> struct xs_handle *xenstore = NULL;
> const char *xen_protocol;
> 
> @@ -214,8 +215,8 @@ static struct XenDevice *xen_be_get_xendev(const char 
> *type, int dom, int dev,
>     xendev->debug      = debug;
>     xendev->local_port = -1;
> 
> -    xendev->evtchndev = xc_evtchn_open();
> -    if (xendev->evtchndev < 0) {
> +    xendev->evtchndev = xc_evtchn_open(NULL, 0);
> +    if (xendev->evtchndev == XC_HANDLER_INITIAL_VALUE) {
>         xen_be_printf(NULL, 0, "can't open evtchn device\n");
>         qemu_free(xendev);
>         return NULL;
> @@ -223,15 +224,15 @@ static struct XenDevice *xen_be_get_xendev(const char 
> *type, int dom, int dev,
>     fcntl(xc_evtchn_fd(xendev->evtchndev), F_SETFD, FD_CLOEXEC);
> 
>     if (ops->flags & DEVOPS_FLAG_NEED_GNTDEV) {
> -        xendev->gnttabdev = xc_gnttab_open();
> -        if (xendev->gnttabdev < 0) {
> +        xendev->gnttabdev = xc_gnttab_open(NULL, 0);
> +        if (xendev->gnttabdev == XC_HANDLER_INITIAL_VALUE) {
>             xen_be_printf(NULL, 0, "can't open gnttab device\n");
>             xc_evtchn_close(xendev->evtchndev);
>             qemu_free(xendev);
>             return NULL;
>         }
>     } else {
> -        xendev->gnttabdev = -1;
> +        xendev->gnttabdev = XC_HANDLER_INITIAL_VALUE;
>     }
> 
>     QTAILQ_INSERT_TAIL(&xendevs, xendev, next);
> @@ -277,10 +278,10 @@ static struct XenDevice *xen_be_del_xendev(int dom, int 
> dev)
>             qemu_free(xendev->fe);
>         }
> 
> -        if (xendev->evtchndev >= 0) {
> +        if (xendev->evtchndev != XC_HANDLER_INITIAL_VALUE) {
>             xc_evtchn_close(xendev->evtchndev);
>         }
> -        if (xendev->gnttabdev >= 0) {
> +        if (xendev->gnttabdev != XC_HANDLER_INITIAL_VALUE) {
>             xc_gnttab_close(xendev->gnttabdev);
>         }
> 
> @@ -664,8 +665,8 @@ int xen_be_init(void)
>         goto err;
>     }
> 
> -    xen_xc = xc_interface_open();
> -    if (xen_xc == -1) {
> +    xen_xc = xc_interface_open(0, 0, 0);
> +    if (xen_xc == XC_HANDLER_INITIAL_VALUE) {
>         xen_be_printf(NULL, 0, "can't open xen interface\n");
>         goto err;
>     }
> diff --git a/hw/xen_backend.h b/hw/xen_backend.h
> index 1b428e3..6401c85 100644
> --- a/hw/xen_backend.h
> +++ b/hw/xen_backend.h
> @@ -45,8 +45,8 @@ struct XenDevice {
>     int                remote_port;
>     int                local_port;
> 
> -    int                evtchndev;
> -    int                gnttabdev;
> +    XenEvtchn          evtchndev;
> +    XenGnttab          gnttabdev;
> 
>     struct XenDevOps   *ops;
>     QTAILQ_ENTRY(XenDevice) next;
> @@ -55,7 +55,7 @@ struct XenDevice {
> /* ------------------------------------------------------------- */
> 
> /* variables */
> -extern int xen_xc;
> +extern XenXC xen_xc;
> extern struct xs_handle *xenstore;
> extern const char *xen_protocol;
> 
> diff --git a/hw/xen_common.h b/hw/xen_common.h
> index 8a55b44..7e123ec 100644
> --- a/hw/xen_common.h
> +++ b/hw/xen_common.h
> @@ -1,6 +1,8 @@
> #ifndef QEMU_HW_XEN_COMMON_H
> #define QEMU_HW_XEN_COMMON_H 1
> 
> +#include "config-host.h"
> +
> #include <stddef.h>
> #include <inttypes.h>
> 
> @@ -13,22 +15,56 @@
> #include "qemu-queue.h"
> 
> /*
> - * tweaks needed to build with different xen versions
> - *  0x00030205 -> 3.1.0
> - *  0x00030207 -> 3.2.0
> - *  0x00030208 -> unstable
> + * We don't support Xen prior to 3.3.0.
>  */
> -#include <xen/xen-compat.h>
> -#if __XEN_LATEST_INTERFACE_VERSION__ < 0x00030205
> -# define evtchn_port_or_error_t int
> -#endif
> -#if __XEN_LATEST_INTERFACE_VERSION__ < 0x00030207
> -# define xc_map_foreign_pages xc_map_foreign_batch
> +
> +/* Xen before 4.0 */
> +#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 400
> +static inline void *xc_map_foreign_bulk(int xc_handle, uint32_t dom, int 
> prot,
> +                                        xen_pfn_t *arr, int *err,
> +                                        unsigned int num)
> +{
> +    return xc_map_foreign_batch(xc_handle, dom, prot, arr, num);
> +}
> #endif
> -#if __XEN_LATEST_INTERFACE_VERSION__ < 0x00030208
> -# define xen_mb()  mb()
> -# define xen_rmb() rmb()
> -# define xen_wmb() wmb()
> +
> +
> +/* Xen before 4.1 */
> +#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 410
> +
> +typedef int XenXC;
> +typedef int XenEvtchn;
> +typedef int XenGnttab;
> +
> +#  define XC_INTERFACE_FMT "%i"
> +#  define XC_HANDLER_INITIAL_VALUE    -1
> +
> +#  define xc_interface_open(logger, dombuild_logger, open_flags) \
> +    xc_interface_open()
> +#  define xc_evtchn_open(logger, open_flags) xc_evtchn_open()
> +#  define xc_gnttab_open(logger, open_flags) xc_gnttab_open()


defines really bite me when abstracting the interface through an indirect 
calling table. I had to change the code this way to make it work with xenner:


diff --git a/hw/xen_common.h b/hw/xen_common.h
index a5fc74b..0c09b37 100644
--- a/hw/xen_common.h
+++ b/hw/xen_common.h
@@ -39,10 +39,23 @@ typedef int XenGnttab;
 #  define XC_INTERFACE_FMT "%i"
 #  define XC_HANDLER_INITIAL_VALUE    -1
 
-#  define xc_interface_open(logger, dombuild_logger, open_flags) \
-    xc_interface_open()
-#  define xc_evtchn_open(logger, open_flags) xc_evtchn_open()
-#  define xc_gnttab_open(logger, open_flags) xc_gnttab_open()
+static inline XenEvtchn xen_xc_evtchn_open(void *logger,
+                                           unsigned int open_flags)
+{
+    return xc_evtchn_open();
+}
+
+static inline XenGnttab xen_xc_gnttab_open(void *logger,
+                                           unsigned int open_flags)
+{
+    return xc_gnttab_open();
+}
+
+static inline XenXC xen_xc_interface_open(void *logger, void *dombuild_logger,
+                                          unsigned int open_flags)
+{
+    return xc_interface_open();
+}
 
 static inline int xc_fd(int xen_xc)
 {
@@ -69,6 +82,24 @@ typedef xc_gnttab *XenGnttab;
 #  define XC_INTERFACE_FMT "%p"
 #  define XC_HANDLER_INITIAL_VALUE    NULL
 
+static inline XenEvtchn xen_xc_evtchn_open(void *logger,
+                                           unsigned int open_flags)
+{
+    return xc_evtchn_open(logger, open_flags);
+}
+
+static inline XenGnttab xen_xc_gnttab_open(void *logger,
+                                           unsigned int open_flags)
+{
+    return xc_gnttab_open(logger, open_flags);
+}
+
+static inline XenXC xen_xc_interface_open(void *logger, void *dombuild_logger,
+                                          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)
 {


Leave it as is in your patch and I'll change it in my xenner patch set again, 
but I wanted to make sure that you're aware of the issues going along with 
#defines.


Alex