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

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Wed, Mar 23, 2011 at 10:43, Alexander Graf <agraf@xxxxxxx> wrote:
>
> On 01.03.2011, at 19:35, Anthony.Perard@xxxxxxxxxx wrote:
>
>> From: Anthony PERARD <anthony.perard@xxxxxxxxxx>
>>
>> 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 <anthony.perard@xxxxxxxxxx>
>> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
>> Acked-by: Alexander Graf <agraf@xxxxxxx>
>> ---
>> 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.

I understand the issues with the defines. I just trying to address
both comment from Anthony and you. But, I think these static inline
instead of the #defines will be ok. So I will put it in my series.

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel