Re: [Xen-devel] [PATCH 1/4] tools/libxendevicemodel: introduce the new library

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, Feb 17, 2017 at 10:27:06AM +0000, Paul Durrant wrote:
> The new xendevicemodel library is intended to be used by all Xen device
> models such that the only hypercall that use will be the dm_op hypercall
> added by commit 524a98c2.
> 
> This patch adds the boilerplate for the new library, with only open() and
> close() entry points, and calls to those from libxenctrl in preparation
> for the compat layer added by a subsequent patch.
> 
> NOTE: After applying this patch libxenctrl will depend on libxendevicemodel
>       and so patches [1] and [2] must be applied to qemu-xen-traditional
>       and mini-os respectively.
> 
> [1] 
> http://xenbits.xen.org/gitweb/?p=people/pauldu/qemu-xen-traditional.git;a=commit;h=a445da7e
> [2] 
> http://xenbits.xen.org/gitweb/?p=people/pauldu/mini-os.git;a=commit;h=41c9f2ae
> 
> Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> ---
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> Cc: Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>
> ---
>  stubdom/Makefile                                | 17 ++++++-
>  tools/Makefile                                  |  1 +
>  tools/Rules.mk                                  | 10 +++-
>  tools/libs/Makefile                             |  1 +
>  tools/libs/devicemodel/Makefile                 | 66 ++++++++++++++++++++++++
>  tools/libs/devicemodel/core.c                   | 68 
> +++++++++++++++++++++++++
>  tools/libs/devicemodel/include/xendevicemodel.h | 40 +++++++++++++++
>  tools/libs/devicemodel/libxendevicemodel.map    |  6 +++
>  tools/libs/devicemodel/private.h                | 22 ++++++++
>  tools/libxc/Makefile                            |  3 +-
>  tools/libxc/xc_private.c                        | 11 +++-
>  tools/libxc/xc_private.h                        |  4 ++
>  12 files changed, 243 insertions(+), 6 deletions(-)
>  create mode 100644 tools/libs/devicemodel/Makefile
>  create mode 100644 tools/libs/devicemodel/core.c
>  create mode 100644 tools/libs/devicemodel/include/xendevicemodel.h
>  create mode 100644 tools/libs/devicemodel/libxendevicemodel.map
>  create mode 100644 tools/libs/devicemodel/private.h
> 

I skimmed through all the Makefile changes, assuming they are copied
from existing files, they look fine.

> diff --git a/tools/libxc/Makefile b/tools/libxc/Makefile
> index 37235e3..af5e49c 100644
> --- a/tools/libxc/Makefile
> +++ b/tools/libxc/Makefile
> @@ -113,6 +113,7 @@ CFLAGS-$(CONFIG_Linux) += -D_GNU_SOURCE
>  CFLAGS       += $(PTHREAD_CFLAGS)
>  CFLAGS       += $(CFLAGS_libxentoollog)
>  CFLAGS       += $(CFLAGS_libxenevtchn)
> +CFLAGS       += $(CFLAGS_libxendevicemodel)
>  
>  CTRL_LIB_OBJS := $(patsubst %.c,%.o,$(CTRL_SRCS-y))
>  CTRL_PIC_OBJS := $(patsubst %.c,%.opic,$(CTRL_SRCS-y))
> @@ -217,7 +218,7 @@ libxenctrl.so.$(MAJOR): libxenctrl.so.$(MAJOR).$(MINOR)
>       $(SYMLINK_SHLIB) $< $@
>  
>  libxenctrl.so.$(MAJOR).$(MINOR): $(CTRL_PIC_OBJS)
> -     $(CC) $(LDFLAGS) $(PTHREAD_LDFLAGS) -Wl,$(SONAME_LDFLAG) 
> -Wl,libxenctrl.so.$(MAJOR) $(SHLIB_LDFLAGS) -o $@ $^ $(LDLIBS_libxentoollog) 
> $(LDLIBS_libxenevtchn) $(LDLIBS_libxengnttab) $(LDLIBS_libxengntshr) 
> $(LDLIBS_libxencall) $(LDLIBS_libxenforeignmemory) $(PTHREAD_LIBS) 
> $(APPEND_LDFLAGS)
> +     $(CC) $(LDFLAGS) $(PTHREAD_LDFLAGS) -Wl,$(SONAME_LDFLAG) 
> -Wl,libxenctrl.so.$(MAJOR) $(SHLIB_LDFLAGS) -o $@ $^ $(LDLIBS_libxentoollog) 
> $(LDLIBS_libxenevtchn) $(LDLIBS_libxengnttab) $(LDLIBS_libxengntshr) 
> $(LDLIBS_libxencall) $(LDLIBS_libxenforeignmemory) 
> $(LDLIBS_libxendevicemodel) $(PTHREAD_LIBS) $(APPEND_LDFLAGS)
>  
>  # libxenguest
>  
> diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c
> index f0e089c..32379b6 100644
> --- a/tools/libxc/xc_private.c
> +++ b/tools/libxc/xc_private.c
> @@ -65,12 +65,18 @@ struct xc_interface_core 
> *xc_interface_open(xentoollog_logger *logger,
>  
>      xch->fmem = xenforeignmemory_open(xch->error_handler, 0);
>  
> -    if ( xch->xcall == NULL )
> +    if ( xch->fmem == NULL )
> +        goto err;
> +

This is a bug fix that should be split out and backported.

Other than this, the code looks fine.

Wei.

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