qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH V10 0/7] libqblock qemu block layer library
@ 2012-11-20  9:46 Wenchao Xia
  2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 1/7] Buildsystem fix distclean error for pixman Wenchao Xia
                   ` (6 more replies)
  0 siblings, 7 replies; 22+ messages in thread
From: Wenchao Xia @ 2012-11-20  9:46 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, peter.maydell, aliguori, stefanha, blauwirbel, kraxel,
	pbonzini, xiawenc

  These patches introduce libqblock API, make subdir-libqblock and make
check-libqblock could build this library.
Functionalities:
 1 create a new image.
 2 sync access of an image.
 3 basic image information retrieving such as backing file.
 4 detect if a sector is allocated in an image.
Supported Formats:
 ALL using file protocols.

  Patch 1 to 2 is independent with libqblock, which fix small bug and improve
qemu, can be applied without libqblock. I am not sure if I should sent them
in another thread, but following patch need these two fixing patch, so include
them here together.

v2:
  Insert reserved bytes into union.
  Use uint64_t instead of size_t, offset.
  Use const char * in filename pointer.
  Initialization function removed and it was automatically executed when
library is loaded.
  Added compile flag visibility=hidden, to avoid name space pollution.
  Structure naming style changed.
  Using byte unit instead of sector for every API.
  Added a member in image static information structure, to report logical
sector size, which is always 512 now.
  Read and write API can take request not aligned to 512 now. It returns the
byte number that have succeed in operation, but now either negative value
or the number requested would be returned, because qemu block sync I/O API
would not return such number.
  Typo fix due to comments and improved documents.

v3:
  Removed the code about OOM error, introduced GError.
  Used a table to map from string to enum types about format.
  Use typedef for every structure.
  Improved the gcc compiler macro to warn if gcc was not used.
  Global variable name changed with prefix libqb_.
  The struct QBlockStaticInfo was changed to folder full format related
information inside, and a new member with pointers pointing to the mostly used
members, such as backing file, virt size, was added. This would allow the user
to get full information about how it is created in the future.
  Each patch in the serial can work with qemu now.
  Typo fixes.

v4:
  Renamed QBroker to QBlockContext.
  Removed tool objs out of libqblock.
  Added a check in initialization about structure size for ABI.
  Added a new helper API to duplicate protocol information, helps to open files
in a backing file chain.
  Check-libqblock will not rebuild libqblock every time now.
  Test case file renamed to "libqblock-[FMT].c".
  Test use gtest framework now.
  Test do random creation of test file now, added check for information API in
it.
  Test do random sync io instead of fixed offset io now.
  Test accept one parameter about where to place the test image, now it is
./tests/libqblock/test_images.

v5:
  Makefile of libqblock was adjusted to be similar as libcacard, added spec
file and install section.
  Removed warning when GCC was not found.
  Structure names were changed to better ones.
  Removed the union typedef that contain reserved bytes to reduce the folder
depth.
  Some format related enum options was changed to better name.
  Added accessors about image static information, hide indirect accessing
member detail in the structure.
  Test Makefile do not create diretory now, test case create it themself.
  Test build system do not use libtool now, and removed qtest-obj-y in its
dependency, make check will automatically execute test anyway now.
  Removed "ifeq ($(LIBTOOL),)" in Makefile.

v6:
  Remove address pointer member in image static info structure.

v7:
  Support out of tree building.

v8:
  Fix a bug in out of tree building.

v9:
  Rebase and splitted out small fix patch for qemu.

v10:
  Rebased to upstream, adjusted libqblock build system according to Paolo's
comments.

Wenchao Xia (7):
  Buildsystem fix distclean error for pixman
  Buildsystem clean tests directory clearly
  block export function path_has_protocol
  libqblock build system
  libqblock type defines
  libqblock API
  libqblock test example

 .gitignore                     |    2 +
 Makefile                       |   33 +-
 block.c                        |    2 +-
 block.h                        |    2 +
 configure                      |   31 +-
 libqblock/Makefile             |   74 +++
 libqblock/libqblock-error.c    |   57 ++
 libqblock/libqblock-error.h    |   49 ++
 libqblock/libqblock-internal.h |   75 +++
 libqblock/libqblock-types.h    |  252 +++++++++
 libqblock/libqblock.c          | 1191 ++++++++++++++++++++++++++++++++++++++++
 libqblock/libqblock.h          |  341 ++++++++++++
 libqblock/libqblock.pc.in      |   13 +
 tests/Makefile                 |   38 ++-
 tests/check-libqblock-qcow2.c  |  392 +++++++++++++
 15 files changed, 2541 insertions(+), 11 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-internal.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/check-libqblock-qcow2.c

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH V10 1/7] Buildsystem fix distclean error for pixman
  2012-11-20  9:46 [Qemu-devel] [PATCH V10 0/7] libqblock qemu block layer library Wenchao Xia
@ 2012-11-20  9:46 ` Wenchao Xia
  2012-11-21 22:54   ` Peter Maydell
  2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 2/7] Buildsystem clean tests directory clearly Wenchao Xia
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 22+ messages in thread
From: Wenchao Xia @ 2012-11-20  9:46 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, peter.maydell, aliguori, stefanha, blauwirbel, kraxel,
	pbonzini, xiawenc

  Currently Makefile test if pixman have configure log, but the script directly
return error if that file do not exist. This patch fix it.

v2: print out the command.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 3e8d441..9ecbcbb 100644
--- a/Makefile
+++ b/Makefile
@@ -286,7 +286,7 @@ distclean: clean
 	for d in $(TARGET_DIRS) $(QEMULIBS); do \
 	rm -rf $$d || exit 1 ; \
         done
-	test -f pixman/config.log && make -C pixman distclean
+	if test -f pixman/config.log; then make -C pixman distclean; fi
 
 KEYMAPS=da     en-gb  et  fr     fr-ch  is  lt  modifiers  no  pt-br  sv \
 ar      de     en-us  fi  fr-be  hr     it  lv  nl         pl  ru     th \
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH V10 2/7] Buildsystem clean tests directory clearly
  2012-11-20  9:46 [Qemu-devel] [PATCH V10 0/7] libqblock qemu block layer library Wenchao Xia
  2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 1/7] Buildsystem fix distclean error for pixman Wenchao Xia
@ 2012-11-20  9:46 ` Wenchao Xia
  2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 3/7] block export function path_has_protocol Wenchao Xia
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 22+ messages in thread
From: Wenchao Xia @ 2012-11-20  9:46 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, peter.maydell, aliguori, stefanha, blauwirbel, kraxel,
	pbonzini, xiawenc

  Currently root Makefile try clean tests/tcg, hard to extend.
This patch added command make check-clean, which clean all
generated files used in tests. With this command root Makefile
do not care tests clean method any more, it simply calls the
command to do it, so any more clean script could be added in
tests/Makefile, make it easier to extend.

v2:
  Discard env MAKEFILES, always include tests/Makefile.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 Makefile       |    6 ++----
 configure      |    2 +-
 tests/Makefile |    7 +++++++
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 9ecbcbb..b04d862 100644
--- a/Makefile
+++ b/Makefile
@@ -211,9 +211,7 @@ qemu-ga$(EXESUF): QEMU_CFLAGS += -I qga/qapi-generated
 
 gen-out-type = $(subst .,-,$(suffix $@))
 
-ifneq ($(wildcard config-host.mak),)
-include $(SRC_PATH)/tests/Makefile
-endif
+include tests/Makefile
 
 qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py
 
@@ -259,7 +257,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
+	$(MAKE) check-clean
 	for d in $(ALL_SUBDIRS) $(QEMULIBS) libcacard; do \
 	if test -d $$d; then $(MAKE) -C $$d $@ || exit 1; fi; \
 	rm -f $$d/qemu-options.def; \
diff --git a/configure b/configure
index 780b19a..801585c 100755
--- a/configure
+++ b/configure
@@ -4166,7 +4166,7 @@ DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas"
 DIRS="$DIRS roms/seabios roms/vgabios"
 DIRS="$DIRS qapi-generated"
 DIRS="$DIRS libcacard libcacard/libcacard libcacard/trace"
-FILES="Makefile tests/tcg/Makefile qdict-test-data.txt"
+FILES="Makefile tests/Makefile tests/tcg/Makefile qdict-test-data.txt"
 FILES="$FILES tests/tcg/cris/Makefile tests/tcg/cris/.gdbinit"
 FILES="$FILES tests/tcg/lm32/Makefile libcacard/Makefile"
 FILES="$FILES pc-bios/optionrom/Makefile pc-bios/keymaps"
diff --git a/tests/Makefile b/tests/Makefile
index ca680e5..ef6c9f2 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -94,6 +94,7 @@ check-help:
 	@echo " make check-unit           Run qobject tests"
 	@echo " make check-block          Run block tests"
 	@echo " make check-report.html    Generates an HTML test report"
+	@echo " make check-clean          Clean the tests"
 	@echo
 	@echo "Please note that HTML reports do not regenerate if the unit tests"
 	@echo "has not changed."
@@ -148,4 +149,10 @@ check-unit: $(patsubst %,check-%, $(check-unit-y))
 check-block: $(patsubst %,check-%, $(check-block-y))
 check: check-unit check-qtest
 
+check-clean:
+	$(MAKE) -C tests/tcg clean
+	rm -f $(check-unit-y)
+	rm -f $(check-qtest-i386-y) $(check-qtest-x86_64-y) $(check-qtest-sparc64-y) $(check-qtest-sparc-y)
+	rm -f tests/*.o
+
 -include $(wildcard tests/*.d)
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH V10 3/7] block export function path_has_protocol
  2012-11-20  9:46 [Qemu-devel] [PATCH V10 0/7] libqblock qemu block layer library Wenchao Xia
  2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 1/7] Buildsystem fix distclean error for pixman Wenchao Xia
  2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 2/7] Buildsystem clean tests directory clearly Wenchao Xia
@ 2012-11-20  9:46 ` Wenchao Xia
  2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 4/7] libqblock build system Wenchao Xia
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 22+ messages in thread
From: Wenchao Xia @ 2012-11-20  9:46 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, peter.maydell, aliguori, stefanha, blauwirbel, kraxel,
	pbonzini, xiawenc

  This function is needed in other module, so export it. There is
already some patch on mail-list try export it, If that patch was applied,
pls ignore this one.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block.c |    2 +-
 block.h |    2 ++
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/block.c b/block.c
index 854ebd6..bfb2be8 100644
--- a/block.c
+++ b/block.c
@@ -199,7 +199,7 @@ static void bdrv_io_limits_intercept(BlockDriverState *bs,
 }
 
 /* check if the path starts with "<protocol>:" */
-static int path_has_protocol(const char *path)
+int path_has_protocol(const char *path)
 {
     const char *p;
 
diff --git a/block.h b/block.h
index 722c620..6805245 100644
--- a/block.h
+++ b/block.h
@@ -428,6 +428,8 @@ typedef enum {
     BLKDBG_EVENT_MAX,
 } BlkDebugEvent;
 
+int path_has_protocol(const char *path);
+
 #define BLKDBG_EVENT(bs, evt) bdrv_debug_event(bs, evt)
 void bdrv_debug_event(BlockDriverState *bs, BlkDebugEvent event);
 
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH V10 4/7] libqblock build system
  2012-11-20  9:46 [Qemu-devel] [PATCH V10 0/7] libqblock qemu block layer library Wenchao Xia
                   ` (2 preceding siblings ...)
  2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 3/7] block export function path_has_protocol Wenchao Xia
@ 2012-11-20  9:46 ` Wenchao Xia
  2012-11-20 10:26   ` Paolo Bonzini
  2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 5/7] libqblock type defines Wenchao Xia
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 22+ messages in thread
From: Wenchao Xia @ 2012-11-20  9:46 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, peter.maydell, aliguori, stefanha, blauwirbel, kraxel,
	pbonzini, xiawenc

  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.

v10:
  Use $(SRC_PATH) in install rules.
  Call install libqblock from install target in root Makefile.
  Make libqblock and check-libqblock is conditional to a configure option and
libtool now, if libtool is not present it is forced to be disabled.
  removed unnest-vars in libqblock Makfile for that it was done in Makefile.objs.
  picked out only needed objects files for libqblock, added stub objects in
libqblock linking.
  use libtool to link check-libqblock.
  removed -fPIC cc flag replacement in tests/Makefile.
  removed seperate directory for libqblock test case.
  added target subdir-libqblock.
  added targets to .phony.
  generate files at root directory instead of ./libqblock.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 .gitignore                    |    2 +
 Makefile                      |   25 +++++++++++++-
 configure                     |   29 ++++++++++++++++
 libqblock/Makefile            |   74 +++++++++++++++++++++++++++++++++++++++++
 libqblock/libqblock.c         |    6 +++
 libqblock/libqblock.h         |    1 +
 libqblock/libqblock.pc.in     |   13 +++++++
 tests/Makefile                |   31 ++++++++++++++++-
 tests/check-libqblock-qcow2.c |    6 +++
 9 files changed, 183 insertions(+), 4 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/check-libqblock-qcow2.c

diff --git a/.gitignore b/.gitignore
index bd6ba1c..77c1910 100644
--- a/.gitignore
+++ b/.gitignore
@@ -93,3 +93,5 @@ cscope.*
 tags
 TAGS
 *~
+tests/check-libqblock-qcow2
+tests/test_images
diff --git a/Makefile b/Makefile
index b04d862..dd01fe7 100644
--- a/Makefile
+++ b/Makefile
@@ -195,6 +195,27 @@ qemu-img$(EXESUF): qemu-img.o $(tools-obj-y) $(block-obj-y) libqemustub.a
 qemu-nbd$(EXESUF): qemu-nbd.o $(tools-obj-y) $(block-obj-y) libqemustub.a
 qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y) $(block-obj-y) libqemustub.a
 
+######################################################################
+# Support building shared library libqblock
+.PHONY: install-libqblock subdir-libqblock
+
+ifeq ($(CONFIG_LIBQBLOCK), y)
+subdir-libqblock: $(GENERATED_HEADERS) $(GENERATED_SOURCES)
+	$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C libqblock V="$(V)" TARGET_DIR="$*/" all,)
+
+install-libqblock: subdir-libqblock
+	$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C libqblock V="$(V)" TARGET_DIR="$*/" install-libqblock,)
+else
+LIBQBLOCK_WARN = "Libqblock was not enabled, skip. Make sure libtool was installed and libqblock was enabled."
+subdir-libqblock: $(GENERATED_HEADERS) $(GENERATED_SOURCES)
+	@echo $(LIBQBLOCK_WARN)
+
+install-libqblock:
+	@echo $(LIBQBLOCK_WARN)
+endif
+
+###########################################################################
+
 qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
 
 vscclient$(EXESUF): $(libcacard-y) $(oslib-obj-y) $(trace-obj-y) libcacard/vscclient.o libqemustub.a
@@ -258,7 +279,7 @@ clean:
 	rm -rf qapi-generated
 	rm -rf qga/qapi-generated
 	$(MAKE) check-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
@@ -331,7 +352,7 @@ install-confdir:
 install-sysconfig: install-datadir install-confdir
 	$(INSTALL_DATA) $(SRC_PATH)/sysconfigs/target/target-x86_64.conf "$(DESTDIR)$(qemu_confdir)"
 
-install: all $(if $(BUILD_DOCS),install-doc) install-sysconfig install-datadir
+install: all $(if $(BUILD_DOCS),install-doc) install-sysconfig install-datadir $(if $(CONFIG_LIBQBLOCK),install-libqblock)
 	$(INSTALL_DIR) "$(DESTDIR)$(bindir)"
 ifneq ($(TOOLS),)
 	$(INSTALL_PROG) $(STRIP_OPT) $(TOOLS) "$(DESTDIR)$(bindir)"
diff --git a/configure b/configure
index 801585c..b0cc044 100755
--- a/configure
+++ b/configure
@@ -223,6 +223,7 @@ libiscsi=""
 coroutine=""
 seccomp=""
 glusterfs=""
+libqblock="yes"
 
 # parse CC options first
 for opt do
@@ -871,6 +872,10 @@ for opt do
   ;;
   --enable-glusterfs) glusterfs="yes"
   ;;
+  --disable-libqblock) libqblock="no"
+  ;;
+  --enable-libqblock) libqblock="yes"
+  ;;
   *) echo "ERROR: unknown option $opt"; show_help="yes"
   ;;
   esac
@@ -1119,6 +1124,8 @@ echo "  --with-coroutine=BACKEND coroutine backend. Supported options:"
 echo "                           gthread, ucontext, sigaltstack, windows"
 echo "  --enable-glusterfs       enable GlusterFS backend"
 echo "  --disable-glusterfs      disable GlusterFS backend"
+echo "  --enable-libqblock       enable dynamic library libqblock"
+echo "  --disable-libqblock      disable dynamic library libqblock"
 echo ""
 echo "NOTE: The object files are built at the place where configure is launched"
 exit 1
@@ -2349,6 +2356,18 @@ EOF
   fi
 fi
 
+##########################################
+# 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
 #
 # Check for xxxat() functions when we are building linux-user
 # emulator.  This is done because older glibc versions don't
@@ -3101,6 +3120,9 @@ if test "$want_tools" = "yes" ; then
   if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then
     tools="qemu-nbd\$(EXESUF) $tools"
   fi
+  if test "$libqblock" = "yes" ; then
+  tools="subdir-libqblock $tools"
+  fi
 fi
 if test "$softmmu" = yes ; then
   if test "$virtfs" != no ; then
@@ -3235,6 +3257,7 @@ echo "build guest agent $guest_agent"
 echo "seccomp support   $seccomp"
 echo "coroutine backend $coroutine_backend"
 echo "GlusterFS support $glusterfs"
+echo "libqblock support $libqblock"
 
 if test "$sdl_too_old" = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -3581,6 +3604,10 @@ if test "$glusterfs" = "yes" ; then
   echo "CONFIG_GLUSTERFS=y" >> $config_host_mak
 fi
 
+if test "$libqblock" = "yes" ; then
+  echo "CONFIG_LIBQBLOCK=y" >> $config_host_mak
+fi
+
 # USB host support
 case "$usb" in
 linux)
@@ -4166,12 +4193,14 @@ DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas"
 DIRS="$DIRS roms/seabios roms/vgabios"
 DIRS="$DIRS qapi-generated"
 DIRS="$DIRS libcacard libcacard/libcacard libcacard/trace"
+DIRS="$DIRS libqblock"
 FILES="Makefile tests/Makefile tests/tcg/Makefile qdict-test-data.txt"
 FILES="$FILES tests/tcg/cris/Makefile tests/tcg/cris/.gdbinit"
 FILES="$FILES tests/tcg/lm32/Makefile libcacard/Makefile"
 FILES="$FILES pc-bios/optionrom/Makefile pc-bios/keymaps"
 FILES="$FILES pc-bios/spapr-rtas/Makefile"
 FILES="$FILES roms/seabios/Makefile roms/vgabios/Makefile"
+FILES="$FILES libqblock/Makefile"
 for bios_file in \
     $source_path/pc-bios/*.bin \
     $source_path/pc-bios/*.rom \
diff --git a/libqblock/Makefile b/libqblock/Makefile
new file mode 100644
index 0000000..256384a
--- /dev/null
+++ b/libqblock/Makefile
@@ -0,0 +1,74 @@
+###########################################################################
+# libqblock Makefile
+##########################################################################
+-include ../config-host.mak
+-include $(SRC_PATH)/rules.mak
+-include $(SRC_PATH)/Makefile.objs
+
+#############################################################################
+# Library settings
+#############################################################################
+# src files are in orignial path
+$(call set-vpath, $(SRC_PATH))
+# generated files are in ../
+$(call set-vpath, ../)
+
+#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
+
+#there are symbol conflict in stub-obj-y with iohandler.o, so set new version
+stub-obj-new-y= stubs/get-fd.o stubs/fdset-get-fd.o stubs/fdset-find-fd.o \
+             stubs/fdset-remove-fd.o stubs/fdset-add-fd.o
+
+
+QEMU_OBJS= $(libqblock-obj-y) $(block-obj-y) $(oslib-obj-y) $(stub-obj-new-y) \
+           $(extra-obj-y)
+QEMU_OBJS_LIB= $(patsubst %.o, %.lo, $(QEMU_OBJS))
+
+QEMU_CFLAGS+= -I$(SRC_PATH) -I$(SRC_PATH)/include -I../
+#adding magic macro define for symbol hiding and exposing
+QEMU_CFLAGS+= -fvisibility=hidden -D LIBQB_BUILD
+
+#dependency libraries
+LIBS+= -lz $(LIBS_TOOLS)
+
+#header files to be installed
+libqblock_includedir= $(includedir)/qblock
+libqblock_srcpath= $(SRC_PATH)/libqblock
+libqblock-pub-headers= $(libqblock_srcpath)/libqblock.h \
+                       $(libqblock_srcpath)/libqblock-types.h \
+                       $(libqblock_srcpath)/libqblock-error.h
+
+#################################################################
+# Runtime rules
+#################################################################
+.PHONY: clean all libqblock.la libqblock.pc install-libqblock
+
+clean:
+	rm -f *.lo *.o *.d *.la *.pc
+	rm -rf .libs block trace audio fsdev hw net qapi qga qom slirp ui libqblock stubs backends
+
+all: libqblock.la libqblock.pc
+# Dummy command so that make thinks it has done something
+	@true
+
+libqblock.la: $(QEMU_OBJS_LIB)
+	$(call quiet-command,$(LIBTOOL) --mode=link --quiet --tag=CC $(CC) -rpath $(libdir) -o $@ $^ $(LIBS),"  lt LINK $@")
+
+libqblock.pc: $(libqblock_srcpath)/libqblock.pc.in
+	$(call quiet-command,sed -e 's|@LIBDIR@|$(libdir)|' \
+		-e 's|@INCLUDEDIR@|$(libqblock_includedir)|' \
+	    -e 's|@VERSION@|$(shell cat $(SRC_PATH)/VERSION)|' \
+		-e 's|@PREFIX@|$(prefix)|' \
+		< $(libqblock_srcpath)/libqblock.pc.in > libqblock.pc,\
+	"  GEN   $@")
+
+install-libqblock: libqblock.la libqblock.pc
+	$(INSTALL_DIR) "$(DESTDIR)$(libdir)"
+	$(INSTALL_DIR) "$(DESTDIR)$(libdir)/pkgconfig"
+	$(INSTALL_DIR) "$(DESTDIR)$(libqblock_includedir)"
+	$(LIBTOOL) --mode=install $(INSTALL_DATA) libqblock.la "$(DESTDIR)$(libdir)"
+	$(LIBTOOL) --mode=install $(INSTALL_DATA) libqblock.pc "$(DESTDIR)$(libdir)/pkgconfig"
+	$(LIBTOOL) --mode=install $(INSTALL_DATA) $(libqblock-pub-headers) "$(DESTDIR)$(libqblock_includedir)"
diff --git a/libqblock/libqblock-error.c b/libqblock/libqblock-error.c
new file mode 100644
index 0000000..e69de29
diff --git a/libqblock/libqblock-error.h b/libqblock/libqblock-error.h
new file mode 100644
index 0000000..e69de29
diff --git a/libqblock/libqblock-types.h b/libqblock/libqblock-types.h
new file mode 100644
index 0000000..e69de29
diff --git a/libqblock/libqblock.c b/libqblock/libqblock.c
new file mode 100644
index 0000000..4c18c41
--- /dev/null
+++ b/libqblock/libqblock.c
@@ -0,0 +1,6 @@
+#include "libqblock.h"
+
+int libqb_test(void)
+{
+    return 0;
+}
diff --git a/libqblock/libqblock.h b/libqblock/libqblock.h
new file mode 100644
index 0000000..b0f9daf
--- /dev/null
+++ b/libqblock/libqblock.h
@@ -0,0 +1 @@
+__attribute__((visibility("default"))) int libqb_test(void);
diff --git a/libqblock/libqblock.pc.in b/libqblock/libqblock.pc.in
new file mode 100644
index 0000000..d2a7d06
--- /dev/null
+++ b/libqblock/libqblock.pc.in
@@ -0,0 +1,13 @@
+prefix=@PREFIX@
+exec_prefix=${prefix}
+libdir=@LIBDIR@
+includedir=@INCLUDEDIR@
+
+Name: qblock
+Description: QEMU block layer library
+Version: @VERSION@
+
+Requires:  rt gthread-2.0 glib-2.0 z curl cap-ng uuid
+Libs: -L${libdir} -lqblock
+Libs.private:
+Cflags: -I${includedir}
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
+
+#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))
+
 .PHONY: check-help
 check-help:
 	@echo "Regression testing targets:"
@@ -94,6 +111,7 @@ check-help:
 	@echo " make check-unit           Run qobject tests"
 	@echo " make check-block          Run block tests"
 	@echo " make check-report.html    Generates an HTML test report"
+	@echo " make check-libqblock      Run libqblock tests"
 	@echo " make check-clean          Clean the tests"
 	@echo
 	@echo "Please note that HTML reports do not regenerate if the unit tests"
@@ -126,6 +144,12 @@ $(patsubst %, check-report-qtest-%.xml, $(QTEST_TARGETS)): check-report-qtest-%.
 check-report-unit.xml: $(check-unit-y)
 	$(call quiet-command,gtester -q $(GTESTER_OPTIONS) -o $@ -m=$(SPEED) $^, "GTESTER $@")
 
+# gtester tests with libqblock
+
+.PHONY: $(patsubst %, check-%, $(check-libqblock-y))
+$(patsubst %, check-%, $(check-libqblock-y)): check-%: %
+	$(call quiet-command, LIBQBLOCK_TEST_DIR=$(LIBQBLOCK_TEST_DIR) gtester $(GTESTER_OPTIONS) -m=$(SPEED) $*,"GTESTER $*")
+
 # Reports and overall runs
 
 check-report.xml: $(patsubst %,check-report-qtest-%.xml, $(QTEST_TARGETS)) check-report-unit.xml
@@ -143,16 +167,19 @@ check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF)
 
 # Consolidated targets
 
-.PHONY: check-qtest check-unit check
+.PHONY: check-qtest check-unit check check-libqblock check-clean
 check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
 check-unit: $(patsubst %,check-%, $(check-unit-y))
 check-block: $(patsubst %,check-%, $(check-block-y))
-check: check-unit check-qtest
+check-libqblock: $(patsubst %,check-%, $(check-libqblock-y))
+check: check-libqblock check-unit check-qtest
 
 check-clean:
 	$(MAKE) -C tests/tcg clean
 	rm -f $(check-unit-y)
 	rm -f $(check-qtest-i386-y) $(check-qtest-x86_64-y) $(check-qtest-sparc64-y) $(check-qtest-sparc-y)
 	rm -f tests/*.o
+	rm -f $(check-libqblock-y)
+	rm -rf $(LIBQBLOCK_TEST_DIR)
 
 -include $(wildcard tests/*.d)
diff --git a/tests/check-libqblock-qcow2.c b/tests/check-libqblock-qcow2.c
new file mode 100644
index 0000000..50a4df3
--- /dev/null
+++ b/tests/check-libqblock-qcow2.c
@@ -0,0 +1,6 @@
+#include "libqblock.h"
+int main(int argc, char **argv)
+{
+    libqb_test();
+    return 0;
+}
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH V10 5/7] libqblock type defines
  2012-11-20  9:46 [Qemu-devel] [PATCH V10 0/7] libqblock qemu block layer library Wenchao Xia
                   ` (3 preceding siblings ...)
  2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 4/7] libqblock build system Wenchao Xia
