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

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 20 February 2017 15:08
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; Ian Jackson <Ian.Jackson@citrix.com>;
> Wei Liu <wei.liu2@citrix.com>; Samuel Thibault <samuel.thibault@ens-
> lyon.org>
> Subject: Re: [PATCH 1/4] tools/libxendevicemodel: introduce the new library
> 
> 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@citrix.com>
> > ---
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > Cc: Samuel Thibault <samuel.thibault@ens-lyon.org>
> > ---
> >  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.

Yeah, I forgot I fixed that along the way. I'll split it out.

> 
> Other than this, the code looks fine.
> 

Thanks,

  Paul

> Wei.

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