Re: [Qemu-devel] [PATCH V9 5/8] libqblock build system

[Wenchao Xia]

于 2012-11-18 22:58, Paolo Bonzini 写道:
> Il 18/11/2012 14:06, Wenchao Xia ha scritto:
> > 于 2012-11-16 19:08, Paolo Bonzini 写道:
> > > Il 16/11/2012 11:12, Wenchao Xia ha scritto:
> > > >     Libqblock was placed in new directory ./libqblock, libtool will build
> > > > dynamic library there, source files of block layer remains in ./block.
> > > > So block related source code will generate 3 sets of binary, first is
> > > > old
> > > > ones used in qemu, second and third are non PIC and PIC ones in
> > > > ./libqblock.
> > > >     GCC compiler flag visibility=hidden was used with special macro,
> > > > to export
> > > > only symbols that was marked as PUBLIC.
> > > >     For testing, make check-libqblock will build binaries and execute
> > > > it, make
> > > > clean or make check-clean will delete generated binaries.
> > > >     By default this library will be built and tested, out of tree
> > > > building is
> > > > supported.
> > > >     Header files added in configure due to install-libqblock from out
> > > > of tree
> > > > build need these them.
> > >
> > > Use $(SRC_PATH) instead in the install rules.
> >    OK.
> >
> > > > Signed-off-by: Wenchao Xia <address@hidden>
> > > > ---
> > > >    .gitignore                        |    2 +
> > > >    Makefile                          |   12 ++++++-
> > > >    configure                         |    4 ++-
> > > >    libqblock/Makefile                |   72
> > > > +++++++++++++++++++++++++++++++++++++
> > > >    libqblock/libqblock.pc.in         |   13 +++++++
> > > >    tests/Makefile                    |   29 ++++++++++++++-
> > > >    tests/libqblock/libqblock-qcow2.c |    4 ++
> > > >    7 files changed, 133 insertions(+), 3 deletions(-)
> > > >    create mode 100644 libqblock/Makefile
> > > >    create mode 100644 libqblock/libqblock-error.c
> > > >    create mode 100644 libqblock/libqblock-error.h
> > > >    create mode 100644 libqblock/libqblock-types.h
> > > >    create mode 100644 libqblock/libqblock.c
> > > >    create mode 100644 libqblock/libqblock.h
> > > >    create mode 100644 libqblock/libqblock.pc.in
> > > >    create mode 100644 tests/libqblock/libqblock-qcow2.c
> > > >
> > > > diff --git a/.gitignore b/.gitignore
> > > > index bd6ba1c..76207fe 100644
> > > > --- a/.gitignore
> > > > +++ b/.gitignore
> > > > @@ -93,3 +93,5 @@ cscope.*
> > > >    tags
> > > >    TAGS
> > > >    *~
> > > > +tests/libqblock/check-*
> > > > +tests/libqblock/test_images
> > > > diff --git a/Makefile b/Makefile
> > > > index 2cde430..4b0755d 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -189,6 +189,16 @@ qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y)
> > > > $(block-obj-y)
> > > >
> > > >    qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
> > > >
> > > > +######################################################################
> > > > +# Support building shared library libqblock
> > > > +libqblock.la:
> > > > +    $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C libqblock
> > > > V="$(V)" TARGET_DIR="$*/" libqblock.la,)
> > > > +
> > > > +install-libqblock:
> > > > +    $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C libqblock
> > > > V="$(V)" TARGET_DIR="$*/" install-libqblock,)
> > >
> > > Who calls install-libqblock?
> >    No one now, just provided an install rules similar to libcacard.
>
> Please call it from the install target (through a dependency).  Also
> remember to make these targets phony.
  right, phony is important, I forgot it before.

> > > > +#library objects
> > > > +libqblock-y=libqblock/libqblock.o libqblock/libqblock-error.o
> > > > +tools-obj-y =  $(oslib-obj-y) $(trace-obj-y) qemu-tool.o qemu-timer.o \
> > > > +       qemu-timer-common.o main-loop.o notify.o \
> > > > +       iohandler.o cutils.o iov.o async.o
> > > > +tools-obj-$(CONFIG_POSIX) += compatfd.o
> > >
> > > Are all of these really needed?  (In fact, after some of the recent
> > > changes none of these should be needed).
> >    OK, I'll try remove tools-objs.
>
> Or perhaps just trim it down.  Moving it to Makefile.objs is another
> idea, but please keep it as small as possible.
  OK.


> > > > +tests/libqblock/%.o: QEMU_CFLAGS:= $(subst -fPIE,-fPIC, $(QEMU_CFLAGS))
> > > > +tests/libqblock/%.o: QEMU_CFLAGS:= $(subst -DPIE,-DPIC, $(QEMU_CFLAGS))
> > >
> > > Why?
> > >
> > > A hint: if you have to do something that is not done anywhere else in
> > > the tree, do not do it.
> >    not sure whether PIE objs can be linked with dynamic library, let me
> > try remove it.
>
> They cannot be part of a dynamic library, but they sure can be linked
> with one.  QEMU itself is PIE and linked with SDL, gnutls, etc.
  Thanks, this extended my knowledge.

> > > Also, I'm not sure we need a completely separate subdirectory for one
> > > file only.  You can just use
> > >
> > > $(check-libqblock-y): QEMU_INCLUDES += ...
> > >
> > > Paolo
> >    A separate directory will allow more test cases come in easily,
> > I thought to added more test cases before.
>
> But you have one for now... :)
  OK.

> Paolo


--
Best Regards

Wenchao Xia