@ 2012-11-20  9:46 ` Wenchao Xia
  2012-11-20 10:30   ` Paolo Bonzini
  2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 6/7] libqblock API Wenchao Xia
  2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 7/7] libqblock test example Wenchao Xia
  6 siblings, 1 reply; 22+ messages in thread
From: Wenchao Xia @ 2012-11-20  9:46 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, peter.maydell, aliguori, stefanha, blauwirbel, kraxel,
	pbonzini, xiawenc

  This patch contains type and macro defines used in APIs, one file for public
usage, one for libqblock internal usage.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 libqblock/libqblock-internal.h |   75 ++++++++++++
 libqblock/libqblock-types.h    |  252 ++++++++++++++++++++++++++++++++++++++++
 libqblock/libqblock.c          |    6 -
 libqblock/libqblock.h          |    1 -
 tests/check-libqblock-qcow2.c  |    1 -
 5 files changed, 327 insertions(+), 8 deletions(-)
 create mode 100644 libqblock/libqblock-internal.h

diff --git a/libqblock/libqblock-internal.h b/libqblock/libqblock-internal.h
new file mode 100644
index 0000000..75c0452
--- /dev/null
+++ b/libqblock/libqblock-internal.h
@@ -0,0 +1,75 @@
+/*
+ * QEMU block layer library
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Wenchao Xia   <xiawenc@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef LIBQBLOCK_INTERNAL
+#define LIBQBLOCK_INTERNAL
+
+#include "glib.h"
+
+#include "block.h"
+#include "block_int.h"
+#include "libqblock-types.h"
+
+/* this file contains defines and types used inside the library. */
+
+#define FUNC_FREE(p) g_free((p))
+#define FUNC_MALLOC(size) g_malloc((size))
+#define FUNC_CALLOC(nmemb, size) g_malloc0((nmemb)*(size))
+#define FUNC_STRDUP(p) g_strdup((p))
+
+#define CLEAN_FREE(p) { \
+        FUNC_FREE(p); \
+        (p) = NULL; \
+}
+
+/* details should be hidden to user */
+struct QBlockState {
+    BlockDriverState *bdrvs;
+    /* internal used file name now, if it is not NULL, it means
+       image was opened.
+    */
+    char *filename;
+};
+
+struct QBlockContext {
+    /* last error */
+    GError *g_error;
+    int err_ret; /* 1st level of error, the libqblock error number */
+    int err_no; /* 2nd level of error, errno what below reports */
+};
+
+/**
+ * QBlockStaticInfoAddr: a structure contains a set of pointer.
+ *
+ *    this struct contains a set of pointer pointing to some
+ *  property related to format or protocol. If a property is not available,
+ *  it will be set as NULL. User could use this to get properties directly.
+ *
+ *  @virt_size: virtual size, it is always not NULL.
+ *  @backing_loc: backing file location.
+ *  @encrypt: encryption flag.
+*/
+
+typedef struct QBlockStaticInfoAddr {
+    uint64_t *virt_size;
+    QBlockLocationInfo *backing_loc;
+    bool *encrypt;
+} QBlockStaticInfoAddr;
+
+#define G_LIBQBLOCK_ERROR g_libqbock_error_quark()
+
+static inline GQuark g_libqbock_error_quark(void)
+{
+    return g_quark_from_static_string("g-libqblock-error-quark");
+}
+#endif
diff --git a/libqblock/libqblock-types.h b/libqblock/libqblock-types.h
index e69de29..77ad325 100644
--- a/libqblock/libqblock-types.h
+++ b/libqblock/libqblock-types.h
@@ -0,0 +1,252 @@
+/*
+ * QEMU block layer library
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Wenchao Xia   <xiawenc@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef LIBQBLOCK_TYPES_H
+#define LIBQBLOCK_TYPES_H
+
+#include <sys/types.h>
+#include <stdint.h>
+#include <stdbool.h>
+
+#if defined(__GNUC__) && __GNUC__ >= 4
+    #ifdef LIBQB_BUILD
+        #define DLL_PUBLIC __attribute__((visibility("default")))
+    #else
+        #define DLL_PUBLIC
+    #endif
+#endif
+
+/* this library is designed around this core struct. */
+typedef struct QBlockState QBlockState;
+
+/* every thread should have a context. */
+typedef struct QBlockContext QBlockContext;
+
+/* flag used in open and create */
+#define LIBQBLOCK_O_RDWR        0x0002
+/* do not use the host page cache */
+#define LIBQBLOCK_O_NOCACHE     0x0020
+/* use write-back caching */
+#define LIBQBLOCK_O_CACHE_WB    0x0040
+/* don't open the backing file */
+#define LIBQBLOCK_O_NO_BACKING  0x0100
+/* disable flushing on this disk */
+#define LIBQBLOCK_O_NO_FLUSH    0x0200
+
+#define LIBQBLOCK_O_CACHE_MASK \
+   (LIBQBLOCK_O_NOCACHE | LIBQBLOCK_O_CACHE_WB | LIBQBLOCK_O_NO_FLUSH)
+
+#define LIBQBLOCK_O_VALID_MASK \
+   (LIBQBLOCK_O_RDWR | LIBQBLOCK_O_NOCACHE | LIBQBLOCK_O_CACHE_WB | \
+    LIBQBLOCK_O_NO_BACKING | LIBQBLOCK_O_NO_FLUSH)
+
+typedef enum QBlockProtocol {
+    QB_PROTO_NONE = 0,
+    QB_PROTO_FILE,
+    QB_PROTO_MAX
+} QBlockProtocol;
+
+typedef struct QBlockProtocolOptionsFile {
+    const char *filename;
+} QBlockProtocolOptionsFile;
+
+/**
+ * struct QBlockLocationInfo: contains information about how to find the image
+ *
+ * @prot_type: protocol type, now only support FILE.
+ * @o_file: file protocol related attributes.
+ * @reserved: reserved bytes for ABI.
+ */
+#define QBLOCK_PROT_OPTIONS_UNION_SIZE (512)
+typedef struct QBlockLocationInfo {
+    QBlockProtocol prot_type;
+    union {
+        QBlockProtocolOptionsFile o_file;
+        uint64_t reserved[QBLOCK_PROT_OPTIONS_UNION_SIZE/8];
+    };
+} QBlockLocationInfo;
+
+
+/* format related options */
+typedef enum QBlockFormat {
+    QB_FMT_NONE = 0,
+    QB_FMT_COW,
+    QB_FMT_QED,
+    QB_FMT_QCOW,
+    QB_FMT_QCOW2,
+    QB_FMT_RAW,
+    QB_FMT_RBD,
+    QB_FMT_SHEEPDOG,
+    QB_FMT_VDI,
+    QB_FMT_VMDK,
+    QB_FMT_VPC,
+    QB_FMT_MAX
+} QBlockFormat;
+
+typedef struct QBlockFormatOptionsCOW {
+    uint64_t virt_size;
+    QBlockLocationInfo backing_loc;
+} QBlockFormatOptionsCOW;
+
+typedef struct QBlockFormatOptionsQED {
+    uint64_t virt_size;
+    QBlockLocationInfo backing_loc;
+    QBlockFormat backing_fmt;
+    uint64_t cluster_size; /* unit is bytes */
+    uint64_t table_size; /* unit is clusters */
+} QBlockFormatOptionsQED;
+
+typedef struct QBlockFormatOptionsQCOW {
+    uint64_t virt_size;
+    QBlockLocationInfo backing_loc;
+    bool encrypt;
+} QBlockFormatOptionsQCOW;
+
+/* "Compatibility level (0.10 or 1.1)" */
+typedef enum QBlockFormatOptionsQCOW2CompatLv {
+    QB_FMT_QCOW2_COMPAT_DEFAULT = 0,
+    QB_FMT_QCOW2_COMPAT_V0_10,
+    QB_FMT_QCOW2_COMPAT_V1_10,
+} QBlockFormatOptionsQCOW2CompatLv;
+
+/* off or metadata */
+typedef enum QBlockFormatOptionsQCOW2PreAlloc {
+    QB_FMT_QCOW2_PREALLOC_DEFAULT = 0,
+    QB_FMT_QCOW2_PREALLOC_OFF,
+    QB_FMT_QCOW2_PREALLOC_METADATA,
+} QBlockFormatOptionsQCOW2PreAlloc;
+
+typedef struct QBlockFormatOptionsQCOW2 {
+    uint64_t virt_size;
+    QBlockLocationInfo backing_loc;
+    QBlockFormat backing_fmt;
+    bool encrypt;
+    uint64_t cluster_size; /* unit is bytes */
+    QBlockFormatOptionsQCOW2CompatLv cpt_lv;
+    QBlockFormatOptionsQCOW2PreAlloc pre_mode;
+} QBlockFormatOptionsQCOW2;
+
+typedef struct QBlockFormatOptionsRAW {
+    uint64_t virt_size;
+} QBlockFormatOptionsRAW;
+
+typedef struct QBlockFormatOptionsRBD {
+    uint64_t virt_size;
+    uint64_t cluster_size;
+} QBlockFormatOptionsRBD;
+
+/* off or full */
+typedef enum QBlockFormatOptionsSDPreAlloc {
+    QB_FMT_SD_PREALLOC_DEFAULT = 0,
+    QB_FMT_SD_PREALLOC_OFF,
+    QB_FMT_SD_PREALLOC_FULL,
+} QBlockFormatOptionsSDPreAlloc;
+
+typedef struct QBlockFormatOptionsSD {
+    uint64_t virt_size;
+    QBlockLocationInfo backing_loc;
+    QBlockFormatOptionsSDPreAlloc pre_mode;
+} QBlockFormatOptionsSD;
+
+typedef enum QBlockFormatOptionsVDIPreAlloc {
+    QB_FMT_VDI_PREALLOC_DEFAULT = 0,
+    QB_FMT_VDI_PREALLOC_OFF,
+    QB_FMT_VDI_PREALLOC_METADATA,
+} QBlockFormatOptionsVDIPreAlloc;
+
+typedef struct QBlockFormatOptionsVDI {
+    uint64_t virt_size;
+    uint64_t cluster_size;
+    QBlockFormatOptionsVDIPreAlloc pre_mode;
+} QBlockFormatOptionsVDI;
+
+/* whether compact to vmdk verion 6 */
+typedef enum QBlockFormatOptionsVMDKCompatLv {
+    QB_FMT_VMDK_COMPAT_DEFAULT = 0,
+    QB_FMT_VMDK_COMPAT_VMDKV6_FALSE,
+    QB_FMT_VMDK_COMPAT_VMDKV6_TRUE,
+} QBlockFormatOptionsVMDKCompatLv;
+
+/* vmdk flat extent format, values:
+"{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse |
+twoGbMaxExtentFlat | streamOptimized} */
+typedef enum QBlockFormatOptionsVMDKSubfmt {
+    QB_FMT_VMDK_SUBFMT_DEFAULT = 0,
+    QB_FMT_VMDK_SUBFMT_MONOLITHIC_SPARSE,
+    QB_FMT_VMDK_SUBFMT_MONOLITHIC_FLAT,
+    QB_FMT_VMDK_SUBFMT_TWOGBMAX_EXTENT_SPARSE,
+    QB_FMT_VMDK_SUBFMT_TWOGBMAX_EXTENT_FLAT,
+    QB_FMT_VMDK_SUBFMT_STREAM_OPTIMIZED,
+} QBlockFormatOptionsVMDKSubfmt;
+
+typedef struct QBlockFormatOptionsVMDK {
+    uint64_t virt_size;
+    QBlockLocationInfo backing_loc;
+    QBlockFormatOptionsVMDKCompatLv cpt_lv;
+    QBlockFormatOptionsVMDKSubfmt subfmt;
+} QBlockFormatOptionsVMDK;
+
+/* "{dynamic (default) | fixed} " */
+typedef enum QBlockFormatOptionsVPCSubfmt {
+    QB_FMT_VPC_SUBFMT_DEFAULT = 0,
+    QB_FMT_VPC_SUBFMT_DYNAMIC,
+    QB_FMT_VPC_SUBFMT_FIXED,
+} QBlockFormatOptionsVPCSubfmt;
+
+typedef struct QBlockFormatOptionsVPC {
+    uint64_t virt_size;
+    QBlockFormatOptionsVPCSubfmt subfmt;
+} QBlockFormatOptionsVPC;
+
+#define QBLOCK_FMT_OPTIONS_UNION_SIZE (QBLOCK_PROT_OPTIONS_UNION_SIZE*2)
+
+/**
+ * struct QBlockFormatInfo: contains information about how to retrieve data
+ *  from the image
+ *
+ * @fmt_type: format type.
+ * @o_cow~@o_vdi: format related attributes.
+ * @reserved: reserved bytes for ABI.
+ */
+typedef struct QBlockFormatInfo {
+    QBlockFormat fmt_type;
+    union {
+        QBlockFormatOptionsCOW       o_cow;
+        QBlockFormatOptionsQED       o_qed;
+        QBlockFormatOptionsQCOW      o_qcow;
+        QBlockFormatOptionsQCOW2     o_qcow2;
+        QBlockFormatOptionsRAW       o_raw;
+        QBlockFormatOptionsRBD       o_rbd;
+        QBlockFormatOptionsSD        o_sd;
+        QBlockFormatOptionsVDI       o_vdi;
+        QBlockFormatOptionsVMDK      o_vmdk;
+        QBlockFormatOptionsVPC       o_vpc;
+        uint64_t reserved[QBLOCK_FMT_OPTIONS_UNION_SIZE/8];
+    };
+} QBlockFormatInfo;
+
+/**
+ * QBlockStaticInfo: information about the block image.
+ *
+ * @loc: location information.
+ * @fmt: format information.
+ * @sector_size: how many bytes in a sector, it is 512 usually.
+ */
+typedef struct QBlockStaticInfo {
+    QBlockLocationInfo loc;
+    QBlockFormatInfo fmt;
+    int sector_size;
+} QBlockStaticInfo;
+
+
+#endif
diff --git a/libqblock/libqblock.c b/libqblock/libqblock.c
index 4c18c41..e69de29 100644
--- a/libqblock/libqblock.c
+++ b/libqblock/libqblock.c
@@ -1,6 +0,0 @@
-#include "libqblock.h"
-
-int libqb_test(void)
-{
-    return 0;
-}
diff --git a/libqblock/libqblock.h b/libqblock/libqblock.h
index b0f9daf..e69de29 100644
--- a/libqblock/libqblock.h
+++ b/libqblock/libqblock.h
@@ -1 +0,0 @@
-__attribute__((visibility("default"))) int libqb_test(void);
diff --git a/tests/check-libqblock-qcow2.c b/tests/check-libqblock-qcow2.c
index 50a4df3..6f5ccac 100644
--- a/tests/check-libqblock-qcow2.c
+++ b/tests/check-libqblock-qcow2.c
@@ -1,6 +1,5 @@
 #include "libqblock.h"
 int main(int argc, char **argv)
 {
-    libqb_test();
     return 0;
 }
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH V10 6/7] libqblock API
  2012-11-20  9:46 [Qemu-devel] [PATCH V10 0/7] libqblock qemu block layer library Wenchao Xia
                   ` (4 preceding siblings ...)
  2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 5/7] libqblock type defines Wenchao Xia
@ 2012-11-20  9:46 ` Wenchao Xia
  2012-11-20 10:41   ` Paolo Bonzini
  2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 7/7] libqblock test example Wenchao Xia
  6 siblings, 1 reply; 22+ messages in thread
From: Wenchao Xia @ 2012-11-20  9:46 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, peter.maydell, aliguori, stefanha, blauwirbel, kraxel,
	pbonzini, xiawenc

  This patch contains the major APIs in the library. For ABI some reserved
members were used.
Important APIs:
  1 QBlockContext. This structure was used to retrieve errors, every thread
must create one first.
  2 QBlockState. It stands for an block image object.
  3 QBlockStaticInfo. It contains static information such as location, backing
file, size.
  4 Sync I/O. It is similar to C file open, read, write and close operations.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 libqblock/libqblock-error.c |   57 ++
 libqblock/libqblock-error.h |   49 ++
 libqblock/libqblock.c       | 1191 +++++++++++++++++++++++++++++++++++++++++++
 libqblock/libqblock.h       |  341 +++++++++++++
 4 files changed, 1638 insertions(+), 0 deletions(-)

diff --git a/libqblock/libqblock-error.c b/libqblock/libqblock-error.c
index e69de29..2a59970 100644
--- a/libqblock/libqblock-error.c
+++ b/libqblock/libqblock-error.c
@@ -0,0 +1,57 @@
+/*
+ * QEMU block layer library
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Wenchao Xia   <xiawenc@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "libqblock-error.h"
+#include "libqblock-internal.h"
+
+void qb_error_get_human_str(QBlockContext *context,
+                            char *buf, size_t buf_size)
+{
+    const char *err_ret_str;
+    switch (context->err_ret) {
+    case QB_ERR_INTERNAL_ERR:
+        err_ret_str = "Internal error.";
+        break;
+    case QB_ERR_INVALID_PARAM:
+        err_ret_str = "Invalid param.";
+        break;
+    case QB_ERR_BLOCK_OUT_OF_RANGE:
+        err_ret_str = "request is out of image's range.";
+        break;
+    default:
+        err_ret_str = "Unknown error.";
+        break;
+    }
+    if (context == NULL) {
+        snprintf(buf, buf_size, "%s", err_ret_str);
+        return;
+    }
+
+    if (context->err_ret == QB_ERR_INTERNAL_ERR) {
+        snprintf(buf, buf_size, "%s %s errno [%d]. strerror [%s].",
+                     err_ret_str, context->g_error->message,
+                     context->err_no, strerror(-context->err_no));
+    } else {
+        snprintf(buf, buf_size, "%s %s",
+                     err_ret_str, context->g_error->message);
+    }
+    return;
+}
+
+int qb_error_get_errno(QBlockContext *context)
+{
+    if (context->err_ret == QB_ERR_INTERNAL_ERR) {
+        return context->err_no;
+    }
+    return 0;
+}
diff --git a/libqblock/libqblock-error.h b/libqblock/libqblock-error.h
index e69de29..4ffd1f1 100644
--- a/libqblock/libqblock-error.h
+++ b/libqblock/libqblock-error.h
@@ -0,0 +1,49 @@
+/*
+ * QEMU block layer library
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Wenchao Xia   <xiawenc@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef LIBQBLOCK_ERROR
+#define LIBQBLOCK_ERROR
+
+#include "libqblock-types.h"
+
+#define QB_ERR_INTERNAL_ERR (-1)
+#define QB_ERR_INVALID_PARAM (-100)
+#define QB_ERR_BLOCK_OUT_OF_RANGE (-101)
+
+/* error handling */
+/**
+ * qb_error_get_human_str: get human readable error string.
+ *
+ * return a human readable string, it would be truncated if buf is not big
+ *  enough.
+ *
+ * @context: operation context, must be valid.
+ * @buf: buf to receive the string.
+ * @buf_size: the size of the string buf.
+ */
+DLL_PUBLIC
+void qb_error_get_human_str(QBlockContext *context,
+                            char *buf, size_t buf_size);
+
+/**
+ * qb_error_get_errno: get error number, only valid when err_ret is
+ *   QB_ERR_INTERNAL_ERR.
+ *
+ * return negative errno if last error is QB_ERR_INTERNAL_ERR, otherwise 0.
+ *
+ * @context: operation context.
+ */
+DLL_PUBLIC
+int qb_error_get_errno(QBlockContext *context);
+
+#endif
diff --git a/libqblock/libqblock.c b/libqblock/libqblock.c
index e69de29..0266d06 100644
--- a/libqblock/libqblock.c
+++ b/libqblock/libqblock.c
@@ -0,0 +1,1191 @@
+/*
+ * QEMU block layer library
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Wenchao Xia   <xiawenc@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include <unistd.h>
+#include <stdarg.h>
+
+#include "libqblock.h"
+#include "libqblock-internal.h"
+
+#include "qemu-aio.h"
+
+#define LIBQB_FILENAME_MAX 4096
+
+typedef struct LibqblockGlobalData {
+    int init_flag;
+} LibqblockGlobalData;
+
+LibqblockGlobalData libqb_global_data;
+
+typedef struct LibqbFormatStrMapping {
+    const char *fmt_str;
+    QBlockFormat fmt_type;
+} LibqbFormatStrMapping;
+
+LibqbFormatStrMapping libqb_fmtstr_table[] = {
+    {"cow", QB_FMT_COW},
+    {"qed", QB_FMT_QED},
+    {"qcow", QB_FMT_QCOW},
+    {"qcow2", QB_FMT_QCOW2},
+    {"raw", QB_FMT_RAW},
+    {"rbd", QB_FMT_RBD},
+    {"sheepdog", QB_FMT_SHEEPDOG},
+    {"vdi", QB_FMT_VDI},
+    {"vmdk", QB_FMT_VMDK},
+    {"vpc", QB_FMT_VPC},
+    {NULL, 0},
+};
+
+__attribute__((constructor))
+static void libqblock_init(void)
+{
+    if (libqb_global_data.init_flag == 0) {
+        bdrv_init();
+        qemu_init_main_loop();
+    }
+    libqb_global_data.init_flag = 1;
+    /* Todo, add an assertion about the ABI */
+}
+
+const char *qb_fmttype2str(QBlockFormat fmt_type)
+{
+    int i = 0;
+    LibqbFormatStrMapping *tb = libqb_fmtstr_table;
+
+    while (tb[i].fmt_str != NULL) {
+        if (tb[i].fmt_type == fmt_type) {
+            return tb[i].fmt_str;
+        }
+        i++;
+    }
+    return NULL;
+}
+
+QBlockFormat qb_str2fmttype(const char *fmt_str)
+{
+    int i = 0;
+    LibqbFormatStrMapping *tb = libqb_fmtstr_table;
+
+    while (tb[i].fmt_str != NULL) {
+        if ((strcmp(fmt_str, tb[i].fmt_str) == 0)) {
+            return tb[i].fmt_type;
+        }
+        i++;
+    }
+    return QB_FMT_NONE;
+}
+
+static void set_context_err(QBlockContext *context, int err_ret,
+                            const char *fmt, ...)
+{
+    va_list ap;
+
+    if (context->g_error != NULL) {
+        g_error_free(context->g_error);
+    }
+
+    va_start(ap, fmt);
+    context->g_error = g_error_new_valist(G_LIBQBLOCK_ERROR, err_ret, fmt, ap);
+    va_end(ap);
+
+    context->err_ret = err_ret;
+    if (err_ret == QB_ERR_INTERNAL_ERR) {
+        context->err_no = -errno;
+    } else {
+        context->err_no = 0;
+    }
+}
+
+int qb_context_new(QBlockContext **context)
+{
+    *context = FUNC_CALLOC(1, sizeof(QBlockContext));
+    return 0;
+}
+
+void qb_context_delete(QBlockContext **context)
+{
+    if ((*context)->g_error != NULL) {
+        g_error_free((*context)->g_error);
+    }
+    CLEAN_FREE(*context);
+    return;
+}
+
+int qb_state_new(QBlockContext *context,
+                 QBlockState **qbs)
+{
+    *qbs = FUNC_CALLOC(1, sizeof(QBlockState));
+    (*qbs)->bdrvs = bdrv_new("hda");
+    if ((*qbs)->bdrvs == NULL) {
+        CLEAN_FREE(*qbs);
+        set_context_err(context, QB_ERR_INTERNAL_ERR,
+                       "failed to create the driver.");
+        return context->err_ret;
+    }
+    return 0;
+}
+
+void qb_state_delete(QBlockContext *context,
+                     QBlockState **qbs)
+{
+    if ((*qbs)->filename != NULL) {
+        qb_close(context, *qbs);
+    }
+    if ((*qbs)->bdrvs != NULL) {
+        bdrv_delete((*qbs)->bdrvs);
+        (*qbs)->bdrvs = NULL;
+    }
+    CLEAN_FREE(*qbs);
+    return;
+}
+
+int qb_loc_info_new(QBlockContext *context,
+                    QBlockLocationInfo **loc)
+{
+    *loc = FUNC_CALLOC(1, sizeof(QBlockLocationInfo));
+    return 0;
+}
+
+void qb_loc_info_delete(QBlockContext *context,
+                         QBlockLocationInfo **loc)
+{
+    CLEAN_FREE(*loc);
+}
+
+int qb_fmt_info_new(QBlockContext *context,
+                    QBlockFormatInfo **fmt)
+{
+    *fmt = FUNC_CALLOC(1, sizeof(QBlockFormatInfo));
+    return 0;
+}
+
+void qb_fmt_info_delete(QBlockContext *context,
+                        QBlockFormatInfo **fmt)
+{
+    CLEAN_FREE(*fmt);
+}
+
+/* return 0 if every thing is fine */
+static int loc_check_params(QBlockContext *context,
+                            QBlockLocationInfo *loc)
+{
+    context->err_ret = 0;
+
+    switch (loc->prot_type) {
+    case QB_PROTO_FILE:
+        if (loc->o_file.filename == NULL) {
+            set_context_err(context, QB_ERR_INVALID_PARAM,
+                           "Filename was not set.");
+            goto out;
+        }
+        if (path_has_protocol(loc->o_file.filename) > 0) {
+            set_context_err(context, QB_ERR_INVALID_PARAM,
+                           "filename [%s] had protocol.",
+                           loc->o_file.filename);
+            goto out;
+        }
+        break;
+    default:
+        set_context_err(context, QB_ERR_INVALID_PARAM,
+                       "Protocol type [%d] was not valid.",
+                       loc->prot_type);
+        break;
+    }
+
+ out:
+    return context->err_ret;
+}
+
+/* translate loc structure to internal filename, returned char* need free,
+ * assuming filename is not NULL. *filename would be set to NULL if no valid
+ * filename found. *filename must be freed later.
+ * return 0 if no error with *filename set.
+ */
+static int loc2filename(QBlockContext *context,
+                        QBlockLocationInfo *loc,
+                        char **filename)
+{
+    context->err_ret = 0;
+
+    if (*filename != NULL) {
+        CLEAN_FREE(*filename);
+    }
+    switch (loc->prot_type) {
+    case QB_PROTO_FILE:
+        *filename = FUNC_STRDUP(loc->o_file.filename);
+        break;
+    default:
+        set_context_err(context, QB_ERR_INVALID_PARAM,
+                 "protocol type [%d] is not supported.",
+                  loc->prot_type);
+        break;
+    }
+
+    return context->err_ret;
+}
+
+/* translate filename to location, loc->prot_type = NONE if fail, filename
+   must be valid. loc internal char pointer must be freed later.
+ * return 0 if no error.
+ */
+static int filename2loc(QBlockContext *context,
+                        QBlockLocationInfo *loc,
+                        const char *filename)
+{
+    context->err_ret = 0;
+
+    if (path_has_protocol(filename) > 0) {
+        set_context_err(context, QB_ERR_INVALID_PARAM,
+                     "Filename [%s] had protocol, not supported now.",
+                     filename);
+        goto out;
+    }
+
+    loc->prot_type = QB_PROTO_FILE;
+    switch (loc->prot_type) {
+    case QB_PROTO_FILE:
+        loc->o_file.filename = FUNC_STRDUP(filename);
+        break;
+    default:
+        break;
+    }
+
+ out:
+    return context->err_ret;
+}
+
+/* return 0 if OK, or qblock error number */
+static int set_backing_file_options(QBlockContext *context,
+                                    QEMUOptionParameter *param,
+                                    QBlockLocationInfo *loc,
+                                    QBlockFormat *fmt)
+{
+    char *backing_filename = NULL;
+    const char *fmtstr_backing = NULL;
+    int ret = 0;
+
+    if (loc == NULL) {
+        goto out;
+    }
+
+    ret = loc2filename(context, loc, &backing_filename);
+    /* ret can < 0 if loc have not been set, mean user did not specify backing
+       file, so need to check return value */
+
+    ret = 0;
+
+    if (backing_filename) {
+        ret = set_option_parameter(param,
+                            BLOCK_OPT_BACKING_FILE, backing_filename);
+        assert(ret == 0);
+        if (fmt == NULL) {
+            goto out;
+        }
+        fmtstr_backing = qb_fmttype2str(*fmt);
+        if (fmtstr_backing) {
+            ret = set_option_parameter(param,
+                                BLOCK_OPT_BACKING_FMT, fmtstr_backing);
+            assert(ret == 0);
+        }
+    }
+
+ out:
+    FUNC_FREE(backing_filename);
+    return ret;
+}
+
+int qb_create(QBlockContext *context,
+              QBlockState *qbs,
+              QBlockLocationInfo *loc,
+              QBlockFormatInfo *fmt,
+              int flag)
+{
+    int ret = 0, bd_ret;
+    char *filename = NULL;
+    BlockDriverState *bs = NULL;
+    BlockDriver *drv = NULL, *backing_drv = NULL;
+    bool tmp_bool;
+
+    const char *fmtstr = NULL, *tmp = NULL;
+    QEMUOptionParameter *param = NULL, *create_options = NULL;
+    QEMUOptionParameter *backing_fmt, *backing_file, *size;
+    QBlockFormatOptionsCOW *o_cow = NULL;
+    QBlockFormatOptionsQED *o_qed = NULL;
+    QBlockFormatOptionsQCOW *o_qcow = NULL;
+    QBlockFormatOptionsQCOW2 *o_qcow2 = NULL;
+    QBlockFormatOptionsRAW *o_raw = NULL;
+    QBlockFormatOptionsRBD *o_rbd = NULL;
+    QBlockFormatOptionsSD *o_sd = NULL;
+    QBlockFormatOptionsVDI *o_vdi = NULL;
+    QBlockFormatOptionsVMDK *o_vmdk = NULL;
+    QBlockFormatOptionsVPC *o_vpc = NULL;
+
+
+    /* check parameters */
+    if (flag & (~LIBQBLOCK_O_VALID_MASK)) {
+        set_context_err(context, QB_ERR_INVALID_PARAM,
+                           "invalid flag was set.");
+        ret = context->err_ret;
+        goto out;
+    }
+
+    if ((loc == NULL) || (qbs == NULL) || (fmt == NULL)) {
+        set_context_err(context, QB_ERR_INVALID_PARAM,
+                          "Got unexpected NULL pointer in parameters.");
+        ret = context->err_ret;
+        goto out;
+    }
+
+    ret = loc_check_params(context, loc);
+    if (ret != 0) {
+        goto out;
+    }
+
+    /* internal translate */
+    ret = loc2filename(context, loc, &filename);
+    if (ret != 0) {
+        goto out;
+    }
+
+    fmtstr = qb_fmttype2str(fmt->fmt_type);
+    if (fmtstr == NULL) {
+        set_context_err(context, QB_ERR_INVALID_PARAM,
+                 "Got unexpected NULL pointer in parameters.");
+        ret = context->err_ret;
+        goto out;
+    }
+
+    drv = bdrv_find_format(fmtstr);
+    assert(drv != NULL);
+
+    create_options = append_option_parameters(create_options,
+                                              drv->create_options);
+    param = parse_option_parameters("", create_options, param);
+
+    switch (fmt->fmt_type) {
+    case QB_FMT_COW:
+        o_cow = &(fmt->o_cow);
+        bd_ret = set_option_parameter_int(param,
+                                BLOCK_OPT_SIZE, o_cow->virt_size);
+        assert(bd_ret == 0);
+        /* do not need to check loc, it may be not set */
+        ret = set_backing_file_options(context, param,
+                                       &o_cow->backing_loc, NULL);
+        if (ret != 0) {
+            goto out;
+        }
+        break;
+    case QB_FMT_QED:
+        o_qed = &(fmt->o_qed);
+        bd_ret = set_option_parameter_int(param,
+                                BLOCK_OPT_SIZE, o_qed->virt_size);
+        assert(bd_ret == 0);
+        ret = set_backing_file_options(context, param,
+                                 &o_qed->backing_loc, &o_qed->backing_fmt);
+        if (ret != 0) {
+            goto out;
+        }
+        bd_ret = set_option_parameter_int(param,
+                                BLOCK_OPT_CLUSTER_SIZE, o_qed->cluster_size);
+        assert(bd_ret == 0);
+        bd_ret = set_option_parameter_int(param,
+                                BLOCK_OPT_TABLE_SIZE, o_qed->table_size);
+        assert(bd_ret == 0);
+        break;
+    case QB_FMT_QCOW:
+        o_qcow = &(fmt->o_qcow);
+        bd_ret = set_option_parameter_int(param,
+                                BLOCK_OPT_SIZE, o_qcow->virt_size);
+        assert(bd_ret == 0);
+        ret = set_backing_file_options(context, param,
+                                       &o_qcow->backing_loc, NULL);
+        if (ret != 0) {
+            goto out;
+        }
+        tmp = o_qcow->encrypt ? "on" : "off";
+        bd_ret = set_option_parameter(param, BLOCK_OPT_ENCRYPT, tmp);
+        assert(bd_ret == 0);
+        break;
+    case QB_FMT_QCOW2:
+        o_qcow2 = &(fmt->o_qcow2);
+        bd_ret = set_option_parameter_int(param,
+                              BLOCK_OPT_SIZE, o_qcow2->virt_size);
+        assert(bd_ret == 0);
+        ret = set_backing_file_options(context, param,
+                              &o_qcow2->backing_loc, &o_qcow2->backing_fmt);
+        if (ret != 0) {
+            goto out;
+        }
+        tmp = o_qcow2->encrypt ? "on" : "off";
+        bd_ret = set_option_parameter(param, BLOCK_OPT_ENCRYPT, tmp);
+        assert(bd_ret == 0);
+        bd_ret = set_option_parameter_int(param,
+                              BLOCK_OPT_CLUSTER_SIZE, o_qcow2->cluster_size);
+        assert(bd_ret == 0);
+
+        if (o_qcow2->cpt_lv != QB_FMT_QCOW2_COMPAT_DEFAULT) {
+            tmp = o_qcow2->cpt_lv == QB_FMT_QCOW2_COMPAT_V0_10 ? "0.10" : "1.1";
+            bd_ret = set_option_parameter(param,
+                              BLOCK_OPT_COMPAT_LEVEL, tmp);
+            assert(bd_ret == 0);
+        }
+
+        if (o_qcow2->pre_mode != QB_FMT_QCOW2_PREALLOC_DEFAULT) {
+            tmp = o_qcow2->pre_mode == QB_FMT_QCOW2_PREALLOC_OFF ?
+                                         "off" : "metadata";
+            bd_ret = set_option_parameter(param,
+                              BLOCK_OPT_PREALLOC, tmp);
+            assert(bd_ret == 0);
+        }
+        break;
+
+    case QB_FMT_RAW:
+        o_raw = &(fmt->o_raw);
+        bd_ret = set_option_parameter_int(param,
+                              BLOCK_OPT_SIZE, o_raw->virt_size);
+        assert(bd_ret == 0);
+        break;
+    case QB_FMT_RBD:
+        o_rbd = &(fmt->o_rbd);
+        bd_ret = set_option_parameter_int(param,
+                              BLOCK_OPT_SIZE, o_rbd->virt_size);
+        assert(bd_ret == 0);
+        bd_ret = set_option_parameter_int(param,
+                              BLOCK_OPT_CLUSTER_SIZE, o_rbd->cluster_size);
+        assert(bd_ret == 0);
+        break;
+    case QB_FMT_SHEEPDOG:
+        o_sd = &(fmt->o_sd);
+        bd_ret = set_option_parameter_int(param,
+                              BLOCK_OPT_SIZE, o_sd->virt_size);
+        assert(bd_ret == 0);
+        ret = set_backing_file_options(context, param,
+                                       &o_sd->backing_loc, NULL);
+        if (ret != 0) {
+            goto out;
+        }
+        if (o_sd->pre_mode != QB_FMT_SD_PREALLOC_DEFAULT) {
+            tmp = o_sd->pre_mode == QB_FMT_SD_PREALLOC_OFF ? "off" : "full";
+            bd_ret = set_option_parameter(param,
+                              BLOCK_OPT_PREALLOC, tmp);
+            assert(bd_ret == 0);
+        }
+        break;
+    case QB_FMT_VDI:
+        o_vdi = &(fmt->o_vdi);
+        bd_ret = set_option_parameter_int(param,
+                              BLOCK_OPT_SIZE, o_vdi->virt_size);
+        assert(bd_ret == 0);
+        /* following option is not always valid depends on configuration */
+        set_option_parameter_int(param,
+                              BLOCK_OPT_CLUSTER_SIZE, o_vdi->cluster_size);
+        if (o_vdi->pre_mode != QB_FMT_VDI_PREALLOC_DEFAULT) {
+            tmp_bool = o_sd->pre_mode == QB_FMT_VDI_PREALLOC_METADATA ?
+                                                     true : false;
+            set_option_parameter_int(param, "static", tmp_bool);
+        }
+        break;
+    case QB_FMT_VMDK:
+        o_vmdk = &(fmt->o_vmdk);
+        bd_ret = set_option_parameter_int(param,
+                              BLOCK_OPT_SIZE, o_vmdk->virt_size);
+        assert(bd_ret == 0);
+        ret = set_backing_file_options(context, param,
+                                       &o_vmdk->backing_loc, NULL);
+        if (ret != 0) {
+            goto out;
+        }
+
+        if (o_vmdk->cpt_lv != QB_FMT_VMDK_COMPAT_DEFAULT) {
+            tmp_bool = o_vmdk->cpt_lv == QB_FMT_VMDK_COMPAT_VMDKV6_TRUE ?
+                                                     true : false;
+            bd_ret = set_option_parameter_int(param, BLOCK_OPT_COMPAT6,
+                                                     tmp_bool);
+            assert(bd_ret == 0);
+        }
+        if (o_vmdk->subfmt != QB_FMT_VMDK_SUBFMT_DEFAULT) {
+            switch (o_vmdk->subfmt) {
+            case QB_FMT_VMDK_SUBFMT_MONOLITHIC_SPARSE:
+                tmp = "monolithicSparse";
+                break;
+            case QB_FMT_VMDK_SUBFMT_MONOLITHIC_FLAT:
+                tmp = "monolithicFlat";
+                break;
+            case QB_FMT_VMDK_SUBFMT_TWOGBMAX_EXTENT_SPARSE:
+                tmp = "twoGbMaxExtentSparse";
+                break;
+            case QB_FMT_VMDK_SUBFMT_TWOGBMAX_EXTENT_FLAT:
+                tmp = "twoGbMaxExtentFlat";
+                break;
+            case QB_FMT_VMDK_SUBFMT_STREAM_OPTIMIZED:
+                tmp = "streamOptimized";
+                break;
+            default:
+                set_context_err(context, QB_ERR_INVALID_PARAM,
+                    "invalid VMDK sumfmt type %d was set.", o_vmdk->subfmt);
+                ret = context->err_ret;
+                goto out;
+                break;
+            }
+            bd_ret = set_option_parameter(param,
+                              BLOCK_OPT_SUBFMT, tmp);
+            assert(bd_ret == 0);
+        }
+        break;
+    case QB_FMT_VPC:
+        o_vpc = &(fmt->o_vpc);
+        bd_ret = set_option_parameter_int(param,
+                               BLOCK_OPT_SIZE, o_vpc->virt_size);
+        assert(bd_ret == 0);
+        if (o_vpc->subfmt != QB_FMT_VPC_SUBFMT_DEFAULT) {
+            tmp = o_vpc->subfmt == QB_FMT_VPC_SUBFMT_DYNAMIC ?
+                                               "dynamic" : "fixed";
+            bd_ret = set_option_parameter(param,
+                               BLOCK_OPT_SUBFMT, tmp);
+            assert(bd_ret == 0);
+        }
+        break;
+    default:
+        set_context_err(context, QB_ERR_INVALID_PARAM,
+                        "invalid format type %d was set.", fmt->fmt_type);
+        ret = context->err_ret;
+        goto out;
+        break;
+    }
+
+    backing_file = get_option_parameter(param, BLOCK_OPT_BACKING_FILE);
+    if (backing_file && backing_file->value.s) {
+        if (!strcmp(filename, backing_file->value.s)) {
+            set_context_err(context, QB_ERR_INVALID_PARAM,
+                          "Backing file is the same with new file.");
+            ret = context->err_ret;
+            goto out;
+        }
+    }
+
+    backing_fmt = get_option_parameter(param, BLOCK_OPT_BACKING_FMT);
+    if (backing_fmt && backing_fmt->value.s) {
+        backing_drv = bdrv_find_format(backing_fmt->value.s);
+        assert(backing_drv != NULL);
+    }
+
+    size = get_option_parameter(param, BLOCK_OPT_SIZE);
+    if (size && size->value.n <= 0) {
+        if (backing_file && backing_file->value.s) {
+            uint64_t size;
+            char buf[32];
+            int back_flags;
+
+            /* backing files always opened read-only */
+            back_flags =
+                flag &
+                ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
+
+            bs = bdrv_new("");
+
+            ret = bdrv_open(bs, backing_file->value.s,
+                                back_flags, backing_drv);
+            if (ret < 0) {
+                set_context_err(context, QB_ERR_INVALID_PARAM,
+                               "Failed to open the backing file.");
+                ret = context->err_ret;
+                goto out;
+            }
+            bdrv_get_geometry(bs, &size);
+            size *= BDRV_SECTOR_SIZE;
+
+            snprintf(buf, sizeof(buf), "%" PRId64, size);
+            set_option_parameter(param, BLOCK_OPT_SIZE, buf);
+        } else {
+            set_context_err(context, QB_ERR_INTERNAL_ERR,
+                           "Neither size or backing file was not set.");
+            ret = context->err_ret;
+            goto out;
+        }
+    }
+
+    bd_ret = bdrv_create(drv, filename, param);
+
+
+    if (bd_ret < 0) {
+        const char *errstr;
+        if (bd_ret == -ENOTSUP) {
+            errstr = "formatting option not supported.";
+        } else if (bd_ret == -EFBIG) {
+            errstr = "The image size is too large.";
+        } else {
+            errstr = "Error in creating the image.";
+        }
+        set_context_err(context, QB_ERR_INTERNAL_ERR, errstr);
+        ret = context->err_ret;
+    }
+
+out:
+    free_option_parameters(create_options);
+    free_option_parameters(param);
+    FUNC_FREE(filename);
+    if (bs) {
+        bdrv_delete(bs);
+    }
+
+    return ret;
+}
+
+int qb_open(QBlockContext *context,
+            QBlockState *qbs,
+            QBlockLocationInfo *loc,
+            QBlockFormatInfo *fmt,
+            int flag)
+{
+    int ret = 0, bd_ret;
+    BlockDriverState *bs;
+    BlockDriver *bd;
+    const char *fmtstr;
+    char *filename = NULL;
+
+    /* take care of user settings */
+    /* do nothing now */
+
+    /* check parameters */
+    if (flag & (~LIBQBLOCK_O_VALID_MASK)) {
+        set_context_err(context, QB_ERR_INVALID_PARAM,
+                      "Invalid flag was set.");
+        ret = context->err_ret;
+        goto out;
+    }
+
+    if ((loc == NULL) || (qbs == NULL)) {
+        set_context_err(context, QB_ERR_INVALID_PARAM,
+                      "Got unexpected NULL pointer in parameters.");
+        ret = context->err_ret;
+        goto out;
+    }
+
+    ret = loc_check_params(context, loc);
+    if (ret != 0) {
+        goto out;
+    }
+
+    /* internal translate */
+    ret = loc2filename(context, loc, &filename);
+    if (ret != 0) {
+        goto out;
+    }
+
+    fmtstr = NULL;
+    bd = NULL;
+    if (fmt != NULL) {
+        fmtstr = qb_fmttype2str(fmt->fmt_type);
+    }
+
+    if (fmtstr != NULL) {
+        bd = bdrv_find_format(fmtstr);
+        assert(bd != NULL);
+    }
+
+    /* do real opening */
+    bs = qbs->bdrvs;
+    bd_ret = bdrv_open(bs, filename, flag, bd);
+    if (bd_ret < 0) {
+        set_context_err(context, QB_ERR_INTERNAL_ERR,
+                      "Failed in opening with driver, bd_ret is %d.", bd_ret);
+        ret = context->err_ret;
+        goto out;
+    }
+
+    if (qbs->filename != NULL) {
+        FUNC_FREE(qbs->filename);
+    }
+    qbs->filename = FUNC_STRDUP(filename);
+
+ out:
+    FUNC_FREE(filename);
+    return ret;
+}
+
+void qb_close(QBlockContext *context,
+              QBlockState *qbs)
+{
+    BlockDriverState *bs;
+
+    bs = qbs->bdrvs;
+
+    if (qbs->filename != NULL) {
+        CLEAN_FREE(qbs->filename);
+        bdrv_close(bs);
+    }
+    return;
+}
+
+int32_t qb_read(QBlockContext *context,
+                QBlockState *qbs,
+                uint8_t *buf,
+                uint32_t len,
+                uint64_t offset)
+{
+    int bd_ret;
+    BlockDriverState *bs;
+    uint8_t temp_buf[BDRV_SECTOR_SIZE], *p;
+    uint64_t sector_start;
+    int sector_num, byte_offset, cp_len;
+    uint32_t remains;
+
+    context->err_ret = 0;
+    bs = qbs->bdrvs;
+
+    if (len <= 0) {
+        set_context_err(context, QB_ERR_INVALID_PARAM,
+                      "Param len is less or equal to zero.");
+        return context->err_ret;
+    }
+
+    p = buf;
+    remains = len;
+
+    sector_start = offset >> BDRV_SECTOR_BITS;
+
+    byte_offset = offset & (~BDRV_SECTOR_MASK);
+    if (byte_offset != 0) {
+        /* the start sector is not alligned, need to read/write this sector. */
+        bd_ret = bdrv_read(bs, sector_start, temp_buf, 1);
+        if (bd_ret < 0) {
+            set_context_err(context, QB_ERR_INTERNAL_ERR,
+                           "QEMU internal block error.");
+            context->err_no = bd_ret;
+            return context->err_ret;
+        }
+        cp_len = BDRV_SECTOR_SIZE - byte_offset;
+        memcpy(p, temp_buf + byte_offset, cp_len);
+
+        remains -= cp_len;
+        p += cp_len;
+        sector_start++;
+    }
+
+    /* now start position is alligned. */
+    if (remains >= BDRV_SECTOR_SIZE) {
+        sector_num = remains >> BDRV_SECTOR_BITS;
+        bd_ret = bdrv_read(bs, sector_start, p, sector_num);
+        if (bd_ret < 0) {
+            set_context_err(context, QB_ERR_INTERNAL_ERR,
+                           "QEMU internal block error.");
+            context->err_no = bd_ret;
+            return context->err_ret;
+        }
+        remains -= sector_num << BDRV_SECTOR_BITS;
+        p += sector_num << BDRV_SECTOR_BITS;
+        sector_start += sector_num;
+    }
+
+    if (remains > 0) {
+        /* there is some request remains, less than 1 sector */
+        bd_ret = bdrv_read(bs, sector_start, temp_buf, 1);
+        if (bd_ret < 0) {
+            set_context_err(context, QB_ERR_INTERNAL_ERR,
+                           "QEMU internal block error.");
+            context->err_no = bd_ret;
+            return context->err_ret;
+        }
+        memcpy(p, temp_buf, remains);
+        remains -= remains;
+    }
+
+    return len-remains;
+}
+
+int32_t qb_write(QBlockContext *context,
+                 QBlockState *qbs,
+                 const uint8_t *buf,
+                 uint32_t len,
+                 uint64_t offset)
+{
+    int bd_ret;
+    BlockDriverState *bs;
+    uint8_t temp_buf[BDRV_SECTOR_SIZE];
+    const uint8_t *p;
+    uint64_t sector_start;
+    int sector_num, byte_offset, cp_len;
+    uint32_t remains;
+
+    context->err_ret = 0;
+    bs = qbs->bdrvs;
+
+    if (len <= 0) {
+        set_context_err(context, QB_ERR_INVALID_PARAM,
+                      "Param len is less or equal to zero.");
+        return context->err_ret;
+    }
+
+    p = buf;
+    remains = len;
+
+    sector_start = offset >> BDRV_SECTOR_BITS;
+
+    byte_offset = offset & (~BDRV_SECTOR_MASK);
+    if (byte_offset != 0) {
+        /* the start sector is not alligned, need to read/write this sector. */
+        bd_ret = bdrv_read(bs, sector_start, temp_buf, 1);
+        if (bd_ret < 0) {
+            set_context_err(context, QB_ERR_INTERNAL_ERR,
+                           "QEMU internal block error.");
+            context->err_no = bd_ret;
+            return context->err_ret;
+        }
+        cp_len = BDRV_SECTOR_SIZE - byte_offset;
+        memcpy(temp_buf + byte_offset, p, cp_len);
+        bd_ret = bdrv_write(bs, sector_start, temp_buf, 1);
+        if (bd_ret < 0) {
+            set_context_err(context, QB_ERR_INTERNAL_ERR,
+                           "QEMU internal block error.");
+            context->err_no = bd_ret;
+            return context->err_ret;
+        }
+        remains -= cp_len;
+        p += cp_len;
+        sector_start++;
+    }
+
+    /* now start position is alligned. */
+    if (remains >= BDRV_SECTOR_SIZE) {
+        sector_num = remains >> BDRV_SECTOR_BITS;
+        bd_ret = bdrv_write(bs, sector_start, p, sector_num);
+        if (bd_ret < 0) {
+            set_context_err(context, QB_ERR_INTERNAL_ERR,
+                           "QEMU internal block error.");
+            context->err_no = bd_ret;
+            return context->err_ret;
+        }
+        remains -= sector_num << BDRV_SECTOR_BITS;
+        p += sector_num << BDRV_SECTOR_BITS;
+        sector_start += sector_num;
+    }
+
+    if (remains > 0) {
+        /* there is some request remains, less than 1 sector */
+        bd_ret = bdrv_read(bs, sector_start, temp_buf, 1);
+        if (bd_ret < 0) {
+            set_context_err(context, QB_ERR_INTERNAL_ERR,
+                           "QEMU internal block error.");
+            context->err_no = bd_ret;
+            return context->err_ret;
+        }
+        memcpy(temp_buf, p, remains);
+        bd_ret = bdrv_write(bs, sector_start, temp_buf, 1);
+        if (bd_ret < 0) {
+            set_context_err(context, QB_ERR_INTERNAL_ERR,
+                           "QEMU internal block error.");
+            context->err_no = bd_ret;
+            return context->err_ret;
+        }
+        remains -= remains;
+    }
+
+    return len-remains;
+}
+
+int qb_flush(QBlockContext *context,
+             QBlockState *qbs)
+{
+    int bd_ret;
+    BlockDriverState *bs;
+
+    context->err_ret = 0;
+    bs = qbs->bdrvs;
+    bd_ret = bdrv_flush(bs);
+    if (bd_ret < 0) {
+        set_context_err(context, QB_ERR_INTERNAL_ERR,
+                       "Internal error.");
+    }
+    return context->err_ret;
+}
+
+int qb_check_allocation(QBlockContext *context,
+                        QBlockState *qbs,
+                        uint64_t start,
+                        int64_t length,
+                        int *pstatus,
+                        int64_t *plength)
+{
+    int ret;
+    int sector_start, sector_num, num;
+    BlockDriverState *bs;
+    unsigned int real_len, ret_len;
+
+    context->err_ret = 0;
+    bs = qbs->bdrvs;
+
+    if (length > 0x1000000000000) {
+        set_context_err(context, QB_ERR_INVALID_PARAM,
+                       "length is too big.");
+        goto out;
+    }
+
+    if (qbs->filename == NULL) {
+        set_context_err(context, QB_ERR_INVALID_PARAM,
+                       "Image was not opened first.");
+        goto out;
+    }
+
+    if (length <= 0) {
+        set_context_err(context, QB_ERR_INVALID_PARAM,
+                       "length is not valid.");
+        goto out;
+    }
+
+    /* translate to sector */
+    sector_start = start >> BDRV_SECTOR_BITS;
+    real_len = (start & (~BDRV_SECTOR_MASK)) + length;
+    sector_num = real_len >> BDRV_SECTOR_BITS;
+    if ((real_len & (~BDRV_SECTOR_MASK)) != 0) {
+        sector_num++;
+    }
+
+    ret = bdrv_is_allocated(bs, sector_start, sector_num, &num);
+    if ((ret == 0) && (num == 0)) {
+        set_context_err(context, QB_ERR_BLOCK_OUT_OF_RANGE,
+                       "Start position was bigger than the image's size.");
+        goto out;
+    }
+
+    *pstatus = ret;
+    ret_len = (num << BDRV_SECTOR_BITS) - (start & (~BDRV_SECTOR_MASK));
+    if (ret_len > length) {
+        ret_len = length;
+    }
+    *plength = ret_len;
+
+ out:
+    return context->err_ret;
+}
+
+static void qb_setup_info_addr(const QBlockStaticInfo *info,
+                               QBlockStaticInfoAddr *info_addr)
+{
+    const uint64_t *virt_size = NULL;
+    const QBlockLocationInfo *backing_loc = NULL;
+    const bool *encrypt = NULL;
+    const QBlockFormatInfo *fmt = &(info->fmt);
+
+    switch (fmt->fmt_type) {
+    case QB_FMT_COW:
+        virt_size = &(fmt->o_cow.virt_size);
+        backing_loc = &(fmt->o_cow.backing_loc);
+        break;
+    case QB_FMT_QED:
+        virt_size = &(fmt->o_qed.virt_size);
+        backing_loc = &(fmt->o_qed.backing_loc);
+        break;
+    case QB_FMT_QCOW:
+        virt_size = &(fmt->o_qcow.virt_size);
+        backing_loc = &(fmt->o_qcow.backing_loc);
+        encrypt = &(fmt->o_qcow.encrypt);
+        break;
+    case QB_FMT_QCOW2:
+        virt_size = &(fmt->o_qcow2.virt_size);
+        backing_loc = &(fmt->o_qcow2.backing_loc);
+        encrypt = &(fmt->o_qcow2.encrypt);
+        break;
+    case QB_FMT_RAW:
+        virt_size = &(fmt->o_raw.virt_size);
+        break;
+    case QB_FMT_RBD:
+        virt_size = &(fmt->o_rbd.virt_size);
+        break;
+    case QB_FMT_SHEEPDOG:
+        virt_size = &(fmt->o_sd.virt_size);
+        backing_loc = &(fmt->o_sd.backing_loc);
+        break;
+    case QB_FMT_VDI:
+        virt_size = &(fmt->o_vdi.virt_size);
+        break;
+    case QB_FMT_VMDK:
+        virt_size = &(fmt->o_vmdk.virt_size);
+        backing_loc = &(fmt->o_vmdk.backing_loc);
+        break;
+    case QB_FMT_VPC:
+        virt_size = &(fmt->o_vpc.virt_size);
+        break;
+    default:
+        break;
+    }
+
+    info_addr->virt_size = (uint64_t *)virt_size;
+    info_addr->backing_loc = (QBlockLocationInfo *)backing_loc;
+    info_addr->encrypt = (bool *)encrypt;
+    return;
+}
+
+const uint64_t *qb_get_virt_size(const QBlockStaticInfo *info)
+{
+    QBlockStaticInfoAddr addr;
+    memset(&addr, 0, sizeof(QBlockStaticInfoAddr));
+    qb_setup_info_addr(info, &addr);
+    return addr.virt_size;
+}
+
+const QBlockLocationInfo *qb_get_backing_loc(const QBlockStaticInfo *info)
+{
+    QBlockStaticInfoAddr addr;
+    memset(&addr, 0, sizeof(QBlockStaticInfoAddr));
+    qb_setup_info_addr(info, &addr);
+    return addr.backing_loc;
+}
+
+const bool *qb_get_encrypt(const QBlockStaticInfo *info)
+{
+    QBlockStaticInfoAddr addr;
+    memset(&addr, 0, sizeof(QBlockStaticInfoAddr));
+    qb_setup_info_addr(info, &addr);
+    return addr.encrypt;
+}
+
+int qb_info_image_static_get(QBlockContext *context,
+                             QBlockState *qbs,
+                             QBlockStaticInfo **info)
+{
+    int ret = 0;
+    BlockDriverState *bs;
+    QBlockStaticInfo *info_tmp;
+    QBlockStaticInfoAddr *member_addr, addr;
+    const char *fmt_str;
+    uint64_t total_sectors;
+    char backing_filename[LIBQB_FILENAME_MAX];
+
+    if (qbs->filename == NULL) {
+        set_context_err(context, QB_ERR_INVALID_PARAM,
+                       "Block Image was not openned.");
+        ret = context->err_ret;
+        goto out;
+    }
+
+    info_tmp = FUNC_CALLOC(1, sizeof(QBlockStaticInfo));
+
+    bs = qbs->bdrvs;
+
+    ret = filename2loc(context,
+                       &(info_tmp->loc),
+                       qbs->filename);
+    if (ret < 0) {
+        goto free;
+    }
+
+    fmt_str = bdrv_get_format_name(bs);
+    info_tmp->fmt.fmt_type = qb_str2fmttype(fmt_str);
+    /* we got the format type and basic location info now, setup the struct
+    pointer to the internal members */
+    memset(&addr, 0, sizeof(QBlockStaticInfoAddr));
+    member_addr = &addr;
+    qb_setup_info_addr(info_tmp, member_addr);
+
+    assert(member_addr->virt_size != NULL);
+    bdrv_get_geometry(bs, &total_sectors);
+    *(member_addr->virt_size) = total_sectors * BDRV_SECTOR_SIZE;
+
+    if (member_addr->encrypt != NULL) {
+        *(member_addr->encrypt) = bdrv_is_encrypted(bs);
+    }
+
+    bdrv_get_full_backing_filename(bs, backing_filename,
+                                   sizeof(backing_filename));
+    if (backing_filename[0] != '\0') {
+        assert(member_addr->backing_loc != NULL);
+        ret = filename2loc(context,
+                           member_addr->backing_loc,
+                           backing_filename);
+        if (ret < 0) {
+            goto free;
+        }
+    }
+
+    info_tmp->sector_size = BDRV_SECTOR_SIZE;
+    *info = info_tmp;
+
+ out:
+    return ret;
+ free:
+    qb_info_image_static_delete(context, &info_tmp);
+    return ret;
+}
+
+/* free locations if it has string allocated on heap. */
+static void loc_free(QBlockLocationInfo *loc)
+{
+    switch (loc->prot_type) {
+    case QB_PROTO_FILE:
+        FUNC_FREE((void *)(loc->o_file.filename));
+        loc->o_file.filename = NULL;
+        break;
+    default:
+        break;
+    }
+}
+
+/* free fmt related resoure. */
+static void fmt_free(QBlockFormatInfo *fmt)
+{
+    switch (fmt->fmt_type) {
+    case QB_FMT_COW:
+        loc_free(&(fmt->o_cow.backing_loc));
+        break;
+    case QB_FMT_QED:
+        loc_free(&(fmt->o_qed.backing_loc));
+        break;
+    case QB_FMT_QCOW:
+        loc_free(&(fmt->o_qcow.backing_loc));
+        break;
+    case QB_FMT_QCOW2:
+        loc_free(&(fmt->o_qcow2.backing_loc));
+        break;
+    case QB_FMT_RAW:
+        break;
+    case QB_FMT_RBD:
+        break;
+    case QB_FMT_SHEEPDOG:
+        loc_free(&(fmt->o_sd.backing_loc));
+        break;
+    case QB_FMT_VDI:
+        break;
+    case QB_FMT_VMDK:
+        loc_free(&(fmt->o_vmdk.backing_loc));
+        break;
+    case QB_FMT_VPC:
+        break;
+    default:
+        break;
+    }
+    return;
+}
+
+
+void qb_info_image_static_delete(QBlockContext *context,
+                                 QBlockStaticInfo **info)
+{
+    loc_free(&(*info)->loc);
+    fmt_free(&(*info)->fmt);
+    CLEAN_FREE(*info);
+}
+
+QBlockLocationInfo *qb_loc_info_dup(const QBlockLocationInfo *prot)
+{
+    QBlockLocationInfo *p = FUNC_CALLOC(1, sizeof(QBlockLocationInfo));
+    p->prot_type = prot->prot_type;
+    switch (p->prot_type) {
+    case QB_PROTO_FILE:
+        p->o_file.filename =
+                          FUNC_STRDUP(prot->o_file.filename);
+        break;
+    default:
+        break;
+    }
+    return p;
+}
diff --git a/libqblock/libqblock.h b/libqblock/libqblock.h
index e69de29..8ca7d28 100644
--- a/libqblock/libqblock.h
+++ b/libqblock/libqblock.h
@@ -0,0 +1,341 @@
+/*
+ * QEMU block layer library
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Wenchao Xia   <xiawenc@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef LIBQBLOCK_H
+#define LIBQBLOCK_H
+
+#include "libqblock-types.h"
+#include "libqblock-error.h"
+
+/**
+ * qb_context_new: allocate a new context.
+ *
+ * Broker is used to pass operation to libqblock, and get feedback from it.
+ *
+ * Returns 0 on success, libqblock negative error value on fail.
+ *
+ * @context: used to receive the created struct.
+ */
+DLL_PUBLIC
+int qb_context_new(QBlockContext **context);
+
+/**
+ * qb_context_delete: delete context.
+ *
+ * Broker will be freed and set to NULL.
+ *
+ * @context: operation context to be deleted.
+ */
+DLL_PUBLIC
+void qb_context_delete(QBlockContext **context);
+
+/**
+ * qb_state_new: allocate a new QBlockState struct
+ *
+ * Subsequent qblock actions will use this struct
+ *
+ * Returns 0 if succeed, libqblock negative error value on fail.
+ *
+ * @context: operation context.
+ * @qbs: used to receive the created struct.
+ */
+DLL_PUBLIC
+int qb_state_new(QBlockContext *context,
+                 QBlockState **qbs);
+
+/**
+ * qb_state_delete: free a QBlockState struct
+ *
+ * if image was opened, qb_close should be called before delete, otherwise
+ *  it would be automatically closed.
+ *
+ * @context: operation context.
+ * @qbs: pointer to the struct's pointer.
+ */
+DLL_PUBLIC
+void qb_state_delete(QBlockContext *context,
+                     QBlockState **qbs);
+
+/**
+ * qb_loc_info_new: create a new QBlockLocationInfo object.
+ *
+ * return 0 on success, libqblock negative error value on fail.
+ *
+ * @context: operation context.
+ * @loc: pointer to receive the new created one.
+ */
+DLL_PUBLIC
+int qb_loc_info_new(QBlockContext *context,
+                    QBlockLocationInfo **loc);
+
+/**
+ * qb_loc_info_delete: free a QBlockLocationInfo.
+ *
+ * @context: operation context.
+ * @loc: pointer to the object, *loc would be set to NULL.
+ */
+DLL_PUBLIC
+void qb_loc_info_delete(QBlockContext *context,
+                        QBlockLocationInfo **loc);
+
+/**
+ * qb_fmt_info_new: create a new QBlockFormatInfo structure.
+ *
+ * return 0 on success, libqblock negative error value on fail.
+ *
+ * @context: operation context.
+ * @op: pointer that will receive created struct.
+ */
+DLL_PUBLIC
+int qb_fmt_info_new(QBlockContext *context,
+                    QBlockFormatInfo **fmt);
+
+/**
+ * qb_fmt_info_delete: free QBlockFormatInfo structure.
+ *
+ * @context: operation context.
+ * @fmt: pointer to the struct, *fmt would be set to NULL.
+ */
+DLL_PUBLIC
+void qb_fmt_info_delete(QBlockContext *context,
+                        QBlockFormatInfo **fmt);
+
+
+/**
+ * qb_open: open a block object.
+ *
+ * return 0 on success, libqblock negative error value on fail.
+ *
+ * @context: operation context.
+ * @qbs: pointer to QBlockState.
+ * @loc: location options for open, how to find the image.
+ * @fmt: format options, how to extract the data, only valid member now is
+ *    fmt->fmt_type, set to NULL if you want to auto discovery the format.
+ * @flag: behavior control flags, it is LIBQBLOCK_O_XXX's combination.
+ *
+ * Note: For raw image, there is a risk that it's content is changed to some
+ *  magic value resulting a wrong probing done by libqblock, so don't do
+ * probing on raw images.
+ */
+DLL_PUBLIC
+int qb_open(QBlockContext *context,
+            QBlockState *qbs,
+            QBlockLocationInfo *loc,
+            QBlockFormatInfo *fmt,
+            int flag);
+
+/**
+ * qb_close: close a block object.
+ *
+ * qb_flush is automatically done inside.
+ *
+ * @context: operation context.
+ * @qbs: pointer to QBlockState.
+ */
+DLL_PUBLIC
+void qb_close(QBlockContext *context,
+              QBlockState *qbs);
+
+/**
+ * qb_create: create a block image or object.
+ *
+ * Note: Create operation would not open the image automatically.
+ *
+ * return 0 on success, libqblock negative error value on fail.
+ *
+ * @context: operation context.
+ * @qbs: pointer to QBlockState.
+ * @loc: location options for open, how to find the image.
+ * @fmt: format options, how to extract the data.
+ * @flag: behavior control flags, LIBQBLOCK_O_XXX's combination.
+ */
+DLL_PUBLIC
+int qb_create(QBlockContext *context,
+              QBlockState *qbs,
+              QBlockLocationInfo *loc,
+              QBlockFormatInfo *fmt,
+              int flag);
+
+
+/* sync access */
+/**
+ * qb_read: block sync read.
+ *
+ * return number of bytes read, libqblock negative error value on fail.
+ *
+ * @context: operation context.
+ * @qbs: pointer to QBlockState.
+ * @buf: buffer that receive the content.
+ * @len: length to read.
+ * @offset: offset in the block data.
+ */
+DLL_PUBLIC
+int32_t qb_read(QBlockContext *context,
+                QBlockState *qbs,
+                uint8_t *buf,
+                uint32_t len,
+                uint64_t offset);
+
+/**
+ * qb_write: block sync write.
+ *
+ * return number of bytes written, libqblock negative error value on fail.
+ *
+ * @context: operation context.
+ * @qbs: pointer to QBlockState.
+ * @buf: buffer that receive the content.
+ * @len: length to write.
+ * @offset: offset in the block data.
+ */
+DLL_PUBLIC
+int32_t qb_write(QBlockContext *context,
+                 QBlockState *qbs,
+                 const uint8_t *buf,
+                 uint32_t len,
+                 uint64_t offset);
+
+/**
+ * qb_flush: block sync flush.
+ *
+ * return 0 on success, libqblock negative error value on fail.
+ *
+ * @context: operation context.
+ * @qbs: pointer to QBlockState.
+ */
+DLL_PUBLIC
+int qb_flush(QBlockContext *context,
+             QBlockState *qbs);
+
+
+/* advance image APIs */
+/**
+ * qb_check_allocation: check if [start, start+lenth-1] was allocated on the
+ *  image.
+ *
+ * return 0 on success, libqblock negative error value on fail.
+ *
+ * @context: operation context.
+ * @qbs: pointer to QBlockState.
+ * @start: start position, unit is byte.
+ * @length: length to check, unit is byte, max is 1TB, otherwise will return
+ *   QB_ERR_INVALID_PARAM.
+ * @pstatus: pointer to receive the status, 1 means allocated,
+ *  0 means unallocated.
+ * @plength: pointer to receive the length that all have the same status as
+ *  *pstatus.
+ *
+ * Note: after return, start+*plength may have the same status as
+ *  start+*plength-1.
+ */
+DLL_PUBLIC
+int qb_check_allocation(QBlockContext *context,
+                        QBlockState *qbs,
+                        uint64_t start,
+                        int64_t length,
+                        int *pstatus,
+                        int64_t *plength);
+
+/* image information */
+/**
+ * qb_get_image_info: get image info.
+ *
+ * return 0 on success, libqblock negative error value on fail.
+ *
+ * @context: operation context.
+ * @qbs: pointer to QBlockState.
+ * @info: pointer that would receive the information.
+ *
+ * *info must be not modified after return, qb_info_image_static_delete will
+ *   use the information in it.
+ */
+DLL_PUBLIC
+int qb_info_image_static_get(QBlockContext *context,
+                             QBlockState *qbs,
+                             QBlockStaticInfo **info);
+
+/**
+ * qb_delete_image_info: free image info.
+ *
+ * @context: operation context.
+ * @info: pointer to the information struct.
+ */
+DLL_PUBLIC
+void qb_info_image_static_delete(QBlockContext *context,
+                                 QBlockStaticInfo **info);
+
+/* helper functions */
+/**
+ * qb_str2fmttype: translate format string to libqblock format enum type.
+ *
+ * return the type, or QB_FMT_NONE if string matches none of supported types.
+ *
+ * @fmt: the format string.
+ */
+DLL_PUBLIC
+QBlockFormat qb_str2fmttype(const char *fmt_str);
+
+/**
+ * qb_fmttype2str: translate libqblock format enum type to a string.
+ *
+ * return a pointer to the string, or NULL if type is not supported, and
+ *  returned pointer must NOT be freed.
+ *
+ * @fmt: the format enum type.
+ */
+DLL_PUBLIC
+const char *qb_fmttype2str(QBlockFormat fmt_type);
+
+/**
+ * qb_loc_info_dup: duplicate a QBlockLocationInfo instance.
+ *
+ * return a pointer to new allocated one having the same values with input,
+ *  it need to be freed by qb_loc_info_delete later. Never fail except OOM.
+ *
+ * @loc: pointer to the source instance.
+ */
+DLL_PUBLIC
+QBlockLocationInfo *qb_loc_info_dup(const QBlockLocationInfo *loc);
+
+/**
+ * qb_get_virt_size: get virtual size.
+ *
+ * return a pointer, which pointer to a member in info, or NULL if info is
+ *  not valid.
+ *
+ * @info: pointer to the QBlockStaticInfo structure.
+ */
+DLL_PUBLIC
+const uint64_t *qb_get_virt_size(const QBlockStaticInfo *info);
+
+/**
+ * qb_get_backing_loc: get backing file location.
+ *
+ * return a pointer, which pointer to a member in info, or NULL if info is
+ *  not valid, or image have no such property.
+ *
+ * @info: pointer to the QBlockStaticInfo structure.
+ */
+DLL_PUBLIC
+const QBlockLocationInfo *qb_get_backing_loc(const QBlockStaticInfo *info);
+
+/**
+ * qb_get_encrypt: get encrytion flag.
+ *
+ * return a pointer, which pointer to a member in info, or NULL if info is
+ *  not valid, or image have no such property.
+ *
+ * @info: pointer to the QBlockStaticInfo structure.
+ */
+DLL_PUBLIC
+const bool *qb_get_encrypt(const QBlockStaticInfo *info);
+#endif
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH V10 7/7] libqblock test example
  2012-11-20  9:46 [Qemu-devel] [PATCH V10 0/7] libqblock qemu block layer library Wenchao Xia
                   ` (5 preceding siblings ...)
  2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 6/7] libqblock API Wenchao Xia
