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

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Thu, Feb 24, 2011 at 17:29, Anthony Liguori <anthony@xxxxxxxxxxxxx> wrote:
> On 02/02/2011 08:49 AM, anthony.perard@xxxxxxxxxx wrote:
>>
>> From: Anthony PERARD<anthony.perard@xxxxxxxxxx>
>>
>> This patch adds a generic layer for xc calls, allowing us to choose
>> between the
>> xenner and xen implementations at runtime.
>>
>> It also update 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>
>> ---
>> ÂMakefile.target   Â|  Â3 +
>> Âconfigure      Â|  62 +++++++++++++++-
>> Âhw/xen_backend.c   |  74 ++++++++++---------
>> Âhw/xen_backend.h   |  Â7 +-
>> Âhw/xen_common.h   Â|  38 ++++++----
>> Âhw/xen_console.c   |  10 +-
>> Âhw/xen_devconfig.c  |  10 +-
>> Âhw/xen_disk.c    Â|  28 ++++---
>> Âhw/xen_domainbuild.c | Â 29 ++++----
>> Âhw/xen_interfaces.c Â| Â191
>> ++++++++++++++++++++++++++++++++++++++++++++++++
>> Âhw/xen_interfaces.h Â| Â198
>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>> Âhw/xen_nic.c     |  36 +++++-----
>> Âhw/xenfb.c      |  14 ++--
>> Â13 files changed, 584 insertions(+), 116 deletions(-)
>> Âcreate mode 100644 hw/xen_interfaces.c
>> Âcreate mode 100644 hw/xen_interfaces.h
>>
>> diff --git a/Makefile.target b/Makefile.target
>> index db29e96..d09719f 100644
>> --- a/Makefile.target
>> +++ b/Makefile.target
>> @@ -205,6 +205,9 @@ QEMU_CFLAGS += $(VNC_SASL_CFLAGS)
>> ÂQEMU_CFLAGS += $(VNC_JPEG_CFLAGS)
>> ÂQEMU_CFLAGS += $(VNC_PNG_CFLAGS)
>>
>> +# xen support
>> +obj-$(CONFIG_XEN) += xen_interfaces.o
>> +
>> Â# xen backend driver support
>> Âobj-$(CONFIG_XEN) += xen_backend.o xen_devconfig.o
>> Âobj-$(CONFIG_XEN) += xen_console.o xenfb.o xen_disk.o xen_nic.o
>> diff --git a/configure b/configure
>> index 5a9121d..fde9bad 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=""
>> @@ -1144,13 +1145,71 @@ 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
>> +
>> + Â# 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"
>> @@ -3009,6 +3068,7 @@ case "$target_arch2" in
>> Â Â Âif test "$xen" = "yes" -a "$target_softmmu" = "yes" ; then
>> Â Â Â Âecho "CONFIG_XEN=y">> Â$config_target_mak
>> Â Â Â Âecho "LIBS+=$xen_libs">> Â$config_target_mak
>> + Â Â Âecho "CONFIG_XEN_CTRL_INTERFACE_VERSION=$xen_ctrl_version">>
>> Â$config_target_mak
>> Â Â Âfi
>> Âesac
>> Âcase "$target_arch2" in
>> diff --git a/hw/xen_backend.c b/hw/xen_backend.c
>> index 860b038..cf081e1 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;
>>
>> @@ -58,7 +59,7 @@ int xenstore_write_str(const char *base, const char
>> *node, const char *val)
>> Â Â Âchar abspath[XEN_BUFSIZE];
>>
>> Â Â Âsnprintf(abspath, sizeof(abspath), "%s/%s", base, node);
>> - Â Âif (!xs_write(xenstore, 0, abspath, val, strlen(val)))
>> + Â Âif (!xs_ops.write(xenstore, 0, abspath, val, strlen(val)))
>> Â Â Â Â Âreturn -1;
>> Â Â Âreturn 0;
>> Â}
>> @@ -70,7 +71,7 @@ char *xenstore_read_str(const char *base, const char
>> *node)
>> Â Â Âchar *str, *ret = NULL;
>>
>> Â Â Âsnprintf(abspath, sizeof(abspath), "%s/%s", base, node);
>> - Â Âstr = xs_read(xenstore, 0, abspath,&len);
>> + Â Âstr = xs_ops.read(xenstore, 0, abspath,&len);
>>
>
> I think I gave this feedback before but I'd really like to see static
> inlines here.
>
> It's very likely that you'll either want to have tracing or some commands
> can have a NULL function pointer in which case having a central location to
> do this is very useful.
>
> Plus, it's more natural to read code that's making a function call instead
> of going through a function pointer in a structure redirection.
>
> Can probably do this with just a sed over the current patch.


Is it good to have a .h with functions like that? :

static inline XenXC qemu_xc_interface_open(xentoollog_logger *logger,
                            xentoollog_logger *dombuild_logger,
                            unsigned open_flags)
{
#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 410
    return xc_interface_open();
#else
    return xc_interface_open(logger, dombuild_logger, open_flags);
#endif
}


So there will have no more structure redirection.

Regards,

-- 
Anthony PERARD

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