From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:57557) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tb0bx-0003jy-C0 for qemu-devel@nongnu.org; Tue, 20 Nov 2012 22:04:26 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tb0bv-0006fP-Ud for qemu-devel@nongnu.org; Tue, 20 Nov 2012 22:04:25 -0500 Received: from e28smtp03.in.ibm.com ([122.248.162.3]:58084) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tb0bv-0006b7-A2 for qemu-devel@nongnu.org; Tue, 20 Nov 2012 22:04:23 -0500 Received: from /spool/local by e28smtp03.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 21 Nov 2012 08:34:18 +0530 Received: from d28av02.in.ibm.com (d28av02.in.ibm.com [9.184.220.64]) by d28relay03.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id qAL34EDo29229264 for ; Wed, 21 Nov 2012 08:34:15 +0530 Received: from d28av02.in.ibm.com (loopback [127.0.0.1]) by d28av02.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id qAL8W5Hn005581 for ; Wed, 21 Nov 2012 19:32:07 +1100 Message-ID: <50AC4467.4050101@linux.vnet.ibm.com> Date: Wed, 21 Nov 2012 11:03:03 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1353404767-4495-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1353404767-4495-5-git-send-email-xiawenc@linux.vnet.ibm.com> <50AB5AF0.8010701@redhat.com> In-Reply-To: <50AB5AF0.8010701@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH V10 4/7] libqblock build system List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: kwolf@redhat.com, peter.maydell@linaro.org, aliguori@us.ibm.com, stefanha@gmail.com, qemu-devel@nongnu.org, blauwirbel@gmail.com, kraxel@redhat.com 于 2012-11-20 18:26, Paolo Bonzini 写道: > Mostly looks good... > > Il 20/11/2012 10:46, Wenchao Xia ha scritto: >> +libqblock="yes" > > Please make the default the empty string, i.e. "build if and only if > libtool is present". > Do you mean adding a probe section later that if libtool is present, its default is "yes", else set it to "no"? >> >> +########################################## >> +# libqblock probe >> +if test "$libqblock" != "no" ; then >> +# libqblock depends on libtool, default to yes if libtool exist >> + if ! has $libtool; then >> + echo >> + echo "Warning: libqblock needs libtool" >> + echo "Setting libqblock option to no." >> + echo >> + libqblock="no" >> + fi >> +fi >> # > > Please fail here if libqblock=yes but libtool is not found. > OK. >> +#library objects >> +libqblock-obj-y= libqblock/libqblock.o libqblock/libqblock-error.o >> +extra-obj-y= main-loop.o qemu-timer.o qemu-tool.o iohandler.o \ >> + compatfd.o > > Are all of these really necessary? Instead of > main-loop/compatfd/iohandler, I think you can just add a new > libqblock/libqblock-aio.c file, and copy these definitions from main-loop.c: > OK, good idea. > QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque) > void qemu_aio_flush(void) > bool qemu_aio_wait(void) > void qemu_aio_set_fd_handler(int fd, > IOHandler *io_read, > IOHandler *io_write, > AioFlushHandler *io_flush, > void *opaque); > void qemu_aio_set_event_notifier(EventNotifier *notifier, > EventNotifierHandler *io_read, > AioFlushEventNotifierHandler *io_flush); > > and also include part of qemu_init_main_loop in the creation of a new > QBlockContext (the first time it's called). We can later map each > QBlockContext to a different AioContext. Please document somewhere that > the library is not yet thread-safe. > OK. >> diff --git a/tests/Makefile b/tests/Makefile >> index ef6c9f2..997dbcc 100644 >> --- a/tests/Makefile >> +++ b/tests/Makefile >> @@ -84,6 +84,23 @@ check-qtest-$(CONFIG_POSIX)=$(foreach TARGET,$(TARGETS), $(check-qtest-$(TARGET) >> qtest-obj-y = tests/libqtest.o $(oslib-obj-y) libqemustub.a >> $(check-qtest-y): $(qtest-obj-y) >> >> +#libqblock build rules >> +check-libqblock-$(CONFIG_LIBQBLOCK) = tests/check-libqblock-qcow2$(EXESUF) >> + >> +$(check-libqblock-y): QEMU_INCLUDES += -I$(SRC_PATH)/tests -I$(SRC_PATH)/$(LIBQBLOCK_DIR) >> +LIBQBLOCK_TEST_DIR = tests/test_images >> +LIBQBLOCK_DIR = libqblock >> +LIBQBLOCK_SO = $(LIBQBLOCK_DIR)/.libs/libqblock.so >> +LIBQBLOCK_SO_LINK_FLAG = -Wl,-rpath,$(LIBQBLOCK_DIR)/.libs > > These two should not be required. Just link in libqblock/libqblock.la > and libtool will do all of its magic. > OK. > Paolo > >> + >> +#use libtool to link check-libqblock, use LIBQBLOCK_SO_LINK_FLAG to add lib search diretory >> +LTLINK_CHECK_LIBQBLOCK = $(call quiet-command, $(LIBTOOL) --mode=link --quiet --tag=CC \ >> + $(CC) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) $(LIBQBLOCK_SO_LINK_FLAG) \ >> + -o $@ $(sort $(1)) $(LIBS), "lt LINK $(TARGET_DIR) $@") >> + >> +$(check-libqblock-y): %$(EXESUF): %.o >> + $(call LTLINK_CHECK_LIBQBLOCK, $^ $(LIBQBLOCK_SO)) > > -- Best Regards Wenchao Xia