Re: [Qemu-devel] [PATCH V14 07/10] libqblock: libqblock API design and type defines

[Eric Blake]

On 01/23/2013 04:18 AM, Wenchao Xia wrote:
>   Public API design header files: libqblock.h, libqblock-error.h.
> Public type define header files: libqblock-types.h. Private internal used
> header files: libqblock-internal. For ABI some reserved bytes are used in
> structure defines.
> 
> Important APIs:
>   1 QBlockContext. This structure was used to retrieve errors, every thread
> must create one first.
>   2 QBlockImage. 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 <address@hidden>
> ---

> +++ b/libqblock/libqblock-error.h
> @@ -0,0 +1,49 @@
> +/*
> + * QEMU block layer library
> + *
> + * Copyright IBM, Corp. 2012

2012-2013 now, for all new files in this series.

> +++ b/libqblock/libqblock-types.h

> +#if defined(__GNUC__) && __GNUC__ >= 4
> +    #ifdef LIBQB_BUILD
> +        #define DLL_PUBLIC __attribute__((visibility("default")))
> +    #else

s/visibility/__visibility__/, to avoid namespace pollution issues.  As
it is, the macro name DLL_PUBLIC is not namespace-safe, and you are
making it harder to use your header as part of a larger project that
might also want to use the name DLL_PUBLIC but for some other purpose.

> +/**
> + * 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];

Magic number 8.  It would be better to use
[QBLOCK_PROG_OPTIONS_UNION_SIZE/sizeof(uint64_t)].

> +
> +/* whether compact to vmdk verion 6 */

s/verion/version/

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature