qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	Anthony Liguori <aliguori@us.ibm.com>,
	Eric Blake <eblake@redhat.com>,
	qemu-devel <qemu-devel@nongnu.org>,
	Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Subject: Re: [Qemu-devel] [PATCH V4 1/5] libqblock build system
Date: Thu, 27 Sep 2012 17:39:16 +0800	[thread overview]
Message-ID: <50641EC4.9000005@linux.vnet.ibm.com> (raw)
In-Reply-To: <506408F5.4080202@redhat.com>

Ok, I'll correct them.

> Il 27/09/2012 04:23, 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.
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>>   Makefile                    |   14 +++++++++-
>>   Makefile.objs               |    6 ++++
>>   libqblock/Makefile          |   56 +++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 74 insertions(+), 2 deletions(-)
>>   create mode 100644 libqblock/Makefile
>>   create mode 100644 libqblock/libqblock-error.c
>>   create mode 100644 libqblock/libqblock.c
>>
>> diff --git a/Makefile b/Makefile
>> index def2ae2..128bc6a 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -164,6 +164,17 @@ 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
>> +ifeq ($(LIBTOOL),)
>> +$(libqblock-lib-la):
>> +	@echo "libtool is missing, please install and rerun configure"; exit 1
>> +else
>> +$(libqblock-lib-la):
>> +	$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C libqblock V="$(V)" TARGET_DIR="$*/" $(libqblock-lib-la),)
>> +endif
>
> Please remove the useless indirection via $(libqblock-lib-la).  In general,
> this patch should be redone more similar to how libcacard is build:
>
> subdir-libcacard: $(oslib-obj-y) $(trace-obj-y) qemu-timer-common.o
> ...
> libcacard.la: $(oslib-obj-y) qemu-timer-common.o $(trace-obj-y)
>          $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C libcacard V="$(V)" TARGET_DIR="$*/" libcacard.la,)
>
> install-libcacard: libcacard.la
>          $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C libcacard V="$(V)" TARGET_DIR="$*/" install-libcacard,)
>
>
> (The ifeq is not necessary, I'm going to remove it from libcacard too.  The
> same error message is already raised in libcacard/Makefile).
>
>> +###########################################################################
>> +
>>   vscclient$(EXESUF): $(libcacard-y) $(oslib-obj-y) $(trace-obj-y) $(tools-obj-y) qemu-timer-common.o libcacard/vscclient.o
>>   	$(call quiet-command,$(CC) $(LDFLAGS) -o $@ $^ $(libcacard_libs) $(LIBS),"  LINK  $@")
>>
>> @@ -226,8 +237,7 @@ clean:
>>   	rm -f $(foreach f,$(GENERATED_SOURCES),$(f) $(f)-timestamp)
>>   	rm -rf qapi-generated
>>   	rm -rf qga/qapi-generated
>> -	$(MAKE) -C tests/tcg clean
>> -	for d in $(ALL_SUBDIRS) $(QEMULIBS) libcacard; do \
>> +	for d in $(ALL_SUBDIRS) $(QEMULIBS) libcacard libqblock; do \
>>   	if test -d $$d; then $(MAKE) -C $$d $@ || exit 1; fi; \
>>   	rm -f $$d/qemu-options.def; \
>>           done
>> diff --git a/Makefile.objs b/Makefile.objs
>> index 4412757..8a4c9fc 100644
>> --- a/Makefile.objs
>> +++ b/Makefile.objs
>> @@ -248,3 +248,9 @@ nested-vars += \
>>   	common-obj-y \
>>   	extra-obj-y
>>   dummy := $(call unnest-vars)
>> +
>> +#############################################################################
>> +# libqblock
>> +
>> +libqblock-lib-la = libqblock.la
>> +libqblock-lib-path = libqblock
>
> Remove these two please.
>
>> diff --git a/libqblock/Makefile b/libqblock/Makefile
>> new file mode 100644
>> index 0000000..5f65613
>> --- /dev/null
>> +++ b/libqblock/Makefile
>> @@ -0,0 +1,56 @@
>> +###########################################################################
>> +# libqblock Makefile
>> +# Todo:
>> +#    1 trace related files is generated in this directory, move
>> +#  them to the root directory.
>> +##########################################################################
>> +-include ../config-host.mak
>> +-include $(SRC_PATH)/Makefile.objs
>> +-include $(SRC_PATH)/rules.mak
>> +
>> +#############################################################################
>> +# Library settings
>> +#############################################################################
>> +$(call set-vpath, $(SRC_PATH))
>> +
>> +#expand the foldered vars,especially ./block
>> +dummy := $(call unnest-vars-1)
>
> Please use unnest-vars.
>
>> +#library objects
>> +libqblock-y=libqblock.o libqblock-error.o
>> +
>> +QEMU_OBJS= $(libqblock-y) $(block-obj-y)
>> +#filter out ./block
>> +QEMU_OBJS_FILTERED=$(filter %.o, $(QEMU_OBJS))
>> +QEMU_OBJS_LIB=$(patsubst %.o, %.lo, $(QEMU_OBJS_FILTERED))
>> +
>> +QEMU_CFLAGS+= -I../ -I../include
>> +#adding magic macro define for symbol hiding and exposing
>> +QEMU_CFLAGS+= -fvisibility=hidden -D LIBQB_BUILD
>> +
>> +#dependency libraries
>> +LIBS+=-lz $(LIBS_TOOLS)
>> +
>> +#################################################################
>> +# Runtime rules
>> +#################################################################
>> +clean:
>> +	rm -f *.lo *.o *.d *.la libqblock-test trace.c trace.c-timestamp
>> +	rm -rf .libs block trace
>> +
>> +help:
>> +	@echo type make libqblock-test at root dirtory, libtool is required
>
> Please remove this rule.
>
>> +#make directory ./block at runtime which would hold the output of block/*.c
>> +block:
>> +	@mkdir block
>> +
>> +ifeq ($(LIBTOOL),)
>> +$(libqblock-lib-la):
>> +	@echo "libtool is missing, please install and rerun configure"; exit 1
>> +else
>> +$(libqblock-lib-la): $(QEMU_OBJS_LIB)
>> +	$(call quiet-command,$(LIBTOOL) --mode=link --quiet --tag=CC $(CC) -rpath $(libdir) -o $@ $^ $(LIBS),"  lt LINK $@")
>> +endif
>> +
>> +.PHONY: libqblock.la
>


-- 
Best Regards

Wenchao Xia

  reply	other threads:[~2012-09-27  9:40 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-27  2:23 [Qemu-devel] [PATCH V4 0/5] libqblock qemu block layer library Wenchao Xia
2012-09-27  2:23 ` [Qemu-devel] [PATCH V4 1/5] libqblock build system Wenchao Xia
2012-09-27  8:06   ` Paolo Bonzini
2012-09-27  9:39     ` Wenchao Xia [this message]
2012-09-27  2:23 ` [Qemu-devel] [PATCH V4 2/5] libqblock type defines Wenchao Xia
2012-09-27  8:25   ` Paolo Bonzini
2012-09-27  9:52     ` Wenchao Xia
2012-09-27 10:16       ` Paolo Bonzini
2012-09-28  3:00         ` Wenchao Xia
2012-09-28  7:51           ` Paolo Bonzini
2012-09-28 10:52             ` Wenchao Xia
2012-09-28 10:58               ` Paolo Bonzini
2012-09-27  2:24 ` [Qemu-devel] [PATCH V4 3/5] libqblock API Wenchao Xia
2012-09-27  2:24 ` [Qemu-devel] [PATCH V4 4/5] libqblock test build system Wenchao Xia
2012-09-27  8:30   ` Paolo Bonzini
2012-09-27  9:56     ` Wenchao Xia
2012-09-27 10:17       ` Paolo Bonzini
2012-09-28  3:04         ` Wenchao Xia
2012-09-28  7:52           ` Paolo Bonzini
2012-09-28  9:46             ` Wenchao Xia
2012-09-27  2:24 ` [Qemu-devel] [PATCH V4 5/5] libqblock test example Wenchao Xia

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50641EC4.9000005@linux.vnet.ibm.com \
    --to=xiawenc@linux.vnet.ibm.com \
    --cc=aliguori@us.ibm.com \
    --cc=eblake@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).