Re: [Qemu-devel] [PATCH V20 00/10] libqblock qemu block layer library

[Wenchao Xia]

Hi, Kevin
  Do you think this version is good enough to upstream?

> Hi,
>    any comments for this version?
> 
>>     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.
>>
>> 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.
>>
>> v11:
>>     Adjusting code in patch 4 to 7, details are in the child patch's commit
>> message.
>>
>> v12:
>>     Split a patch to add a function in stubs, other change are in patch 4 to 
>> 7
>> commit messages.
>>
>> v13:
>>     Moved another function into stubs, added xml rule in tests/makefile, 
>> little
>> changes in patch 4, 6, 7.
>>
>> v14:
>>     all: Rebased.
>>     1/10, 2/10: automatically call subdir's clean command if subdir's 
>> Makefile
>> added $SUBDIR_CLEAN_RULES, so tests/Makefile do not need to be always 
>> included,
>> libqblock's rule can also use it.
>>     3/10: seperated patch for configure support, modified as libcacard's 
>> style.
>>     4/10: modifed as libcacard's rule.
>>     5/10: seperated patch, also changed a bit to be a mirror as libcacard's 
>> rule.
>>     8/10: use bdrv_pread/bdrv_pwrite, instead of handling the buf allignment 
>> by
>> libqblock itself. Removed libqblock-aio.c because most function are in
>> block-obj-y now.
>>     9/10: seperated patch, use LINK instead of custom LT_LINK rule, and now
>> libqblock.la is a dependence in the link rule of test program, to make
>> LINK invoke libtool.
>>
>> v15:
>>     1/9: merged from 1/10, 2/10 of previous version, and use dependce of 
>> "clean"
>> in sub Makefile instead of a intermedia variable.
>>     2/9: drop $TOOLS adding in libqblock's Makefile, the rule is added in 
>> "all".
>>     3/9: drop libqblock-y in Makefile.objs, directly add them in libqblock's
>> Makfile.
>>     6/9: use __visibility__ in special public marking macro, change macro 
>> name
>> to LIBQB_DLL_PUBLIC to avoid confilict in name space, use sizeof(uint64_t)
>> instead of magic number 8 in reserved member, spelling fix. Date of file was
>> changed to 2013.
>>     7/9: date of file was changed to 2013.
>>     8/9: test rule was added to check-unit, drop correspond rule for
>> check-libqblock, drop varible $LIBQBLOCK_LA.
>>
>> v16:
>>     7/9: remove *o_raw and correct the enum type to fix error in building.
>>
>> v17:
>>     Tested on Fedora 17, build pass on RH6.2, windows.
>>     1/10: remove variable CHECK_CLEAN_TARGETS.
>>     2/10: new added. Because libcacard share some .lo with libqblock and they
>> are compiled differently with '-f visibility', so build sequence will affect
>> the build result. This patch introduce macro QEMU_DLL_PULIC to hide symbols
>> in all .lo file, so they can use same .lo files now, to avoid build failure.
>> All symbols in libcacard.syms are marked as public.
>>     3/10: only support libqblock on LINUX now, to avoid trouble on Windows. 
>> Add
>> glib-thread and glib check in configure. Dependency library was set in
>> configure instead of libqblock/Makefile.
>>     4/10: remove variable LIBQBLOCK_CLEAN_TARGETS, removed -fvisibility flag
>> because it is added in rules.mak before.
>>     7/10: change name qb_fmt to qb_format, qb_loc to qb_location. Use macro
>> QEMU_DLL_PUBLIC.
>>     8/10: change name qb_fmt to qb_format, qb_loc to qb_location, use 
>> g_malloc0
>> in qb_context_new(). Moved init code from __constructure__ function to
>> qb_context_new, to avoid it get called before block layer init, related lock
>> is added.
>>     9/10: remove clean for check-libqblock-y, because it is added in 
>> check-unit-y.
>> removed unused variale LIBQBLOCK_TEST_DIR, moved exe file adding rule to next
>> patch to make build success in this patch.
>>     10/10: add rule for itself in this patch. Default test dir is changed to
>> /tmp/libqblock_test.
>>
>> v18:
>>     Tested on RH6.2.
>>     3/10: using probing result before instead of re-probing for libqblock.
>>     4/10: add comments about the reasion to filter out one .o file in stubs.
>>     7/10:
>>       error handlering:
>>       remove qb_error_get_errno(), return a complete string which need to
>> be freed in qb_error_get_human_str().
>>       API and defines:
>>       remove small macros QB_FREE and G_LIBQBLOCK_ERROR, rename
>> g_libqblock_error_quark() to qb_error_quark(). Add comments about
>> caching. Added reserved function pointers for every public structure,
>> remove related helper functions for those public structure. Remove
>> QBlockContext from public header, it is associated with QBlockImage
>> now. Use QBlockImage* instead of QBlockImage** in qb_image_ref().
>> remove "const" for filename in QBlockLocationInfo. Return 0 on success
>> for qb_read()/qb_write(). Add comments about the flag usage in qb_create().
>> use uint64_t for both length and *plenth in qb_check_allocation(). Spell
>> fix.
>>     8/10: related change for API.
>>     10/10: related change for API. In information getting case, the code
>> handle the public structure's allocation itself.
>>
>> v19:
>>     8/10: add static const flag for libqb_formatstr_table[], use ARRAY_SIZE
>> instead of NULL condition in traversal, and directly use 
>> libqb_formatstr_table
>> instead of *tb in qb_formattype2str() and qb_str2formattype(). add 
>> GCC_FMT_ATTR
>> flag to set_context_err(), remove useless return in void functions.
>>     10/10: remove useless return in void functions.
>>
>> v20:
>>     Rebased.
>>     address Eric's comments:
>>     07/10: add space around binrary operators in macro, drop comments for
>> default sector_size, remove limitation of length for qb_check_allocation().
>>     08/10: check multiple times with lower function in qb_check_allocation().
>>     address Stefan's function:
>>     08/10: use g-once for block init function.
>>
>> Wenchao Xia (10):
>>     1 build: add command check-clean
>>     2 build: hide symbols in *.lo
>>     3 libqblock: build: add configure support
>>     4 libqblock: build: add rule for libqblock.la
>>     5 libqblock: build: add packaging support
>>     6 block: export function path_has_protocol()
>>     7 libqblock: libqblock API design and type defines
>>     8 libqblock: libqblock API implement
>>     9 libqblock: build: add rules for test case
>>     10 libqblock: test: libqblock test example
>>
>>    Makefile                       |    4 +-
>>    block.c                        |    2 +-
>>    configure                      |   50 ++
>>    include/block/block.h          |    2 +
>>    include/qemu/sockets.h         |    1 +
>>    include/qemu/thread.h          |    6 +
>>    libcacard/cac.h                |    2 +
>>    libcacard/card_7816.h          |    9 +
>>    libcacard/vcard.h              |   22 +
>>    libcacard/vcard_emul.h         |   12 +
>>    libcacard/vcard_emul_type.h    |    3 +
>>    libcacard/vcardt.h             |    8 +
>>    libcacard/vevent.h             |    6 +
>>    libcacard/vreader.h            |   23 +
>>    libqblock/Makefile             |   54 +++
>>    libqblock/libqblock-error.c    |   49 ++
>>    libqblock/libqblock-error.h    |   35 ++
>>    libqblock/libqblock-internal.h |   62 +++
>>    libqblock/libqblock-types.h    |  264 +++++++++++
>>    libqblock/libqblock.c          | 1005 
>> ++++++++++++++++++++++++++++++++++++++++
>>    libqblock/libqblock.h          |  245 ++++++++++
>>    libqblock/libqblock.pc.in      |   13 +
>>    rules.mak                      |    1 +
>>    tests/Makefile                 |   19 +-
>>    tests/check-libqblock-qcow2.c  |  385 +++++++++++++++
>>    25 files changed, 2279 insertions(+), 3 deletions(-)
>>    create mode 100644 libqblock/Makefile
>>    create mode 100644 libqblock/libqblock-error.c
>>    create mode 100644 libqblock/libqblock-error.h
>>    create mode 100644 libqblock/libqblock-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
>>
>>
> 
> 


-- 
Best Regards

Wenchao Xia