@ 2012-11-20  9:46 ` Wenchao Xia
  6 siblings, 0 replies; 22+ messages in thread
From: Wenchao Xia @ 2012-11-20  9:46 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, peter.maydell, aliguori, stefanha, blauwirbel, kraxel,
	pbonzini, xiawenc

  In this example, first it will create some qcow2 images, then try get
information including backing file relationship, then it will do sync IO on
the image.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 tests/check-libqblock-qcow2.c |  389 ++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 388 insertions(+), 1 deletions(-)

diff --git a/tests/check-libqblock-qcow2.c b/tests/check-libqblock-qcow2.c
index 6f5ccac..15e81b8 100644
--- a/tests/check-libqblock-qcow2.c
+++ b/tests/check-libqblock-qcow2.c
@@ -1,5 +1,392 @@
+/*
+ * QEMU block layer library test
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Wenchao Xia   <xiawenc@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ * Limitation:
+ *    1 filename do not support relative path, to save trouble in creating
+ * backing files.
+ */
+
+#include <glib.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <inttypes.h>
+#include <string.h>
+#include <stdlib.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+
+
 #include "libqblock.h"
+#include "libqtest.h"
+
+#define LIBQB_TEST_ENV_DIR "LIBQBLOCK_TEST_DIR"
+#define LIBQB_TEST_DEFAULT_DIR "/tmp"
+#define LIBQB_TEST_DEFAULT_FILENAME "libqblock_qcow2_test_img"
+
+typedef struct LibqbTestSettings {
+    const char *image_filename;
+    uint64_t image_size;
+    unsigned int num_backings;
+    unsigned int io_buf_size;
+    uint64_t io_offset;
+    int print_flag;
+} LibqbTestSettings;
+
+LibqbTestSettings libqb_test_settings;
+
+static void print_loc(const QBlockLocationInfo *loc)
+{
+    if (loc == NULL) {
+        printf("loc is NULL.");
+        return;
+    }
+    switch (loc->prot_type) {
+    case QB_PROTO_NONE:
+        printf("protocol type [none].");
+        break;
+    case QB_PROTO_FILE:
+        printf("protocol type [file], filename [%s].",
+               loc->o_file.filename);
+        break;
+    default:
+        printf("protocol type not supported.");
+        break;
+    }
+}
+
+static void print_info_image_static(QBlockStaticInfo *info)
+{
+    const uint64_t *virt_size = qb_get_virt_size(info);
+    const QBlockLocationInfo *backing_loc = qb_get_backing_loc(info);
+    g_assert(virt_size != NULL);
+
+    printf("=======image location:\n");
+    print_loc(&info->loc);
+    printf("\nvirtual_size %" PRId64 ", format type %d [%s]",
+           *(virt_size),
+           info->fmt.fmt_type, qb_fmttype2str(info->fmt.fmt_type));
+    printf("\nbacking image location:\n");
+    print_loc(backing_loc);
+    printf("\n");
+}
+
+static char *generate_backing_filename(const char *filename, int index)
+{
+    char *backing_filename = NULL;
+
+    backing_filename = g_strdup_printf("%s_backing_%d", filename, index);
+    return backing_filename;
+}
+
+/* get filename in a full path */
+static const char *get_filename(const char *path)
+{
+    const char *filename;
+    filename = strrchr(path, '/');
+    if (filename == NULL) {
+        filename = path;
+    } else {
+        filename++;
+    }
+    return filename;
+}
+
+/* create a chain of files, num_backings must >= 0. */
+static void files_create_qcow2(const char *filename,
+                               int num_backings,
+                               uint64_t virt_size)
+{
+    QBlockContext *context = NULL;
+    QBlockState *qbs = NULL;
+    QBlockLocationInfo *loc_info = NULL;
+    QBlockFormatInfo *fmt_info = NULL;
+    int ret;
+    int index;
+    int flag;
+    char *backing_filename = NULL, *new_filename = NULL;
+    const char *relative_filename = NULL;
+
+    ret = qb_context_new(&context);
+    g_assert(ret == 0);
+
+    ret = qb_state_new(context, &qbs);
+    g_assert(ret == 0);
+
+    ret = qb_loc_info_new(context, &loc_info);
+    g_assert(ret == 0);
+
+    ret = qb_fmt_info_new(context, &fmt_info);
+    g_assert(ret == 0);
+
+    loc_info->prot_type = QB_PROTO_FILE;
+    fmt_info->fmt_type = QB_FMT_QCOW2;
+    fmt_info->o_qcow2.virt_size = virt_size;
+    flag = 0;
+
+    index = 0;
+    while (index < num_backings) {
+        new_filename = generate_backing_filename(filename, index);
+        loc_info->o_file.filename = new_filename;
+        if (backing_filename != NULL) {
+            fmt_info->o_qcow2.backing_loc.prot_type = QB_PROTO_FILE;
+            relative_filename = get_filename(backing_filename);
+            fmt_info->o_qcow2.backing_loc.o_file.filename =
+                                                             relative_filename;
+        }
+        ret = qb_create(context, qbs, loc_info, fmt_info, flag);
+        g_assert(ret == 0);
+        free(backing_filename);
+        backing_filename = new_filename;
+        new_filename = NULL;
+        index++;
+    }
+
+    loc_info->o_file.filename = filename;
+    if (backing_filename != NULL) {
+        fmt_info->o_qcow2.backing_loc.prot_type = QB_PROTO_FILE;
+        relative_filename = get_filename(backing_filename);
+        fmt_info->o_qcow2.backing_loc.o_file.filename =
+                                                         relative_filename;
+    }
+    ret = qb_create(context, qbs, loc_info, fmt_info, flag);
+    g_assert(ret == 0);
+    free(backing_filename);
+
+    qb_fmt_info_delete(context, &fmt_info);
+    qb_loc_info_delete(context, &loc_info);
+    qb_state_delete(context, &qbs);
+    qb_context_delete(&context);
+    return;
+}
+
+static void files_delete(const char *filename,
+                         int num_backings,
+                         uint64_t virt_size)
+{
+    char *new_filename = NULL;
+    int index = 0;
+    while (index < num_backings) {
+        new_filename = generate_backing_filename(filename, index);
+        unlink(new_filename);
+        free(new_filename);
+        index++;
+    }
+    unlink(filename);
+}
+
+static void testcase_info_image_static(void)
+{
+    const char *filename = libqb_test_settings.image_filename;
+    uint64_t image_size = libqb_test_settings.image_size;
+    int num_backings = libqb_test_settings.num_backings;
+    QBlockContext *context = NULL;
+    QBlockState *qbs = NULL;
+    QBlockLocationInfo *loc_info = NULL;
+    int ret, flag;
+    QBlockStaticInfo *info_st = NULL;
+    int i;
+    char *backing_filename = NULL;
+    const uint64_t *virt_size = NULL;
+    const QBlockLocationInfo *backing_loc = NULL;
+
+    ret = qb_context_new(&context);
+    g_assert(ret == 0);
+
+    ret = qb_state_new(context, &qbs);
+    g_assert(ret == 0);
+
+    ret = qb_loc_info_new(context, &loc_info);
+    g_assert(ret == 0);
+
+    loc_info->prot_type = QB_PROTO_FILE;
+    loc_info->o_file.filename = filename;
+    flag = LIBQBLOCK_O_NO_BACKING;
+    ret = qb_open(context, qbs, loc_info, NULL, flag);
+    g_assert(ret == 0);
+
+    i = num_backings - 1;
+    while (1) {
+        ret = qb_info_image_static_get(context, qbs, &info_st);
+        g_assert(ret == 0);
+        if (libqb_test_settings.print_flag > 0) {
+            print_info_image_static(info_st);
+        }
+        qb_close(context, qbs);
+
+        /* checking the information */
+        g_assert(info_st->loc.prot_type == loc_info->prot_type);
+        ret = strcmp(info_st->loc.o_file.filename,
+                     loc_info->o_file.filename);
+
+        g_assert(ret == 0);
+        g_assert(info_st->fmt.fmt_type == QB_FMT_QCOW2);
+        virt_size = qb_get_virt_size(info_st);
+        g_assert(virt_size != NULL);
+        g_assert(*virt_size == image_size);
+        backing_loc = qb_get_backing_loc(info_st);
+        /* qcow2 have always backing file property */
+        g_assert(backing_loc != NULL);
+        if (i >= 0) {
+            /* it should have backing file */
+            g_assert(backing_loc->prot_type == QB_PROTO_FILE);
+            backing_filename = generate_backing_filename(filename, i);
+            ret = strcmp(backing_filename, backing_loc->o_file.filename);
+            g_assert(ret == 0);
+            g_free(backing_filename);
+        } else {
+            g_assert(backing_loc->prot_type == QB_FMT_NONE);
+        }
+        i--;
+        /* see if there is backing file need to check. */
+        if (backing_loc->prot_type == QB_FMT_NONE) {
+            g_assert(i < 0);
+            break;
+        }
+        qb_loc_info_delete(context, &loc_info);
+        loc_info = qb_loc_info_dup(backing_loc);
+        ret = qb_open(context, qbs, loc_info, NULL, flag);
+        g_assert(ret == 0);
+        qb_info_image_static_delete(context, &info_st);
+    }
+    qb_info_image_static_delete(context, &info_st);
+
+    qb_loc_info_delete(context, &loc_info);
+    qb_state_delete(context, &qbs);
+    qb_context_delete(&context);
+    return;
+}
+
+/* assuming the image is new created */
+static void testcase_sync_io(void)
+{
+    const char *filename = libqb_test_settings.image_filename;
+    uint64_t io_buf_size = libqb_test_settings.io_buf_size;
+    uint64_t io_offset = libqb_test_settings.io_offset;;
+    unsigned char *buf_r, *buf_w;
+    uint64_t i;
+    unsigned int rand_value;
+
+    buf_r = g_malloc0(io_buf_size);
+    buf_w = g_malloc0(io_buf_size);
+    for (i = 0; i < io_buf_size; i++) {
+        rand_value = g_test_rand_int_range(0, 255);
+        buf_w[i] = (unsigned char)rand_value;
+    }
+
+    QBlockContext *context = NULL;
+    QBlockState *qbs = NULL;
+    QBlockLocationInfo *loc_info = NULL;
+    int ret, flag;
+
+    ret = qb_context_new(&context);
+    g_assert(ret == 0);
+
+    ret = qb_state_new(context, &qbs);
+    g_assert(ret == 0);
+
+    ret = qb_loc_info_new(context, &loc_info);
+    g_assert(ret == 0);
+
+
+    loc_info->prot_type = QB_PROTO_FILE;
+    loc_info->o_file.filename = filename;
+    flag = LIBQBLOCK_O_RDWR;
+    ret = qb_open(context, qbs, loc_info, NULL, flag);
+    g_assert(ret == 0);
+
+    int status;
+    int64_t len;
+    ret = qb_check_allocation(context, qbs, io_offset, io_buf_size,
+                                       &status, &len);
+    g_assert(ret == 0);
+    g_assert(status == 0);
+    g_assert(len == io_buf_size);
+
+    ret = qb_write(context, qbs, buf_w, io_buf_size, io_offset);
+    g_assert(ret == io_buf_size);
+
+    ret = qb_read(context, qbs, buf_r, io_buf_size, io_offset);
+    g_assert(ret == io_buf_size);
+
+    int cmp = memcmp(buf_r, buf_w, io_buf_size);
+    g_assert(cmp == 0);
+
+    ret = qb_check_allocation(context, qbs, io_offset, io_buf_size,
+                                       &status, &len);
+    g_assert(ret == 0);
+    g_assert(status == 1);
+    g_assert(len == io_buf_size);
+
+    qb_close(context, qbs);
+
+    qb_loc_info_delete(context, &loc_info);
+    qb_state_delete(context, &qbs);
+    qb_context_delete(&context);
+    g_free(buf_r);
+    g_free(buf_w);
+    return;
+}
+
 int main(int argc, char **argv)
 {
-    return 0;
+    const char *root_dir;
+    const char *name = LIBQB_TEST_DEFAULT_FILENAME;
+    int ret;
+    int dir_created = 0;
+    char *filename;
+
+    root_dir = getenv(LIBQB_TEST_ENV_DIR);
+    if (root_dir == NULL) {
+        root_dir = LIBQB_TEST_DEFAULT_DIR;
+    }
+
+    ret = access(root_dir, F_OK);
+    if (ret != 0) {
+        /*try mkdir */
+        ret = mkdir(root_dir, 0755);
+        g_assert(ret == 0);
+        dir_created = 1;
+    }
+
+    filename = g_strdup_printf("%s/%s", root_dir, name);
+
+    g_test_init(&argc, &argv, NULL);
+    g_test_rand_int();
+
+    libqb_test_settings.image_filename = filename;
+    libqb_test_settings.image_size =
+                            1024 * 1024 * g_test_rand_int_range(1, 1024);
+    libqb_test_settings.num_backings = g_test_rand_int_range(0, 10);
+    libqb_test_settings.io_buf_size = g_test_rand_int_range(1, 1024 * 1024);
+    libqb_test_settings.io_offset = g_test_rand_int_range(0,
+            libqb_test_settings.image_size - libqb_test_settings.io_buf_size);
+
+    files_create_qcow2(libqb_test_settings.image_filename,
+                       libqb_test_settings.num_backings,
+                       libqb_test_settings.image_size);
+
+    g_test_add_func("/libqblock/sync_info_image_static",
+                      testcase_info_image_static);
+    g_test_add_func("/libqblock/sync_io", testcase_sync_io);
+
+    ret = g_test_run();
+    files_delete(libqb_test_settings.image_filename,
+                 libqb_test_settings.num_backings,
+                 libqb_test_settings.image_size);
+
+    g_free(filename);
+    if (dir_created) {
+        ret = rmdir(root_dir);
+        g_assert(ret == 0);
+    }
+    return ret;
 }
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH V10 4/7] libqblock build system
  2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 4/7] libqblock build system Wenchao Xia
@ 2012-11-20 10:26   ` Paolo Bonzini
  2012-11-21  3:03     ` Wenchao Xia
  0 siblings, 1 reply; 22+ messages in thread
From: Paolo Bonzini @ 2012-11-20 10:26 UTC (permalink / raw)
  To: Wenchao Xia
  Cc: kwolf, peter.maydell, aliguori, stefanha, qemu-devel, blauwirbel,
	kraxel

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH V10 5/7] libqblock type defines
  2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 5/7] libqblock type defines Wenchao Xia
@ 2012-11-20 10:30   ` Paolo Bonzini
  2012-11-21  3:12     ` Wenchao Xia
  0 siblings, 1 reply; 22+ messages in thread
From: Paolo Bonzini @ 2012-11-20 10:30 UTC (permalink / raw)
  To: Wenchao Xia
  Cc: kwolf, peter.maydell, aliguori, stefanha, qemu-devel, blauwirbel,
	kraxel

Il 20/11/2012 10:46, Wenchao Xia ha scritto:
>   This patch contains type and macro defines used in APIs, one file for public
> usage, one for libqblock internal usage.

Mostly looks good, just a few questions below.

> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  libqblock/libqblock-internal.h |   75 ++++++++++++
>  libqblock/libqblock-types.h    |  252 ++++++++++++++++++++++++++++++++++++++++
>  libqblock/libqblock.c          |    6 -
>  libqblock/libqblock.h          |    1 -
>  tests/check-libqblock-qcow2.c  |    1 -
>  5 files changed, 327 insertions(+), 8 deletions(-)
>  create mode 100644 libqblock/libqblock-internal.h
> 
> diff --git a/libqblock/libqblock-internal.h b/libqblock/libqblock-internal.h
> new file mode 100644
> index 0000000..75c0452
> --- /dev/null
> +++ b/libqblock/libqblock-internal.h
> @@ -0,0 +1,75 @@
> +/*
> + * QEMU block layer library
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + *  Wenchao Xia   <xiawenc@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#ifndef LIBQBLOCK_INTERNAL
> +#define LIBQBLOCK_INTERNAL
> +
> +#include "glib.h"

Use <glib.h> please.

> +
> +#include "block.h"
> +#include "block_int.h"

Is block_int.h really needed?

> +#include "libqblock-types.h"
> +
> +/* this file contains defines and types used inside the library. */
> +
> +#define FUNC_FREE(p) g_free((p))
> +#define FUNC_MALLOC(size) g_malloc((size))
> +#define FUNC_CALLOC(nmemb, size) g_malloc0((nmemb)*(size))
> +#define FUNC_STRDUP(p) g_strdup((p))

Why keep these?
> +
> +#define CLEAN_FREE(p) { \
> +        FUNC_FREE(p); \
> +        (p) = NULL; \
> +}

What about calling it QB_FREE?

> +
> +/* details should be hidden to user */
> +struct QBlockState {
> +    BlockDriverState *bdrvs;
> +    /* internal used file name now, if it is not NULL, it means
> +       image was opened.
> +    */
> +    char *filename;
> +};
> +
> +struct QBlockContext {
> +    /* last error */
> +    GError *g_error;
> +    int err_ret; /* 1st level of error, the libqblock error number */
> +    int err_no; /* 2nd level of error, errno what below reports */
> +};
> +
> +/**
> + * QBlockStaticInfoAddr: a structure contains a set of pointer.
> + *
> + *    this struct contains a set of pointer pointing to some
> + *  property related to format or protocol. If a property is not available,
> + *  it will be set as NULL. User could use this to get properties directly.
> + *
> + *  @virt_size: virtual size, it is always not NULL.
> + *  @backing_loc: backing file location.
> + *  @encrypt: encryption flag.
> +*/
> +
> +typedef struct QBlockStaticInfoAddr {
> +    uint64_t *virt_size;
> +    QBlockLocationInfo *backing_loc;
> +    bool *encrypt;
> +} QBlockStaticInfoAddr;
> +
> +#define G_LIBQBLOCK_ERROR g_libqbock_error_quark()
> +
> +static inline GQuark g_libqbock_error_quark(void)

libqblock, not libqbock.

> +{
> +    return g_quark_from_static_string("g-libqblock-error-quark");
> +}
> +#endif
> diff --git a/libqblock/libqblock-types.h b/libqblock/libqblock-types.h
> index e69de29..77ad325 100644
> --- a/libqblock/libqblock-types.h
> +++ b/libqblock/libqblock-types.h
> @@ -0,0 +1,252 @@
> +/*
> + * QEMU block layer library
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + *  Wenchao Xia   <xiawenc@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#ifndef LIBQBLOCK_TYPES_H
> +#define LIBQBLOCK_TYPES_H
> +
> +#include <sys/types.h>
> +#include <stdint.h>
> +#include <stdbool.h>
> +
> +#if defined(__GNUC__) && __GNUC__ >= 4
> +    #ifdef LIBQB_BUILD
> +        #define DLL_PUBLIC __attribute__((visibility("default")))
> +    #else
> +        #define DLL_PUBLIC
> +    #endif
> +#endif
> +
> +/* this library is designed around this core struct. */
> +typedef struct QBlockState QBlockState;
> +
> +/* every thread should have a context. */
> +typedef struct QBlockContext QBlockContext;
> +
> +/* flag used in open and create */
> +#define LIBQBLOCK_O_RDWR        0x0002
> +/* do not use the host page cache */
> +#define LIBQBLOCK_O_NOCACHE     0x0020
> +/* use write-back caching */
> +#define LIBQBLOCK_O_CACHE_WB    0x0040
> +/* don't open the backing file */
> +#define LIBQBLOCK_O_NO_BACKING  0x0100
> +/* disable flushing on this disk */
> +#define LIBQBLOCK_O_NO_FLUSH    0x0200
> +
> +#define LIBQBLOCK_O_CACHE_MASK \
> +   (LIBQBLOCK_O_NOCACHE | LIBQBLOCK_O_CACHE_WB | LIBQBLOCK_O_NO_FLUSH)
> +
> +#define LIBQBLOCK_O_VALID_MASK \
> +   (LIBQBLOCK_O_RDWR | LIBQBLOCK_O_NOCACHE | LIBQBLOCK_O_CACHE_WB | \
> +    LIBQBLOCK_O_NO_BACKING | LIBQBLOCK_O_NO_FLUSH)
> +
> +typedef enum QBlockProtocol {
> +    QB_PROTO_NONE = 0,
> +    QB_PROTO_FILE,
> +    QB_PROTO_MAX
> +} QBlockProtocol;
> +
> +typedef struct QBlockProtocolOptionsFile {
> +    const char *filename;
> +} QBlockProtocolOptionsFile;
> +
> +/**
> + * struct QBlockLocationInfo: contains information about how to find the image
> + *
> + * @prot_type: protocol type, now only support FILE.
> + * @o_file: file protocol related attributes.
> + * @reserved: reserved bytes for ABI.
> + */
> +#define QBLOCK_PROT_OPTIONS_UNION_SIZE (512)
> +typedef struct QBlockLocationInfo {
> +    QBlockProtocol prot_type;
> +    union {
> +        QBlockProtocolOptionsFile o_file;
> +        uint64_t reserved[QBLOCK_PROT_OPTIONS_UNION_SIZE/8];
> +    };
> +} QBlockLocationInfo;
> +
> +
> +/* format related options */
> +typedef enum QBlockFormat {
> +    QB_FMT_NONE = 0,
> +    QB_FMT_COW,
> +    QB_FMT_QED,
> +    QB_FMT_QCOW,
> +    QB_FMT_QCOW2,
> +    QB_FMT_RAW,
> +    QB_FMT_RBD,
> +    QB_FMT_SHEEPDOG,
> +    QB_FMT_VDI,
> +    QB_FMT_VMDK,
> +    QB_FMT_VPC,
> +    QB_FMT_MAX
> +} QBlockFormat;
> +
> +typedef struct QBlockFormatOptionsCOW {
> +    uint64_t virt_size;
> +    QBlockLocationInfo backing_loc;
> +} QBlockFormatOptionsCOW;
> +
> +typedef struct QBlockFormatOptionsQED {
> +    uint64_t virt_size;
> +    QBlockLocationInfo backing_loc;
> +    QBlockFormat backing_fmt;
> +    uint64_t cluster_size; /* unit is bytes */
> +    uint64_t table_size; /* unit is clusters */
> +} QBlockFormatOptionsQED;
> +
> +typedef struct QBlockFormatOptionsQCOW {
> +    uint64_t virt_size;
> +    QBlockLocationInfo backing_loc;
> +    bool encrypt;
> +} QBlockFormatOptionsQCOW;
> +
> +/* "Compatibility level (0.10 or 1.1)" */
> +typedef enum QBlockFormatOptionsQCOW2CompatLv {
> +    QB_FMT_QCOW2_COMPAT_DEFAULT = 0,
> +    QB_FMT_QCOW2_COMPAT_V0_10,
> +    QB_FMT_QCOW2_COMPAT_V1_10,
> +} QBlockFormatOptionsQCOW2CompatLv;
> +
> +/* off or metadata */
> +typedef enum QBlockFormatOptionsQCOW2PreAlloc {
> +    QB_FMT_QCOW2_PREALLOC_DEFAULT = 0,
> +    QB_FMT_QCOW2_PREALLOC_OFF,
> +    QB_FMT_QCOW2_PREALLOC_METADATA,
> +} QBlockFormatOptionsQCOW2PreAlloc;
> +
> +typedef struct QBlockFormatOptionsQCOW2 {
> +    uint64_t virt_size;
> +    QBlockLocationInfo backing_loc;
> +    QBlockFormat backing_fmt;
> +    bool encrypt;
> +    uint64_t cluster_size; /* unit is bytes */
> +    QBlockFormatOptionsQCOW2CompatLv cpt_lv;
> +    QBlockFormatOptionsQCOW2PreAlloc pre_mode;
> +} QBlockFormatOptionsQCOW2;
> +
> +typedef struct QBlockFormatOptionsRAW {
> +    uint64_t virt_size;
> +} QBlockFormatOptionsRAW;
> +
> +typedef struct QBlockFormatOptionsRBD {
> +    uint64_t virt_size;
> +    uint64_t cluster_size;
> +} QBlockFormatOptionsRBD;
> +
> +/* off or full */
> +typedef enum QBlockFormatOptionsSDPreAlloc {
> +    QB_FMT_SD_PREALLOC_DEFAULT = 0,
> +    QB_FMT_SD_PREALLOC_OFF,
> +    QB_FMT_SD_PREALLOC_FULL,
> +} QBlockFormatOptionsSDPreAlloc;
> +
> +typedef struct QBlockFormatOptionsSD {
> +    uint64_t virt_size;
> +    QBlockLocationInfo backing_loc;
> +    QBlockFormatOptionsSDPreAlloc pre_mode;
> +} QBlockFormatOptionsSD;
> +
> +typedef enum QBlockFormatOptionsVDIPreAlloc {
> +    QB_FMT_VDI_PREALLOC_DEFAULT = 0,
> +    QB_FMT_VDI_PREALLOC_OFF,
> +    QB_FMT_VDI_PREALLOC_METADATA,
> +} QBlockFormatOptionsVDIPreAlloc;
> +
> +typedef struct QBlockFormatOptionsVDI {
> +    uint64_t virt_size;
> +    uint64_t cluster_size;
> +    QBlockFormatOptionsVDIPreAlloc pre_mode;
> +} QBlockFormatOptionsVDI;
> +
> +/* whether compact to vmdk verion 6 */
> +typedef enum QBlockFormatOptionsVMDKCompatLv {
> +    QB_FMT_VMDK_COMPAT_DEFAULT = 0,
> +    QB_FMT_VMDK_COMPAT_VMDKV6_FALSE,
> +    QB_FMT_VMDK_COMPAT_VMDKV6_TRUE,
> +} QBlockFormatOptionsVMDKCompatLv;
> +
> +/* vmdk flat extent format, values:
> +"{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse |
> +twoGbMaxExtentFlat | streamOptimized} */
> +typedef enum QBlockFormatOptionsVMDKSubfmt {
> +    QB_FMT_VMDK_SUBFMT_DEFAULT = 0,
> +    QB_FMT_VMDK_SUBFMT_MONOLITHIC_SPARSE,
> +    QB_FMT_VMDK_SUBFMT_MONOLITHIC_FLAT,
> +    QB_FMT_VMDK_SUBFMT_TWOGBMAX_EXTENT_SPARSE,
> +    QB_FMT_VMDK_SUBFMT_TWOGBMAX_EXTENT_FLAT,
> +    QB_FMT_VMDK_SUBFMT_STREAM_OPTIMIZED,
> +} QBlockFormatOptionsVMDKSubfmt;
> +
> +typedef struct QBlockFormatOptionsVMDK {
> +    uint64_t virt_size;
> +    QBlockLocationInfo backing_loc;
> +    QBlockFormatOptionsVMDKCompatLv cpt_lv;
> +    QBlockFormatOptionsVMDKSubfmt subfmt;
> +} QBlockFormatOptionsVMDK;
> +
> +/* "{dynamic (default) | fixed} " */
> +typedef enum QBlockFormatOptionsVPCSubfmt {
> +    QB_FMT_VPC_SUBFMT_DEFAULT = 0,
> +    QB_FMT_VPC_SUBFMT_DYNAMIC,
> +    QB_FMT_VPC_SUBFMT_FIXED,
> +} QBlockFormatOptionsVPCSubfmt;
> +
> +typedef struct QBlockFormatOptionsVPC {
> +    uint64_t virt_size;
> +    QBlockFormatOptionsVPCSubfmt subfmt;
> +} QBlockFormatOptionsVPC;
> +
> +#define QBLOCK_FMT_OPTIONS_UNION_SIZE (QBLOCK_PROT_OPTIONS_UNION_SIZE*2)

Why?

> +
> +/**
> + * struct QBlockFormatInfo: contains information about how to retrieve data
> + *  from the image
> + *
> + * @fmt_type: format type.
> + * @o_cow~@o_vdi: format related attributes.
> + * @reserved: reserved bytes for ABI.
> + */
> +typedef struct QBlockFormatInfo {
> +    QBlockFormat fmt_type;
> +    union {
> +        QBlockFormatOptionsCOW       o_cow;
> +        QBlockFormatOptionsQED       o_qed;
> +        QBlockFormatOptionsQCOW      o_qcow;
> +        QBlockFormatOptionsQCOW2     o_qcow2;
> +        QBlockFormatOptionsRAW       o_raw;
> +        QBlockFormatOptionsRBD       o_rbd;
> +        QBlockFormatOptionsSD        o_sd;
> +        QBlockFormatOptionsVDI       o_vdi;
> +        QBlockFormatOptionsVMDK      o_vmdk;
> +        QBlockFormatOptionsVPC       o_vpc;
> +        uint64_t reserved[QBLOCK_FMT_OPTIONS_UNION_SIZE/8];
> +    };
> +} QBlockFormatInfo;
> +
> +/**
> + * QBlockStaticInfo: information about the block image.
> + *
> + * @loc: location information.
> + * @fmt: format information.
> + * @sector_size: how many bytes in a sector, it is 512 usually.
> + */
> +typedef struct QBlockStaticInfo {
> +    QBlockLocationInfo loc;
> +    QBlockFormatInfo fmt;
> +    int sector_size;
> +} QBlockStaticInfo;
> +
> +
> +#endif
> diff --git a/libqblock/libqblock.c b/libqblock/libqblock.c
> index 4c18c41..e69de29 100644
> --- a/libqblock/libqblock.c
> +++ b/libqblock/libqblock.c
> @@ -1,6 +0,0 @@
> -#include "libqblock.h"
> -
> -int libqb_test(void)
> -{
> -    return 0;
> -}

Why?

> diff --git a/libqblock/libqblock.h b/libqblock/libqblock.h
> index b0f9daf..e69de29 100644
> --- a/libqblock/libqblock.h
> +++ b/libqblock/libqblock.h
> @@ -1 +0,0 @@
> -__attribute__((visibility("default"))) int libqb_test(void);
> diff --git a/tests/check-libqblock-qcow2.c b/tests/check-libqblock-qcow2.c
> index 50a4df3..6f5ccac 100644
> --- a/tests/check-libqblock-qcow2.c
> +++ b/tests/check-libqblock-qcow2.c
> @@ -1,6 +1,5 @@
>  #include "libqblock.h"
>  int main(int argc, char **argv)
>  {
> -    libqb_test();

Why?

>      return 0;
>  }
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH V10 6/7] libqblock API
  2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 6/7] libqblock API Wenchao Xia
@ 2012-11-20 10:41   ` Paolo Bonzini
  2012-11-21  3:40     ` Wenchao Xia
  0 siblings, 1 reply; 22+ messages in thread
From: Paolo Bonzini @ 2012-11-20 10:41 UTC (permalink / raw)
  To: Wenchao Xia
  Cc: kwolf, peter.maydell, aliguori, stefanha, qemu-devel, blauwirbel,
	kraxel

Il 20/11/2012 10:46, Wenchao Xia ha scritto:
> diff --git a/libqblock/libqblock.h b/libqblock/libqblock.h
> index e69de29..8ca7d28 100644
> --- a/libqblock/libqblock.h
> +++ b/libqblock/libqblock.h

Please move the creation of this file to the previous patch.

I dislike QBlockState and would prefer something like QBlockImage...

Also, it would be nicer to use reference counting for QBlockImage,
rather than just a new/delete pair.

There are some spelling mistakes (openned->opened).

But overall we're converging, it's good work that you've done.  Thanks
for persisting!

> 
> +    byte_offset = offset & (~BDRV_SECTOR_MASK);
> +    if (byte_offset != 0) {
> +        /* the start sector is not alligned, need to read/write this sector. */
> +        bd_ret = bdrv_read(bs, sector_start, temp_buf, 1);
> +        if (bd_ret < 0) {
> +            set_context_err(context, QB_ERR_INTERNAL_ERR,
> +                           "QEMU internal block error.");

These are I/O errors, not internal errors.

> 
> +    if (length > 0x1000000000000) {

What is this magic number?  I think, just remove the check.

> +        set_context_err(context, QB_ERR_INVALID_PARAM,
> +                       "length is too big.");
> +        goto out;
> +    }

...

> +            context->err_no = bd_ret;
> +            return context->err_ret;
> +        }
> +        cp_len = BDRV_SECTOR_SIZE - byte_offset;
> +        memcpy(p, temp_buf + byte_offset, cp_len);
> +
> +        remains -= cp_len;
> +        p += cp_len;
> +        sector_start++;
> +    }
> +
> +    /* now start position is alligned. */
> +    if (remains >= BDRV_SECTOR_SIZE) {
> +        sector_num = remains >> BDRV_SECTOR_BITS;
> +        bd_ret = bdrv_read(bs, sector_start, p, sector_num);
> +        if (bd_ret < 0) {
> +            set_context_err(context, QB_ERR_INTERNAL_ERR,
> +                           "QEMU internal block error.");
> +            context->err_no = bd_ret;
> +            return context->err_ret;
> +        }
> +        remains -= sector_num << BDRV_SECTOR_BITS;
> +        p += sector_num << BDRV_SECTOR_BITS;
> +        sector_start += sector_num;
> +    }
> +
> +    if (remains > 0) {
> +        /* there is some request remains, less than 1 sector */
> +        bd_ret = bdrv_read(bs, sector_start, temp_buf, 1);
> +        if (bd_ret < 0) {
> +            set_context_err(context, QB_ERR_INTERNAL_ERR,
> +                           "QEMU internal block error.");
> +            context->err_no = bd_ret;
> +            return context->err_ret;
> +        }
> +        memcpy(p, temp_buf, remains);
> +        remains -= remains;
> +    }
> +



> +int qb_info_image_static_get(QBlockContext *context,
> +                             QBlockState *qbs,
> +                             QBlockStaticInfo **info)
> +{
> +    int ret = 0;
> +    BlockDriverState *bs;
> +    QBlockStaticInfo *info_tmp;
> +    QBlockStaticInfoAddr *member_addr, addr;
> +    const char *fmt_str;
> +    uint64_t total_sectors;
> +    char backing_filename[LIBQB_FILENAME_MAX];
> +
> +    if (qbs->filename == NULL) {
> +        set_context_err(context, QB_ERR_INVALID_PARAM,
> +                       "Block Image was not openned.");
> +        ret = context->err_ret;
> +        goto out;
> +    }
> +
> +    info_tmp = FUNC_CALLOC(1, sizeof(QBlockStaticInfo));

Please rename info to p_info and info_tmp to info.

> +    bs = qbs->bdrvs;
> +
> +    ret = filename2loc(context,
> +                       &(info_tmp->loc),

Useless parentheses around & argument.  This is very common, please
remove them.

> +                       qbs->filename);
> +    if (ret < 0) {
> +        goto free;
> +    }
> +
> +    fmt_str = bdrv_get_format_name(bs);
> +    info_tmp->fmt.fmt_type = qb_str2fmttype(fmt_str);
> +    /* we got the format type and basic location info now, setup the struct
> +    pointer to the internal members */
> +    memset(&addr, 0, sizeof(QBlockStaticInfoAddr));

Please move these memsets to qb_setup_info_addr.

> +    member_addr = &addr;
> +    qb_setup_info_addr(info_tmp, member_addr);

Just use qb_setup_info_addr(info_tmp, &addr).

> +
> +    assert(member_addr->virt_size != NULL);
> +    bdrv_get_geometry(bs, &total_sectors);
> +    *(member_addr->virt_size) = total_sectors * BDRV_SECTOR_SIZE;

virt_size is always valid, please move it out of the unions and in the
main struct.

> +    if (member_addr->encrypt != NULL) {
> +        *(member_addr->encrypt) = bdrv_is_encrypted(bs);
> +    }
> +    bdrv_get_full_backing_filename(bs, backing_filename,
> +                                   sizeof(backing_filename));
> +    if (backing_filename[0] != '\0') {
> +        assert(member_addr->backing_loc != NULL);
> +        ret = filename2loc(context,
> +                           member_addr->backing_loc,
> +                           backing_filename);
> +        if (ret < 0) {
> +            goto free;
> +        }
> +    }
> +
> +    info_tmp->sector_size = BDRV_SECTOR_SIZE;
> +    *info = info_tmp;
> +
> + out:
> +    return ret;
> + free:
> +    qb_info_image_static_delete(context, &info_tmp);
> +    return ret;
> +}

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH V10 4/7] libqblock build system
  2012-11-20 10:26   ` Paolo Bonzini
