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".
>
> +##########################################
> +# 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.
> +#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:
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.
> 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.
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))
