From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=51079 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PsyNo-0007bd-Uj for qemu-devel@nongnu.org; Fri, 25 Feb 2011 09:11:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PsyNn-0002i8-DA for qemu-devel@nongnu.org; Fri, 25 Feb 2011 09:11:00 -0500 Received: from mail-vw0-f45.google.com ([209.85.212.45]:36936) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PsyNn-0002hu-9t for qemu-devel@nongnu.org; Fri, 25 Feb 2011 09:10:59 -0500 Received: by vws19 with SMTP id 19so1530834vws.4 for ; Fri, 25 Feb 2011 06:10:58 -0800 (PST) Message-ID: <4D67B87F.4020108@codemonkey.ws> Date: Fri, 25 Feb 2011 08:11:11 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Xen-devel] Re: [Qemu-devel] [PATCH V10 03/15] xen: Support new libxc calls from xen unstable. References: <1296658172-16609-1-git-send-email-anthony.perard@citrix.com> <1296658172-16609-4-git-send-email-anthony.perard@citrix.com> <4D669572.2010606@codemonkey.ws> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony PERARD Cc: Xen Devel , QEMU-devel , Stefano Stabellini On 02/25/2011 08:06 AM, Anthony PERARD wrote: > On Thu, Feb 24, 2011 at 17:29, Anthony Liguori wrote: > >> On 02/02/2011 08:49 AM, anthony.perard@citrix.com wrote: >> >>> From: Anthony PERARD >>> >>> 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 >>> Signed-off-by: Stefano Stabellini >>> Acked-by: Alexander Graf >>> --- >>> 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<>> #include >>> #include >>> -int main(void) { xs_daemon_open(); xc_interface_open(); return 0; } >>> +#include >>> +#include >>> +#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<>> +#include >>> +#include >>> +#include >>> +#include >>> +#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<>> +#include >>> +#include >>> +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. > It would be better to have two versions of the header, one that implemented the < 410 functions and one that implemented the newer functions. If you're just using the new signature for everything, you could even just #define in the later header. Regards, Anthony Liguori > Regards, > >