@ 2012-11-21  3:03     ` Wenchao Xia
  2012-11-21  7:56       ` Paolo Bonzini
  0 siblings, 1 reply; 22+ messages in thread
From: Wenchao Xia @ 2012-11-21  3:03 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: kwolf, peter.maydell, aliguori, stefanha, qemu-devel, blauwirbel,
	kraxel

于 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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH V10 5/7] libqblock type defines
  2012-11-20 10:30   ` Paolo Bonzini
@ 2012-11-21  3:12     ` Wenchao Xia
  2012-11-21  8:05       ` Paolo Bonzini
  0 siblings, 1 reply; 22+ messages in thread
From: Wenchao Xia @ 2012-11-21  3:12 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: kwolf, peter.maydell, aliguori, stefanha, qemu-devel, blauwirbel,
	kraxel

于 2012-11-20 18:30, Paolo Bonzini 写道:
> Il 20/11/2012 10:46, Wenchao Xia ha scritto:
>>    This patch contains type and macro defines used in APIs, one file for public
>> usage, one for libqblock internal usage.
>
> Mostly looks good, just a few questions below.
>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>>   libqblock/libqblock-internal.h |   75 ++++++++++++
>>   libqblock/libqblock-types.h    |  252 ++++++++++++++++++++++++++++++++++++++++
>>   libqblock/libqblock.c          |    6 -
>>   libqblock/libqblock.h          |    1 -
>>   tests/check-libqblock-qcow2.c  |    1 -
>>   5 files changed, 327 insertions(+), 8 deletions(-)
>>   create mode 100644 libqblock/libqblock-internal.h
>>
>> diff --git a/libqblock/libqblock-internal.h b/libqblock/libqblock-internal.h
>> new file mode 100644
>> index 0000000..75c0452
>> --- /dev/null
>> +++ b/libqblock/libqblock-internal.h
>> @@ -0,0 +1,75 @@
>> +/*
>> + * QEMU block layer library
>> + *
>> + * Copyright IBM, Corp. 2012
>> + *
>> + * Authors:
>> + *  Wenchao Xia   <xiawenc@linux.vnet.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
>> + * See the COPYING.LIB file in the top-level directory.
>> + *
>> + */
>> +
>> +#ifndef LIBQBLOCK_INTERNAL
>> +#define LIBQBLOCK_INTERNAL
>> +
>> +#include "glib.h"
>
> Use <glib.h> please.
>
   OK.

>> +
>> +#include "block.h"
>> +#include "block_int.h"
>
> Is block_int.h really needed?
>
   Some type define in it was used before, let me check if it can be
removed.

>> +#include "libqblock-types.h"
>> +
>> +/* this file contains defines and types used inside the library. */
>> +
>> +#define FUNC_FREE(p) g_free((p))
>> +#define FUNC_MALLOC(size) g_malloc((size))
>> +#define FUNC_CALLOC(nmemb, size) g_malloc0((nmemb)*(size))
>> +#define FUNC_STRDUP(p) g_strdup((p))
>
> Why keep these?

   This macro make it easy to switch mem related functions if we change
our mind in the future.

>> +
>> +#define CLEAN_FREE(p) { \
>> +        FUNC_FREE(p); \
>> +        (p) = NULL; \
>> +}
>
> What about calling it QB_FREE?
>
   OK.

>> +
>> +/* details should be hidden to user */
>> +struct QBlockState {
>> +    BlockDriverState *bdrvs;
>> +    /* internal used file name now, if it is not NULL, it means
>> +       image was opened.
>> +    */
>> +    char *filename;
>> +};
>> +
>> +struct QBlockContext {
>> +    /* last error */
>> +    GError *g_error;
>> +    int err_ret; /* 1st level of error, the libqblock error number */
>> +    int err_no; /* 2nd level of error, errno what below reports */
>> +};
>> +
>> +/**
>> + * QBlockStaticInfoAddr: a structure contains a set of pointer.
>> + *
>> + *    this struct contains a set of pointer pointing to some
>> + *  property related to format or protocol. If a property is not available,
>> + *  it will be set as NULL. User could use this to get properties directly.
>> + *
>> + *  @virt_size: virtual size, it is always not NULL.
>> + *  @backing_loc: backing file location.
>> + *  @encrypt: encryption flag.
>> +*/
>> +
>> +typedef struct QBlockStaticInfoAddr {
>> +    uint64_t *virt_size;
>> +    QBlockLocationInfo *backing_loc;
>> +    bool *encrypt;
>> +} QBlockStaticInfoAddr;
>> +
>> +#define G_LIBQBLOCK_ERROR g_libqbock_error_quark()
>> +
>> +static inline GQuark g_libqbock_error_quark(void)
>
> libqblock, not libqbock.
>
   my mistake.

>> +{
>> +    return g_quark_from_static_string("g-libqblock-error-quark");
>> +}
>> +#endif
>> diff --git a/libqblock/libqblock-types.h b/libqblock/libqblock-types.h
>> index e69de29..77ad325 100644
>> --- a/libqblock/libqblock-types.h
>> +++ b/libqblock/libqblock-types.h
>> @@ -0,0 +1,252 @@
>> +/*
>> + * QEMU block layer library
>> + *
>> + * Copyright IBM, Corp. 2012
>> + *
>> + * Authors:
>> + *  Wenchao Xia   <xiawenc@linux.vnet.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
>> + * See the COPYING.LIB file in the top-level directory.
>> + *
>> + */
>> +
>> +#ifndef LIBQBLOCK_TYPES_H
>> +#define LIBQBLOCK_TYPES_H
>> +
>> +#include <sys/types.h>
>> +#include <stdint.h>
>> +#include <stdbool.h>
>> +
>> +#if defined(__GNUC__) && __GNUC__ >= 4
>> +    #ifdef LIBQB_BUILD
>> +        #define DLL_PUBLIC __attribute__((visibility("default")))
>> +    #else
>> +        #define DLL_PUBLIC
>> +    #endif
>> +#endif
>> +
>> +/* this library is designed around this core struct. */
>> +typedef struct QBlockState QBlockState;
>> +
>> +/* every thread should have a context. */
>> +typedef struct QBlockContext QBlockContext;
>> +
>> +/* flag used in open and create */
>> +#define LIBQBLOCK_O_RDWR        0x0002
>> +/* do not use the host page cache */
>> +#define LIBQBLOCK_O_NOCACHE     0x0020
>> +/* use write-back caching */
>> +#define LIBQBLOCK_O_CACHE_WB    0x0040
>> +/* don't open the backing file */
>> +#define LIBQBLOCK_O_NO_BACKING  0x0100
>> +/* disable flushing on this disk */
>> +#define LIBQBLOCK_O_NO_FLUSH    0x0200
>> +
>> +#define LIBQBLOCK_O_CACHE_MASK \
>> +   (LIBQBLOCK_O_NOCACHE | LIBQBLOCK_O_CACHE_WB | LIBQBLOCK_O_NO_FLUSH)
>> +
>> +#define LIBQBLOCK_O_VALID_MASK \
>> +   (LIBQBLOCK_O_RDWR | LIBQBLOCK_O_NOCACHE | LIBQBLOCK_O_CACHE_WB | \
>> +    LIBQBLOCK_O_NO_BACKING | LIBQBLOCK_O_NO_FLUSH)
>> +
>> +typedef enum QBlockProtocol {
>> +    QB_PROTO_NONE = 0,
>> +    QB_PROTO_FILE,
>> +    QB_PROTO_MAX
>> +} QBlockProtocol;
>> +
>> +typedef struct QBlockProtocolOptionsFile {
>> +    const char *filename;
>> +} QBlockProtocolOptionsFile;
>> +
>> +/**
>> + * struct QBlockLocationInfo: contains information about how to find the image
>> + *
>> + * @prot_type: protocol type, now only support FILE.
>> + * @o_file: file protocol related attributes.
>> + * @reserved: reserved bytes for ABI.
>> + */
>> +#define QBLOCK_PROT_OPTIONS_UNION_SIZE (512)
>> +typedef struct QBlockLocationInfo {
>> +    QBlockProtocol prot_type;
>> +    union {
>> +        QBlockProtocolOptionsFile o_file;
>> +        uint64_t reserved[QBLOCK_PROT_OPTIONS_UNION_SIZE/8];
>> +    };
>> +} QBlockLocationInfo;
>> +
>> +
>> +/* format related options */
>> +typedef enum QBlockFormat {
>> +    QB_FMT_NONE = 0,
>> +    QB_FMT_COW,
>> +    QB_FMT_QED,
>> +    QB_FMT_QCOW,
>> +    QB_FMT_QCOW2,
>> +    QB_FMT_RAW,
>> +    QB_FMT_RBD,
>> +    QB_FMT_SHEEPDOG,
>> +    QB_FMT_VDI,
>> +    QB_FMT_VMDK,
>> +    QB_FMT_VPC,
>> +    QB_FMT_MAX
>> +} QBlockFormat;
>> +
>> +typedef struct QBlockFormatOptionsCOW {
>> +    uint64_t virt_size;
>> +    QBlockLocationInfo backing_loc;
>> +} QBlockFormatOptionsCOW;
>> +
>> +typedef struct QBlockFormatOptionsQED {
>> +    uint64_t virt_size;
>> +    QBlockLocationInfo backing_loc;
>> +    QBlockFormat backing_fmt;
>> +    uint64_t cluster_size; /* unit is bytes */
>> +    uint64_t table_size; /* unit is clusters */
>> +} QBlockFormatOptionsQED;
>> +
>> +typedef struct QBlockFormatOptionsQCOW {
>> +    uint64_t virt_size;
>> +    QBlockLocationInfo backing_loc;
>> +    bool encrypt;
>> +} QBlockFormatOptionsQCOW;
>> +
>> +/* "Compatibility level (0.10 or 1.1)" */
>> +typedef enum QBlockFormatOptionsQCOW2CompatLv {
>> +    QB_FMT_QCOW2_COMPAT_DEFAULT = 0,
>> +    QB_FMT_QCOW2_COMPAT_V0_10,
>> +    QB_FMT_QCOW2_COMPAT_V1_10,
>> +} QBlockFormatOptionsQCOW2CompatLv;
>> +
>> +/* off or metadata */
>> +typedef enum QBlockFormatOptionsQCOW2PreAlloc {
>> +    QB_FMT_QCOW2_PREALLOC_DEFAULT = 0,
>> +    QB_FMT_QCOW2_PREALLOC_OFF,
>> +    QB_FMT_QCOW2_PREALLOC_METADATA,
>> +} QBlockFormatOptionsQCOW2PreAlloc;
>> +
>> +typedef struct QBlockFormatOptionsQCOW2 {
>> +    uint64_t virt_size;
>> +    QBlockLocationInfo backing_loc;
>> +    QBlockFormat backing_fmt;
>> +    bool encrypt;
>> +    uint64_t cluster_size; /* unit is bytes */
>> +    QBlockFormatOptionsQCOW2CompatLv cpt_lv;
>> +    QBlockFormatOptionsQCOW2PreAlloc pre_mode;
>> +} QBlockFormatOptionsQCOW2;
>> +
>> +typedef struct QBlockFormatOptionsRAW {
>> +    uint64_t virt_size;
>> +} QBlockFormatOptionsRAW;
>> +
>> +typedef struct QBlockFormatOptionsRBD {
>> +    uint64_t virt_size;
>> +    uint64_t cluster_size;
>> +} QBlockFormatOptionsRBD;
>> +
>> +/* off or full */
>> +typedef enum QBlockFormatOptionsSDPreAlloc {
>> +    QB_FMT_SD_PREALLOC_DEFAULT = 0,
>> +    QB_FMT_SD_PREALLOC_OFF,
>> +    QB_FMT_SD_PREALLOC_FULL,
>> +} QBlockFormatOptionsSDPreAlloc;
>> +
>> +typedef struct QBlockFormatOptionsSD {
>> +    uint64_t virt_size;
>> +    QBlockLocationInfo backing_loc;
>> +    QBlockFormatOptionsSDPreAlloc pre_mode;
>> +} QBlockFormatOptionsSD;
>> +
>> +typedef enum QBlockFormatOptionsVDIPreAlloc {
>> +    QB_FMT_VDI_PREALLOC_DEFAULT = 0,
>> +    QB_FMT_VDI_PREALLOC_OFF,
>> +    QB_FMT_VDI_PREALLOC_METADATA,
>> +} QBlockFormatOptionsVDIPreAlloc;
>> +
>> +typedef struct QBlockFormatOptionsVDI {
>> +    uint64_t virt_size;
>> +    uint64_t cluster_size;
>> +    QBlockFormatOptionsVDIPreAlloc pre_mode;
>> +} QBlockFormatOptionsVDI;
>> +
>> +/* whether compact to vmdk verion 6 */
>> +typedef enum QBlockFormatOptionsVMDKCompatLv {
>> +    QB_FMT_VMDK_COMPAT_DEFAULT = 0,
>> +    QB_FMT_VMDK_COMPAT_VMDKV6_FALSE,
>> +    QB_FMT_VMDK_COMPAT_VMDKV6_TRUE,
>> +} QBlockFormatOptionsVMDKCompatLv;
>> +
>> +/* vmdk flat extent format, values:
>> +"{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse |
>> +twoGbMaxExtentFlat | streamOptimized} */
>> +typedef enum QBlockFormatOptionsVMDKSubfmt {
>> +    QB_FMT_VMDK_SUBFMT_DEFAULT = 0,
>> +    QB_FMT_VMDK_SUBFMT_MONOLITHIC_SPARSE,
>> +    QB_FMT_VMDK_SUBFMT_MONOLITHIC_FLAT,
>> +    QB_FMT_VMDK_SUBFMT_TWOGBMAX_EXTENT_SPARSE,
>> +    QB_FMT_VMDK_SUBFMT_TWOGBMAX_EXTENT_FLAT,
>> +    QB_FMT_VMDK_SUBFMT_STREAM_OPTIMIZED,
>> +} QBlockFormatOptionsVMDKSubfmt;
>> +
>> +typedef struct QBlockFormatOptionsVMDK {
>> +    uint64_t virt_size;
>> +    QBlockLocationInfo backing_loc;
>> +    QBlockFormatOptionsVMDKCompatLv cpt_lv;
>> +    QBlockFormatOptionsVMDKSubfmt subfmt;
>> +} QBlockFormatOptionsVMDK;
>> +
>> +/* "{dynamic (default) | fixed} " */
>> +typedef enum QBlockFormatOptionsVPCSubfmt {
>> +    QB_FMT_VPC_SUBFMT_DEFAULT = 0,
>> +    QB_FMT_VPC_SUBFMT_DYNAMIC,
>> +    QB_FMT_VPC_SUBFMT_FIXED,
>> +} QBlockFormatOptionsVPCSubfmt;
>> +
>> +typedef struct QBlockFormatOptionsVPC {
>> +    uint64_t virt_size;
>> +    QBlockFormatOptionsVPCSubfmt subfmt;
>> +} QBlockFormatOptionsVPC;
>> +
>> +#define QBLOCK_FMT_OPTIONS_UNION_SIZE (QBLOCK_PROT_OPTIONS_UNION_SIZE*2)
>
> Why?
   QBlockFormatInfo contains QBlockLocationInfo for backing file in some
formats, so make this structure twice size as QBlockLocationInfo.

>
>> +
>> +/**
>> + * struct QBlockFormatInfo: contains information about how to retrieve data
>> + *  from the image
>> + *
>> + * @fmt_type: format type.
>> + * @o_cow~@o_vdi: format related attributes.
>> + * @reserved: reserved bytes for ABI.
>> + */
>> +typedef struct QBlockFormatInfo {
>> +    QBlockFormat fmt_type;
>> +    union {
>> +        QBlockFormatOptionsCOW       o_cow;
>> +        QBlockFormatOptionsQED       o_qed;
>> +        QBlockFormatOptionsQCOW      o_qcow;
>> +        QBlockFormatOptionsQCOW2     o_qcow2;
>> +        QBlockFormatOptionsRAW       o_raw;
>> +        QBlockFormatOptionsRBD       o_rbd;
>> +        QBlockFormatOptionsSD        o_sd;
>> +        QBlockFormatOptionsVDI       o_vdi;
>> +        QBlockFormatOptionsVMDK      o_vmdk;
>> +        QBlockFormatOptionsVPC       o_vpc;
>> +        uint64_t reserved[QBLOCK_FMT_OPTIONS_UNION_SIZE/8];
>> +    };
>> +} QBlockFormatInfo;
>> +
>> +/**
>> + * QBlockStaticInfo: information about the block image.
>> + *
>> + * @loc: location information.
>> + * @fmt: format information.
>> + * @sector_size: how many bytes in a sector, it is 512 usually.
>> + */
>> +typedef struct QBlockStaticInfo {
>> +    QBlockLocationInfo loc;
>> +    QBlockFormatInfo fmt;
>> +    int sector_size;
>> +} QBlockStaticInfo;
>> +
>> +
>> +#endif
>> diff --git a/libqblock/libqblock.c b/libqblock/libqblock.c
>> index 4c18c41..e69de29 100644
>> --- a/libqblock/libqblock.c
>> +++ b/libqblock/libqblock.c
>> @@ -1,6 +0,0 @@
>> -#include "libqblock.h"
>> -
>> -int libqb_test(void)
>> -{
>> -    return 0;
>> -}
>
> Why?
>
   This function is used in build system patch to verify final
link is OK, it should be removed in next API patch, my mistake.

>> diff --git a/libqblock/libqblock.h b/libqblock/libqblock.h
>> index b0f9daf..e69de29 100644
>> --- a/libqblock/libqblock.h
>> +++ b/libqblock/libqblock.h
>> @@ -1 +0,0 @@
>> -__attribute__((visibility("default"))) int libqb_test(void);
>> diff --git a/tests/check-libqblock-qcow2.c b/tests/check-libqblock-qcow2.c
>> index 50a4df3..6f5ccac 100644
>> --- a/tests/check-libqblock-qcow2.c
>> +++ b/tests/check-libqblock-qcow2.c
>> @@ -1,6 +1,5 @@
>>   #include "libqblock.h"
>>   int main(int argc, char **argv)
>>   {
>> -    libqb_test();
>
> Why?
>
>>       return 0;
>>   }
>>
>


-- 
Best Regards

Wenchao Xia

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH V10 6/7] libqblock API
  2012-11-20 10:41   ` Paolo Bonzini
@ 2012-11-21  3:40     ` Wenchao Xia
  2012-11-21  8:04       ` Paolo Bonzini
  0 siblings, 1 reply; 22+ messages in thread
From: Wenchao Xia @ 2012-11-21  3:40 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: kwolf, peter.maydell, aliguori, stefanha, qemu-devel, blauwirbel,
	kraxel

于 2012-11-20 18:41, Paolo Bonzini 写道:
> Il 20/11/2012 10:46, Wenchao Xia ha scritto:
>> diff --git a/libqblock/libqblock.h b/libqblock/libqblock.h
>> index e69de29..8ca7d28 100644
>> --- a/libqblock/libqblock.h
>> +++ b/libqblock/libqblock.h
>
> Please move the creation of this file to the previous patch.
>
   It is first created in 4/7 build system patch, should it be changed or
add API defines into that patch?

> I dislike QBlockState and would prefer something like QBlockImage...
>
   OK, QBlockImage seems good to me too.

> Also, it would be nicer to use reference counting for QBlockImage,
> rather than just a new/delete pair.
   OK, will add.

>
> There are some spelling mistakes (openned->opened).
   My mistake, will fix it.

>
> But overall we're converging, it's good work that you've done.  Thanks
> for persisting!
   It is my pleasure, thank u for carefully reviewing the patch.

>
>>
>> +    byte_offset = offset & (~BDRV_SECTOR_MASK);
>> +    if (byte_offset != 0) {
>> +        /* the start sector is not alligned, need to read/write this sector. */
>> +        bd_ret = bdrv_read(bs, sector_start, temp_buf, 1);
>> +        if (bd_ret < 0) {
>> +            set_context_err(context, QB_ERR_INTERNAL_ERR,
>> +                           "QEMU internal block error.");
>
> These are I/O errors, not internal errors.
>
   OK.

>>
>> +    if (length > 0x1000000000000) {
>
> What is this magic number?  I think, just remove the check.
>
>> +        set_context_err(context, QB_ERR_INVALID_PARAM,
>> +                       "length is too big.");
>> +        goto out;
>> +    }
>
> ...
   This check is done to avoid silent fail in translating bytes to
sector number used in API
bdrv_is_allocated(bs, sector_start, sector_num, &num)
   sector_num is uint_32, so this length should be no larger than
uint32<<BDRV_SECTOR_MASK.
   Sorry this section had some problem and lacks document, make it
looks joke.... Will fix the check with precise size.

>
>> +            context->err_no = bd_ret;
>> +            return context->err_ret;
>> +        }
>> +        cp_len = BDRV_SECTOR_SIZE - byte_offset;
>> +        memcpy(p, temp_buf + byte_offset, cp_len);
>> +
>> +        remains -= cp_len;
>> +        p += cp_len;
>> +        sector_start++;
>> +    }
>> +
>> +    /* now start position is alligned. */
>> +    if (remains >= BDRV_SECTOR_SIZE) {
>> +        sector_num = remains >> BDRV_SECTOR_BITS;
>> +        bd_ret = bdrv_read(bs, sector_start, p, sector_num);
>> +        if (bd_ret < 0) {
>> +            set_context_err(context, QB_ERR_INTERNAL_ERR,
>> +                           "QEMU internal block error.");
>> +            context->err_no = bd_ret;
>> +            return context->err_ret;
>> +        }
>> +        remains -= sector_num << BDRV_SECTOR_BITS;
>> +        p += sector_num << BDRV_SECTOR_BITS;
>> +        sector_start += sector_num;
>> +    }
>> +
>> +    if (remains > 0) {
>> +        /* there is some request remains, less than 1 sector */
>> +        bd_ret = bdrv_read(bs, sector_start, temp_buf, 1);
>> +        if (bd_ret < 0) {
>> +            set_context_err(context, QB_ERR_INTERNAL_ERR,
>> +                           "QEMU internal block error.");
>> +            context->err_no = bd_ret;
>> +            return context->err_ret;
>> +        }
>> +        memcpy(p, temp_buf, remains);
>> +        remains -= remains;
>> +    }
>> +
>
>
>
>> +int qb_info_image_static_get(QBlockContext *context,
>> +                             QBlockState *qbs,
>> +                             QBlockStaticInfo **info)
>> +{
>> +    int ret = 0;
>> +    BlockDriverState *bs;
>> +    QBlockStaticInfo *info_tmp;
>> +    QBlockStaticInfoAddr *member_addr, addr;
>> +    const char *fmt_str;
>> +    uint64_t total_sectors;
>> +    char backing_filename[LIBQB_FILENAME_MAX];
>> +
>> +    if (qbs->filename == NULL) {
>> +        set_context_err(context, QB_ERR_INVALID_PARAM,
>> +                       "Block Image was not openned.");
>> +        ret = context->err_ret;
>> +        goto out;
>> +    }
>> +
>> +    info_tmp = FUNC_CALLOC(1, sizeof(QBlockStaticInfo));
>
> Please rename info to p_info and info_tmp to info.
>
   OK.

>> +    bs = qbs->bdrvs;
>> +
>> +    ret = filename2loc(context,
>> +                       &(info_tmp->loc),
>
> Useless parentheses around & argument.  This is very common, please
> remove them.
>
   OK.

>> +                       qbs->filename);
>> +    if (ret < 0) {
>> +        goto free;
>> +    }
>> +
>> +    fmt_str = bdrv_get_format_name(bs);
>> +    info_tmp->fmt.fmt_type = qb_str2fmttype(fmt_str);
>> +    /* we got the format type and basic location info now, setup the struct
>> +    pointer to the internal members */
>> +    memset(&addr, 0, sizeof(QBlockStaticInfoAddr));
>
> Please move these memsets to qb_setup_info_addr.
>
   OK.

>> +    member_addr = &addr;
>> +    qb_setup_info_addr(info_tmp, member_addr);
>
> Just use qb_setup_info_addr(info_tmp, &addr).
>
   OK.

>> +
>> +    assert(member_addr->virt_size != NULL);
>> +    bdrv_get_geometry(bs, &total_sectors);
>> +    *(member_addr->virt_size) = total_sectors * BDRV_SECTOR_SIZE;
>
> virt_size is always valid, please move it out of the unions and in the
> main struct.
>
   OK.

>> +    if (member_addr->encrypt != NULL) {
>> +        *(member_addr->encrypt) = bdrv_is_encrypted(bs);
>> +    }
>> +    bdrv_get_full_backing_filename(bs, backing_filename,
>> +                                   sizeof(backing_filename));
>> +    if (backing_filename[0] != '\0') {
>> +        assert(member_addr->backing_loc != NULL);
>> +        ret = filename2loc(context,
>> +                           member_addr->backing_loc,
>> +                           backing_filename);
>> +        if (ret < 0) {
>> +            goto free;
>> +        }
>> +    }
>> +
>> +    info_tmp->sector_size = BDRV_SECTOR_SIZE;
>> +    *info = info_tmp;
>> +
>> + out:
>> +    return ret;
>> + free:
>> +    qb_info_image_static_delete(context, &info_tmp);
>> +    return ret;
>> +}
>


-- 
Best Regards

Wenchao Xia

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH V10 4/7] libqblock build system
  2012-11-21  3:03     ` Wenchao Xia
@ 2012-11-21  7:56       ` Paolo Bonzini
  2012-11-22  1:47         ` Wenchao Xia
  0 siblings, 1 reply; 22+ messages in thread
From: Paolo Bonzini @ 2012-11-21  7:56 UTC (permalink / raw)
  To: Wenchao Xia
  Cc: kwolf, peter.maydell, aliguori, stefanha, qemu-devel, blauwirbel,
	kraxel

Il 21/11/2012 04:03, 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"?

Here, you just do libqblock="".

Then --enable-libqblock and --disable-libqblock set it to yes or no.

Then, if $libqblock = yes, and no libtool, fail.

If $libqblock is empty, and no libtool, set it to no.

If $libqblock is empty, and libtool is there, set it to yes.

Paolo

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH V10 6/7] libqblock API
  2012-11-21  3:40     ` Wenchao Xia
@ 2012-11-21  8:04       ` Paolo Bonzini
  2012-11-22  1:48         ` Wenchao Xia
  0 siblings, 1 reply; 22+ messages in thread
From: Paolo Bonzini @ 2012-11-21  8:04 UTC (permalink / raw)
  To: Wenchao Xia
  Cc: kwolf, peter.maydell, aliguori, stefanha, qemu-devel, blauwirbel,
	kraxel

Il 21/11/2012 04:40, Wenchao Xia ha scritto:
>>
>> Please move the creation of this file to the previous patch.
>>
>   It is first created in 4/7 build system patch, should it be changed or
> add API defines into that patch?

I mean add API defines in 5/7.

Paolo

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH V10 5/7] libqblock type defines
  2012-11-21  3:12     ` Wenchao Xia
@ 2012-11-21  8:05       ` Paolo Bonzini
  2012-11-22  1:50         ` Wenchao Xia
  0 siblings, 1 reply; 22+ messages in thread
From: Paolo Bonzini @ 2012-11-21  8:05 UTC (permalink / raw)
  To: Wenchao Xia
  Cc: kwolf, peter.maydell, aliguori, stefanha, qemu-devel, blauwirbel,
	kraxel

Il 21/11/2012 04:12, Wenchao Xia ha scritto:
>>>
>>> +#define FUNC_FREE(p) g_free((p))
>>> +#define FUNC_MALLOC(size) g_malloc((size))
>>> +#define FUNC_CALLOC(nmemb, size) g_malloc0((nmemb)*(size))
>>> +#define FUNC_STRDUP(p) g_strdup((p))
>>
>> Why keep these?
> 
>   This macro make it easy to switch mem related functions if we change
> our mind in the future.

I don't like it particularly.

If you really want it, keep it, but please use g_malloc0_n for FUNC_CALLOC.

Paolo

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH V10 1/7] Buildsystem fix distclean error for pixman
  2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 1/7] Buildsystem fix distclean error for pixman Wenchao Xia
@ 2012-11-21 22:54   ` Peter Maydell
  2012-11-22  1:50     ` Wenchao Xia
  0 siblings, 1 reply; 22+ messages in thread
From: Peter Maydell @ 2012-11-21 22:54 UTC (permalink / raw)
  To: Wenchao Xia
  Cc: kwolf, aliguori, stefanha, qemu-devel, blauwirbel, kraxel,
	pbonzini

On 20 November 2012 09:46, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>   Currently Makefile test if pixman have configure log, but the script directly
> return error if that file do not exist. This patch fix it.
>
> v2: print out the command.

We need this patch in 1.3 because otherwise 'distclean' barfs
if you're using internal pixman. This would be clearer if it
was a separate patch and not part of this libqblock series
(which is not for 1.3 as far as I know).

> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

> ---
>  Makefile |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 3e8d441..9ecbcbb 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -286,7 +286,7 @@ distclean: clean
>         for d in $(TARGET_DIRS) $(QEMULIBS); do \
>         rm -rf $$d || exit 1 ; \
>          done
> -       test -f pixman/config.log && make -C pixman distclean
> +       if test -f pixman/config.log; then make -C pixman distclean; fi
>
>  KEYMAPS=da     en-gb  et  fr     fr-ch  is  lt  modifiers  no  pt-br  sv \
>  ar      de     en-us  fi  fr-be  hr     it  lv  nl         pl  ru     th \
> --
> 1.7.1
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH V10 4/7] libqblock build system
  2012-11-21  7:56       ` Paolo Bonzini
@ 2012-11-22  1:47         ` Wenchao Xia
  0 siblings, 0 replies; 22+ messages in thread
From: Wenchao Xia @ 2012-11-22  1:47 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: kwolf, peter.maydell, aliguori, stefanha, qemu-devel, blauwirbel,
	kraxel

于 2012-11-21 15:56, Paolo Bonzini 写道:
> Il 21/11/2012 04:03, 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"?
>
> Here, you just do libqblock="".
>
> Then --enable-libqblock and --disable-libqblock set it to yes or no.
>
> Then, if $libqblock = yes, and no libtool, fail.
>
> If $libqblock is empty, and no libtool, set it to no.
>
> If $libqblock is empty, and libtool is there, set it to yes.
>
> Paolo
>

   got it, will change it as comments.


-- 
Best Regards

Wenchao Xia

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH V10 6/7] libqblock API
  2012-11-21  8:04       ` Paolo Bonzini
@ 2012-11-22  1:48         ` Wenchao Xia
  0 siblings, 0 replies; 22+ messages in thread
From: Wenchao Xia @ 2012-11-22  1:48 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: kwolf, peter.maydell, aliguori, stefanha, qemu-devel, blauwirbel,
	kraxel

于 2012-11-21 16:04, Paolo Bonzini 写道:
> Il 21/11/2012 04:40, Wenchao Xia ha scritto:
>>>
>>> Please move the creation of this file to the previous patch.
>>>
>>    It is first created in 4/7 build system patch, should it be changed or
>> add API defines into that patch?
>
> I mean add API defines in 5/7.
>
> Paolo
>
   OK.

-- 
Best Regards

Wenchao Xia

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH V10 5/7] libqblock type defines
  2012-11-21  8:05       ` Paolo Bonzini
@ 2012-11-22  1:50         ` Wenchao Xia
  0 siblings, 0 replies; 22+ messages in thread
From: Wenchao Xia @ 2012-11-22  1:50 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: kwolf, peter.maydell, aliguori, stefanha, qemu-devel, blauwirbel,
	kraxel

于 2012-11-21 16:05, Paolo Bonzini 写道:
> Il 21/11/2012 04:12, Wenchao Xia ha scritto:
>>>>
>>>> +#define FUNC_FREE(p) g_free((p))
>>>> +#define FUNC_MALLOC(size) g_malloc((size))
>>>> +#define FUNC_CALLOC(nmemb, size) g_malloc0((nmemb)*(size))
>>>> +#define FUNC_STRDUP(p) g_strdup((p))
>>>
>>> Why keep these?
>>
>>    This macro make it easy to switch mem related functions if we change
>> our mind in the future.
>
> I don't like it particularly.
>
> If you really want it, keep it, but please use g_malloc0_n for FUNC_CALLOC.
>
> Paolo
>
   OK.

-- 
Best Regards

Wenchao Xia

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH V10 1/7] Buildsystem fix distclean error for pixman
  2012-11-21 22:54   ` Peter Maydell
@ 2012-11-22  1:50     ` Wenchao Xia
  0 siblings, 0 replies; 22+ messages in thread
From: Wenchao Xia @ 2012-11-22  1:50 UTC (permalink / raw)
  To: Peter Maydell
  Cc: kwolf, aliguori, stefanha, qemu-devel, blauwirbel, kraxel,
	pbonzini

于 2012-11-22 6:54, Peter Maydell 写道:
> On 20 November 2012 09:46, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>    Currently Makefile test if pixman have configure log, but the script directly
>> return error if that file do not exist. This patch fix it.
>>
>> v2: print out the command.
>
> We need this patch in 1.3 because otherwise 'distclean' barfs
> if you're using internal pixman. This would be clearer if it
> was a separate patch and not part of this libqblock series
> (which is not for 1.3 as far as I know).
>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> thanks
> -- PMM
>
>> ---
>>   Makefile |    2 +-
>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 3e8d441..9ecbcbb 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -286,7 +286,7 @@ distclean: clean
>>          for d in $(TARGET_DIRS) $(QEMULIBS); do \
>>          rm -rf $$d || exit 1 ; \
>>           done
>> -       test -f pixman/config.log && make -C pixman distclean
>> +       if test -f pixman/config.log; then make -C pixman distclean; fi
>>
>>   KEYMAPS=da     en-gb  et  fr     fr-ch  is  lt  modifiers  no  pt-br  sv \
>>   ar      de     en-us  fi  fr-be  hr     it  lv  nl         pl  ru     th \
>> --
>> 1.7.1
>>
>
   will sent a separate patch, thanks.

-- 
Best Regards

Wenchao Xia

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2012-11-22  1:51 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-20  9:46 [Qemu-devel] [PATCH V10 0/7] libqblock qemu block layer library Wenchao Xia
2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 1/7] Buildsystem fix distclean error for pixman Wenchao Xia
2012-11-21 22:54   ` Peter Maydell
2012-11-22  1:50     ` Wenchao Xia
2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 2/7] Buildsystem clean tests directory clearly Wenchao Xia
2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 3/7] block export function path_has_protocol Wenchao Xia
2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 4/7] libqblock build system Wenchao Xia
2012-11-20 10:26   ` Paolo Bonzini
2012-11-21  3:03     ` Wenchao Xia
2012-11-21  7:56       ` Paolo Bonzini
2012-11-22  1:47         ` Wenchao Xia
2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 5/7] libqblock type defines Wenchao Xia
2012-11-20 10:30   ` Paolo Bonzini
2012-11-21  3:12     ` Wenchao Xia
2012-11-21  8:05       ` Paolo Bonzini
2012-11-22  1:50         ` Wenchao Xia
2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 6/7] libqblock API Wenchao Xia
2012-11-20 10:41   ` Paolo Bonzini
2012-11-21  3:40     ` Wenchao Xia
2012-11-21  8:04       ` Paolo Bonzini
2012-11-22  1:48         ` Wenchao Xia
2012-11-20  9:46 ` [Qemu-devel] [PATCH V10 7/7] libqblock test example Wenchao Xia

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