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

[Paolo Bonzini <pbonzini@redhat.com>]

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 <xiawenc@linux.vnet.ibm.com>
>>> ---
>>>   .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.


>>> +#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.


>>> +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.

>> 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... :)

Paolo