[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V4 2/5] libqblock type defines
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH V4 2/5] libqblock type defines |
Date: |
Thu, 27 Sep 2012 10:25:52 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120911 Thunderbird/15.0.1 |
Il 27/09/2012 04:23, Wenchao Xia ha scritto:
> This patch contains type and macro defines used in APIs, one file for public
> usage by user, one for libqblock internal usage.
>
> Signed-off-by: Wenchao Xia <address@hidden>
> ---
> libqblock/libqblock-internal.h | 57 +++++++++
> libqblock/libqblock-types.h | 268
> ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 325 insertions(+), 0 deletions(-)
> create mode 100644 libqblock/libqblock-internal.h
> create mode 100644 libqblock/libqblock-types.h
>
> diff --git a/libqblock/libqblock-internal.h b/libqblock/libqblock-internal.h
> new file mode 100644
> index 0000000..d3e983c
> --- /dev/null
> +++ b/libqblock/libqblock-internal.h
> @@ -0,0 +1,57 @@
> +/*
> + * QEMU block layer library
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + * Wenchao Xia <address@hidden>
> + *
> + * 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 */
> +};
> +
> +#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
> new file mode 100644
> index 0000000..948ab8a
> --- /dev/null
> +++ b/libqblock/libqblock-types.h
> @@ -0,0 +1,268 @@
> +/*
> + * QEMU block layer library
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + * Wenchao Xia <address@hidden>
> + *
> + * 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
> +#else
> + #warning : gcc compiler version < 4, symbols can not be hidden.
You don't need to warn. If LIBQB_BUILD is not defined, DLL_PUBLIC would
be empty anyway. If LIBQB_BUILD is defined, you know GCC >= 4.0 because
you pass -fvisibility=hidden from the Makefile.
> +#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 QBlockProtType {
> + QB_PROT_NONE = 0,
> + QB_PROT_FILE,
> + QB_PROT_MAX
> +} QBlockProtType;
Use QBlockProtocol, QB_PROTO_*.
> +typedef struct QBlockProtOptionFile {
> + const char *filename;
> +} QBlockProtOptionFile;
Use QBlockProtocolOptionsFile
> +#define QBLOCK_PROT_OPTIONS_UNION_SIZE (512)
> +typedef union QBlockProtOptionsUnion {
> + QBlockProtOptionFile o_file;
> + uint8_t reserved[QBLOCK_PROT_OPTIONS_UNION_SIZE];
> +} QBlockProtOptionsUnion;
Not really options, because they are required. I would just not name
the union, and embed it in the struct. Also please change it to
uint64_t reserved[QBLOCK_PROT_OPTIONS_UNION_SIZE / 8];
(Sorry for not thinking about this before). This will fix the alignment
and ensure that future changes to the union do not change the ABI.
> +/**
> + * struct QBlockProtInfo: contains information about how to find the image
> + *
> + * @prot_type: protocol type, now only support FILE.
> + * @prot_op: protocol related options.
> + */
> +typedef struct QBlockProtInfo {
> + QBlockProtType prot_type;
> + QBlockProtOptionsUnion prot_op;
> +} QBlockProtInfo;
What about QBlockLocation instead? Or QBlockLocationInfo.
> +
> +/* format related options */
> +typedef enum QBlockFmtType {
> + 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
> +} QBlockFmtType;
QBlockFormat; similarly remove "Type" from all enums.
> +typedef struct QBlockFmtOptionCow {
> + uint64_t virt_size;
> + QBlockProtInfo backing_loc;
> +} QBlockFmtOptionCow;
QBlockFormatOptionsCOW. Similarly use the plural for all other formats.
> +typedef struct QBlockFmtOptionQed {
> + uint64_t virt_size;
> + QBlockProtInfo backing_loc;
> + QBlockFmtType backing_fmt;
> + uint64_t cluster_size; /* unit is bytes */
> + uint64_t table_size; /* unit is clusters */
> +} QBlockFmtOptionQed;
Similarly use uppercase for QED, QCOW, etc.
> +typedef struct QBlockFmtOptionQcow {
> + uint64_t virt_size;
> + QBlockProtInfo backing_loc;
> + bool encrypt;
> +} QBlockFmtOptionQcow;
> +
> +/* "Compatibility level (0.10 or 1.1)" */
> +typedef enum QBlockFmtOptionQcow2CptLv {
> + QBO_FMT_QCOW2_CPT_NONE = 0,
> + QBO_FMT_QCOW2_CPT_V010,
> + QBO_FMT_QCOW2_CPT_V110,
> +} QBlockFmtOptionQcow2CptLv;
Please use QBO_ instead of QB_ throughout. Also write COMPAT instead of
CPT, and remove CPT_NONE since 0.10 is the default:
typedef enum QBlockFmtOptionQCOW2Compat {
QB_FMT_QCOW2_COMPAT_V0_10,
QB_FMT_QCOW2_COMPAT_V1_1,
} QBlockFmtOptionQCOW2Compat
> +/* off or metadata */
> +typedef enum QBlockFmtOptionQcow2PreAllocType {
> + QBO_FMT_QCOW2_PREALLOC_NONE = 0,
> + QBO_FMT_QCOW2_PREALLOC_OFF,
> + QBO_FMT_QCOW2_PREALLOC_METADATA,
> +} QBlockFmtOptionQcow2PreAllocType;
Please remove NONE since OFF is the default. However, I would use the enum.
> +typedef struct QBlockFmtOptionQcow2 {
> + uint64_t virt_size;
> + QBlockProtInfo backing_loc;
> + QBlockFmtType backing_fmt;
> + bool encrypt;
> + uint64_t cluster_size; /* unit is bytes */
> + QBlockFmtOptionQcow2CptLv cpt_lv;
> + QBlockFmtOptionQcow2PreAllocType pre_mode;
> +} QBlockFmtOptionQcow2;
> +
> +typedef struct QBlockFmtOptionRaw {
> + uint64_t virt_size;
> +} QBlockFmtOptionRaw;
> +
> +typedef struct QBlockFmtOptionRbd {
> + uint64_t virt_size;
> + uint64_t cluster_size;
> +} QBlockFmtOptionRbd;
> +
> +/* off or full */
> +typedef enum QBlockFmtOptionSheepdogPreAllocType {
> + QBO_FMT_SD_PREALLOC_NONE = 0,
> + QBO_FMT_SD_PREALLOC_OFF,
> + QBO_FMT_SD_PREALLOC_FULL,
> +} QBlockFmtOptionSheepdogPreAllocType;
The default is false, so just make it a bool.
> +typedef struct QBlockFmtOptionSheepdog {
> + uint64_t virt_size;
> + QBlockProtInfo backing_loc;
> + QBlockFmtOptionSheepdogPreAllocType pre_mode;
> +} QBlockFmtOptionSheepdog;
> +
> +typedef enum QBlockFmtOptionVdiPreAllocType {
> + QBO_FMT_VDI_PREALLOC_NONE = 0,
> + QBO_FMT_VDI_PREALLOC_FALSE,
> + QBO_FMT_VDI_PREALLOC_TRUE,
> +} QBlockFmtOptionVdiPreAllocType;
Please remove NONE, and replace FALSE/TRUE with OFF/METADATA (same as
qcow2).
> +typedef struct QBlockFmtOptionVdi {
> + uint64_t virt_size;
> + uint64_t cluster_size;
> + QBlockFmtOptionVdiPreAllocType pre_mode;
> +} QBlockFmtOptionVdi;
> +
> +/* whether compact to vmdk verion 6 */
> +typedef enum QBlockFmtOptionVmdkCptLv {
> + QBO_FMT_VMDK_CPT_NONE = 0,
> + QBO_FMT_VMDK_CPT_VMDKV6_FALSE,
> + QBO_FMT_VMDK_CPT_VMDKV6_TRUE,
> +} QBlockFmtOptionVmdkCptLv;
Here the default is false, so just make it a bool.
> +/* vmdk flat extent format, values:
> +"{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse |
> +twoGbMaxExtentFlat | streamOptimized} */
> +typedef enum QBlockFmtOptionVmdkSubfmtType {
> + QBO_FMT_VMDK_SUBFMT_MONOLITHIC_NONE = 0,
> + QBO_FMT_VMDK_SUBFMT_MONOLITHIC_SPARSE,
You can remove QBO_FMT_VMDK_SUBFMT_MONOLITHIC_NONE (whose more
appropriate name would be just ...SUBFMT_NONE) since the default is
QBO_FMT_VMDK_SUBFMT_MONOLITHIC_SPARSE.
> + QBO_FMT_VMDK_SUBFMT_MONOLITHIC_FLAT,
> + QBO_FMT_VMDK_SUBFMT_TWOGBMAX_EXTENT_SPARSE,
> + QBO_FMT_VMDK_SUBFMT_TWOGBMAX_EXTENT_FLAT,
> + QBO_FMT_VMDK_SUBFMT_STREAM_OPTIMIZED,
> +} QBlockFmtOptionVmdkSubfmtType;
> +
> +typedef struct QBlockFmtOptionVmdk {
> + uint64_t virt_size;
> + QBlockProtInfo backing_loc;
> + QBlockFmtOptionVmdkCptLv cpt_lv;
> + QBlockFmtOptionVmdkSubfmtType subfmt;
> +} QBlockFmtOptionVmdk;
> +
> +/* "{dynamic (default) | fixed} " */
> +typedef enum QBlockFmtOptionVpcSubfmtType {
> + QBO_FMT_VPC_SUBFMT_NONE = 0,
> + QBO_FMT_VPC_SUBFMT_DYNAMIC,
> + QBO_FMT_VPC_SUBFMT_FIXED,
> +} QBlockFmtOptionVpcSubfmtType;
Again, please remove NONE.
> +typedef struct QBlockFmtOptionVpc {
> + uint64_t virt_size;
> + QBlockFmtOptionVpcSubfmtType subfmt;
> +} QBlockFmtOptionVpc;
> +
> +#define QBLOCK_FMT_OPTIONS_UNION_SIZE (QBLOCK_PROT_OPTIONS_UNION_SIZE*2)
> +typedef union QBlockFmtOptionsUnion {
> + QBlockFmtOptionCow o_cow;
> + QBlockFmtOptionQed o_qed;
> + QBlockFmtOptionQcow o_qcow;
> + QBlockFmtOptionQcow2 o_qcow2;
> + QBlockFmtOptionRaw o_raw;
> + QBlockFmtOptionRbd o_rbd;
> + QBlockFmtOptionSheepdog o_sheepdog;
> + QBlockFmtOptionVdi o_vdi;
> + QBlockFmtOptionVmdk o_vmdk;
> + QBlockFmtOptionVpc o_vpc;
> + uint8_t reserved[QBLOCK_FMT_OPTIONS_UNION_SIZE];
Similarly use uint64_t here, and just embed the union in the struct.
> +} QBlockFmtOptionsUnion;
> +typedef struct QBlockFmtInfo {
> + QBlockFmtType fmt_type;
> + QBlockFmtOptionsUnion fmt_op;
> +} QBlockFmtInfo;
QBlockFormatInfo.
> +/**
> + * 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;
> + QBlockProtInfo *backing_loc;
> + bool *encrypt;
> +} QBlockStaticInfoAddr;
Why the indirection?
> +/**
> + * QBlockStaticInfo: information about the block image.
> + *
> + * @member_addr: contains pointer which can be used to access the structure.
> + * @loc: location information.
> + * @fmt: format information.
> + * @sector_size: how many bytes in a sector, it is 512 usually.
> + */
> +typedef struct QBlockStaticInfo {
> + QBlockStaticInfoAddr *member_addr;
> + QBlockProtInfo loc;
> + QBlockFmtInfo fmt;
> + int sector_size;
> +} QBlockStaticInfo;
> +
> +
> +#endif
>
- [Qemu-devel] [PATCH V4 0/5] libqblock qemu block layer library, Wenchao Xia, 2012/09/26
- [Qemu-devel] [PATCH V4 1/5] libqblock build system, Wenchao Xia, 2012/09/26
- [Qemu-devel] [PATCH V4 2/5] libqblock type defines, Wenchao Xia, 2012/09/26
- Re: [Qemu-devel] [PATCH V4 2/5] libqblock type defines,
Paolo Bonzini <=
- Re: [Qemu-devel] [PATCH V4 2/5] libqblock type defines, Wenchao Xia, 2012/09/27
- Re: [Qemu-devel] [PATCH V4 2/5] libqblock type defines, Paolo Bonzini, 2012/09/27
- Re: [Qemu-devel] [PATCH V4 2/5] libqblock type defines, Wenchao Xia, 2012/09/27
- Re: [Qemu-devel] [PATCH V4 2/5] libqblock type defines, Paolo Bonzini, 2012/09/28
- Re: [Qemu-devel] [PATCH V4 2/5] libqblock type defines, Wenchao Xia, 2012/09/28
- Re: [Qemu-devel] [PATCH V4 2/5] libqblock type defines, Paolo Bonzini, 2012/09/28
[Qemu-devel] [PATCH V4 4/5] libqblock test build system, Wenchao Xia, 2012/09/26