qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH V2 0/6] libqblock, qemu block layer library
@ 2012-09-10  8:26 Wenchao Xia
  2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 1/6] libqblock API design Wenchao Xia
                   ` (5 more replies)
  0 siblings, 6 replies; 34+ messages in thread
From: Wenchao Xia @ 2012-09-10  8:26 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, aliguori, stefanha, blauwirbel, pbonzini, eblake,
	Wenchao Xia

  This patch introduce libqblock API, make libqblock.la 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 neither 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.

Wenchao Xia (6):
  libqblock API design
  libqblock type and structure defines
  libqblock error handling
  libqblock export some qemu block function
  libqblock building system
  libqblock test example

 Makefile                         |   22 +-
 Makefile.objs                    |    6 +
 block.c                          |    2 +-
 block.h                          |    1 +
 libqblock/Makefile               |   64 +++
 libqblock/libqblock-error.c      |   60 +++
 libqblock/libqblock-error.h      |   50 ++
 libqblock/libqblock-internal.h   |   50 ++
 libqblock/libqblock-types.h      |  251 +++++++++
 libqblock/libqblock.c            | 1077 ++++++++++++++++++++++++++++++++++++++
 libqblock/libqblock.h            |  292 +++++++++++
 tests/Makefile                   |    3 +
 tests/libqblock/Makefile         |   28 +
 tests/libqblock/libqblock-test.c |  231 ++++++++
 14 files changed, 2135 insertions(+), 2 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 tests/libqblock/Makefile
 create mode 100644 tests/libqblock/libqblock-test.c

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

* [Qemu-devel] [PATCH V2 1/6] libqblock API design
  2012-09-10  8:26 [Qemu-devel] [PATCH V2 0/6] libqblock, qemu block layer library Wenchao Xia
@ 2012-09-10  8:26 ` Wenchao Xia
  2012-09-10 21:07   ` Eric Blake
  2012-09-11 20:28   ` Blue Swirl
  2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 2/6] libqblock type and structure defines Wenchao Xia
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 34+ messages in thread
From: Wenchao Xia @ 2012-09-10  8:26 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, aliguori, stefanha, blauwirbel, pbonzini, eblake,
	Wenchao Xia

  This patch contains the major APIs in the library.
Important APIs:
  1 QBroker. These structure was used to retrieve errors, every thread must
create one first, later maybe thread related staff could be added into it.
  2 QBlockState. It stands for an block image object.
  3 QBlockStaticInfo. It contains static information such as location, backing
file, size.
  4 ABI was kept with reserved members.
  5 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.c | 1077 +++++++++++++++++++++++++++++++++++++++++++++++++
 libqblock/libqblock.h |  292 +++++++++++++
 2 files changed, 1369 insertions(+), 0 deletions(-)
 create mode 100644 libqblock/libqblock.c
 create mode 100644 libqblock/libqblock.h

diff --git a/libqblock/libqblock.c b/libqblock/libqblock.c
new file mode 100644
index 0000000..133ac0f
--- /dev/null
+++ b/libqblock/libqblock.c
@@ -0,0 +1,1077 @@
+/*
+ * 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"
+
+struct LibqblockGlobalData {
+    int init_flag;
+};
+
+struct LibqblockGlobalData g_libqblock_data;
+
+__attribute__((constructor))
+static void libqblock_init(void)
+{
+    if (g_libqblock_data.init_flag == 0) {
+        bdrv_init();
+        qemu_init_main_loop();
+    }
+    g_libqblock_data.init_flag = 1;
+}
+
+const char *qb_fmttype2str(enum QBlockFmtType fmt_type)
+{
+    const char *ret = NULL;
+    switch (fmt_type) {
+    case QB_FMT_COW:
+        ret = "cow";
+        break;
+    case QB_FMT_QED:
+        ret = "qed";
+        break;
+    case QB_FMT_QCOW:
+        ret = "qcow";
+        break;
+    case QB_FMT_QCOW2:
+        ret = "qcow2";
+        break;
+    case QB_FMT_RAW:
+        ret = "raw";
+        break;
+    case QB_FMT_RBD:
+        ret = "rbd";
+        break;
+    case QB_FMT_SHEEPDOG:
+        ret = "sheepdog";
+        break;
+    case QB_FMT_VDI:
+        ret = "vdi";
+        break;
+    case QB_FMT_VMDK:
+        ret = "vmdk";
+        break;
+    case QB_FMT_VPC:
+        ret = "vpc";
+        break;
+    default:
+        break;
+    }
+    return ret;
+}
+
+enum QBlockFmtType qb_str2fmttype(const char *fmt_str)
+{
+    enum QBlockFmtType ret = QB_FMT_NONE;
+    if (0 == strcmp(fmt_str, "cow")) {
+        ret = QB_FMT_COW;
+    } else if (0 == strcmp(fmt_str, "qed")) {
+        ret = QB_FMT_QED;
+    } else if (0 == strcmp(fmt_str, "qcow")) {
+        ret = QB_FMT_QCOW;
+    } else if (0 == strcmp(fmt_str, "qcow2")) {
+        ret = QB_FMT_QCOW2;
+    } else if (0 == strcmp(fmt_str, "raw")) {
+        ret = QB_FMT_RAW;
+    } else if (0 == strcmp(fmt_str, "rbd")) {
+        ret = QB_FMT_RBD;
+    } else if (0 == strcmp(fmt_str, "sheepdog")) {
+        ret = QB_FMT_SHEEPDOG;
+    } else if (0 == strcmp(fmt_str, "vdi")) {
+        ret = QB_FMT_VDI;
+    } else if (0 == strcmp(fmt_str, "vmdk")) {
+        ret = QB_FMT_VMDK;
+    } else if (0 == strcmp(fmt_str, "vpc")) {
+        ret = QB_FMT_VPC;
+    }
+    return ret;
+}
+
+static void set_broker_err(struct QBroker *broker, int err_ret,
+                           const char *fmt, ...)
+{
+    va_list ap;
+
+    broker->err_ret = err_ret;
+    if (err_ret == QB_ERR_INTERNAL_ERR) {
+        broker->err_no = -errno;
+    } else {
+        broker->err_no = 0;
+    }
+
+    va_start(ap, fmt);
+    vsnprintf(broker->err_msg, sizeof(broker->err_msg), fmt, ap);
+    va_end(ap);
+}
+
+static void set_broker_err_nomem(struct QBroker *broker)
+{
+    set_broker_err(broker, QB_ERR_MEM_ERR, "No Memory.");
+}
+
+int qb_broker_new(struct QBroker **broker)
+{
+    *broker = FUNC_CALLOC(1, sizeof(struct QBroker));
+    if (*broker == NULL) {
+        return QB_ERR_MEM_ERR;
+    }
+    return 0;
+}
+
+void qb_broker_delete(struct QBroker **broker)
+{
+    CLEAN_FREE(*broker);
+    return;
+}
+
+int qb_state_new(struct QBroker *broker,
+                 struct QBlockState **qbs)
+{
+    *qbs = FUNC_CALLOC(1, sizeof(struct QBlockState));
+    if (*qbs == NULL) {
+        set_broker_err_nomem(broker);
+        return broker->err_ret;
+    }
+    (*qbs)->bdrvs = bdrv_new("hda");
+    if ((*qbs)->bdrvs == NULL) {
+        CLEAN_FREE(*qbs);
+        set_broker_err(broker, QB_ERR_INTERNAL_ERR,
+                       "failed to create the driver.");
+        return broker->err_ret;
+    }
+    return 0;
+}
+
+void qb_state_delete(struct QBroker *broker,
+                     struct QBlockState **qbs)
+{
+    CLEAN_FREE((*qbs)->filename);
+    if ((*qbs)->bdrvs != NULL) {
+        bdrv_delete((*qbs)->bdrvs);
+        (*qbs)->bdrvs = NULL;
+    }
+    CLEAN_FREE(*qbs);
+    return;
+}
+
+int qb_prot_info_new(struct QBroker *broker,
+                     struct QBlockProtInfo **op)
+{
+    *op = FUNC_CALLOC(1, sizeof(struct QBlockProtInfo));
+    if (*op == NULL) {
+        set_broker_err_nomem(broker);
+        return broker->err_ret;
+    }
+    return 0;
+}
+
+void qb_prot_info_delete(struct QBroker *broker,
+                         struct QBlockProtInfo **op)
+{
+    CLEAN_FREE(*op);
+}
+
+int qb_fmt_info_new(struct QBroker *broker,
+                    struct QBlockFmtInfo **op)
+{
+    *op = FUNC_CALLOC(1, sizeof(struct QBlockFmtInfo));
+    if (*op == NULL) {
+        set_broker_err_nomem(broker);
+        return broker->err_ret;
+    }
+    return 0;
+}
+
+void qb_fmt_info_delete(struct QBroker *broker,
+                        struct QBlockFmtInfo **op)
+{
+    CLEAN_FREE(*op);
+}
+
+/* return 0 if every thing is fine */
+static int loc_check_params(struct QBroker *broker,
+                            struct QBlockProtInfo *loc)
+{
+    broker->err_ret = 0;
+
+    switch (loc->prot_type) {
+    case QB_PROT_FILE:
+        if (loc->prot_op.o_file.filename == NULL) {
+            set_broker_err(broker, QB_ERR_INVALID_PARAM,
+                           "Filename was not set.");
+            goto out;
+        }
+        if (path_has_protocol(loc->prot_op.o_file.filename) > 0) {
+            set_broker_err(broker, QB_ERR_INVALID_PARAM,
+                           "filename [%s] had protocol.",
+                           loc->prot_op.o_file.filename);
+            goto out;
+        }
+        break;
+    default:
+        set_broker_err(broker, QB_ERR_INVALID_PARAM,
+                       "Protocol type [%d] was not valid.",
+                       loc->prot_type);
+        break;
+    }
+
+ out:
+    return broker->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(struct QBroker *broker,
+                        struct QBlockProtInfo *loc,
+                        char **filename)
+{
+    broker->err_ret = 0;
+
+    if (*filename != NULL) {
+        CLEAN_FREE(*filename);
+    }
+    switch (loc->prot_type) {
+    case QB_PROT_FILE:
+        *filename = strdup(loc->prot_op.o_file.filename);
+        if (*filename == NULL) {
+            set_broker_err_nomem(broker);
+        }
+        break;
+    default:
+        set_broker_err(broker, QB_ERR_INVALID_PARAM,
+                 "protocol type [%d] is not supported.",
+                  loc->prot_type);
+        break;
+    }
+
+    return broker->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(struct QBroker *broker,
+                        struct QBlockProtInfo *loc,
+                        const char *filename)
+{
+    broker->err_ret = 0;
+
+    if (path_has_protocol(filename) > 0) {
+        set_broker_err(broker, QB_ERR_INVALID_PARAM,
+                     "Filename [%s] had protocol, not supported now.",
+                     filename);
+        goto out;
+    }
+
+    loc->prot_type = QB_PROT_FILE;
+    switch (loc->prot_type) {
+    case QB_PROT_FILE:
+        loc->prot_op.o_file.filename = strdup(filename);
+        if (loc->prot_op.o_file.filename == NULL) {
+            set_broker_err_nomem(broker);
+            goto out;
+        }
+        break;
+    default:
+        break;
+    }
+
+ out:
+    return broker->err_ret;
+}
+
+/* return 0 if OK, or qblock error number */
+static int set_backing_file_options(struct QBroker *broker,
+                                    QEMUOptionParameter *param,
+                                    struct QBlockProtInfo *loc,
+                                    enum QBlockFmtType *fmt)
+{
+    char *backing_filename = NULL;
+    const char *fmtstr_backing = NULL;
+    int ret = 0;
+
+    if (loc == NULL) {
+        goto out;
+    }
+
+    ret = loc2filename(broker, loc, &backing_filename);
+    /* ret can < 0 if loc have not been set, mean user did not specify backing
+       file */
+    if (ret == QB_ERR_MEM_ERR) {
+        goto out;
+    }
+    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(struct QBroker *broker,
+              struct QBlockState *qbs,
+              struct QBlockProtInfo *loc,
+              struct QBlockFmtInfo *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;
+    struct QBlockFmtOptionCow *o_cow = NULL;
+    struct QBlockFmtOptionQed *o_qed = NULL;
+    struct QBlockFmtOptionQcow *o_qcow = NULL;
+    struct QBlockFmtOptionQcow2 *o_qcow2 = NULL;
+    struct QBlockFmtOptionRaw *o_raw = NULL;
+    struct QBlockFmtOptionRbd *o_rbd = NULL;
+    struct QBlockFmtOptionSheepdog *o_sd = NULL;
+    struct QBlockFmtOptionVdi *o_vdi = NULL;
+    struct QBlockFmtOptionVmdk *o_vmdk = NULL;
+    struct QBlockFmtOptionVpc *o_vpc = NULL;
+
+
+    /* check parameters */
+    if (flag & (~LIBQBLOCK_O_VALID_MASK)) {
+        set_broker_err(broker, QB_ERR_INVALID_PARAM,
+                           "invalid flag was set.");
+        ret = broker->err_ret;
+        goto out;
+    }
+
+    if ((loc == NULL) || (qbs == NULL) || (fmt == NULL)) {
+        set_broker_err(broker, QB_ERR_INVALID_PARAM,
+                          "Got unexpected NULL pointer in parameters.");
+        ret = broker->err_ret;
+        goto out;
+    }
+
+    ret = loc_check_params(broker, loc);
+    if (ret != 0) {
+        goto out;
+    }
+
+    /* internal translate */
+    ret = loc2filename(broker, loc, &filename);
+    if (ret != 0) {
+        goto out;
+    }
+
+    fmtstr = qb_fmttype2str(fmt->fmt_type);
+    if (fmtstr == NULL) {
+        set_broker_err(broker, QB_ERR_INVALID_PARAM,
+                 "Got unexpected NULL pointer in parameters.");
+        ret = broker->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->fmt_op.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(broker, param,
+                                       &o_cow->backing_loc, NULL);
+        if (ret != 0) {
+            goto out;
+        }
+        break;
+    case QB_FMT_QED:
+        o_qed = &(fmt->fmt_op.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(broker, 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->fmt_op.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(broker, 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->fmt_op.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(broker, 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 != QBO_FMT_QCOW2_CPT_NONE) {
+            tmp = o_qcow2->cpt_lv == QBO_FMT_QCOW2_CPT_V010 ? "0.10" : "1.1";
+            bd_ret = set_option_parameter(param,
+                              BLOCK_OPT_COMPAT_LEVEL, tmp);
+            assert(bd_ret == 0);
+        }
+
+        if (o_qcow2->pre_mode != QBO_FMT_QCOW2_PREALLOC_NONE) {
+            tmp = o_qcow2->pre_mode == QBO_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->fmt_op.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->fmt_op.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->fmt_op.o_sheepdog);
+        bd_ret = set_option_parameter_int(param,
+                              BLOCK_OPT_SIZE, o_sd->virt_size);
+        assert(bd_ret == 0);
+        ret = set_backing_file_options(broker, param,
+                                       &o_sd->backing_loc, NULL);
+        if (ret != 0) {
+            goto out;
+        }
+        if (o_sd->pre_mode != QBO_FMT_SD_PREALLOC_NONE) {
+            tmp = o_sd->pre_mode == QBO_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->fmt_op.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 != QBO_FMT_VDI_PREALLOC_NONE) {
+            tmp_bool = o_sd->pre_mode == QBO_FMT_VDI_PREALLOC_TRUE ?
+                                                     true : false;
+            set_option_parameter_int(param, "static", tmp_bool);
+        }
+        break;
+    case QB_FMT_VMDK:
+        o_vmdk = &(fmt->fmt_op.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(broker, param,
+                                       &o_vmdk->backing_loc, NULL);
+        if (ret != 0) {
+            goto out;
+        }
+
+        if (o_vmdk->cpt_lv != QBO_FMT_VMDK_CPT_NONE) {
+            tmp_bool = o_vmdk->cpt_lv == QBO_FMT_VMDK_CPT_VMDKV6_TRUE ?
+                                                     true : false;
+            bd_ret = set_option_parameter_int(param, BLOCK_OPT_COMPAT6,
+                                                     tmp_bool);
+            assert(bd_ret == 0);
+        }
+        if (o_vmdk->subfmt != QBO_FMT_VMDK_SUBFMT_MONOLITHIC_NONE) {
+            switch (o_vmdk->subfmt) {
+            case QBO_FMT_VMDK_SUBFMT_MONOLITHIC_SPARSE:
+                tmp = "monolithicSparse";
+                break;
+            case QBO_FMT_VMDK_SUBFMT_MONOLITHIC_FLAT:
+                tmp = "monolithicFlat";
+                break;
+            case QBO_FMT_VMDK_SUBFMT_TWOGBMAX_EXTENT_SPARSE:
+                tmp = "twoGbMaxExtentSparse";
+                break;
+            case QBO_FMT_VMDK_SUBFMT_TWOGBMAX_EXTENT_FLAT:
+                tmp = "twoGbMaxExtentFlat";
+                break;
+            case QBO_FMT_VMDK_SUBFMT_STREAM_OPTIMIZED:
+                tmp = "streamOptimized";
+                break;
+            default:
+                assert(false);
+                break;
+            }
+            bd_ret = set_option_parameter(param,
+                              BLOCK_OPT_SUBFMT, tmp);
+            assert(bd_ret == 0);
+        }
+        break;
+    case QB_FMT_VPC:
+        o_vpc = &(fmt->fmt_op.o_vpc);
+        bd_ret = set_option_parameter_int(param,
+                               BLOCK_OPT_SIZE, o_vpc->virt_size);
+        assert(bd_ret == 0);
+        if (o_vpc->subfmt != QBO_FMT_VPC_SUBFMT_NONE) {
+            tmp = o_vpc->subfmt == QBO_FMT_VPC_SUBFMT_DYNAMIC ?
+                                               "dynamic" : "fixed";
+            bd_ret = set_option_parameter(param,
+                               BLOCK_OPT_SUBFMT, tmp);
+            assert(bd_ret == 0);
+        }
+        break;
+    default:
+        assert(false);
+        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_broker_err(broker, QB_ERR_INVALID_PARAM,
+                          "Backing file is the same with new file.");
+            ret = broker->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_broker_err(broker, QB_ERR_INVALID_PARAM,
+                               "Failed to open the backing file.");
+                ret = broker->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_broker_err(broker, QB_ERR_INTERNAL_ERR,
+                           "Neither size or backing file was not set.");
+            ret = broker->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_broker_err(broker, QB_ERR_INTERNAL_ERR, errstr);
+        ret = broker->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(struct QBroker *broker,
+            struct QBlockState *qbs,
+            struct QBlockProtInfo *loc,
+            struct QBlockFmtInfo *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_broker_err(broker, QB_ERR_INVALID_PARAM,
+                      "Invalid flag was set.");
+        ret = broker->err_ret;
+        goto out;
+    }
+
+    if ((loc == NULL) || (qbs == NULL)) {
+        set_broker_err(broker, QB_ERR_INVALID_PARAM,
+                      "Got unexpected NULL pointer in parameters.");
+        ret = broker->err_ret;
+        goto out;
+    }
+
+    ret = loc_check_params(broker, loc);
+    if (ret != 0) {
+        goto out;
+    }
+
+    /* internal translate */
+    ret = loc2filename(broker, 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 openning */
+    bs = qbs->bdrvs;
+    bd_ret = bdrv_open(bs, filename, flag, bd);
+    if (bd_ret < 0) {
+        set_broker_err(broker, QB_ERR_INTERNAL_ERR,
+                      "Failed in opening with driver, bd_ret is %d.", bd_ret);
+        ret = broker->err_ret;
+        goto out;
+    }
+
+    if (qbs->filename != NULL) {
+        FUNC_FREE(qbs->filename);
+    }
+    qbs->filename = strdup(filename);
+    if (qbs->filename == NULL) {
+        set_broker_err_nomem(broker);
+        ret = broker->err_ret;
+        bdrv_close(bs);
+        goto out;
+    }
+
+ out:
+    FUNC_FREE(filename);
+    return ret;
+}
+
+void qb_close(struct QBroker *broker,
+              struct QBlockState *qbs)
+{
+    BlockDriverState *bs;
+
+    bs = qbs->bdrvs;
+
+    if (qbs->filename != NULL) {
+        CLEAN_FREE(qbs->filename);
+        bdrv_close(bs);
+    }
+    return;
+}
+
+int64_t qb_read(struct QBroker *broker,
+                struct 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;
+    int remains;
+
+    broker->err_ret = 0;
+    bs = qbs->bdrvs;
+
+    if (len <= 0) {
+        set_broker_err(broker, QB_ERR_INVALID_PARAM,
+                      "Param len is less or equal to zero.");
+        return broker->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 aligned, need to read/write this sector. */
+        bd_ret = bdrv_read(bs, sector_start, temp_buf, 1);
+        if (bd_ret < 0) {
+            set_broker_err(broker, QB_ERR_INTERNAL_ERR,
+                           "QEMU internal block error.");
+            return broker->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 aligned. */
+    if (remains >= BDRV_SECTOR_SIZE) {
+        sector_num = len >> BDRV_SECTOR_BITS;
+        bd_ret = bdrv_read(bs, sector_start, p, sector_num);
+        if (bd_ret < 0) {
+            set_broker_err(broker, QB_ERR_INTERNAL_ERR,
+                           "QEMU internal block error.");
+            return broker->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_broker_err(broker, QB_ERR_INTERNAL_ERR,
+                           "QEMU internal block error.");
+            return broker->err_ret;
+        }
+        memcpy(p, temp_buf, remains);
+        remains -= remains;
+    }
+
+    return len-remains;
+}
+
+int64_t qb_write(struct QBroker *broker,
+                 struct 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;
+    int remains;
+
+    broker->err_ret = 0;
+    bs = qbs->bdrvs;
+
+    if (len <= 0) {
+        set_broker_err(broker, QB_ERR_INVALID_PARAM,
+                      "Param len is less or equal to zero.");
+        return broker->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 aligned, need to read/write this sector. */
+        bd_ret = bdrv_read(bs, sector_start, temp_buf, 1);
+        if (bd_ret < 0) {
+            set_broker_err(broker, QB_ERR_INTERNAL_ERR,
+                           "QEMU internal block error.");
+            return broker->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_broker_err(broker, QB_ERR_INTERNAL_ERR,
+                           "QEMU internal block error.");
+            return broker->err_ret;
+        }
+        remains -= cp_len;
+        p += cp_len;
+        sector_start++;
+    }
+
+    /* now start position is aligned. */
+    if (remains >= BDRV_SECTOR_SIZE) {
+        sector_num = len >> BDRV_SECTOR_BITS;
+        bd_ret = bdrv_write(bs, sector_start, p, sector_num);
+        if (bd_ret < 0) {
+            set_broker_err(broker, QB_ERR_INTERNAL_ERR,
+                           "QEMU internal block error.");
+            return broker->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_broker_err(broker, QB_ERR_INTERNAL_ERR,
+                           "QEMU internal block error.");
+            return broker->err_ret;
+        }
+        memcpy(temp_buf, p, remains);
+        bd_ret = bdrv_write(bs, sector_start, temp_buf, 1);
+        if (bd_ret < 0) {
+            set_broker_err(broker, QB_ERR_INTERNAL_ERR,
+                           "QEMU internal block error.");
+            return broker->err_ret;
+        }
+        remains -= remains;
+    }
+
+    return len-remains;
+}
+
+int qb_flush(struct QBroker *broker,
+             struct QBlockState *qbs)
+{
+    int bd_ret;
+    BlockDriverState *bs;
+
+    broker->err_ret = 0;
+    bs = qbs->bdrvs;
+    bd_ret = bdrv_flush(bs);
+    if (bd_ret < 0) {
+        set_broker_err(broker, QB_ERR_INTERNAL_ERR,
+                       "Internal error.");
+    }
+    return broker->err_ret;
+}
+
+int qb_check_allocation(struct QBroker *broker,
+                        struct QBlockState *qbs,
+                        uint64_t start,
+                        int length,
+                        int *pstatus,
+                        int *plength)
+{
+    int ret;
+    int sector_start, sector_num, num;
+    BlockDriverState *bs;
+    unsigned int real_len, ret_len;
+
+    broker->err_ret = 0;
+    bs = qbs->bdrvs;
+
+    if (qbs->filename == NULL) {
+        set_broker_err(broker, QB_ERR_INVALID_PARAM,
+                       "Image was not opened first.");
+        goto out;
+    }
+
+    if (length <= 0) {
+        set_broker_err(broker, 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_broker_err(broker, 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 broker->err_ret;
+}
+
+int qb_info_image_static_get(struct QBroker *broker,
+                             struct QBlockState *qbs,
+                             struct QBlockStaticInfo **info)
+{
+    int ret = 0;
+    BlockDriverState *bs;
+    struct QBlockStaticInfo *info_tmp;
+    const char *fmt_str;
+    size_t total_sectors;
+    char backing_filename[1024];
+
+    if (qbs->filename == NULL) {
+        set_broker_err(broker, QB_ERR_INVALID_PARAM,
+                       "Block Image was not openned.");
+        ret = broker->err_ret;
+        goto out;
+    }
+
+    info_tmp = FUNC_CALLOC(1, sizeof(struct QBlockStaticInfo));
+    if (info_tmp == NULL) {
+        set_broker_err_nomem(broker);
+        ret = broker->err_ret;
+        goto out;
+    }
+
+    bs = qbs->bdrvs;
+
+    ret = filename2loc(broker,
+                       &(info_tmp->loc),
+                       qbs->filename);
+    if (ret < 0) {
+        goto free;
+    }
+
+    fmt_str = bdrv_get_format_name(bs);
+    info_tmp->fmt_type = qb_str2fmttype(fmt_str);
+
+    bdrv_get_geometry(bs, &total_sectors);
+    info_tmp->virt_size = total_sectors * BDRV_SECTOR_SIZE;
+
+    info_tmp->encrypt = bdrv_is_encrypted(bs);
+
+    bdrv_get_full_backing_filename(bs, backing_filename,
+                                   sizeof(backing_filename));
+    if (backing_filename[0] != '\0') {
+        ret = filename2loc(broker,
+                           &(info_tmp->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(broker, &info_tmp);
+    return ret;
+}
+
+/* free locations if it has string allocated on heap. */
+static void loc_free(struct QBlockProtInfo *loc)
+{
+    switch (loc->prot_type) {
+    case QB_PROT_FILE:
+        FUNC_FREE((void *)(loc->prot_op.o_file.filename));
+        loc->prot_op.o_file.filename = NULL;
+        break;
+    default:
+        break;
+    }
+}
+
+void qb_info_image_static_delete(struct QBroker *broker,
+                                 struct QBlockStaticInfo **info)
+{
+    loc_free(&(*info)->loc);
+    loc_free(&(*info)->backing_loc);
+
+    CLEAN_FREE(*info);
+}
diff --git a/libqblock/libqblock.h b/libqblock/libqblock.h
new file mode 100644
index 0000000..97e6c7c
--- /dev/null
+++ b/libqblock/libqblock.h
@@ -0,0 +1,292 @@
+/*
+ * 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_broker_new: allocate a new broker
+ *
+ * Broker is used to pass operation to libqblock, and get feed back from it.
+ *
+ * Returns 0 on success, libqblock negative error value on fail.
+ *
+ * @broker: used to receive the created struct.
+ */
+DLL_PUBLIC
+int qb_broker_new(struct QBroker **broker);
+
+/**
+ * qb_broker_delete: delete broker
+ *
+ * Broker will be freed and set to NULL.
+ *
+ * @broker: operation broker to be deleted.
+ */
+DLL_PUBLIC
+void qb_broker_delete(struct QBroker **broker);
+
+/**
+ * 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.
+ *
+ * @broker: operation broker.
+ * @qbs: used to receive the created struct.
+ */
+DLL_PUBLIC
+int qb_state_new(struct QBroker *broker,
+                 struct QBlockState **qbs);
+
+/**
+ * qb_state_delete: free a QBlockState struct
+ *
+ * if image was opened, qb_close must be called before delete.
+ *
+ * @broker: operation broker.
+ * @qbs: pointer to the struct's pointer.
+ */
+DLL_PUBLIC
+void qb_state_delete(struct QBroker *broker,
+                     struct QBlockState **qbs);
+
+/**
+ * qb_prot_info_new: create a new struct QBlockProtInfo.
+ *
+ * return 0 on success, libqblock negative error value on fail.
+ *
+ * @broker: operation broker.
+ * @op: pointer to receive the new created one.
+ */
+DLL_PUBLIC
+int qb_prot_info_new(struct QBroker *broker,
+                     struct QBlockProtInfo **op);
+
+/**
+ * qb_prot_info_delete: free a struct QBlockProtInfo.
+ *
+ * @broker: operation broker.
+ * @op: pointer to the object, *op would be set to NULL.
+ */
+DLL_PUBLIC
+void qb_prot_info_delete(struct QBroker *broker,
+                         struct QBlockProtInfo **op);
+
+/**
+ * qb_fmt_info_new: create a new QBlockFmtInfo structure.
+ *
+ * return 0 on success, libqblock negative error value on fail.
+ *
+ * @broker: operation broker.
+ * @op: pointer that will receive created struct.
+ */
+DLL_PUBLIC
+int qb_fmt_info_new(struct QBroker *broker,
+                    struct QBlockFmtInfo **op);
+
+/**
+ * qb_fmt_info_delete: free QBlockFmtInfo structure.
+ *
+ * @broker: operation broker.
+ * @op: pointer to the struct, *op would be set to NULL.
+ */
+DLL_PUBLIC
+void qb_fmt_info_delete(struct QBroker *broker,
+                        struct QBlockFmtInfo **op);
+
+
+/**
+ * qb_open: open a block object.
+ *
+ * return 0 on success, libqblock negative error value on fail.
+ *
+ * @broker: operation broker.
+ * @qbs: pointer to struct 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(struct QBroker *broker,
+            struct QBlockState *qbs,
+            struct QBlockProtInfo *loc,
+            struct QBlockFmtInfo *fmt,
+            int flag);
+
+/**
+ * qb_close: close a block object.
+ *
+ * qb_flush is automatically done inside.
+ *
+ * @broker: operation broker.
+ * @qbs: pointer to struct QBlockState.
+ */
+DLL_PUBLIC
+void qb_close(struct QBroker *broker,
+              struct 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.
+ *
+ * @broker: operation broker.
+ * @qbs: pointer to struct 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(struct QBroker *broker,
+              struct QBlockState *qbs,
+              struct QBlockProtInfo *loc,
+              struct QBlockFmtInfo *fmt,
+              int flag);
+
+
+/* sync access */
+/**
+ * qb_read: block sync read.
+ *
+ * return number of bytes read, libqblock negative error value on fail.
+ *
+ * @broker: operation broker.
+ * @qbs: pointer to struct QBlockState.
+ * @buf: buffer that receive the content.
+ * @len: length to read.
+ * @offset: offset in the block data.
+ */
+DLL_PUBLIC
+int64_t qb_read(struct QBroker *broker,
+                struct 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.
+ *
+ * @broker: operation broker.
+ * @qbs: pointer to struct QBlockState.
+ * @buf: buffer that receive the content.
+ * @len: length to write.
+ * @offset: offset in the block data.
+ */
+DLL_PUBLIC
+int64_t qb_write(struct QBroker *broker,
+                 struct 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.
+ *
+ * @broker: operation broker.
+ * @qbs: pointer to struct QBlockState.
+ */
+DLL_PUBLIC
+int qb_flush(struct QBroker *broker,
+             struct 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.
+ *
+ * @broker: operation broker.
+ * @qbs: pointer to struct QBlockState.
+ * @start: start position, unit is byte.
+ * @length: length to check, unit is byte.
+ * @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(struct QBroker *broker,
+                        struct QBlockState *qbs,
+                        uint64_t start,
+                        int length,
+                        int *pstatus,
+                        int *plength);
+
+/* image information */
+/**
+ * qb_get_image_info: get image info.
+ *
+ * return 0 on success, libqblock negative error value on fail.
+ *
+ * @broker: operation broker.
+ * @qbs: pointer to struct QBlockState.
+ * @info: pointer that would receive the information.
+ */
+DLL_PUBLIC
+int qb_info_image_static_get(struct QBroker *broker,
+                             struct QBlockState *qbs,
+                             struct QBlockStaticInfo **info);
+
+/**
+ * qb_delete_image_info: free image info.
+ *
+ * @broker: operation broker.
+ * @info: pointer to the information struct.
+ */
+DLL_PUBLIC
+void qb_info_image_static_delete(struct QBroker *broker,
+                                 struct 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
+enum QBlockFmtType qb_str2fmttype(const char *fmt_str);
+
+/**
+ * qb_fmttype2str: libqblock format enum type to a string.
+ *
+ * return a pointer to the string, or NULL if type is not supported, and
+ *  returned pointer do NOT need to free.
+ *
+ * @fmt: the format enum type.
+ */
+DLL_PUBLIC
+const char *qb_fmttype2str(enum QBlockFmtType fmt_type);
+#endif
-- 
1.7.1

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

* [Qemu-devel] [PATCH V2 2/6] libqblock type and structure defines
  2012-09-10  8:26 [Qemu-devel] [PATCH V2 0/6] libqblock, qemu block layer library Wenchao Xia
  2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 1/6] libqblock API design Wenchao Xia
@ 2012-09-10  8:26 ` Wenchao Xia
  2012-09-10 21:27   ` Eric Blake
  2012-09-11 20:31   ` Blue Swirl
  2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 3/6] libqblock error handling Wenchao Xia
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 34+ messages in thread
From: Wenchao Xia @ 2012-09-10  8:26 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, aliguori, stefanha, blauwirbel, pbonzini, eblake,
	Wenchao Xia

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

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 libqblock/libqblock-internal.h |   50 ++++++++
 libqblock/libqblock-types.h    |  251 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 301 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..fa27ed4
--- /dev/null
+++ b/libqblock/libqblock-internal.h
@@ -0,0 +1,50 @@
+/*
+ * 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 "block.h"
+#include "block_int.h"
+
+#include "libqblock-types.h"
+
+/* this file contains defines and types used inside the library. */
+
+#define FUNC_FREE free
+#define FUNC_MALLOC malloc
+#define FUNC_CALLOC calloc
+
+#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;
+};
+
+#define QB_ERR_STRING_SIZE (1024)
+struct QBroker {
+    /* last error */
+    char err_msg[QB_ERR_STRING_SIZE];
+    int err_ret; /* last error return of libqblock. */
+    int err_no; /* 2nd level of error, errno what below reports */
+};
+
+#endif
diff --git a/libqblock/libqblock-types.h b/libqblock/libqblock-types.h
new file mode 100644
index 0000000..9d4e3fc
--- /dev/null
+++ b/libqblock/libqblock-types.h
@@ -0,0 +1,251 @@
+/*
+ * 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 __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. */
+struct QBlockState;
+
+/* every thread would have a broker. */
+struct QBroker;
+
+/* 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)
+
+enum QBlockProtType {
+    QB_PROT_NONE = 0,
+    QB_PROT_FILE,
+    QB_PROT_MAX
+};
+
+struct QBlockProtOptionFile {
+    const char *filename;
+};
+
+#define QBLOCK_PROT_OPTIONS_UNION_SIZE (512)
+union QBlockProtOptionsUnion {
+    struct QBlockProtOptionFile o_file;
+    uint8_t reserved[QBLOCK_PROT_OPTIONS_UNION_SIZE];
+};
+
+/**
+ * struct QBlockProtInfo: contains information about how to find the image
+ *
+ * @prot_type: protocol type, now only support FILE.
+ * @prot_op: protocol related options.
+ */
+struct QBlockProtInfo {
+    enum QBlockProtType prot_type;
+    union QBlockProtOptionsUnion prot_op;
+};
+
+
+/* format related options */
+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
+};
+
+struct QBlockFmtOptionCow {
+    uint64_t virt_size;
+    struct QBlockProtInfo backing_loc;
+};
+
+struct QBlockFmtOptionQed {
+    uint64_t virt_size;
+    struct QBlockProtInfo backing_loc;
+    enum QBlockFmtType backing_fmt;
+    uint64_t cluster_size; /* unit is bytes */
+    uint64_t table_size; /* unit is clusters */
+};
+
+struct QBlockFmtOptionQcow {
+    uint64_t virt_size;
+    struct QBlockProtInfo backing_loc;
+    bool encrypt;
+};
+
+/* "Compatibility level (0.10 or 1.1)" */
+enum QBlockFmtOptionQcow2CptLv {
+    QBO_FMT_QCOW2_CPT_NONE = 0,
+    QBO_FMT_QCOW2_CPT_V010,
+    QBO_FMT_QCOW2_CPT_V110,
+};
+
+/* off or metadata */
+enum QBlockFmtOptionQcow2PreAllocType {
+    QBO_FMT_QCOW2_PREALLOC_NONE = 0,
+    QBO_FMT_QCOW2_PREALLOC_OFF,
+    QBO_FMT_QCOW2_PREALLOC_METADATA,
+};
+
+struct QBlockFmtOptionQcow2 {
+    uint64_t virt_size;
+    struct QBlockProtInfo backing_loc;
+    enum QBlockFmtType backing_fmt;
+    bool encrypt;
+    uint64_t cluster_size; /* unit is bytes */
+    enum QBlockFmtOptionQcow2CptLv cpt_lv;
+    enum QBlockFmtOptionQcow2PreAllocType pre_mode;
+};
+
+struct QBlockFmtOptionRaw {
+    uint64_t virt_size;
+};
+
+struct QBlockFmtOptionRbd {
+    uint64_t virt_size;
+    uint64_t cluster_size;
+};
+
+/* off or full */
+enum QBlockFmtOptionSheepdogPreAllocType {
+    QBO_FMT_SD_PREALLOC_NONE = 0,
+    QBO_FMT_SD_PREALLOC_OFF,
+    QBO_FMT_SD_PREALLOC_FULL,
+};
+
+struct QBlockFmtOptionSheepdog {
+    uint64_t virt_size;
+    struct QBlockProtInfo backing_loc;
+    enum QBlockFmtOptionSheepdogPreAllocType pre_mode;
+};
+
+enum QBlockFmtOptionVdiPreAllocType {
+    QBO_FMT_VDI_PREALLOC_NONE = 0,
+    QBO_FMT_VDI_PREALLOC_FALSE,
+    QBO_FMT_VDI_PREALLOC_TRUE,
+};
+
+struct QBlockFmtOptionVdi {
+    uint64_t virt_size;
+    uint64_t cluster_size;
+    enum QBlockFmtOptionVdiPreAllocType pre_mode;
+};
+
+/* whether compact to vmdk verion 6 */
+enum QBlockFmtOptionVmdkCptLv {
+    QBO_FMT_VMDK_CPT_NONE = 0,
+    QBO_FMT_VMDK_CPT_VMDKV6_FALSE,
+    QBO_FMT_VMDK_CPT_VMDKV6_TRUE,
+};
+
+/* vmdk flat extent format, values:
+"{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse |
+twoGbMaxExtentFlat | streamOptimized} */
+enum QBlockFmtOptionVmdkSubfmtType {
+    QBO_FMT_VMDK_SUBFMT_MONOLITHIC_NONE = 0,
+    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,
+};
+
+struct QBlockFmtOptionVmdk {
+    uint64_t virt_size;
+    struct QBlockProtInfo backing_loc;
+    enum QBlockFmtOptionVmdkCptLv cpt_lv;
+    enum QBlockFmtOptionVmdkSubfmtType subfmt;
+};
+
+/* "{dynamic (default) | fixed} " */
+enum QBlockFmtOptionVpcSubfmtType {
+    QBO_FMT_VPC_SUBFMT_NONE = 0,
+    QBO_FMT_VPC_SUBFMT_DYNAMIC,
+    QBO_FMT_VPC_SUBFMT_FIXED,
+};
+
+struct QBlockFmtOptionVpc {
+    uint64_t virt_size;
+    enum QBlockFmtOptionVpcSubfmtType subfmt;
+};
+
+#define QBLOCK_FMT_OPTIONS_UNION_SIZE (QBLOCK_PROT_OPTIONS_UNION_SIZE*2)
+union QBlockFmtOptionsUnion {
+    struct QBlockFmtOptionCow       o_cow;
+    struct QBlockFmtOptionQed       o_qed;
+    struct QBlockFmtOptionQcow      o_qcow;
+    struct QBlockFmtOptionQcow2     o_qcow2;
+    struct QBlockFmtOptionRaw       o_raw;
+    struct QBlockFmtOptionRbd       o_rbd;
+    struct QBlockFmtOptionSheepdog  o_sheepdog;
+    struct QBlockFmtOptionVdi       o_vdi;
+    struct QBlockFmtOptionVmdk      o_vmdk;
+    struct QBlockFmtOptionVpc       o_vpc;
+    uint8_t reserved[QBLOCK_FMT_OPTIONS_UNION_SIZE];
+};
+
+struct QBlockFmtInfo {
+    enum QBlockFmtType fmt_type;
+    union QBlockFmtOptionsUnion fmt_op;
+};
+
+/**
+ * QBlockStaticInfo: information about the block image.
+ *
+ * @loc: location info.
+ * @fmt_type: format type.
+ * @virt_size: virtual size in bytes.
+ * @backing_loc: backing file location, its type is QB_PROT_NONE if not exist.
+ * @encrypt: encrypt flag.
+ * @sector_size: how many bytes in a sector, it is 512 usually.
+ */
+struct QBlockStaticInfo {
+    struct QBlockProtInfo loc;
+    enum QBlockFmtType fmt_type;
+    uint64_t virt_size;
+    /* advance info */
+    struct QBlockProtInfo backing_loc;
+    bool encrypt;
+    int sector_size;
+};
+#endif
-- 
1.7.1

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

* [Qemu-devel] [PATCH V2 3/6] libqblock error handling
  2012-09-10  8:26 [Qemu-devel] [PATCH V2 0/6] libqblock, qemu block layer library Wenchao Xia
  2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 1/6] libqblock API design Wenchao Xia
  2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 2/6] libqblock type and structure defines Wenchao Xia
@ 2012-09-10  8:26 ` Wenchao Xia
  2012-09-10 21:33   ` Eric Blake
  2012-09-11 20:32   ` Blue Swirl
  2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 4/6] libqblock export some qemu block function Wenchao Xia
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 34+ messages in thread
From: Wenchao Xia @ 2012-09-10  8:26 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, aliguori, stefanha, blauwirbel, pbonzini, eblake,
	Wenchao Xia

  This patch contains error handling APIs, which user could call them to
get error details.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 libqblock/libqblock-error.c |   60 +++++++++++++++++++++++++++++++++++++++++++
 libqblock/libqblock-error.h |   50 +++++++++++++++++++++++++++++++++++
 2 files changed, 110 insertions(+), 0 deletions(-)
 create mode 100644 libqblock/libqblock-error.c
 create mode 100644 libqblock/libqblock-error.h

diff --git a/libqblock/libqblock-error.c b/libqblock/libqblock-error.c
new file mode 100644
index 0000000..d5ebabf
--- /dev/null
+++ b/libqblock/libqblock-error.c
@@ -0,0 +1,60 @@
+/*
+ * 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(struct QBroker *broker,
+                            char *buf, int buf_size)
+{
+    const char *err_ret_str;
+    switch (broker->err_ret) {
+    case QB_ERR_MEM_ERR:
+        err_ret_str = "Not enough memory.";
+        break;
+    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 = "Unknow error.";
+        break;
+    }
+    if (broker == NULL) {
+        snprintf(buf, buf_size, "%s", err_ret_str);
+        return;
+    }
+
+    if (broker->err_ret == QB_ERR_INTERNAL_ERR) {
+        snprintf(buf, buf_size, "%s %s errno [%d]. strerror [%s].",
+                     err_ret_str, broker->err_msg,
+                     broker->err_no, strerror(-broker->err_no));
+    } else {
+        snprintf(buf, buf_size, "%s %s",
+                     err_ret_str, broker->err_msg);
+    }
+    return;
+}
+
+int qb_error_get_errno(struct QBroker *broker)
+{
+    if (broker->err_ret == QB_ERR_INTERNAL_ERR) {
+        return broker->err_no;
+    }
+    return 0;
+}
diff --git a/libqblock/libqblock-error.h b/libqblock/libqblock-error.h
new file mode 100644
index 0000000..0690cfb
--- /dev/null
+++ b/libqblock/libqblock-error.h
@@ -0,0 +1,50 @@
+/*
+ * 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_MEM_ERR (-1)
+#define QB_ERR_INTERNAL_ERR (-2)
+#define QB_ERR_INVALID_PARAM (-3)
+#define QB_ERR_BLOCK_OUT_OF_RANGE (-100)
+
+/* 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.
+ *
+ * @broker: operation broker, 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(struct QBroker *broker,
+                            char *buf, int buf_size);
+
+/**
+ * qb_error_get_errno: get error number, only valid when err_ret is
+ *   QB_ERR_INTERNAL_ERR.
+ *
+ * return negative errno or 0 if last error is not QB_ERR_INTERNAL_ERR.
+ *
+ * @broker: operation broker.
+ */
+DLL_PUBLIC
+int qb_error_get_errno(struct QBroker *broker);
+
+#endif
-- 
1.7.1

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

* [Qemu-devel] [PATCH V2 4/6] libqblock export some qemu block function
  2012-09-10  8:26 [Qemu-devel] [PATCH V2 0/6] libqblock, qemu block layer library Wenchao Xia
                   ` (2 preceding siblings ...)
  2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 3/6] libqblock error handling Wenchao Xia
@ 2012-09-10  8:26 ` Wenchao Xia
  2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 5/6] libqblock building system Wenchao Xia
  2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 6/6] libqblock test example Wenchao Xia
  5 siblings, 0 replies; 34+ messages in thread
From: Wenchao Xia @ 2012-09-10  8:26 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, aliguori, stefanha, blauwirbel, pbonzini, eblake,
	Wenchao Xia

  Export some qemu qblock functions for libqblock.

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

diff --git a/block.c b/block.c
index 470bdcc..8b312f8 100644
--- a/block.c
+++ b/block.c
@@ -196,7 +196,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 2e2be11..e7da711 100644
--- a/block.h
+++ b/block.h
@@ -405,4 +405,5 @@ typedef enum {
 #define BLKDBG_EVENT(bs, evt) bdrv_debug_event(bs, evt)
 void bdrv_debug_event(BlockDriverState *bs, BlkDebugEvent event);
 
+int path_has_protocol(const char *path);
 #endif
-- 
1.7.1

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

* [Qemu-devel] [PATCH V2 5/6] libqblock building system
  2012-09-10  8:26 [Qemu-devel] [PATCH V2 0/6] libqblock, qemu block layer library Wenchao Xia
                   ` (3 preceding siblings ...)
  2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 4/6] libqblock export some qemu block function Wenchao Xia
@ 2012-09-10  8:26 ` Wenchao Xia
  2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 6/6] libqblock test example Wenchao Xia
  5 siblings, 0 replies; 34+ messages in thread
From: Wenchao Xia @ 2012-09-10  8:26 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, aliguori, stefanha, blauwirbel, pbonzini, eblake,
	Wenchao Xia

  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.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 Makefile           |   22 +++++++++++++++++-
 Makefile.objs      |    6 +++++
 libqblock/Makefile |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 91 insertions(+), 1 deletions(-)
 create mode 100644 libqblock/Makefile

diff --git a/Makefile b/Makefile
index 1cd5bc8..9f3c0a0 100644
--- a/Makefile
+++ b/Makefile
@@ -163,6 +163,25 @@ qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y) $(block-obj-y)
 
 qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
 
+######################################################################
+# Support building shared library libqblock
+
+.PHONY: libqblock.la
+ifeq ($(LIBTOOL),)
+$(libqblock-lib-la):
+	@echo "libtool is missing, please install and rerun configure"; exit 1
+else
+$(libqblock-lib-la):
+	$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C libqblock V="$(V)" TARGET_DIR="$*/" $(libqblock-lib-la),)
+endif
+
+#make native test file instead of libtool, as a test for development
+QEMU_CFLAGS+=-I./libqblock
+libqblock-objs=libqblock/libqblock.o libqblock/libqblock-helper.o libqblock/libqblock-error.o
+libqblock-test-native$(EXESUF): $(libqblock-objs) tests/libqblock/libqblock-test.o $(tools-obj-y) $(block-obj-y)
+	$(call quiet-command,$(CC) $(LDFLAGS) -o $@ $^ $(LIBS),"  LINK  $@")
+###########################################################################
+
 vscclient$(EXESUF): $(libcacard-y) $(oslib-obj-y) $(trace-obj-y) $(tools-obj-y) qemu-timer-common.o libcacard/vscclient.o
 	$(call quiet-command,$(CC) $(LDFLAGS) -o $@ $^ $(libcacard_libs) $(LIBS),"  LINK  $@")
 
@@ -226,7 +245,8 @@ clean:
 	rm -rf qapi-generated
 	rm -rf qga/qapi-generated
 	$(MAKE) -C tests/tcg clean
-	for d in $(ALL_SUBDIRS) $(QEMULIBS) libcacard; do \
+	$(MAKE) -C tests/libqblock clean
+	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
diff --git a/Makefile.objs b/Makefile.objs
index 4412757..8a4c9fc 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -248,3 +248,9 @@ nested-vars += \
 	common-obj-y \
 	extra-obj-y
 dummy := $(call unnest-vars)
+
+#############################################################################
+# libqblock
+
+libqblock-lib-la = libqblock.la
+libqblock-lib-path = libqblock
diff --git a/libqblock/Makefile b/libqblock/Makefile
new file mode 100644
index 0000000..bf7abcc
--- /dev/null
+++ b/libqblock/Makefile
@@ -0,0 +1,64 @@
+###########################################################################
+# libqblock Makefile
+# Todo:
+#    1 trace related files is generated in this directory, move
+#  them to the root directory.
+##########################################################################
+-include ../config-host.mak
+-include $(SRC_PATH)/Makefile.objs
+-include $(SRC_PATH)/rules.mak
+
+#############################################################################
+# Library settings
+#############################################################################
+$(call set-vpath, $(SRC_PATH))
+
+#expand the foldered vars,especially ./block
+dummy := $(call unnest-vars-1)
+
+#library objects
+tools-obj-y = $(oslib-obj-y) $(trace-obj-y) qemu-tool.o qemu-timer.o \
+	qemu-timer-common.o main-loop.o notify.o \
+	iohandler.o cutils.o iov.o async.o
+tools-obj-$(CONFIG_POSIX) += compatfd.o
+
+libqblock-y=libqblock.o libqblock-error.o
+libqblock-lib-y=$(patsubst %.o,%.lo,$(libqblock-y))
+
+QEMU_OBJS=$(tools-obj-y) $(block-obj-y)
+QEMU_OBJS_FILTERED=$(filter %.o,$(QEMU_OBJS))
+QEMU_OBJS_LIB=$(patsubst %.o, %.lo,$(QEMU_OBJS_FILTERED))
+
+QEMU_CFLAGS+= -I../ -I../include
+#adding magic macro define for symbol hiding and exposing
+QEMU_CFLAGS+= -fvisibility=hidden -D LIBQB_BUILD
+
+#dependency libraries
+LIBS+=-lz $(LIBS_TOOLS)
+
+#################################################################
+# Runtime rules
+#################################################################
+clean:
+	rm -f *.lo *.o *.d *.la libqblock-test trace.c trace.c-timestamp
+	rm -rf .libs block trace
+
+all: libqblock-test
+	@true
+
+help:
+	@echo type make libqblock-test at root dirtory, libtool is required
+
+#make dir block at runtime which would hold the output of block/*.c
+block:
+	@mkdir block
+
+ifeq ($(LIBTOOL),)
+$(libqblock-lib-la):
+	@echo "libtool is missing, please install and rerun configure"; exit 1
+else
+$(libqblock-lib-la): $(libqblock-lib-y) $(QEMU_OBJS_LIB)
+	$(call quiet-command,$(LIBTOOL) --mode=link --quiet --tag=CC $(CC) -rpath $(libdir) -o $@ $^ $(LIBS),"  lt LINK $@")
+endif
+
+.PHONY: libqblock.la
-- 
1.7.1

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

* [Qemu-devel] [PATCH V2 6/6] libqblock test example
  2012-09-10  8:26 [Qemu-devel] [PATCH V2 0/6] libqblock, qemu block layer library Wenchao Xia
                   ` (4 preceding siblings ...)
  2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 5/6] libqblock building system Wenchao Xia
@ 2012-09-10  8:26 ` Wenchao Xia
  5 siblings, 0 replies; 34+ messages in thread
From: Wenchao Xia @ 2012-09-10  8:26 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, aliguori, stefanha, blauwirbel, pbonzini, eblake,
	Wenchao Xia

  Created a new directory in tests, and added a simple test case in it.
In this example, user first create two qcow2 images, and then get the
backing file relationship information of them. Then does write and read
sync IO on them.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 tests/Makefile                   |    3 +
 tests/libqblock/Makefile         |   28 +++++
 tests/libqblock/libqblock-test.c |  231 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 262 insertions(+), 0 deletions(-)
 create mode 100644 tests/libqblock/Makefile
 create mode 100644 tests/libqblock/libqblock-test.c

diff --git a/tests/Makefile b/tests/Makefile
index 26a67ce..69af1e2 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -148,4 +148,7 @@ check-unit: $(patsubst %,check-%, $(check-unit-y))
 check-block: $(patsubst %,check-%, $(check-block-y))
 check: check-unit check-qtest
 
+check-libqblock:
+	$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C tests/libqblock V="$(V)" TARGET_DIR="$*/" check-libqblock,)
+
 -include $(wildcard tests/*.d)
diff --git a/tests/libqblock/Makefile b/tests/libqblock/Makefile
new file mode 100644
index 0000000..dc5acda
--- /dev/null
+++ b/tests/libqblock/Makefile
@@ -0,0 +1,28 @@
+-include ../../config-host.mak
+-include $(SRC_PATH)/Makefile.objs
+-include $(SRC_PATH)/rules.mak
+
+$(call set-vpath, $(SRC_PATH))
+
+#library test case objects
+libqblock-test-objs=libqblock-test.lo
+
+QEMU_CFLAGS+=-I $(SRC_PATH)/$(libqblock-lib-path)
+libqblock-la-path = $(SRC_PATH)/$(libqblock-lib-path)/$(libqblock-lib-la)
+
+##########################################################################
+#runtime rules:
+ifeq ($(LIBTOOL),)
+libqblock-test.bin:
+	@echo "libtool is missing, please install and rerun configure"; exit 1
+else
+libqblock-test.bin: $(libqblock-test-objs) $(libqblock-la-path)
+	$(call quiet-command,$(LIBTOOL) --mode=link --quiet --tag=CC $(CC) -shared -rpath $(libdir) -o $@ $^ ,"  lt LINK $@")
+endif
+
+check-libqblock: libqblock-test.bin
+	./libqblock-test.bin
+
+clean:
+	rm -f *.lo *.o *.d *.la *.bin
+	rm -rf .libs
diff --git a/tests/libqblock/libqblock-test.c b/tests/libqblock/libqblock-test.c
new file mode 100644
index 0000000..2d1a119
--- /dev/null
+++ b/tests/libqblock/libqblock-test.c
@@ -0,0 +1,231 @@
+/*
+ * 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.
+ *
+ */
+
+#include <stdarg.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <inttypes.h>
+#include <string.h>
+#include <stdlib.h>
+#include "libqblock.h"
+
+#define TEST_BUF_SIZE 1024
+static unsigned char buf_r[TEST_BUF_SIZE];
+static unsigned char buf_w[TEST_BUF_SIZE] = {0, 0, 0, 0};
+
+struct VerifyData {
+    unsigned char *buf_r;
+    unsigned char *buf_w;
+    int len;
+};
+
+static void print_loc(struct QBlockProtInfo *loc)
+{
+    switch (loc->prot_type) {
+    case QB_PROT_NONE:
+        printf("protocol type [none].");
+        break;
+    case QB_PROT_FILE:
+        printf("protocol type [file], filename [%s].",
+               loc->prot_op.o_file.filename);
+        break;
+    default:
+        printf("protocol type not supported.");
+        break;
+    }
+}
+
+static void print_info_image_static(struct QBlockStaticInfo *info)
+{
+    printf("=======image location:\n");
+    print_loc(&info->loc);
+    printf("\nvirtual_size %" PRId64 ", format type %d [%s]",
+           info->virt_size, info->fmt_type, qb_fmttype2str(info->fmt_type));
+    printf("\nbacking image location:\n");
+    print_loc(&info->backing_loc);
+    printf("\n");
+}
+
+static void test_check(struct VerifyData *vdata)
+{
+    int cmp;
+    cmp = memcmp(vdata->buf_r, vdata->buf_w, vdata->len);
+    if (cmp == 0) {
+        printf("compare succeed, %d.\n", vdata->buf_r[24]);
+    } else {
+        printf("!!! compare fail, %d.\n", vdata->buf_r[24]);
+        exit(1);
+    }
+}
+
+int main(int argc, char **argv)
+{
+    const char *filename1, *filename2;
+    struct QBroker *broker = NULL;
+    struct QBlockState *qbs = NULL;
+    struct QBlockProtInfo *ol = NULL;
+    struct QBlockFmtInfo *of = NULL;
+    struct QBlockStaticInfo *info_st = NULL;
+    int ret, flag;
+    int test_offset = 510;
+    int test_len = 520;
+    struct VerifyData vdata;
+    char err_str[1024];
+
+    vdata.buf_r = buf_r;
+    vdata.buf_w = buf_w;
+    vdata.len = test_len;
+
+    filename1 = "./qemu_image1";
+    filename2 = "./qemu_image2";
+    printf("qemu test, filename1 is %s, filename2 is %s.\n",
+                                       filename1, filename2);
+
+    ret = qb_broker_new(&broker);
+    if (ret < 0) {
+        goto free;
+    }
+
+    ret = qb_state_new(broker, &qbs);
+    if (ret < 0) {
+        goto free;
+    }
+
+    ret = qb_prot_info_new(broker, &ol);
+    if (ret < 0) {
+        goto free;
+    }
+
+    ret = qb_fmt_info_new(broker, &of);
+    if (ret < 0) {
+        goto free;
+    }
+
+    /* create a new image */
+
+    ol->prot_type = QB_PROT_FILE;
+    ol->prot_op.o_file.filename = filename2;
+    of->fmt_type = QB_FMT_QCOW2;
+    of->fmt_op.o_qcow2.virt_size = 100 * 1024;
+    flag = 0;
+
+    ret = qb_create(broker, qbs, ol, of, flag);
+    if (ret < 0) {
+        qb_error_get_human_str(broker, err_str, sizeof(err_str));
+        printf("create fail 1. %s.\n", err_str);
+        goto unlink;
+    }
+
+    ol->prot_type = QB_PROT_FILE;
+    ol->prot_op.o_file.filename = filename1;
+    of->fmt_type = QB_FMT_QCOW2;
+    of->fmt_op.o_qcow2.backing_loc.prot_type = QB_PROT_FILE;
+    of->fmt_op.o_qcow2.backing_loc.prot_op.o_file.filename = filename2;
+    flag = 0;
+    ret = qb_create(broker, qbs, ol, of, flag);
+    if (ret < 0) {
+        qb_error_get_human_str(broker, err_str, sizeof(err_str));
+        printf("create fail 2. %s.\n", err_str);
+        goto unlink;
+    }
+
+    /* get informations */
+    ol->prot_type = QB_PROT_FILE;
+    ol->prot_op.o_file.filename = filename1;
+    of->fmt_type = QB_FMT_NONE;
+    flag = LIBQBLOCK_O_NO_BACKING;
+    ret = qb_open(broker, qbs, ol, of, flag);
+    if (ret < 0) {
+        qb_error_get_human_str(broker, err_str, sizeof(err_str));
+        printf("info getting, open failed. %s.\n", err_str);
+        goto free;
+    }
+
+    while (1) {
+        ret = qb_info_image_static_get(broker, qbs, &info_st);
+        if (ret < 0) {
+            qb_error_get_human_str(broker, err_str, sizeof(err_str));
+            printf("info get error. %s.\n", err_str);
+            goto close;
+        }
+        print_info_image_static(info_st);
+        qb_close(broker, qbs);
+        if (info_st->backing_loc.prot_type == QB_FMT_NONE) {
+            break;
+        }
+        *ol = info_st->backing_loc;
+        ret = qb_open(broker, qbs, ol, of, flag);
+        if (ret < 0) {
+            qb_error_get_human_str(broker, err_str, sizeof(err_str));
+            printf("info getting, open failed in backing file. %s.\n",
+                                                       err_str);
+            goto free;
+        }
+        qb_info_image_static_delete(broker, &info_st);
+    }
+    /* read and write the image */
+    ol->prot_type = QB_PROT_FILE;
+    ol->prot_op.o_file.filename = filename1;
+    of->fmt_type = QB_FMT_NONE;
+    flag = LIBQBLOCK_O_RDWR;
+    ret = qb_open(broker, qbs, ol, of, flag);
+    if (ret < 0) {
+        qb_error_get_human_str(broker, err_str, sizeof(err_str));
+        printf("open failed. %s.\n", err_str);
+        goto free;
+    }
+
+    buf_w[1] = 1;
+    buf_w[2] = 2;
+    buf_w[514] = 4;
+    memset(buf_r, 0, sizeof(buf_r));
+
+    ret = qb_write(broker, qbs, buf_w, test_len, test_offset);
+    if (ret < 0) {
+        qb_error_get_human_str(broker, err_str, sizeof(err_str));
+        printf("%s.\n", err_str);
+        goto close;
+    }
+
+    ret = qb_read(broker, qbs, buf_r, test_len, test_offset);
+    if (ret < 0) {
+        qb_error_get_human_str(broker, err_str, sizeof(err_str));
+        printf("%s.\n", err_str);
+        goto close;
+    }
+
+    test_check(&vdata);
+
+ close:
+    qb_close(broker, qbs);
+ unlink:
+    unlink(filename1);
+    unlink(filename2);
+ free:
+    if (info_st != NULL) {
+        qb_info_image_static_delete(broker, &info_st);
+    }
+    if (qbs != NULL) {
+        qb_state_delete(broker, &qbs);
+    }
+    if (ol != NULL) {
+        qb_prot_info_delete(broker, &ol);
+    }
+    if (of != NULL) {
+        qb_fmt_info_delete(broker, &of);
+    }
+    if (broker != NULL) {
+        qb_broker_delete(&broker);
+    }
+    return 0;
+}
-- 
1.7.1

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

* Re: [Qemu-devel] [PATCH V2 1/6] libqblock API design
  2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 1/6] libqblock API design Wenchao Xia
@ 2012-09-10 21:07   ` Eric Blake
  2012-09-11  3:16     ` Wenchao Xia
  2012-09-11 20:28   ` Blue Swirl
  1 sibling, 1 reply; 34+ messages in thread
From: Eric Blake @ 2012-09-10 21:07 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: kwolf, aliguori, stefanha, qemu-devel, blauwirbel, pbonzini

[-- Attachment #1: Type: text/plain, Size: 5921 bytes --]

On 09/10/2012 02:26 AM, Wenchao Xia wrote:
>   This patch contains the major APIs in the library.
> Important APIs:
>   1 QBroker. These structure was used to retrieve errors, every thread must
> create one first, later maybe thread related staff could be added into it.
>   2 QBlockState. It stands for an block image object.
>   3 QBlockStaticInfo. It contains static information such as location, backing
> file, size.
>   4 ABI was kept with reserved members.
>   5 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.c | 1077 +++++++++++++++++++++++++++++++++++++++++++++++++
>  libqblock/libqblock.h |  292 +++++++++++++

Two new files, but no build machinery to build them to see if they have
blatant errors.  Yes, it is bisectable, but no, the bisection is not
useful.  I'd rather see the Makefile patches with stub files at the
beginning, then flesh out the stubs, where each patch builds along the way.

In particular,

> +++ b/libqblock/libqblock.c

> +#include "libqblock.h"
> +#include "libqblock-internal.h"

There is no libqblock-internal.h with just this patch applied, making it
impossible to validate this patch in order (the fact that 'make' isn't
flagging this incomplete patch is because you aren't building
libqblock-o yet).  I'm planning on overlooking the .c file and focusing
on the user interface (I'd rather leave the detailed review to those
more familiar with qemu, while I'm personally worried about how libvirt
would ever use libqblock if you ever solved the glib-aborts-on-OOM issue
to make the library even worth using).

> +++ b/libqblock/libqblock.h
> @@ -0,0 +1,292 @@
> +/*
> + * 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"

Even worse - you've introduced a public header that I'm supposed to be
able to use, but I can't use it because libqblock-types.h and
libqblock-error.h don't exist.  I'd much rather review a patch series
built incrementally from ground up, with each piece working, rather than
reviewing an API that depends on constructs that aren't defined until
later patches.

> +/**
> + * qb_broker_new: allocate a new broker
> + *
> + * Broker is used to pass operation to libqblock, and get feed back from it.

s/feed back/feedback/

> +/**
> + * qb_state_delete: free a QBlockState struct
> + *
> + * if image was opened, qb_close must be called before delete.

And if it wasn't closed, what happens?  Should this function return int,
instead of void, to error out in the case that it is called out of order?

> +/**
> + * qb_prot_info_new: create a new struct QBlockProtInfo.

Inconsistent on whether your descriptions end in '.' or not.

> +/* sync access */
> +/**
> + * qb_read: block sync read.
> + *
> + * return number of bytes read, libqblock negative error value on fail.
> + *
> + * @broker: operation broker.
> + * @qbs: pointer to struct QBlockState.
> + * @buf: buffer that receive the content.
> + * @len: length to read.
> + * @offset: offset in the block data.
> + */
> +DLL_PUBLIC
> +int64_t qb_read(struct QBroker *broker,
> +                struct QBlockState *qbs,
> +                uint8_t *buf,
> +                uint32_t len,
> +                uint64_t offset);

Seems odd to have 32 bit limit on input, when output handles 64 bit.

> +
> +/**
> + * qb_write: block sync write.
> + *
> + * return number of bytes written, libqblock negative error value on fail.
> + *
> + * @broker: operation broker.
> + * @qbs: pointer to struct QBlockState.
> + * @buf: buffer that receive the content.
> + * @len: length to write.
> + * @offset: offset in the block data.
> + */
> +DLL_PUBLIC
> +int64_t qb_write(struct QBroker *broker,
> +                 struct QBlockState *qbs,
> +                 const uint8_t *buf,
> +                 uint32_t len,
> +                 uint64_t offset);

and again.

> +/* 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.
> + *
> + * @broker: operation broker.
> + * @qbs: pointer to struct QBlockState.
> + * @start: start position, unit is byte.
> + * @length: length to check, unit is byte.

Needs to be at least int32_t to match your read/write; or better yet
int64_t to allow probes of more than 2G at a time.

> + * @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(struct QBroker *broker,
> +                        struct QBlockState *qbs,
> +                        uint64_t start,
> +                        int length,
> +                        int *pstatus,
> +                        int *plength);

If you change the type of length, then plength needs to match.

> +/**
> + * qb_fmttype2str: libqblock format enum type to a string.
> + *
> + * return a pointer to the string, or NULL if type is not supported, and
> + *  returned pointer do NOT need to free.

grammar; I suggest:
returned pointer must not be freed

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]

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

* Re: [Qemu-devel] [PATCH V2 2/6] libqblock type and structure defines
  2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 2/6] libqblock type and structure defines Wenchao Xia
@ 2012-09-10 21:27   ` Eric Blake
  2012-09-11  3:26     ` Wenchao Xia
  2012-09-11 20:31   ` Blue Swirl
  1 sibling, 1 reply; 34+ messages in thread
From: Eric Blake @ 2012-09-10 21:27 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: kwolf, aliguori, stefanha, qemu-devel, blauwirbel, pbonzini

[-- Attachment #1: Type: text/plain, Size: 2246 bytes --]

On 09/10/2012 02:26 AM, Wenchao Xia wrote:
>   This patch contains type and defines used in APIs, one file for public usage
> by user, one for libqblock internal usage.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  libqblock/libqblock-internal.h |   50 ++++++++
>  libqblock/libqblock-types.h    |  251 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 301 insertions(+), 0 deletions(-)
>  create mode 100644 libqblock/libqblock-internal.h
>  create mode 100644 libqblock/libqblock-types.h
> 

As mentioned in 1/6, this should come earlier in the series, as it lays
the fundamentals used by other new files.

> diff --git a/libqblock/libqblock-internal.h b/libqblock/libqblock-internal.h
> new file mode 100644
> index 0000000..fa27ed4
> --- /dev/null
> +++ b/libqblock/libqblock-internal.h
> @@ -0,0 +1,50 @@

> +
> +#define QB_ERR_STRING_SIZE (1024)
> +struct QBroker {
> +    /* last error */
> +    char err_msg[QB_ERR_STRING_SIZE];

Is this fixed-width struct going to bite us in the future?  Suppose I
pass in a file name that is already 1000 bytes long; it seems like I
might be able to get you to overflow this buffer if your error message
includes the name of my offending file.

> +++ b/libqblock/libqblock-types.h
> +/* this library is designed around this core struct. */
> +struct QBlockState;
> +
> +/* every thread would have a broker. */

s/would/should/

> +
> +/**
> + * QBlockStaticInfo: information about the block image.
> + *
> + * @loc: location info.
> + * @fmt_type: format type.
> + * @virt_size: virtual size in bytes.
> + * @backing_loc: backing file location, its type is QB_PROT_NONE if not exist.
> + * @encrypt: encrypt flag.
> + * @sector_size: how many bytes in a sector, it is 512 usually.
> + */
> +struct QBlockStaticInfo {
> +    struct QBlockProtInfo loc;
> +    enum QBlockFmtType fmt_type;
> +    uint64_t virt_size;
> +    /* advance info */
> +    struct QBlockProtInfo backing_loc;
> +    bool encrypt;
> +    int sector_size;
> +};

No reserved space for potential growth in this structure?

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]

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

* Re: [Qemu-devel] [PATCH V2 3/6] libqblock error handling
  2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 3/6] libqblock error handling Wenchao Xia
@ 2012-09-10 21:33   ` Eric Blake
  2012-09-11  4:36     ` Wenchao Xia
  2012-09-11 20:32   ` Blue Swirl
  1 sibling, 1 reply; 34+ messages in thread
From: Eric Blake @ 2012-09-10 21:33 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: kwolf, aliguori, stefanha, qemu-devel, blauwirbel, pbonzini

[-- Attachment #1: Type: text/plain, Size: 1291 bytes --]

On 09/10/2012 02:26 AM, Wenchao Xia wrote:
>   This patch contains error handling APIs, which user could call them to
> get error details.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  libqblock/libqblock-error.c |   60 +++++++++++++++++++++++++++++++++++++++++++
>  libqblock/libqblock-error.h |   50 +++++++++++++++++++++++++++++++++++
>  2 files changed, 110 insertions(+), 0 deletions(-)
>  create mode 100644 libqblock/libqblock-error.c
>  create mode 100644 libqblock/libqblock-error.h

Again, this should come earlier in the series, and I'm focusing on the
.h as a potential user, rather than on the .c.

> +    default:
> +        err_ret_str = "Unknow error.";

s/Unknow/Unknown/

> +++ b/libqblock/libqblock-error.h
> +/**
> + * qb_error_get_errno: get error number, only valid when err_ret is
> + *   QB_ERR_INTERNAL_ERR.
> + *
> + * return negative errno or 0 if last error is not QB_ERR_INTERNAL_ERR.

So does this return EINVAL or -EINVAL?  If you return positive errno
values, then you can reserve 0 for no error, and a QB_*-specific
negative value in the case where QB_ERR_INTERNAL_ERR was not the last error.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]

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

* Re: [Qemu-devel] [PATCH V2 1/6] libqblock API design
  2012-09-10 21:07   ` Eric Blake
@ 2012-09-11  3:16     ` Wenchao Xia
  2012-09-14  2:03       ` Wenchao Xia
  0 siblings, 1 reply; 34+ messages in thread
From: Wenchao Xia @ 2012-09-11  3:16 UTC (permalink / raw)
  To: Eric Blake; +Cc: kwolf, aliguori, stefanha, qemu-devel, blauwirbel, pbonzini

于 2012-9-11 5:07, Eric Blake 写道:
> On 09/10/2012 02:26 AM, Wenchao Xia wrote:
>>    This patch contains the major APIs in the library.
>> Important APIs:
>>    1 QBroker. These structure was used to retrieve errors, every thread must
>> create one first, later maybe thread related staff could be added into it.
>>    2 QBlockState. It stands for an block image object.
>>    3 QBlockStaticInfo. It contains static information such as location, backing
>> file, size.
>>    4 ABI was kept with reserved members.
>>    5 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.c | 1077 +++++++++++++++++++++++++++++++++++++++++++++++++
>>   libqblock/libqblock.h |  292 +++++++++++++
>
> Two new files, but no build machinery to build them to see if they have
> blatant errors.  Yes, it is bisectable, but no, the bisection is not
> useful.  I'd rather see the Makefile patches with stub files at the
> beginning, then flesh out the stubs, where each patch builds along the way.
>
> In particular,
>
   OK, I understand the importance to make each patch workable to
maintainer, will adjust the patches to make each one works.

>> +++ b/libqblock/libqblock.c
>
>> +#include "libqblock.h"
>> +#include "libqblock-internal.h"
>
> There is no libqblock-internal.h with just this patch applied, making it
> impossible to validate this patch in order (the fact that 'make' isn't
> flagging this incomplete patch is because you aren't building
> libqblock-o yet).  I'm planning on overlooking the .c file and focusing
> on the user interface (I'd rather leave the detailed review to those
> more familiar with qemu, while I'm personally worried about how libvirt
> would ever use libqblock if you ever solved the glib-aborts-on-OOM issue
> to make the library even worth using).
>
   About the OOM issue, I think there are potential 3 ways to solve it:
1 modify qemu block layer code, in this way providing libqblock at first
will help, for that it will encapsulate and isolate all codes needed
for block layer, and we got test case on the top to validate OOM
behavior.
2 Using glib and forgot OOM now. Then at higher layer, they have two
choice: if they want no exiting on OOM, wrap the API with xmlrpc or
something like; otherwise directly use the API.
3 switch the implemention of libqblock, do not link qemu block code
directly, fork and execute qemu-img, qemu-nbd. This require a
re-implement about AIO in libqblock, with GSource AIO framework I am
not sure if it would exit on OOM, but I guess better to not involve any
glib in this case. Additional challenge would be, any more
functionalities adding require patch for qemu-img, qemu-io, qemu-nbd
first, such as image information retrieving, allocation detection,
and libqblock need to parse string output carefully, better to get
all output in json format. Personally I am worried to handle many
exception in string parsing.

   To me, the second way seems most reasonable, it allows qemu block
remain tightly bind to glib. The first way also make sense, but
need to loose the tight between qemu and glib. what do you think?

>> +++ b/libqblock/libqblock.h
>> @@ -0,0 +1,292 @@
>> +/*
>> + * 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"
>
> Even worse - you've introduced a public header that I'm supposed to be
> able to use, but I can't use it because libqblock-types.h and
> libqblock-error.h don't exist.  I'd much rather review a patch series
> built incrementally from ground up, with each piece working, rather than
> reviewing an API that depends on constructs that aren't defined until
> later patches.
>
>> +/**
>> + * qb_broker_new: allocate a new broker
>> + *
>> + * Broker is used to pass operation to libqblock, and get feed back from it.
>
> s/feed back/feedback/
>
>> +/**
>> + * qb_state_delete: free a QBlockState struct
>> + *
>> + * if image was opened, qb_close must be called before delete.
>
> And if it wasn't closed, what happens?  Should this function return int,
> instead of void, to error out in the case that it is called out of order?
>
>> +/**
>> + * qb_prot_info_new: create a new struct QBlockProtInfo.
>
> Inconsistent on whether your descriptions end in '.' or not.
>
>> +/* sync access */
>> +/**
>> + * qb_read: block sync read.
>> + *
>> + * return number of bytes read, libqblock negative error value on fail.
>> + *
>> + * @broker: operation broker.
>> + * @qbs: pointer to struct QBlockState.
>> + * @buf: buffer that receive the content.
>> + * @len: length to read.
>> + * @offset: offset in the block data.
>> + */
>> +DLL_PUBLIC
>> +int64_t qb_read(struct QBroker *broker,
>> +                struct QBlockState *qbs,
>> +                uint8_t *buf,
>> +                uint32_t len,
>> +                uint64_t offset);
>
> Seems odd to have 32 bit limit on input, when output handles 64 bit.
>
>> +
>> +/**
>> + * qb_write: block sync write.
>> + *
>> + * return number of bytes written, libqblock negative error value on fail.
>> + *
>> + * @broker: operation broker.
>> + * @qbs: pointer to struct QBlockState.
>> + * @buf: buffer that receive the content.
>> + * @len: length to write.
>> + * @offset: offset in the block data.
>> + */
>> +DLL_PUBLIC
>> +int64_t qb_write(struct QBroker *broker,
>> +                 struct QBlockState *qbs,
>> +                 const uint8_t *buf,
>> +                 uint32_t len,
>> +                 uint64_t offset);
>
> and again.
>
>> +/* 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.
>> + *
>> + * @broker: operation broker.
>> + * @qbs: pointer to struct QBlockState.
>> + * @start: start position, unit is byte.
>> + * @length: length to check, unit is byte.
>
> Needs to be at least int32_t to match your read/write; or better yet
> int64_t to allow probes of more than 2G at a time.
>
>> + * @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(struct QBroker *broker,
>> +                        struct QBlockState *qbs,
>> +                        uint64_t start,
>> +                        int length,
>> +                        int *pstatus,
>> +                        int *plength);
>
> If you change the type of length, then plength needs to match.
>
>> +/**
>> + * qb_fmttype2str: libqblock format enum type to a string.
>> + *
>> + * return a pointer to the string, or NULL if type is not supported, and
>> + *  returned pointer do NOT need to free.
>
> grammar; I suggest:
> returned pointer must not be freed
>


-- 
Best Regards

Wenchao Xia

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

* Re: [Qemu-devel] [PATCH V2 2/6] libqblock type and structure defines
  2012-09-10 21:27   ` Eric Blake
@ 2012-09-11  3:26     ` Wenchao Xia
  2012-09-11  4:12       ` Eric Blake
  0 siblings, 1 reply; 34+ messages in thread
From: Wenchao Xia @ 2012-09-11  3:26 UTC (permalink / raw)
  To: Eric Blake; +Cc: kwolf, aliguori, stefanha, qemu-devel, blauwirbel, pbonzini

于 2012-9-11 5:27, Eric Blake 写道:
> On 09/10/2012 02:26 AM, Wenchao Xia wrote:
>>    This patch contains type and defines used in APIs, one file for public usage
>> by user, one for libqblock internal usage.
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>>   libqblock/libqblock-internal.h |   50 ++++++++
>>   libqblock/libqblock-types.h    |  251 ++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 301 insertions(+), 0 deletions(-)
>>   create mode 100644 libqblock/libqblock-internal.h
>>   create mode 100644 libqblock/libqblock-types.h
>>
>
> As mentioned in 1/6, this should come earlier in the series, as it lays
> the fundamentals used by other new files.
>
   OK.

>> diff --git a/libqblock/libqblock-internal.h b/libqblock/libqblock-internal.h
>> new file mode 100644
>> index 0000000..fa27ed4
>> --- /dev/null
>> +++ b/libqblock/libqblock-internal.h
>> @@ -0,0 +1,50 @@
>
>> +
>> +#define QB_ERR_STRING_SIZE (1024)
>> +struct QBroker {
>> +    /* last error */
>> +    char err_msg[QB_ERR_STRING_SIZE];
>
> Is this fixed-width struct going to bite us in the future?  Suppose I
> pass in a file name that is already 1000 bytes long; it seems like I
> might be able to get you to overflow this buffer if your error message
> includes the name of my offending file.
>
   Yes it will, thanks for mention me. The resource will always have a
limit, I guess I can just increase the size to 4k to solve the issue.

>> +++ b/libqblock/libqblock-types.h
>> +/* this library is designed around this core struct. */
>> +struct QBlockState;
>> +
>> +/* every thread would have a broker. */
>
> s/would/should/
>
  OK.

>> +
>> +/**
>> + * QBlockStaticInfo: information about the block image.
>> + *
>> + * @loc: location info.
>> + * @fmt_type: format type.
>> + * @virt_size: virtual size in bytes.
>> + * @backing_loc: backing file location, its type is QB_PROT_NONE if not exist.
>> + * @encrypt: encrypt flag.
>> + * @sector_size: how many bytes in a sector, it is 512 usually.
>> + */
>> +struct QBlockStaticInfo {
>> +    struct QBlockProtInfo loc;
>> +    enum QBlockFmtType fmt_type;
>> +    uint64_t virt_size;
>> +    /* advance info */
>> +    struct QBlockProtInfo backing_loc;
>> +    bool encrypt;
>> +    int sector_size;
>> +};
>
> No reserved space for potential growth in this structure?
>
   It could have or not. If new members are added in the tail, then
reserved bytes is not needed, for that this structure's instance
will be allocated in libqblock API.


-- 
Best Regards

Wenchao Xia

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

* Re: [Qemu-devel] [PATCH V2 2/6] libqblock type and structure defines
  2012-09-11  3:26     ` Wenchao Xia
@ 2012-09-11  4:12       ` Eric Blake
  0 siblings, 0 replies; 34+ messages in thread
From: Eric Blake @ 2012-09-11  4:12 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: kwolf, aliguori, stefanha, qemu-devel, blauwirbel, pbonzini

[-- Attachment #1: Type: text/plain, Size: 1029 bytes --]

On 09/10/2012 09:26 PM, Wenchao Xia wrote:

>>> +#define QB_ERR_STRING_SIZE (1024)
>>> +struct QBroker {
>>> +    /* last error */
>>> +    char err_msg[QB_ERR_STRING_SIZE];
>>
>> Is this fixed-width struct going to bite us in the future?  Suppose I
>> pass in a file name that is already 1000 bytes long; it seems like I
>> might be able to get you to overflow this buffer if your error message
>> includes the name of my offending file.
>>
>   Yes it will, thanks for mention me. The resource will always have a
> limit, I guess I can just increase the size to 4k to solve the issue.

A 4k limit is still an easily reachable limit.  PATH_MAX is typically
4k, and it is quite possible to create and access files in a hierarchy
so deep that they are longer than PATH_MAX.  I still think you are
better off malloc'ing a pointer than trying to claim a fixed width field
solves all possible messages.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]

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

* Re: [Qemu-devel] [PATCH V2 3/6] libqblock error handling
  2012-09-10 21:33   ` Eric Blake
@ 2012-09-11  4:36     ` Wenchao Xia
  0 siblings, 0 replies; 34+ messages in thread
From: Wenchao Xia @ 2012-09-11  4:36 UTC (permalink / raw)
  To: Eric Blake; +Cc: kwolf, aliguori, stefanha, qemu-devel, blauwirbel, pbonzini

于 2012-9-11 5:33, Eric Blake 写道:
> On 09/10/2012 02:26 AM, Wenchao Xia wrote:
>>    This patch contains error handling APIs, which user could call them to
>> get error details.
>>
OK.

>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>>   libqblock/libqblock-error.c |   60 +++++++++++++++++++++++++++++++++++++++++++
>>   libqblock/libqblock-error.h |   50 +++++++++++++++++++++++++++++++++++
>>   2 files changed, 110 insertions(+), 0 deletions(-)
>>   create mode 100644 libqblock/libqblock-error.c
>>   create mode 100644 libqblock/libqblock-error.h
>
> Again, this should come earlier in the series, and I'm focusing on the
> .h as a potential user, rather than on the .c.
>
>> +    default:
>> +        err_ret_str = "Unknow error.";
>
> s/Unknow/Unknown/
>
   OK, will fix it.

>> +++ b/libqblock/libqblock-error.h
>> +/**
>> + * qb_error_get_errno: get error number, only valid when err_ret is
>> + *   QB_ERR_INTERNAL_ERR.
>> + *
>> + * return negative errno or 0 if last error is not QB_ERR_INTERNAL_ERR.
>
> So does this return EINVAL or -EINVAL?  If you return positive errno
> values, then you can reserve 0 for no error, and a QB_*-specific
> negative value in the case where QB_ERR_INTERNAL_ERR was not the last error.
>
   I think better to split these kind of errors, they are two level of
errors, level one is libqblock's error, and errno is 2nd level of error,
belong to one kind of level 1 error, QB_ERR_INTERNAL_ERR.


-- 
Best Regards

Wenchao Xia

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

* Re: [Qemu-devel] [PATCH V2 1/6] libqblock API design
  2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 1/6] libqblock API design Wenchao Xia
  2012-09-10 21:07   ` Eric Blake
@ 2012-09-11 20:28   ` Blue Swirl
  2012-09-12  2:54     ` Wenchao Xia
  2012-09-12  8:19     ` Kevin Wolf
  1 sibling, 2 replies; 34+ messages in thread
From: Blue Swirl @ 2012-09-11 20:28 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: kwolf, aliguori, stefanha, qemu-devel, pbonzini, eblake

On Mon, Sep 10, 2012 at 8:26 AM, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>   This patch contains the major APIs in the library.
> Important APIs:
>   1 QBroker. These structure was used to retrieve errors, every thread must
> create one first, later maybe thread related staff could be added into it.
>   2 QBlockState. It stands for an block image object.
>   3 QBlockStaticInfo. It contains static information such as location, backing
> file, size.
>   4 ABI was kept with reserved members.
>   5 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.c | 1077 +++++++++++++++++++++++++++++++++++++++++++++++++
>  libqblock/libqblock.h |  292 +++++++++++++
>  2 files changed, 1369 insertions(+), 0 deletions(-)
>  create mode 100644 libqblock/libqblock.c
>  create mode 100644 libqblock/libqblock.h
>
> diff --git a/libqblock/libqblock.c b/libqblock/libqblock.c
> new file mode 100644
> index 0000000..133ac0f
> --- /dev/null
> +++ b/libqblock/libqblock.c
> @@ -0,0 +1,1077 @@
> +/*
> + * 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"
> +
> +struct LibqblockGlobalData {
> +    int init_flag;
> +};
> +
> +struct LibqblockGlobalData g_libqblock_data;
> +
> +__attribute__((constructor))
> +static void libqblock_init(void)
> +{
> +    if (g_libqblock_data.init_flag == 0) {
> +        bdrv_init();
> +        qemu_init_main_loop();
> +    }
> +    g_libqblock_data.init_flag = 1;
> +}
> +
> +const char *qb_fmttype2str(enum QBlockFmtType fmt_type)
> +{
> +    const char *ret = NULL;
> +    switch (fmt_type) {
> +    case QB_FMT_COW:
> +        ret = "cow";
> +        break;
> +    case QB_FMT_QED:
> +        ret = "qed";
> +        break;
> +    case QB_FMT_QCOW:
> +        ret = "qcow";
> +        break;
> +    case QB_FMT_QCOW2:
> +        ret = "qcow2";
> +        break;
> +    case QB_FMT_RAW:
> +        ret = "raw";
> +        break;
> +    case QB_FMT_RBD:
> +        ret = "rbd";
> +        break;
> +    case QB_FMT_SHEEPDOG:
> +        ret = "sheepdog";
> +        break;
> +    case QB_FMT_VDI:
> +        ret = "vdi";
> +        break;
> +    case QB_FMT_VMDK:
> +        ret = "vmdk";
> +        break;
> +    case QB_FMT_VPC:
> +        ret = "vpc";
> +        break;
> +    default:
> +        break;
> +    }
> +    return ret;
> +}
> +
> +enum QBlockFmtType qb_str2fmttype(const char *fmt_str)
> +{
> +    enum QBlockFmtType ret = QB_FMT_NONE;
> +    if (0 == strcmp(fmt_str, "cow")) {

This order is not common in QEMU.

> +        ret = QB_FMT_COW;
> +    } else if (0 == strcmp(fmt_str, "qed")) {
> +        ret = QB_FMT_QED;
> +    } else if (0 == strcmp(fmt_str, "qcow")) {
> +        ret = QB_FMT_QCOW;
> +    } else if (0 == strcmp(fmt_str, "qcow2")) {
> +        ret = QB_FMT_QCOW2;
> +    } else if (0 == strcmp(fmt_str, "raw")) {
> +        ret = QB_FMT_RAW;
> +    } else if (0 == strcmp(fmt_str, "rbd")) {
> +        ret = QB_FMT_RBD;
> +    } else if (0 == strcmp(fmt_str, "sheepdog")) {
> +        ret = QB_FMT_SHEEPDOG;
> +    } else if (0 == strcmp(fmt_str, "vdi")) {
> +        ret = QB_FMT_VDI;
> +    } else if (0 == strcmp(fmt_str, "vmdk")) {
> +        ret = QB_FMT_VMDK;
> +    } else if (0 == strcmp(fmt_str, "vpc")) {
> +        ret = QB_FMT_VPC;
> +    }
> +    return ret;
> +}
> +
> +static void set_broker_err(struct QBroker *broker, int err_ret,
> +                           const char *fmt, ...)
> +{
> +    va_list ap;
> +
> +    broker->err_ret = err_ret;
> +    if (err_ret == QB_ERR_INTERNAL_ERR) {
> +        broker->err_no = -errno;
> +    } else {
> +        broker->err_no = 0;
> +    }
> +
> +    va_start(ap, fmt);
> +    vsnprintf(broker->err_msg, sizeof(broker->err_msg), fmt, ap);
> +    va_end(ap);
> +}
> +
> +static void set_broker_err_nomem(struct QBroker *broker)
> +{
> +    set_broker_err(broker, QB_ERR_MEM_ERR, "No Memory.");
> +}
> +
> +int qb_broker_new(struct QBroker **broker)
> +{
> +    *broker = FUNC_CALLOC(1, sizeof(struct QBroker));
> +    if (*broker == NULL) {
> +        return QB_ERR_MEM_ERR;
> +    }
> +    return 0;
> +}
> +
> +void qb_broker_delete(struct QBroker **broker)
> +{
> +    CLEAN_FREE(*broker);
> +    return;
> +}
> +
> +int qb_state_new(struct QBroker *broker,
> +                 struct QBlockState **qbs)
> +{
> +    *qbs = FUNC_CALLOC(1, sizeof(struct QBlockState));
> +    if (*qbs == NULL) {
> +        set_broker_err_nomem(broker);
> +        return broker->err_ret;
> +    }
> +    (*qbs)->bdrvs = bdrv_new("hda");
> +    if ((*qbs)->bdrvs == NULL) {
> +        CLEAN_FREE(*qbs);
> +        set_broker_err(broker, QB_ERR_INTERNAL_ERR,
> +                       "failed to create the driver.");
> +        return broker->err_ret;
> +    }
> +    return 0;
> +}
> +
> +void qb_state_delete(struct QBroker *broker,
> +                     struct QBlockState **qbs)
> +{
> +    CLEAN_FREE((*qbs)->filename);
> +    if ((*qbs)->bdrvs != NULL) {
> +        bdrv_delete((*qbs)->bdrvs);
> +        (*qbs)->bdrvs = NULL;
> +    }
> +    CLEAN_FREE(*qbs);
> +    return;
> +}
> +
> +int qb_prot_info_new(struct QBroker *broker,
> +                     struct QBlockProtInfo **op)
> +{
> +    *op = FUNC_CALLOC(1, sizeof(struct QBlockProtInfo));
> +    if (*op == NULL) {
> +        set_broker_err_nomem(broker);
> +        return broker->err_ret;
> +    }
> +    return 0;
> +}
> +
> +void qb_prot_info_delete(struct QBroker *broker,
> +                         struct QBlockProtInfo **op)
> +{
> +    CLEAN_FREE(*op);
> +}
> +
> +int qb_fmt_info_new(struct QBroker *broker,
> +                    struct QBlockFmtInfo **op)
> +{
> +    *op = FUNC_CALLOC(1, sizeof(struct QBlockFmtInfo));
> +    if (*op == NULL) {
> +        set_broker_err_nomem(broker);
> +        return broker->err_ret;
> +    }
> +    return 0;
> +}
> +
> +void qb_fmt_info_delete(struct QBroker *broker,
> +                        struct QBlockFmtInfo **op)
> +{
> +    CLEAN_FREE(*op);
> +}
> +
> +/* return 0 if every thing is fine */
> +static int loc_check_params(struct QBroker *broker,
> +                            struct QBlockProtInfo *loc)
> +{
> +    broker->err_ret = 0;
> +
> +    switch (loc->prot_type) {
> +    case QB_PROT_FILE:
> +        if (loc->prot_op.o_file.filename == NULL) {
> +            set_broker_err(broker, QB_ERR_INVALID_PARAM,
> +                           "Filename was not set.");
> +            goto out;
> +        }
> +        if (path_has_protocol(loc->prot_op.o_file.filename) > 0) {
> +            set_broker_err(broker, QB_ERR_INVALID_PARAM,
> +                           "filename [%s] had protocol.",
> +                           loc->prot_op.o_file.filename);
> +            goto out;
> +        }
> +        break;
> +    default:
> +        set_broker_err(broker, QB_ERR_INVALID_PARAM,
> +                       "Protocol type [%d] was not valid.",
> +                       loc->prot_type);
> +        break;
> +    }
> +
> + out:
> +    return broker->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(struct QBroker *broker,
> +                        struct QBlockProtInfo *loc,
> +                        char **filename)
> +{
> +    broker->err_ret = 0;
> +
> +    if (*filename != NULL) {
> +        CLEAN_FREE(*filename);
> +    }
> +    switch (loc->prot_type) {
> +    case QB_PROT_FILE:
> +        *filename = strdup(loc->prot_op.o_file.filename);
> +        if (*filename == NULL) {
> +            set_broker_err_nomem(broker);
> +        }
> +        break;
> +    default:
> +        set_broker_err(broker, QB_ERR_INVALID_PARAM,
> +                 "protocol type [%d] is not supported.",
> +                  loc->prot_type);
> +        break;
> +    }
> +
> +    return broker->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(struct QBroker *broker,
> +                        struct QBlockProtInfo *loc,
> +                        const char *filename)
> +{
> +    broker->err_ret = 0;
> +
> +    if (path_has_protocol(filename) > 0) {
> +        set_broker_err(broker, QB_ERR_INVALID_PARAM,
> +                     "Filename [%s] had protocol, not supported now.",
> +                     filename);
> +        goto out;
> +    }
> +
> +    loc->prot_type = QB_PROT_FILE;
> +    switch (loc->prot_type) {
> +    case QB_PROT_FILE:
> +        loc->prot_op.o_file.filename = strdup(filename);
> +        if (loc->prot_op.o_file.filename == NULL) {
> +            set_broker_err_nomem(broker);
> +            goto out;
> +        }
> +        break;
> +    default:
> +        break;
> +    }
> +
> + out:
> +    return broker->err_ret;
> +}
> +
> +/* return 0 if OK, or qblock error number */
> +static int set_backing_file_options(struct QBroker *broker,
> +                                    QEMUOptionParameter *param,
> +                                    struct QBlockProtInfo *loc,
> +                                    enum QBlockFmtType *fmt)
> +{
> +    char *backing_filename = NULL;
> +    const char *fmtstr_backing = NULL;
> +    int ret = 0;
> +
> +    if (loc == NULL) {
> +        goto out;
> +    }
> +
> +    ret = loc2filename(broker, loc, &backing_filename);
> +    /* ret can < 0 if loc have not been set, mean user did not specify backing
> +       file */
> +    if (ret == QB_ERR_MEM_ERR) {
> +        goto out;
> +    }
> +    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(struct QBroker *broker,
> +              struct QBlockState *qbs,
> +              struct QBlockProtInfo *loc,
> +              struct QBlockFmtInfo *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;
> +    struct QBlockFmtOptionCow *o_cow = NULL;
> +    struct QBlockFmtOptionQed *o_qed = NULL;
> +    struct QBlockFmtOptionQcow *o_qcow = NULL;
> +    struct QBlockFmtOptionQcow2 *o_qcow2 = NULL;
> +    struct QBlockFmtOptionRaw *o_raw = NULL;
> +    struct QBlockFmtOptionRbd *o_rbd = NULL;
> +    struct QBlockFmtOptionSheepdog *o_sd = NULL;
> +    struct QBlockFmtOptionVdi *o_vdi = NULL;
> +    struct QBlockFmtOptionVmdk *o_vmdk = NULL;
> +    struct QBlockFmtOptionVpc *o_vpc = NULL;
> +
> +
> +    /* check parameters */
> +    if (flag & (~LIBQBLOCK_O_VALID_MASK)) {
> +        set_broker_err(broker, QB_ERR_INVALID_PARAM,
> +                           "invalid flag was set.");
> +        ret = broker->err_ret;
> +        goto out;
> +    }
> +
> +    if ((loc == NULL) || (qbs == NULL) || (fmt == NULL)) {
> +        set_broker_err(broker, QB_ERR_INVALID_PARAM,
> +                          "Got unexpected NULL pointer in parameters.");
> +        ret = broker->err_ret;
> +        goto out;
> +    }
> +
> +    ret = loc_check_params(broker, loc);
> +    if (ret != 0) {
> +        goto out;
> +    }
> +
> +    /* internal translate */
> +    ret = loc2filename(broker, loc, &filename);
> +    if (ret != 0) {
> +        goto out;
> +    }
> +
> +    fmtstr = qb_fmttype2str(fmt->fmt_type);
> +    if (fmtstr == NULL) {
> +        set_broker_err(broker, QB_ERR_INVALID_PARAM,
> +                 "Got unexpected NULL pointer in parameters.");
> +        ret = broker->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->fmt_op.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(broker, param,
> +                                       &o_cow->backing_loc, NULL);
> +        if (ret != 0) {
> +            goto out;
> +        }
> +        break;
> +    case QB_FMT_QED:
> +        o_qed = &(fmt->fmt_op.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(broker, 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->fmt_op.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(broker, 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->fmt_op.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(broker, 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 != QBO_FMT_QCOW2_CPT_NONE) {
> +            tmp = o_qcow2->cpt_lv == QBO_FMT_QCOW2_CPT_V010 ? "0.10" : "1.1";
> +            bd_ret = set_option_parameter(param,
> +                              BLOCK_OPT_COMPAT_LEVEL, tmp);
> +            assert(bd_ret == 0);
> +        }
> +
> +        if (o_qcow2->pre_mode != QBO_FMT_QCOW2_PREALLOC_NONE) {
> +            tmp = o_qcow2->pre_mode == QBO_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->fmt_op.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->fmt_op.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->fmt_op.o_sheepdog);
> +        bd_ret = set_option_parameter_int(param,
> +                              BLOCK_OPT_SIZE, o_sd->virt_size);
> +        assert(bd_ret == 0);
> +        ret = set_backing_file_options(broker, param,
> +                                       &o_sd->backing_loc, NULL);
> +        if (ret != 0) {
> +            goto out;
> +        }
> +        if (o_sd->pre_mode != QBO_FMT_SD_PREALLOC_NONE) {
> +            tmp = o_sd->pre_mode == QBO_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->fmt_op.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 != QBO_FMT_VDI_PREALLOC_NONE) {
> +            tmp_bool = o_sd->pre_mode == QBO_FMT_VDI_PREALLOC_TRUE ?
> +                                                     true : false;
> +            set_option_parameter_int(param, "static", tmp_bool);
> +        }
> +        break;
> +    case QB_FMT_VMDK:
> +        o_vmdk = &(fmt->fmt_op.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(broker, param,
> +                                       &o_vmdk->backing_loc, NULL);
> +        if (ret != 0) {
> +            goto out;
> +        }
> +
> +        if (o_vmdk->cpt_lv != QBO_FMT_VMDK_CPT_NONE) {
> +            tmp_bool = o_vmdk->cpt_lv == QBO_FMT_VMDK_CPT_VMDKV6_TRUE ?
> +                                                     true : false;
> +            bd_ret = set_option_parameter_int(param, BLOCK_OPT_COMPAT6,
> +                                                     tmp_bool);
> +            assert(bd_ret == 0);
> +        }
> +        if (o_vmdk->subfmt != QBO_FMT_VMDK_SUBFMT_MONOLITHIC_NONE) {
> +            switch (o_vmdk->subfmt) {
> +            case QBO_FMT_VMDK_SUBFMT_MONOLITHIC_SPARSE:
> +                tmp = "monolithicSparse";
> +                break;
> +            case QBO_FMT_VMDK_SUBFMT_MONOLITHIC_FLAT:
> +                tmp = "monolithicFlat";
> +                break;
> +            case QBO_FMT_VMDK_SUBFMT_TWOGBMAX_EXTENT_SPARSE:
> +                tmp = "twoGbMaxExtentSparse";
> +                break;
> +            case QBO_FMT_VMDK_SUBFMT_TWOGBMAX_EXTENT_FLAT:
> +                tmp = "twoGbMaxExtentFlat";
> +                break;
> +            case QBO_FMT_VMDK_SUBFMT_STREAM_OPTIMIZED:
> +                tmp = "streamOptimized";
> +                break;
> +            default:
> +                assert(false);
> +                break;
> +            }
> +            bd_ret = set_option_parameter(param,
> +                              BLOCK_OPT_SUBFMT, tmp);
> +            assert(bd_ret == 0);
> +        }
> +        break;
> +    case QB_FMT_VPC:
> +        o_vpc = &(fmt->fmt_op.o_vpc);
> +        bd_ret = set_option_parameter_int(param,
> +                               BLOCK_OPT_SIZE, o_vpc->virt_size);
> +        assert(bd_ret == 0);
> +        if (o_vpc->subfmt != QBO_FMT_VPC_SUBFMT_NONE) {
> +            tmp = o_vpc->subfmt == QBO_FMT_VPC_SUBFMT_DYNAMIC ?
> +                                               "dynamic" : "fixed";
> +            bd_ret = set_option_parameter(param,
> +                               BLOCK_OPT_SUBFMT, tmp);
> +            assert(bd_ret == 0);
> +        }
> +        break;
> +    default:
> +        assert(false);

abort()

> +        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_broker_err(broker, QB_ERR_INVALID_PARAM,
> +                          "Backing file is the same with new file.");
> +            ret = broker->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_broker_err(broker, QB_ERR_INVALID_PARAM,
> +                               "Failed to open the backing file.");
> +                ret = broker->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_broker_err(broker, QB_ERR_INTERNAL_ERR,
> +                           "Neither size or backing file was not set.");
> +            ret = broker->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_broker_err(broker, QB_ERR_INTERNAL_ERR, errstr);
> +        ret = broker->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(struct QBroker *broker,
> +            struct QBlockState *qbs,
> +            struct QBlockProtInfo *loc,
> +            struct QBlockFmtInfo *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_broker_err(broker, QB_ERR_INVALID_PARAM,
> +                      "Invalid flag was set.");
> +        ret = broker->err_ret;
> +        goto out;
> +    }
> +
> +    if ((loc == NULL) || (qbs == NULL)) {
> +        set_broker_err(broker, QB_ERR_INVALID_PARAM,
> +                      "Got unexpected NULL pointer in parameters.");
> +        ret = broker->err_ret;
> +        goto out;
> +    }
> +
> +    ret = loc_check_params(broker, loc);
> +    if (ret != 0) {
> +        goto out;
> +    }
> +
> +    /* internal translate */
> +    ret = loc2filename(broker, 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 openning */

opening

> +    bs = qbs->bdrvs;
> +    bd_ret = bdrv_open(bs, filename, flag, bd);
> +    if (bd_ret < 0) {
> +        set_broker_err(broker, QB_ERR_INTERNAL_ERR,
> +                      "Failed in opening with driver, bd_ret is %d.", bd_ret);
> +        ret = broker->err_ret;
> +        goto out;
> +    }
> +
> +    if (qbs->filename != NULL) {
> +        FUNC_FREE(qbs->filename);
> +    }
> +    qbs->filename = strdup(filename);
> +    if (qbs->filename == NULL) {
> +        set_broker_err_nomem(broker);
> +        ret = broker->err_ret;
> +        bdrv_close(bs);
> +        goto out;
> +    }
> +
> + out:
> +    FUNC_FREE(filename);
> +    return ret;
> +}
> +
> +void qb_close(struct QBroker *broker,
> +              struct QBlockState *qbs)
> +{
> +    BlockDriverState *bs;
> +
> +    bs = qbs->bdrvs;
> +
> +    if (qbs->filename != NULL) {
> +        CLEAN_FREE(qbs->filename);
> +        bdrv_close(bs);
> +    }
> +    return;
> +}
> +
> +int64_t qb_read(struct QBroker *broker,
> +                struct 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;
> +    int remains;
> +
> +    broker->err_ret = 0;
> +    bs = qbs->bdrvs;
> +
> +    if (len <= 0) {
> +        set_broker_err(broker, QB_ERR_INVALID_PARAM,
> +                      "Param len is less or equal to zero.");
> +        return broker->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 aligned, need to read/write this sector. */
> +        bd_ret = bdrv_read(bs, sector_start, temp_buf, 1);
> +        if (bd_ret < 0) {
> +            set_broker_err(broker, QB_ERR_INTERNAL_ERR,
> +                           "QEMU internal block error.");
> +            return broker->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 aligned. */
> +    if (remains >= BDRV_SECTOR_SIZE) {
> +        sector_num = len >> BDRV_SECTOR_BITS;
> +        bd_ret = bdrv_read(bs, sector_start, p, sector_num);
> +        if (bd_ret < 0) {
> +            set_broker_err(broker, QB_ERR_INTERNAL_ERR,
> +                           "QEMU internal block error.");
> +            return broker->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_broker_err(broker, QB_ERR_INTERNAL_ERR,
> +                           "QEMU internal block error.");
> +            return broker->err_ret;
> +        }
> +        memcpy(p, temp_buf, remains);
> +        remains -= remains;
> +    }
> +
> +    return len-remains;
> +}
> +
> +int64_t qb_write(struct QBroker *broker,
> +                 struct 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;
> +    int remains;
> +
> +    broker->err_ret = 0;
> +    bs = qbs->bdrvs;
> +
> +    if (len <= 0) {
> +        set_broker_err(broker, QB_ERR_INVALID_PARAM,
> +                      "Param len is less or equal to zero.");
> +        return broker->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 aligned, need to read/write this sector. */
> +        bd_ret = bdrv_read(bs, sector_start, temp_buf, 1);
> +        if (bd_ret < 0) {
> +            set_broker_err(broker, QB_ERR_INTERNAL_ERR,
> +                           "QEMU internal block error.");
> +            return broker->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_broker_err(broker, QB_ERR_INTERNAL_ERR,
> +                           "QEMU internal block error.");
> +            return broker->err_ret;
> +        }
> +        remains -= cp_len;
> +        p += cp_len;
> +        sector_start++;
> +    }
> +
> +    /* now start position is aligned. */
> +    if (remains >= BDRV_SECTOR_SIZE) {
> +        sector_num = len >> BDRV_SECTOR_BITS;
> +        bd_ret = bdrv_write(bs, sector_start, p, sector_num);
> +        if (bd_ret < 0) {
> +            set_broker_err(broker, QB_ERR_INTERNAL_ERR,
> +                           "QEMU internal block error.");
> +            return broker->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_broker_err(broker, QB_ERR_INTERNAL_ERR,
> +                           "QEMU internal block error.");
> +            return broker->err_ret;
> +        }
> +        memcpy(temp_buf, p, remains);
> +        bd_ret = bdrv_write(bs, sector_start, temp_buf, 1);
> +        if (bd_ret < 0) {
> +            set_broker_err(broker, QB_ERR_INTERNAL_ERR,
> +                           "QEMU internal block error.");
> +            return broker->err_ret;
> +        }
> +        remains -= remains;
> +    }
> +
> +    return len-remains;
> +}
> +
> +int qb_flush(struct QBroker *broker,
> +             struct QBlockState *qbs)
> +{
> +    int bd_ret;
> +    BlockDriverState *bs;
> +
> +    broker->err_ret = 0;
> +    bs = qbs->bdrvs;
> +    bd_ret = bdrv_flush(bs);
> +    if (bd_ret < 0) {
> +        set_broker_err(broker, QB_ERR_INTERNAL_ERR,
> +                       "Internal error.");
> +    }
> +    return broker->err_ret;
> +}
> +
> +int qb_check_allocation(struct QBroker *broker,
> +                        struct QBlockState *qbs,
> +                        uint64_t start,
> +                        int length,
> +                        int *pstatus,
> +                        int *plength)
> +{
> +    int ret;
> +    int sector_start, sector_num, num;
> +    BlockDriverState *bs;
> +    unsigned int real_len, ret_len;
> +
> +    broker->err_ret = 0;
> +    bs = qbs->bdrvs;
> +
> +    if (qbs->filename == NULL) {
> +        set_broker_err(broker, QB_ERR_INVALID_PARAM,
> +                       "Image was not opened first.");
> +        goto out;
> +    }
> +
> +    if (length <= 0) {
> +        set_broker_err(broker, 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_broker_err(broker, 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 broker->err_ret;
> +}
> +
> +int qb_info_image_static_get(struct QBroker *broker,
> +                             struct QBlockState *qbs,
> +                             struct QBlockStaticInfo **info)
> +{
> +    int ret = 0;
> +    BlockDriverState *bs;
> +    struct QBlockStaticInfo *info_tmp;
> +    const char *fmt_str;
> +    size_t total_sectors;
> +    char backing_filename[1024];
> +
> +    if (qbs->filename == NULL) {
> +        set_broker_err(broker, QB_ERR_INVALID_PARAM,
> +                       "Block Image was not openned.");
> +        ret = broker->err_ret;
> +        goto out;
> +    }
> +
> +    info_tmp = FUNC_CALLOC(1, sizeof(struct QBlockStaticInfo));
> +    if (info_tmp == NULL) {
> +        set_broker_err_nomem(broker);
> +        ret = broker->err_ret;
> +        goto out;
> +    }
> +
> +    bs = qbs->bdrvs;
> +
> +    ret = filename2loc(broker,
> +                       &(info_tmp->loc),
> +                       qbs->filename);
> +    if (ret < 0) {
> +        goto free;
> +    }
> +
> +    fmt_str = bdrv_get_format_name(bs);
> +    info_tmp->fmt_type = qb_str2fmttype(fmt_str);
> +
> +    bdrv_get_geometry(bs, &total_sectors);
> +    info_tmp->virt_size = total_sectors * BDRV_SECTOR_SIZE;
> +
> +    info_tmp->encrypt = bdrv_is_encrypted(bs);
> +
> +    bdrv_get_full_backing_filename(bs, backing_filename,
> +                                   sizeof(backing_filename));
> +    if (backing_filename[0] != '\0') {
> +        ret = filename2loc(broker,
> +                           &(info_tmp->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(broker, &info_tmp);
> +    return ret;
> +}
> +
> +/* free locations if it has string allocated on heap. */
> +static void loc_free(struct QBlockProtInfo *loc)
> +{
> +    switch (loc->prot_type) {
> +    case QB_PROT_FILE:
> +        FUNC_FREE((void *)(loc->prot_op.o_file.filename));
> +        loc->prot_op.o_file.filename = NULL;
> +        break;
> +    default:
> +        break;
> +    }
> +}
> +
> +void qb_info_image_static_delete(struct QBroker *broker,
> +                                 struct QBlockStaticInfo **info)
> +{
> +    loc_free(&(*info)->loc);
> +    loc_free(&(*info)->backing_loc);
> +
> +    CLEAN_FREE(*info);
> +}
> diff --git a/libqblock/libqblock.h b/libqblock/libqblock.h
> new file mode 100644
> index 0000000..97e6c7c
> --- /dev/null
> +++ b/libqblock/libqblock.h
> @@ -0,0 +1,292 @@
> +/*
> + * 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_broker_new: allocate a new broker
> + *
> + * Broker is used to pass operation to libqblock, and get feed back from it.
> + *
> + * Returns 0 on success, libqblock negative error value on fail.
> + *
> + * @broker: used to receive the created struct.
> + */
> +DLL_PUBLIC
> +int qb_broker_new(struct QBroker **broker);
> +
> +/**
> + * qb_broker_delete: delete broker
> + *
> + * Broker will be freed and set to NULL.
> + *
> + * @broker: operation broker to be deleted.
> + */
> +DLL_PUBLIC
> +void qb_broker_delete(struct QBroker **broker);
> +
> +/**
> + * 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.
> + *
> + * @broker: operation broker.
> + * @qbs: used to receive the created struct.
> + */
> +DLL_PUBLIC
> +int qb_state_new(struct QBroker *broker,
> +                 struct QBlockState **qbs);
> +
> +/**
> + * qb_state_delete: free a QBlockState struct
> + *
> + * if image was opened, qb_close must be called before delete.
> + *
> + * @broker: operation broker.
> + * @qbs: pointer to the struct's pointer.
> + */
> +DLL_PUBLIC
> +void qb_state_delete(struct QBroker *broker,
> +                     struct QBlockState **qbs);
> +
> +/**
> + * qb_prot_info_new: create a new struct QBlockProtInfo.
> + *
> + * return 0 on success, libqblock negative error value on fail.
> + *
> + * @broker: operation broker.
> + * @op: pointer to receive the new created one.
> + */
> +DLL_PUBLIC
> +int qb_prot_info_new(struct QBroker *broker,
> +                     struct QBlockProtInfo **op);
> +
> +/**
> + * qb_prot_info_delete: free a struct QBlockProtInfo.
> + *
> + * @broker: operation broker.
> + * @op: pointer to the object, *op would be set to NULL.
> + */
> +DLL_PUBLIC
> +void qb_prot_info_delete(struct QBroker *broker,
> +                         struct QBlockProtInfo **op);
> +
> +/**
> + * qb_fmt_info_new: create a new QBlockFmtInfo structure.
> + *
> + * return 0 on success, libqblock negative error value on fail.
> + *
> + * @broker: operation broker.
> + * @op: pointer that will receive created struct.
> + */
> +DLL_PUBLIC
> +int qb_fmt_info_new(struct QBroker *broker,
> +                    struct QBlockFmtInfo **op);
> +
> +/**
> + * qb_fmt_info_delete: free QBlockFmtInfo structure.
> + *
> + * @broker: operation broker.
> + * @op: pointer to the struct, *op would be set to NULL.
> + */
> +DLL_PUBLIC
> +void qb_fmt_info_delete(struct QBroker *broker,
> +                        struct QBlockFmtInfo **op);
> +
> +
> +/**
> + * qb_open: open a block object.
> + *
> + * return 0 on success, libqblock negative error value on fail.
> + *
> + * @broker: operation broker.
> + * @qbs: pointer to struct 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(struct QBroker *broker,
> +            struct QBlockState *qbs,
> +            struct QBlockProtInfo *loc,
> +            struct QBlockFmtInfo *fmt,
> +            int flag);
> +
> +/**
> + * qb_close: close a block object.
> + *
> + * qb_flush is automatically done inside.
> + *
> + * @broker: operation broker.
> + * @qbs: pointer to struct QBlockState.
> + */
> +DLL_PUBLIC
> +void qb_close(struct QBroker *broker,
> +              struct 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.
> + *
> + * @broker: operation broker.
> + * @qbs: pointer to struct 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(struct QBroker *broker,
> +              struct QBlockState *qbs,
> +              struct QBlockProtInfo *loc,
> +              struct QBlockFmtInfo *fmt,
> +              int flag);
> +
> +
> +/* sync access */
> +/**
> + * qb_read: block sync read.
> + *
> + * return number of bytes read, libqblock negative error value on fail.
> + *
> + * @broker: operation broker.
> + * @qbs: pointer to struct QBlockState.
> + * @buf: buffer that receive the content.
> + * @len: length to read.
> + * @offset: offset in the block data.
> + */
> +DLL_PUBLIC
> +int64_t qb_read(struct QBroker *broker,
> +                struct 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.
> + *
> + * @broker: operation broker.
> + * @qbs: pointer to struct QBlockState.
> + * @buf: buffer that receive the content.
> + * @len: length to write.
> + * @offset: offset in the block data.
> + */
> +DLL_PUBLIC
> +int64_t qb_write(struct QBroker *broker,
> +                 struct 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.
> + *
> + * @broker: operation broker.
> + * @qbs: pointer to struct QBlockState.
> + */
> +DLL_PUBLIC
> +int qb_flush(struct QBroker *broker,
> +             struct 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.
> + *
> + * @broker: operation broker.
> + * @qbs: pointer to struct QBlockState.
> + * @start: start position, unit is byte.
> + * @length: length to check, unit is byte.
> + * @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(struct QBroker *broker,
> +                        struct QBlockState *qbs,
> +                        uint64_t start,
> +                        int length,
> +                        int *pstatus,
> +                        int *plength);
> +
> +/* image information */
> +/**
> + * qb_get_image_info: get image info.
> + *
> + * return 0 on success, libqblock negative error value on fail.
> + *
> + * @broker: operation broker.
> + * @qbs: pointer to struct QBlockState.
> + * @info: pointer that would receive the information.
> + */
> +DLL_PUBLIC
> +int qb_info_image_static_get(struct QBroker *broker,
> +                             struct QBlockState *qbs,
> +                             struct QBlockStaticInfo **info);
> +
> +/**
> + * qb_delete_image_info: free image info.
> + *
> + * @broker: operation broker.
> + * @info: pointer to the information struct.
> + */
> +DLL_PUBLIC
> +void qb_info_image_static_delete(struct QBroker *broker,
> +                                 struct 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
> +enum QBlockFmtType qb_str2fmttype(const char *fmt_str);
> +
> +/**
> + * qb_fmttype2str: libqblock format enum type to a string.
> + *
> + * return a pointer to the string, or NULL if type is not supported, and
> + *  returned pointer do NOT need to free.
> + *
> + * @fmt: the format enum type.
> + */
> +DLL_PUBLIC
> +const char *qb_fmttype2str(enum QBlockFmtType fmt_type);
> +#endif
> --
> 1.7.1
>
>

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

* Re: [Qemu-devel] [PATCH V2 2/6] libqblock type and structure defines
  2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 2/6] libqblock type and structure defines Wenchao Xia
  2012-09-10 21:27   ` Eric Blake
@ 2012-09-11 20:31   ` Blue Swirl
  2012-09-11 22:52     ` Eric Blake
  1 sibling, 1 reply; 34+ messages in thread
From: Blue Swirl @ 2012-09-11 20:31 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: kwolf, aliguori, stefanha, qemu-devel, pbonzini, eblake

On Mon, Sep 10, 2012 at 8:26 AM, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>   This patch contains type and defines used in APIs, one file for public usage
> by user, one for libqblock internal usage.
>
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  libqblock/libqblock-internal.h |   50 ++++++++
>  libqblock/libqblock-types.h    |  251 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 301 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..fa27ed4
> --- /dev/null
> +++ b/libqblock/libqblock-internal.h
> @@ -0,0 +1,50 @@
> +/*
> + * 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 "block.h"
> +#include "block_int.h"
> +
> +#include "libqblock-types.h"
> +
> +/* this file contains defines and types used inside the library. */
> +
> +#define FUNC_FREE free
> +#define FUNC_MALLOC malloc
> +#define FUNC_CALLOC calloc
> +
> +#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;
> +};
> +
> +#define QB_ERR_STRING_SIZE (1024)
> +struct QBroker {
> +    /* last error */
> +    char err_msg[QB_ERR_STRING_SIZE];
> +    int err_ret; /* last error return of libqblock. */
> +    int err_no; /* 2nd level of error, errno what below reports */
> +};
> +
> +#endif
> diff --git a/libqblock/libqblock-types.h b/libqblock/libqblock-types.h
> new file mode 100644
> index 0000000..9d4e3fc
> --- /dev/null
> +++ b/libqblock/libqblock-types.h
> @@ -0,0 +1,251 @@
> +/*
> + * 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 __GNUC__ >= 4

#if defined(__GNUC__) && __GNUC__ ...

> +    #ifdef LIBQB_BUILD
> +        #define DLL_PUBLIC __attribute__((visibility("default")))
> +    #else
> +        #define DLL_PUBLIC
> +    #endif
> +#endif
> +
> +/* this library is designed around this core struct. */
> +struct QBlockState;

typedefs missing

> +
> +/* every thread would have a broker. */
> +struct QBroker;
> +
> +/* 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)
> +
> +enum QBlockProtType {
> +    QB_PROT_NONE = 0,
> +    QB_PROT_FILE,
> +    QB_PROT_MAX
> +};
> +
> +struct QBlockProtOptionFile {
> +    const char *filename;
> +};
> +
> +#define QBLOCK_PROT_OPTIONS_UNION_SIZE (512)
> +union QBlockProtOptionsUnion {
> +    struct QBlockProtOptionFile o_file;
> +    uint8_t reserved[QBLOCK_PROT_OPTIONS_UNION_SIZE];
> +};
> +
> +/**
> + * struct QBlockProtInfo: contains information about how to find the image
> + *
> + * @prot_type: protocol type, now only support FILE.
> + * @prot_op: protocol related options.
> + */
> +struct QBlockProtInfo {
> +    enum QBlockProtType prot_type;
> +    union QBlockProtOptionsUnion prot_op;
> +};
> +
> +
> +/* format related options */
> +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
> +};
> +
> +struct QBlockFmtOptionCow {
> +    uint64_t virt_size;
> +    struct QBlockProtInfo backing_loc;
> +};
> +
> +struct QBlockFmtOptionQed {
> +    uint64_t virt_size;
> +    struct QBlockProtInfo backing_loc;
> +    enum QBlockFmtType backing_fmt;
> +    uint64_t cluster_size; /* unit is bytes */
> +    uint64_t table_size; /* unit is clusters */
> +};
> +
> +struct QBlockFmtOptionQcow {
> +    uint64_t virt_size;
> +    struct QBlockProtInfo backing_loc;
> +    bool encrypt;
> +};
> +
> +/* "Compatibility level (0.10 or 1.1)" */
> +enum QBlockFmtOptionQcow2CptLv {
> +    QBO_FMT_QCOW2_CPT_NONE = 0,
> +    QBO_FMT_QCOW2_CPT_V010,
> +    QBO_FMT_QCOW2_CPT_V110,
> +};
> +
> +/* off or metadata */
> +enum QBlockFmtOptionQcow2PreAllocType {
> +    QBO_FMT_QCOW2_PREALLOC_NONE = 0,
> +    QBO_FMT_QCOW2_PREALLOC_OFF,
> +    QBO_FMT_QCOW2_PREALLOC_METADATA,
> +};
> +
> +struct QBlockFmtOptionQcow2 {
> +    uint64_t virt_size;
> +    struct QBlockProtInfo backing_loc;
> +    enum QBlockFmtType backing_fmt;
> +    bool encrypt;
> +    uint64_t cluster_size; /* unit is bytes */
> +    enum QBlockFmtOptionQcow2CptLv cpt_lv;
> +    enum QBlockFmtOptionQcow2PreAllocType pre_mode;
> +};
> +
> +struct QBlockFmtOptionRaw {
> +    uint64_t virt_size;
> +};
> +
> +struct QBlockFmtOptionRbd {
> +    uint64_t virt_size;
> +    uint64_t cluster_size;
> +};
> +
> +/* off or full */
> +enum QBlockFmtOptionSheepdogPreAllocType {
> +    QBO_FMT_SD_PREALLOC_NONE = 0,
> +    QBO_FMT_SD_PREALLOC_OFF,
> +    QBO_FMT_SD_PREALLOC_FULL,
> +};
> +
> +struct QBlockFmtOptionSheepdog {
> +    uint64_t virt_size;
> +    struct QBlockProtInfo backing_loc;
> +    enum QBlockFmtOptionSheepdogPreAllocType pre_mode;
> +};
> +
> +enum QBlockFmtOptionVdiPreAllocType {
> +    QBO_FMT_VDI_PREALLOC_NONE = 0,
> +    QBO_FMT_VDI_PREALLOC_FALSE,
> +    QBO_FMT_VDI_PREALLOC_TRUE,
> +};
> +
> +struct QBlockFmtOptionVdi {
> +    uint64_t virt_size;
> +    uint64_t cluster_size;
> +    enum QBlockFmtOptionVdiPreAllocType pre_mode;
> +};
> +
> +/* whether compact to vmdk verion 6 */
> +enum QBlockFmtOptionVmdkCptLv {
> +    QBO_FMT_VMDK_CPT_NONE = 0,
> +    QBO_FMT_VMDK_CPT_VMDKV6_FALSE,
> +    QBO_FMT_VMDK_CPT_VMDKV6_TRUE,
> +};
> +
> +/* vmdk flat extent format, values:
> +"{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse |
> +twoGbMaxExtentFlat | streamOptimized} */
> +enum QBlockFmtOptionVmdkSubfmtType {
> +    QBO_FMT_VMDK_SUBFMT_MONOLITHIC_NONE = 0,
> +    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,
> +};
> +
> +struct QBlockFmtOptionVmdk {
> +    uint64_t virt_size;
> +    struct QBlockProtInfo backing_loc;
> +    enum QBlockFmtOptionVmdkCptLv cpt_lv;
> +    enum QBlockFmtOptionVmdkSubfmtType subfmt;
> +};
> +
> +/* "{dynamic (default) | fixed} " */
> +enum QBlockFmtOptionVpcSubfmtType {
> +    QBO_FMT_VPC_SUBFMT_NONE = 0,
> +    QBO_FMT_VPC_SUBFMT_DYNAMIC,
> +    QBO_FMT_VPC_SUBFMT_FIXED,
> +};
> +
> +struct QBlockFmtOptionVpc {
> +    uint64_t virt_size;
> +    enum QBlockFmtOptionVpcSubfmtType subfmt;
> +};
> +
> +#define QBLOCK_FMT_OPTIONS_UNION_SIZE (QBLOCK_PROT_OPTIONS_UNION_SIZE*2)
> +union QBlockFmtOptionsUnion {
> +    struct QBlockFmtOptionCow       o_cow;
> +    struct QBlockFmtOptionQed       o_qed;
> +    struct QBlockFmtOptionQcow      o_qcow;
> +    struct QBlockFmtOptionQcow2     o_qcow2;
> +    struct QBlockFmtOptionRaw       o_raw;
> +    struct QBlockFmtOptionRbd       o_rbd;
> +    struct QBlockFmtOptionSheepdog  o_sheepdog;
> +    struct QBlockFmtOptionVdi       o_vdi;
> +    struct QBlockFmtOptionVmdk      o_vmdk;
> +    struct QBlockFmtOptionVpc       o_vpc;
> +    uint8_t reserved[QBLOCK_FMT_OPTIONS_UNION_SIZE];
> +};
> +
> +struct QBlockFmtInfo {
> +    enum QBlockFmtType fmt_type;
> +    union QBlockFmtOptionsUnion fmt_op;
> +};
> +
> +/**
> + * QBlockStaticInfo: information about the block image.
> + *
> + * @loc: location info.
> + * @fmt_type: format type.
> + * @virt_size: virtual size in bytes.
> + * @backing_loc: backing file location, its type is QB_PROT_NONE if not exist.
> + * @encrypt: encrypt flag.
> + * @sector_size: how many bytes in a sector, it is 512 usually.
> + */
> +struct QBlockStaticInfo {
> +    struct QBlockProtInfo loc;
> +    enum QBlockFmtType fmt_type;
> +    uint64_t virt_size;
> +    /* advance info */
> +    struct QBlockProtInfo backing_loc;
> +    bool encrypt;
> +    int sector_size;
> +};
> +#endif
> --
> 1.7.1
>
>

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

* Re: [Qemu-devel] [PATCH V2 3/6] libqblock error handling
  2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 3/6] libqblock error handling Wenchao Xia
  2012-09-10 21:33   ` Eric Blake
@ 2012-09-11 20:32   ` Blue Swirl
  2012-09-12  2:58     ` Wenchao Xia
  1 sibling, 1 reply; 34+ messages in thread
From: Blue Swirl @ 2012-09-11 20:32 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: kwolf, aliguori, stefanha, qemu-devel, pbonzini, eblake

On Mon, Sep 10, 2012 at 8:26 AM, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>   This patch contains error handling APIs, which user could call them to
> get error details.
>
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  libqblock/libqblock-error.c |   60 +++++++++++++++++++++++++++++++++++++++++++
>  libqblock/libqblock-error.h |   50 +++++++++++++++++++++++++++++++++++
>  2 files changed, 110 insertions(+), 0 deletions(-)
>  create mode 100644 libqblock/libqblock-error.c
>  create mode 100644 libqblock/libqblock-error.h
>
> diff --git a/libqblock/libqblock-error.c b/libqblock/libqblock-error.c
> new file mode 100644
> index 0000000..d5ebabf
> --- /dev/null
> +++ b/libqblock/libqblock-error.c
> @@ -0,0 +1,60 @@
> +/*
> + * 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(struct QBroker *broker,
> +                            char *buf, int buf_size)

size_t buf_size

> +{
> +    const char *err_ret_str;
> +    switch (broker->err_ret) {
> +    case QB_ERR_MEM_ERR:
> +        err_ret_str = "Not enough memory.";
> +        break;
> +    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 = "Unknow error.";
> +        break;
> +    }
> +    if (broker == NULL) {
> +        snprintf(buf, buf_size, "%s", err_ret_str);
> +        return;
> +    }
> +
> +    if (broker->err_ret == QB_ERR_INTERNAL_ERR) {
> +        snprintf(buf, buf_size, "%s %s errno [%d]. strerror [%s].",
> +                     err_ret_str, broker->err_msg,
> +                     broker->err_no, strerror(-broker->err_no));
> +    } else {
> +        snprintf(buf, buf_size, "%s %s",
> +                     err_ret_str, broker->err_msg);
> +    }

Please NUL terminate the string.

> +    return;
> +}
> +
> +int qb_error_get_errno(struct QBroker *broker)
> +{
> +    if (broker->err_ret == QB_ERR_INTERNAL_ERR) {
> +        return broker->err_no;
> +    }
> +    return 0;
> +}
> diff --git a/libqblock/libqblock-error.h b/libqblock/libqblock-error.h
> new file mode 100644
> index 0000000..0690cfb
> --- /dev/null
> +++ b/libqblock/libqblock-error.h
> @@ -0,0 +1,50 @@
> +/*
> + * 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_MEM_ERR (-1)
> +#define QB_ERR_INTERNAL_ERR (-2)
> +#define QB_ERR_INVALID_PARAM (-3)
> +#define QB_ERR_BLOCK_OUT_OF_RANGE (-100)
> +
> +/* 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.
> + *
> + * @broker: operation broker, 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(struct QBroker *broker,
> +                            char *buf, int buf_size);
> +
> +/**
> + * qb_error_get_errno: get error number, only valid when err_ret is
> + *   QB_ERR_INTERNAL_ERR.
> + *
> + * return negative errno or 0 if last error is not QB_ERR_INTERNAL_ERR.
> + *
> + * @broker: operation broker.
> + */
> +DLL_PUBLIC
> +int qb_error_get_errno(struct QBroker *broker);
> +
> +#endif
> --
> 1.7.1
>
>

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

* Re: [Qemu-devel] [PATCH V2 2/6] libqblock type and structure defines
  2012-09-11 20:31   ` Blue Swirl
@ 2012-09-11 22:52     ` Eric Blake
  2012-09-12  3:05       ` Wenchao Xia
  2012-09-14 18:02       ` Blue Swirl
  0 siblings, 2 replies; 34+ messages in thread
From: Eric Blake @ 2012-09-11 22:52 UTC (permalink / raw)
  To: Blue Swirl; +Cc: kwolf, aliguori, stefanha, qemu-devel, pbonzini, Wenchao Xia

[-- Attachment #1: Type: text/plain, Size: 893 bytes --]

On 09/11/2012 02:31 PM, Blue Swirl wrote:
> On Mon, Sep 10, 2012 at 8:26 AM, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>   This patch contains type and defines used in APIs, one file for public usage
>> by user, one for libqblock internal usage.
>>

>> +
>> +#if __GNUC__ >= 4
> 
> #if defined(__GNUC__) && __GNUC__ ...

Seriously?  We require a C99-compliant compiler, which is required to
treat the more compact version identically (all undefined names evaluate
to 0 in the preprocessor), and HACKING doesn't mandate that we spell out
a defined-ness check first.  Okay, so configure adds -Wundef to the set
of CFLAGS, but I fail to see why we want -Wundef (that's an
anachronistic warning, only there to help you if you are writing code
portable to K&R).

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]

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

* Re: [Qemu-devel] [PATCH V2 1/6] libqblock API design
  2012-09-11 20:28   ` Blue Swirl
@ 2012-09-12  2:54     ` Wenchao Xia
  2012-09-12  8:19     ` Kevin Wolf
  1 sibling, 0 replies; 34+ messages in thread
From: Wenchao Xia @ 2012-09-12  2:54 UTC (permalink / raw)
  To: Blue Swirl; +Cc: kwolf, aliguori, stefanha, qemu-devel, pbonzini, eblake

   All changed, thanks.

> On Mon, Sep 10, 2012 at 8:26 AM, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>    This patch contains the major APIs in the library.
>> Important APIs:
>>    1 QBroker. These structure was used to retrieve errors, every thread must
>> create one first, later maybe thread related staff could be added into it.
>>    2 QBlockState. It stands for an block image object.
>>    3 QBlockStaticInfo. It contains static information such as location, backing
>> file, size.
>>    4 ABI was kept with reserved members.
>>    5 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.c | 1077 +++++++++++++++++++++++++++++++++++++++++++++++++
>>   libqblock/libqblock.h |  292 +++++++++++++
>>   2 files changed, 1369 insertions(+), 0 deletions(-)
>>   create mode 100644 libqblock/libqblock.c
>>   create mode 100644 libqblock/libqblock.h
>>
>> diff --git a/libqblock/libqblock.c b/libqblock/libqblock.c
>> new file mode 100644
>> index 0000000..133ac0f
>> --- /dev/null
>> +++ b/libqblock/libqblock.c
>> @@ -0,0 +1,1077 @@
>> +/*
>> + * 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"
>> +
>> +struct LibqblockGlobalData {
>> +    int init_flag;
>> +};
>> +
>> +struct LibqblockGlobalData g_libqblock_data;
>> +
>> +__attribute__((constructor))
>> +static void libqblock_init(void)
>> +{
>> +    if (g_libqblock_data.init_flag == 0) {
>> +        bdrv_init();
>> +        qemu_init_main_loop();
>> +    }
>> +    g_libqblock_data.init_flag = 1;
>> +}
>> +
>> +const char *qb_fmttype2str(enum QBlockFmtType fmt_type)
>> +{
>> +    const char *ret = NULL;
>> +    switch (fmt_type) {
>> +    case QB_FMT_COW:
>> +        ret = "cow";
>> +        break;
>> +    case QB_FMT_QED:
>> +        ret = "qed";
>> +        break;
>> +    case QB_FMT_QCOW:
>> +        ret = "qcow";
>> +        break;
>> +    case QB_FMT_QCOW2:
>> +        ret = "qcow2";
>> +        break;
>> +    case QB_FMT_RAW:
>> +        ret = "raw";
>> +        break;
>> +    case QB_FMT_RBD:
>> +        ret = "rbd";
>> +        break;
>> +    case QB_FMT_SHEEPDOG:
>> +        ret = "sheepdog";
>> +        break;
>> +    case QB_FMT_VDI:
>> +        ret = "vdi";
>> +        break;
>> +    case QB_FMT_VMDK:
>> +        ret = "vmdk";
>> +        break;
>> +    case QB_FMT_VPC:
>> +        ret = "vpc";
>> +        break;
>> +    default:
>> +        break;
>> +    }
>> +    return ret;
>> +}
>> +
>> +enum QBlockFmtType qb_str2fmttype(const char *fmt_str)
>> +{
>> +    enum QBlockFmtType ret = QB_FMT_NONE;
>> +    if (0 == strcmp(fmt_str, "cow")) {
>
> This order is not common in QEMU.
>
>> +        ret = QB_FMT_COW;
>> +    } else if (0 == strcmp(fmt_str, "qed")) {
>> +        ret = QB_FMT_QED;
>> +    } else if (0 == strcmp(fmt_str, "qcow")) {
>> +        ret = QB_FMT_QCOW;
>> +    } else if (0 == strcmp(fmt_str, "qcow2")) {
>> +        ret = QB_FMT_QCOW2;
>> +    } else if (0 == strcmp(fmt_str, "raw")) {
>> +        ret = QB_FMT_RAW;
>> +    } else if (0 == strcmp(fmt_str, "rbd")) {
>> +        ret = QB_FMT_RBD;
>> +    } else if (0 == strcmp(fmt_str, "sheepdog")) {
>> +        ret = QB_FMT_SHEEPDOG;
>> +    } else if (0 == strcmp(fmt_str, "vdi")) {
>> +        ret = QB_FMT_VDI;
>> +    } else if (0 == strcmp(fmt_str, "vmdk")) {
>> +        ret = QB_FMT_VMDK;
>> +    } else if (0 == strcmp(fmt_str, "vpc")) {
>> +        ret = QB_FMT_VPC;
>> +    }
>> +    return ret;
>> +}
>> +
>> +static void set_broker_err(struct QBroker *broker, int err_ret,
>> +                           const char *fmt, ...)
>> +{
>> +    va_list ap;
>> +
>> +    broker->err_ret = err_ret;
>> +    if (err_ret == QB_ERR_INTERNAL_ERR) {
>> +        broker->err_no = -errno;
>> +    } else {
>> +        broker->err_no = 0;
>> +    }
>> +
>> +    va_start(ap, fmt);
>> +    vsnprintf(broker->err_msg, sizeof(broker->err_msg), fmt, ap);
>> +    va_end(ap);
>> +}
>> +
>> +static void set_broker_err_nomem(struct QBroker *broker)
>> +{
>> +    set_broker_err(broker, QB_ERR_MEM_ERR, "No Memory.");
>> +}
>> +
>> +int qb_broker_new(struct QBroker **broker)
>> +{
>> +    *broker = FUNC_CALLOC(1, sizeof(struct QBroker));
>> +    if (*broker == NULL) {
>> +        return QB_ERR_MEM_ERR;
>> +    }
>> +    return 0;
>> +}
>> +
>> +void qb_broker_delete(struct QBroker **broker)
>> +{
>> +    CLEAN_FREE(*broker);
>> +    return;
>> +}
>> +
>> +int qb_state_new(struct QBroker *broker,
>> +                 struct QBlockState **qbs)
>> +{
>> +    *qbs = FUNC_CALLOC(1, sizeof(struct QBlockState));
>> +    if (*qbs == NULL) {
>> +        set_broker_err_nomem(broker);
>> +        return broker->err_ret;
>> +    }
>> +    (*qbs)->bdrvs = bdrv_new("hda");
>> +    if ((*qbs)->bdrvs == NULL) {
>> +        CLEAN_FREE(*qbs);
>> +        set_broker_err(broker, QB_ERR_INTERNAL_ERR,
>> +                       "failed to create the driver.");
>> +        return broker->err_ret;
>> +    }
>> +    return 0;
>> +}
>> +
>> +void qb_state_delete(struct QBroker *broker,
>> +                     struct QBlockState **qbs)
>> +{
>> +    CLEAN_FREE((*qbs)->filename);
>> +    if ((*qbs)->bdrvs != NULL) {
>> +        bdrv_delete((*qbs)->bdrvs);
>> +        (*qbs)->bdrvs = NULL;
>> +    }
>> +    CLEAN_FREE(*qbs);
>> +    return;
>> +}
>> +
>> +int qb_prot_info_new(struct QBroker *broker,
>> +                     struct QBlockProtInfo **op)
>> +{
>> +    *op = FUNC_CALLOC(1, sizeof(struct QBlockProtInfo));
>> +    if (*op == NULL) {
>> +        set_broker_err_nomem(broker);
>> +        return broker->err_ret;
>> +    }
>> +    return 0;
>> +}
>> +
>> +void qb_prot_info_delete(struct QBroker *broker,
>> +                         struct QBlockProtInfo **op)
>> +{
>> +    CLEAN_FREE(*op);
>> +}
>> +
>> +int qb_fmt_info_new(struct QBroker *broker,
>> +                    struct QBlockFmtInfo **op)
>> +{
>> +    *op = FUNC_CALLOC(1, sizeof(struct QBlockFmtInfo));
>> +    if (*op == NULL) {
>> +        set_broker_err_nomem(broker);
>> +        return broker->err_ret;
>> +    }
>> +    return 0;
>> +}
>> +
>> +void qb_fmt_info_delete(struct QBroker *broker,
>> +                        struct QBlockFmtInfo **op)
>> +{
>> +    CLEAN_FREE(*op);
>> +}
>> +
>> +/* return 0 if every thing is fine */
>> +static int loc_check_params(struct QBroker *broker,
>> +                            struct QBlockProtInfo *loc)
>> +{
>> +    broker->err_ret = 0;
>> +
>> +    switch (loc->prot_type) {
>> +    case QB_PROT_FILE:
>> +        if (loc->prot_op.o_file.filename == NULL) {
>> +            set_broker_err(broker, QB_ERR_INVALID_PARAM,
>> +                           "Filename was not set.");
>> +            goto out;
>> +        }
>> +        if (path_has_protocol(loc->prot_op.o_file.filename) > 0) {
>> +            set_broker_err(broker, QB_ERR_INVALID_PARAM,
>> +                           "filename [%s] had protocol.",
>> +                           loc->prot_op.o_file.filename);
>> +            goto out;
>> +        }
>> +        break;
>> +    default:
>> +        set_broker_err(broker, QB_ERR_INVALID_PARAM,
>> +                       "Protocol type [%d] was not valid.",
>> +                       loc->prot_type);
>> +        break;
>> +    }
>> +
>> + out:
>> +    return broker->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(struct QBroker *broker,
>> +                        struct QBlockProtInfo *loc,
>> +                        char **filename)
>> +{
>> +    broker->err_ret = 0;
>> +
>> +    if (*filename != NULL) {
>> +        CLEAN_FREE(*filename);
>> +    }
>> +    switch (loc->prot_type) {
>> +    case QB_PROT_FILE:
>> +        *filename = strdup(loc->prot_op.o_file.filename);
>> +        if (*filename == NULL) {
>> +            set_broker_err_nomem(broker);
>> +        }
>> +        break;
>> +    default:
>> +        set_broker_err(broker, QB_ERR_INVALID_PARAM,
>> +                 "protocol type [%d] is not supported.",
>> +                  loc->prot_type);
>> +        break;
>> +    }
>> +
>> +    return broker->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(struct QBroker *broker,
>> +                        struct QBlockProtInfo *loc,
>> +                        const char *filename)
>> +{
>> +    broker->err_ret = 0;
>> +
>> +    if (path_has_protocol(filename) > 0) {
>> +        set_broker_err(broker, QB_ERR_INVALID_PARAM,
>> +                     "Filename [%s] had protocol, not supported now.",
>> +                     filename);
>> +        goto out;
>> +    }
>> +
>> +    loc->prot_type = QB_PROT_FILE;
>> +    switch (loc->prot_type) {
>> +    case QB_PROT_FILE:
>> +        loc->prot_op.o_file.filename = strdup(filename);
>> +        if (loc->prot_op.o_file.filename == NULL) {
>> +            set_broker_err_nomem(broker);
>> +            goto out;
>> +        }
>> +        break;
>> +    default:
>> +        break;
>> +    }
>> +
>> + out:
>> +    return broker->err_ret;
>> +}
>> +
>> +/* return 0 if OK, or qblock error number */
>> +static int set_backing_file_options(struct QBroker *broker,
>> +                                    QEMUOptionParameter *param,
>> +                                    struct QBlockProtInfo *loc,
>> +                                    enum QBlockFmtType *fmt)
>> +{
>> +    char *backing_filename = NULL;
>> +    const char *fmtstr_backing = NULL;
>> +    int ret = 0;
>> +
>> +    if (loc == NULL) {
>> +        goto out;
>> +    }
>> +
>> +    ret = loc2filename(broker, loc, &backing_filename);
>> +    /* ret can < 0 if loc have not been set, mean user did not specify backing
>> +       file */
>> +    if (ret == QB_ERR_MEM_ERR) {
>> +        goto out;
>> +    }
>> +    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(struct QBroker *broker,
>> +              struct QBlockState *qbs,
>> +              struct QBlockProtInfo *loc,
>> +              struct QBlockFmtInfo *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;
>> +    struct QBlockFmtOptionCow *o_cow = NULL;
>> +    struct QBlockFmtOptionQed *o_qed = NULL;
>> +    struct QBlockFmtOptionQcow *o_qcow = NULL;
>> +    struct QBlockFmtOptionQcow2 *o_qcow2 = NULL;
>> +    struct QBlockFmtOptionRaw *o_raw = NULL;
>> +    struct QBlockFmtOptionRbd *o_rbd = NULL;
>> +    struct QBlockFmtOptionSheepdog *o_sd = NULL;
>> +    struct QBlockFmtOptionVdi *o_vdi = NULL;
>> +    struct QBlockFmtOptionVmdk *o_vmdk = NULL;
>> +    struct QBlockFmtOptionVpc *o_vpc = NULL;
>> +
>> +
>> +    /* check parameters */
>> +    if (flag & (~LIBQBLOCK_O_VALID_MASK)) {
>> +        set_broker_err(broker, QB_ERR_INVALID_PARAM,
>> +                           "invalid flag was set.");
>> +        ret = broker->err_ret;
>> +        goto out;
>> +    }
>> +
>> +    if ((loc == NULL) || (qbs == NULL) || (fmt == NULL)) {
>> +        set_broker_err(broker, QB_ERR_INVALID_PARAM,
>> +                          "Got unexpected NULL pointer in parameters.");
>> +        ret = broker->err_ret;
>> +        goto out;
>> +    }
>> +
>> +    ret = loc_check_params(broker, loc);
>> +    if (ret != 0) {
>> +        goto out;
>> +    }
>> +
>> +    /* internal translate */
>> +    ret = loc2filename(broker, loc, &filename);
>> +    if (ret != 0) {
>> +        goto out;
>> +    }
>> +
>> +    fmtstr = qb_fmttype2str(fmt->fmt_type);
>> +    if (fmtstr == NULL) {
>> +        set_broker_err(broker, QB_ERR_INVALID_PARAM,
>> +                 "Got unexpected NULL pointer in parameters.");
>> +        ret = broker->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->fmt_op.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(broker, param,
>> +                                       &o_cow->backing_loc, NULL);
>> +        if (ret != 0) {
>> +            goto out;
>> +        }
>> +        break;
>> +    case QB_FMT_QED:
>> +        o_qed = &(fmt->fmt_op.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(broker, 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->fmt_op.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(broker, 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->fmt_op.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(broker, 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 != QBO_FMT_QCOW2_CPT_NONE) {
>> +            tmp = o_qcow2->cpt_lv == QBO_FMT_QCOW2_CPT_V010 ? "0.10" : "1.1";
>> +            bd_ret = set_option_parameter(param,
>> +                              BLOCK_OPT_COMPAT_LEVEL, tmp);
>> +            assert(bd_ret == 0);
>> +        }
>> +
>> +        if (o_qcow2->pre_mode != QBO_FMT_QCOW2_PREALLOC_NONE) {
>> +            tmp = o_qcow2->pre_mode == QBO_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->fmt_op.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->fmt_op.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->fmt_op.o_sheepdog);
>> +        bd_ret = set_option_parameter_int(param,
>> +                              BLOCK_OPT_SIZE, o_sd->virt_size);
>> +        assert(bd_ret == 0);
>> +        ret = set_backing_file_options(broker, param,
>> +                                       &o_sd->backing_loc, NULL);
>> +        if (ret != 0) {
>> +            goto out;
>> +        }
>> +        if (o_sd->pre_mode != QBO_FMT_SD_PREALLOC_NONE) {
>> +            tmp = o_sd->pre_mode == QBO_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->fmt_op.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 != QBO_FMT_VDI_PREALLOC_NONE) {
>> +            tmp_bool = o_sd->pre_mode == QBO_FMT_VDI_PREALLOC_TRUE ?
>> +                                                     true : false;
>> +            set_option_parameter_int(param, "static", tmp_bool);
>> +        }
>> +        break;
>> +    case QB_FMT_VMDK:
>> +        o_vmdk = &(fmt->fmt_op.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(broker, param,
>> +                                       &o_vmdk->backing_loc, NULL);
>> +        if (ret != 0) {
>> +            goto out;
>> +        }
>> +
>> +        if (o_vmdk->cpt_lv != QBO_FMT_VMDK_CPT_NONE) {
>> +            tmp_bool = o_vmdk->cpt_lv == QBO_FMT_VMDK_CPT_VMDKV6_TRUE ?
>> +                                                     true : false;
>> +            bd_ret = set_option_parameter_int(param, BLOCK_OPT_COMPAT6,
>> +                                                     tmp_bool);
>> +            assert(bd_ret == 0);
>> +        }
>> +        if (o_vmdk->subfmt != QBO_FMT_VMDK_SUBFMT_MONOLITHIC_NONE) {
>> +            switch (o_vmdk->subfmt) {
>> +            case QBO_FMT_VMDK_SUBFMT_MONOLITHIC_SPARSE:
>> +                tmp = "monolithicSparse";
>> +                break;
>> +            case QBO_FMT_VMDK_SUBFMT_MONOLITHIC_FLAT:
>> +                tmp = "monolithicFlat";
>> +                break;
>> +            case QBO_FMT_VMDK_SUBFMT_TWOGBMAX_EXTENT_SPARSE:
>> +                tmp = "twoGbMaxExtentSparse";
>> +                break;
>> +            case QBO_FMT_VMDK_SUBFMT_TWOGBMAX_EXTENT_FLAT:
>> +                tmp = "twoGbMaxExtentFlat";
>> +                break;
>> +            case QBO_FMT_VMDK_SUBFMT_STREAM_OPTIMIZED:
>> +                tmp = "streamOptimized";
>> +                break;
>> +            default:
>> +                assert(false);
>> +                break;
>> +            }
>> +            bd_ret = set_option_parameter(param,
>> +                              BLOCK_OPT_SUBFMT, tmp);
>> +            assert(bd_ret == 0);
>> +        }
>> +        break;
>> +    case QB_FMT_VPC:
>> +        o_vpc = &(fmt->fmt_op.o_vpc);
>> +        bd_ret = set_option_parameter_int(param,
>> +                               BLOCK_OPT_SIZE, o_vpc->virt_size);
>> +        assert(bd_ret == 0);
>> +        if (o_vpc->subfmt != QBO_FMT_VPC_SUBFMT_NONE) {
>> +            tmp = o_vpc->subfmt == QBO_FMT_VPC_SUBFMT_DYNAMIC ?
>> +                                               "dynamic" : "fixed";
>> +            bd_ret = set_option_parameter(param,
>> +                               BLOCK_OPT_SUBFMT, tmp);
>> +            assert(bd_ret == 0);
>> +        }
>> +        break;
>> +    default:
>> +        assert(false);
>
> abort()
>
>> +        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_broker_err(broker, QB_ERR_INVALID_PARAM,
>> +                          "Backing file is the same with new file.");
>> +            ret = broker->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_broker_err(broker, QB_ERR_INVALID_PARAM,
>> +                               "Failed to open the backing file.");
>> +                ret = broker->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_broker_err(broker, QB_ERR_INTERNAL_ERR,
>> +                           "Neither size or backing file was not set.");
>> +            ret = broker->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_broker_err(broker, QB_ERR_INTERNAL_ERR, errstr);
>> +        ret = broker->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(struct QBroker *broker,
>> +            struct QBlockState *qbs,
>> +            struct QBlockProtInfo *loc,
>> +            struct QBlockFmtInfo *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_broker_err(broker, QB_ERR_INVALID_PARAM,
>> +                      "Invalid flag was set.");
>> +        ret = broker->err_ret;
>> +        goto out;
>> +    }
>> +
>> +    if ((loc == NULL) || (qbs == NULL)) {
>> +        set_broker_err(broker, QB_ERR_INVALID_PARAM,
>> +                      "Got unexpected NULL pointer in parameters.");
>> +        ret = broker->err_ret;
>> +        goto out;
>> +    }
>> +
>> +    ret = loc_check_params(broker, loc);
>> +    if (ret != 0) {
>> +        goto out;
>> +    }
>> +
>> +    /* internal translate */
>> +    ret = loc2filename(broker, 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 openning */
>
> opening
>
>> +    bs = qbs->bdrvs;
>> +    bd_ret = bdrv_open(bs, filename, flag, bd);
>> +    if (bd_ret < 0) {
>> +        set_broker_err(broker, QB_ERR_INTERNAL_ERR,
>> +                      "Failed in opening with driver, bd_ret is %d.", bd_ret);
>> +        ret = broker->err_ret;
>> +        goto out;
>> +    }
>> +
>> +    if (qbs->filename != NULL) {
>> +        FUNC_FREE(qbs->filename);
>> +    }
>> +    qbs->filename = strdup(filename);
>> +    if (qbs->filename == NULL) {
>> +        set_broker_err_nomem(broker);
>> +        ret = broker->err_ret;
>> +        bdrv_close(bs);
>> +        goto out;
>> +    }
>> +
>> + out:
>> +    FUNC_FREE(filename);
>> +    return ret;
>> +}
>> +
>> +void qb_close(struct QBroker *broker,
>> +              struct QBlockState *qbs)
>> +{
>> +    BlockDriverState *bs;
>> +
>> +    bs = qbs->bdrvs;
>> +
>> +    if (qbs->filename != NULL) {
>> +        CLEAN_FREE(qbs->filename);
>> +        bdrv_close(bs);
>> +    }
>> +    return;
>> +}
>> +
>> +int64_t qb_read(struct QBroker *broker,
>> +                struct 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;
>> +    int remains;
>> +
>> +    broker->err_ret = 0;
>> +    bs = qbs->bdrvs;
>> +
>> +    if (len <= 0) {
>> +        set_broker_err(broker, QB_ERR_INVALID_PARAM,
>> +                      "Param len is less or equal to zero.");
>> +        return broker->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 aligned, need to read/write this sector. */
>> +        bd_ret = bdrv_read(bs, sector_start, temp_buf, 1);
>> +        if (bd_ret < 0) {
>> +            set_broker_err(broker, QB_ERR_INTERNAL_ERR,
>> +                           "QEMU internal block error.");
>> +            return broker->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 aligned. */
>> +    if (remains >= BDRV_SECTOR_SIZE) {
>> +        sector_num = len >> BDRV_SECTOR_BITS;
>> +        bd_ret = bdrv_read(bs, sector_start, p, sector_num);
>> +        if (bd_ret < 0) {
>> +            set_broker_err(broker, QB_ERR_INTERNAL_ERR,
>> +                           "QEMU internal block error.");
>> +            return broker->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_broker_err(broker, QB_ERR_INTERNAL_ERR,
>> +                           "QEMU internal block error.");
>> +            return broker->err_ret;
>> +        }
>> +        memcpy(p, temp_buf, remains);
>> +        remains -= remains;
>> +    }
>> +
>> +    return len-remains;
>> +}
>> +
>> +int64_t qb_write(struct QBroker *broker,
>> +                 struct 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;
>> +    int remains;
>> +
>> +    broker->err_ret = 0;
>> +    bs = qbs->bdrvs;
>> +
>> +    if (len <= 0) {
>> +        set_broker_err(broker, QB_ERR_INVALID_PARAM,
>> +                      "Param len is less or equal to zero.");
>> +        return broker->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 aligned, need to read/write this sector. */
>> +        bd_ret = bdrv_read(bs, sector_start, temp_buf, 1);
>> +        if (bd_ret < 0) {
>> +            set_broker_err(broker, QB_ERR_INTERNAL_ERR,
>> +                           "QEMU internal block error.");
>> +            return broker->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_broker_err(broker, QB_ERR_INTERNAL_ERR,
>> +                           "QEMU internal block error.");
>> +            return broker->err_ret;
>> +        }
>> +        remains -= cp_len;
>> +        p += cp_len;
>> +        sector_start++;
>> +    }
>> +
>> +    /* now start position is aligned. */
>> +    if (remains >= BDRV_SECTOR_SIZE) {
>> +        sector_num = len >> BDRV_SECTOR_BITS;
>> +        bd_ret = bdrv_write(bs, sector_start, p, sector_num);
>> +        if (bd_ret < 0) {
>> +            set_broker_err(broker, QB_ERR_INTERNAL_ERR,
>> +                           "QEMU internal block error.");
>> +            return broker->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_broker_err(broker, QB_ERR_INTERNAL_ERR,
>> +                           "QEMU internal block error.");
>> +            return broker->err_ret;
>> +        }
>> +        memcpy(temp_buf, p, remains);
>> +        bd_ret = bdrv_write(bs, sector_start, temp_buf, 1);
>> +        if (bd_ret < 0) {
>> +            set_broker_err(broker, QB_ERR_INTERNAL_ERR,
>> +                           "QEMU internal block error.");
>> +            return broker->err_ret;
>> +        }
>> +        remains -= remains;
>> +    }
>> +
>> +    return len-remains;
>> +}
>> +
>> +int qb_flush(struct QBroker *broker,
>> +             struct QBlockState *qbs)
>> +{
>> +    int bd_ret;
>> +    BlockDriverState *bs;
>> +
>> +    broker->err_ret = 0;
>> +    bs = qbs->bdrvs;
>> +    bd_ret = bdrv_flush(bs);
>> +    if (bd_ret < 0) {
>> +        set_broker_err(broker, QB_ERR_INTERNAL_ERR,
>> +                       "Internal error.");
>> +    }
>> +    return broker->err_ret;
>> +}
>> +
>> +int qb_check_allocation(struct QBroker *broker,
>> +                        struct QBlockState *qbs,
>> +                        uint64_t start,
>> +                        int length,
>> +                        int *pstatus,
>> +                        int *plength)
>> +{
>> +    int ret;
>> +    int sector_start, sector_num, num;
>> +    BlockDriverState *bs;
>> +    unsigned int real_len, ret_len;
>> +
>> +    broker->err_ret = 0;
>> +    bs = qbs->bdrvs;
>> +
>> +    if (qbs->filename == NULL) {
>> +        set_broker_err(broker, QB_ERR_INVALID_PARAM,
>> +                       "Image was not opened first.");
>> +        goto out;
>> +    }
>> +
>> +    if (length <= 0) {
>> +        set_broker_err(broker, 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_broker_err(broker, 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 broker->err_ret;
>> +}
>> +
>> +int qb_info_image_static_get(struct QBroker *broker,
>> +                             struct QBlockState *qbs,
>> +                             struct QBlockStaticInfo **info)
>> +{
>> +    int ret = 0;
>> +    BlockDriverState *bs;
>> +    struct QBlockStaticInfo *info_tmp;
>> +    const char *fmt_str;
>> +    size_t total_sectors;
>> +    char backing_filename[1024];
>> +
>> +    if (qbs->filename == NULL) {
>> +        set_broker_err(broker, QB_ERR_INVALID_PARAM,
>> +                       "Block Image was not openned.");
>> +        ret = broker->err_ret;
>> +        goto out;
>> +    }
>> +
>> +    info_tmp = FUNC_CALLOC(1, sizeof(struct QBlockStaticInfo));
>> +    if (info_tmp == NULL) {
>> +        set_broker_err_nomem(broker);
>> +        ret = broker->err_ret;
>> +        goto out;
>> +    }
>> +
>> +    bs = qbs->bdrvs;
>> +
>> +    ret = filename2loc(broker,
>> +                       &(info_tmp->loc),
>> +                       qbs->filename);
>> +    if (ret < 0) {
>> +        goto free;
>> +    }
>> +
>> +    fmt_str = bdrv_get_format_name(bs);
>> +    info_tmp->fmt_type = qb_str2fmttype(fmt_str);
>> +
>> +    bdrv_get_geometry(bs, &total_sectors);
>> +    info_tmp->virt_size = total_sectors * BDRV_SECTOR_SIZE;
>> +
>> +    info_tmp->encrypt = bdrv_is_encrypted(bs);
>> +
>> +    bdrv_get_full_backing_filename(bs, backing_filename,
>> +                                   sizeof(backing_filename));
>> +    if (backing_filename[0] != '\0') {
>> +        ret = filename2loc(broker,
>> +                           &(info_tmp->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(broker, &info_tmp);
>> +    return ret;
>> +}
>> +
>> +/* free locations if it has string allocated on heap. */
>> +static void loc_free(struct QBlockProtInfo *loc)
>> +{
>> +    switch (loc->prot_type) {
>> +    case QB_PROT_FILE:
>> +        FUNC_FREE((void *)(loc->prot_op.o_file.filename));
>> +        loc->prot_op.o_file.filename = NULL;
>> +        break;
>> +    default:
>> +        break;
>> +    }
>> +}
>> +
>> +void qb_info_image_static_delete(struct QBroker *broker,
>> +                                 struct QBlockStaticInfo **info)
>> +{
>> +    loc_free(&(*info)->loc);
>> +    loc_free(&(*info)->backing_loc);
>> +
>> +    CLEAN_FREE(*info);
>> +}
>> diff --git a/libqblock/libqblock.h b/libqblock/libqblock.h
>> new file mode 100644
>> index 0000000..97e6c7c
>> --- /dev/null
>> +++ b/libqblock/libqblock.h
>> @@ -0,0 +1,292 @@
>> +/*
>> + * 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_broker_new: allocate a new broker
>> + *
>> + * Broker is used to pass operation to libqblock, and get feed back from it.
>> + *
>> + * Returns 0 on success, libqblock negative error value on fail.
>> + *
>> + * @broker: used to receive the created struct.
>> + */
>> +DLL_PUBLIC
>> +int qb_broker_new(struct QBroker **broker);
>> +
>> +/**
>> + * qb_broker_delete: delete broker
>> + *
>> + * Broker will be freed and set to NULL.
>> + *
>> + * @broker: operation broker to be deleted.
>> + */
>> +DLL_PUBLIC
>> +void qb_broker_delete(struct QBroker **broker);
>> +
>> +/**
>> + * 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.
>> + *
>> + * @broker: operation broker.
>> + * @qbs: used to receive the created struct.
>> + */
>> +DLL_PUBLIC
>> +int qb_state_new(struct QBroker *broker,
>> +                 struct QBlockState **qbs);
>> +
>> +/**
>> + * qb_state_delete: free a QBlockState struct
>> + *
>> + * if image was opened, qb_close must be called before delete.
>> + *
>> + * @broker: operation broker.
>> + * @qbs: pointer to the struct's pointer.
>> + */
>> +DLL_PUBLIC
>> +void qb_state_delete(struct QBroker *broker,
>> +                     struct QBlockState **qbs);
>> +
>> +/**
>> + * qb_prot_info_new: create a new struct QBlockProtInfo.
>> + *
>> + * return 0 on success, libqblock negative error value on fail.
>> + *
>> + * @broker: operation broker.
>> + * @op: pointer to receive the new created one.
>> + */
>> +DLL_PUBLIC
>> +int qb_prot_info_new(struct QBroker *broker,
>> +                     struct QBlockProtInfo **op);
>> +
>> +/**
>> + * qb_prot_info_delete: free a struct QBlockProtInfo.
>> + *
>> + * @broker: operation broker.
>> + * @op: pointer to the object, *op would be set to NULL.
>> + */
>> +DLL_PUBLIC
>> +void qb_prot_info_delete(struct QBroker *broker,
>> +                         struct QBlockProtInfo **op);
>> +
>> +/**
>> + * qb_fmt_info_new: create a new QBlockFmtInfo structure.
>> + *
>> + * return 0 on success, libqblock negative error value on fail.
>> + *
>> + * @broker: operation broker.
>> + * @op: pointer that will receive created struct.
>> + */
>> +DLL_PUBLIC
>> +int qb_fmt_info_new(struct QBroker *broker,
>> +                    struct QBlockFmtInfo **op);
>> +
>> +/**
>> + * qb_fmt_info_delete: free QBlockFmtInfo structure.
>> + *
>> + * @broker: operation broker.
>> + * @op: pointer to the struct, *op would be set to NULL.
>> + */
>> +DLL_PUBLIC
>> +void qb_fmt_info_delete(struct QBroker *broker,
>> +                        struct QBlockFmtInfo **op);
>> +
>> +
>> +/**
>> + * qb_open: open a block object.
>> + *
>> + * return 0 on success, libqblock negative error value on fail.
>> + *
>> + * @broker: operation broker.
>> + * @qbs: pointer to struct 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(struct QBroker *broker,
>> +            struct QBlockState *qbs,
>> +            struct QBlockProtInfo *loc,
>> +            struct QBlockFmtInfo *fmt,
>> +            int flag);
>> +
>> +/**
>> + * qb_close: close a block object.
>> + *
>> + * qb_flush is automatically done inside.
>> + *
>> + * @broker: operation broker.
>> + * @qbs: pointer to struct QBlockState.
>> + */
>> +DLL_PUBLIC
>> +void qb_close(struct QBroker *broker,
>> +              struct 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.
>> + *
>> + * @broker: operation broker.
>> + * @qbs: pointer to struct 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(struct QBroker *broker,
>> +              struct QBlockState *qbs,
>> +              struct QBlockProtInfo *loc,
>> +              struct QBlockFmtInfo *fmt,
>> +              int flag);
>> +
>> +
>> +/* sync access */
>> +/**
>> + * qb_read: block sync read.
>> + *
>> + * return number of bytes read, libqblock negative error value on fail.
>> + *
>> + * @broker: operation broker.
>> + * @qbs: pointer to struct QBlockState.
>> + * @buf: buffer that receive the content.
>> + * @len: length to read.
>> + * @offset: offset in the block data.
>> + */
>> +DLL_PUBLIC
>> +int64_t qb_read(struct QBroker *broker,
>> +                struct 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.
>> + *
>> + * @broker: operation broker.
>> + * @qbs: pointer to struct QBlockState.
>> + * @buf: buffer that receive the content.
>> + * @len: length to write.
>> + * @offset: offset in the block data.
>> + */
>> +DLL_PUBLIC
>> +int64_t qb_write(struct QBroker *broker,
>> +                 struct 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.
>> + *
>> + * @broker: operation broker.
>> + * @qbs: pointer to struct QBlockState.
>> + */
>> +DLL_PUBLIC
>> +int qb_flush(struct QBroker *broker,
>> +             struct 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.
>> + *
>> + * @broker: operation broker.
>> + * @qbs: pointer to struct QBlockState.
>> + * @start: start position, unit is byte.
>> + * @length: length to check, unit is byte.
>> + * @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(struct QBroker *broker,
>> +                        struct QBlockState *qbs,
>> +                        uint64_t start,
>> +                        int length,
>> +                        int *pstatus,
>> +                        int *plength);
>> +
>> +/* image information */
>> +/**
>> + * qb_get_image_info: get image info.
>> + *
>> + * return 0 on success, libqblock negative error value on fail.
>> + *
>> + * @broker: operation broker.
>> + * @qbs: pointer to struct QBlockState.
>> + * @info: pointer that would receive the information.
>> + */
>> +DLL_PUBLIC
>> +int qb_info_image_static_get(struct QBroker *broker,
>> +                             struct QBlockState *qbs,
>> +                             struct QBlockStaticInfo **info);
>> +
>> +/**
>> + * qb_delete_image_info: free image info.
>> + *
>> + * @broker: operation broker.
>> + * @info: pointer to the information struct.
>> + */
>> +DLL_PUBLIC
>> +void qb_info_image_static_delete(struct QBroker *broker,
>> +                                 struct 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
>> +enum QBlockFmtType qb_str2fmttype(const char *fmt_str);
>> +
>> +/**
>> + * qb_fmttype2str: libqblock format enum type to a string.
>> + *
>> + * return a pointer to the string, or NULL if type is not supported, and
>> + *  returned pointer do NOT need to free.
>> + *
>> + * @fmt: the format enum type.
>> + */
>> +DLL_PUBLIC
>> +const char *qb_fmttype2str(enum QBlockFmtType fmt_type);
>> +#endif
>> --
>> 1.7.1
>>
>>
>
>


-- 
Best Regards

Wenchao Xia

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

* Re: [Qemu-devel] [PATCH V2 3/6] libqblock error handling
  2012-09-11 20:32   ` Blue Swirl
@ 2012-09-12  2:58     ` Wenchao Xia
  2012-09-14 17:09       ` Blue Swirl
  0 siblings, 1 reply; 34+ messages in thread
From: Wenchao Xia @ 2012-09-12  2:58 UTC (permalink / raw)
  To: Blue Swirl; +Cc: kwolf, aliguori, stefanha, qemu-devel, pbonzini, eblake

于 2012-9-12 4:32, Blue Swirl 写道:
> On Mon, Sep 10, 2012 at 8:26 AM, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>    This patch contains error handling APIs, which user could call them to
>> get error details.
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>>   libqblock/libqblock-error.c |   60 +++++++++++++++++++++++++++++++++++++++++++
>>   libqblock/libqblock-error.h |   50 +++++++++++++++++++++++++++++++++++
>>   2 files changed, 110 insertions(+), 0 deletions(-)
>>   create mode 100644 libqblock/libqblock-error.c
>>   create mode 100644 libqblock/libqblock-error.h
>>
>> diff --git a/libqblock/libqblock-error.c b/libqblock/libqblock-error.c
>> new file mode 100644
>> index 0000000..d5ebabf
>> --- /dev/null
>> +++ b/libqblock/libqblock-error.c
>> @@ -0,0 +1,60 @@
>> +/*
>> + * 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(struct QBroker *broker,
>> +                            char *buf, int buf_size)
>
> size_t buf_size
>
   OK.

>> +{
>> +    const char *err_ret_str;
>> +    switch (broker->err_ret) {
>> +    case QB_ERR_MEM_ERR:
>> +        err_ret_str = "Not enough memory.";
>> +        break;
>> +    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 = "Unknow error.";
>> +        break;
>> +    }
>> +    if (broker == NULL) {
>> +        snprintf(buf, buf_size, "%s", err_ret_str);
>> +        return;
>> +    }
>> +
>> +    if (broker->err_ret == QB_ERR_INTERNAL_ERR) {
>> +        snprintf(buf, buf_size, "%s %s errno [%d]. strerror [%s].",
>> +                     err_ret_str, broker->err_msg,
>> +                     broker->err_no, strerror(-broker->err_no));
>> +    } else {
>> +        snprintf(buf, buf_size, "%s %s",
>> +                     err_ret_str, broker->err_msg);
>> +    }
>
> Please NUL terminate the string.
>
   snprintf will add "\0" automatically, could u explain more about this?

>> +    return;
>> +}
>> +
>> +int qb_error_get_errno(struct QBroker *broker)
>> +{
>> +    if (broker->err_ret == QB_ERR_INTERNAL_ERR) {
>> +        return broker->err_no;
>> +    }
>> +    return 0;
>> +}
>> diff --git a/libqblock/libqblock-error.h b/libqblock/libqblock-error.h
>> new file mode 100644
>> index 0000000..0690cfb
>> --- /dev/null
>> +++ b/libqblock/libqblock-error.h
>> @@ -0,0 +1,50 @@
>> +/*
>> + * 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_MEM_ERR (-1)
>> +#define QB_ERR_INTERNAL_ERR (-2)
>> +#define QB_ERR_INVALID_PARAM (-3)
>> +#define QB_ERR_BLOCK_OUT_OF_RANGE (-100)
>> +
>> +/* 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.
>> + *
>> + * @broker: operation broker, 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(struct QBroker *broker,
>> +                            char *buf, int buf_size);
>> +
>> +/**
>> + * qb_error_get_errno: get error number, only valid when err_ret is
>> + *   QB_ERR_INTERNAL_ERR.
>> + *
>> + * return negative errno or 0 if last error is not QB_ERR_INTERNAL_ERR.
>> + *
>> + * @broker: operation broker.
>> + */
>> +DLL_PUBLIC
>> +int qb_error_get_errno(struct QBroker *broker);
>> +
>> +#endif
>> --
>> 1.7.1
>>
>>
>


-- 
Best Regards

Wenchao Xia

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

* Re: [Qemu-devel] [PATCH V2 2/6] libqblock type and structure defines
  2012-09-11 22:52     ` Eric Blake
@ 2012-09-12  3:05       ` Wenchao Xia
  2012-09-12 12:59         ` Eric Blake
  2012-09-14 18:02       ` Blue Swirl
  1 sibling, 1 reply; 34+ messages in thread
From: Wenchao Xia @ 2012-09-12  3:05 UTC (permalink / raw)
  To: Eric Blake; +Cc: kwolf, aliguori, stefanha, qemu-devel, Blue Swirl, pbonzini

于 2012-9-12 6:52, Eric Blake 写道:
> On 09/11/2012 02:31 PM, Blue Swirl wrote:
>> On Mon, Sep 10, 2012 at 8:26 AM, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>>    This patch contains type and defines used in APIs, one file for public usage
>>> by user, one for libqblock internal usage.
>>>
>
>>> +
>>> +#if __GNUC__ >= 4
>>
>> #if defined(__GNUC__) && __GNUC__ ...
>
> Seriously?  We require a C99-compliant compiler, which is required to
> treat the more compact version identically (all undefined names evaluate
> to 0 in the preprocessor), and HACKING doesn't mandate that we spell out
> a defined-ness check first.  Okay, so configure adds -Wundef to the set
> of CFLAGS, but I fail to see why we want -Wundef (that's an
> anachronistic warning, only there to help you if you are writing code
> portable to K&R).
>
   So if the preprocessor replaced __GNUC__ to 0, is there difference
between these two kinds of macoros?
#if __GNUC__ >= 4
#if defined(__GNUC__) && __GNUC__ >=4

   I guess they are the same.

-- 
Best Regards

Wenchao Xia

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

* Re: [Qemu-devel] [PATCH V2 1/6] libqblock API design
  2012-09-11 20:28   ` Blue Swirl
  2012-09-12  2:54     ` Wenchao Xia
@ 2012-09-12  8:19     ` Kevin Wolf
  2012-09-12  9:21       ` Wenchao Xia
  2012-09-14 19:08       ` Blue Swirl
  1 sibling, 2 replies; 34+ messages in thread
From: Kevin Wolf @ 2012-09-12  8:19 UTC (permalink / raw)
  To: Blue Swirl; +Cc: aliguori, stefanha, qemu-devel, pbonzini, eblake, Wenchao Xia

Am 11.09.2012 22:28, schrieb Blue Swirl:
> On Mon, Sep 10, 2012 at 8:26 AM, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>   This patch contains the major APIs in the library.
>> Important APIs:
>>   1 QBroker. These structure was used to retrieve errors, every thread must
>> create one first, later maybe thread related staff could be added into it.
>>   2 QBlockState. It stands for an block image object.
>>   3 QBlockStaticInfo. It contains static information such as location, backing
>> file, size.
>>   4 ABI was kept with reserved members.
>>   5 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.c | 1077 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  libqblock/libqblock.h |  292 +++++++++++++
>>  2 files changed, 1369 insertions(+), 0 deletions(-)
>>  create mode 100644 libqblock/libqblock.c
>>  create mode 100644 libqblock/libqblock.h
>>
>> diff --git a/libqblock/libqblock.c b/libqblock/libqblock.c
>> new file mode 100644
>> index 0000000..133ac0f
>> --- /dev/null
>> +++ b/libqblock/libqblock.c
>> @@ -0,0 +1,1077 @@
>> +/*
>> + * 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"
>> +
>> +struct LibqblockGlobalData {
>> +    int init_flag;
>> +};
>> +
>> +struct LibqblockGlobalData g_libqblock_data;
>> +
>> +__attribute__((constructor))
>> +static void libqblock_init(void)
>> +{
>> +    if (g_libqblock_data.init_flag == 0) {
>> +        bdrv_init();
>> +        qemu_init_main_loop();
>> +    }
>> +    g_libqblock_data.init_flag = 1;
>> +}
>> +
>> +const char *qb_fmttype2str(enum QBlockFmtType fmt_type)
>> +{
>> +    const char *ret = NULL;
>> +    switch (fmt_type) {
>> +    case QB_FMT_COW:
>> +        ret = "cow";
>> +        break;
>> +    case QB_FMT_QED:
>> +        ret = "qed";
>> +        break;
>> +    case QB_FMT_QCOW:
>> +        ret = "qcow";
>> +        break;
>> +    case QB_FMT_QCOW2:
>> +        ret = "qcow2";
>> +        break;
>> +    case QB_FMT_RAW:
>> +        ret = "raw";
>> +        break;
>> +    case QB_FMT_RBD:
>> +        ret = "rbd";
>> +        break;
>> +    case QB_FMT_SHEEPDOG:
>> +        ret = "sheepdog";
>> +        break;
>> +    case QB_FMT_VDI:
>> +        ret = "vdi";
>> +        break;
>> +    case QB_FMT_VMDK:
>> +        ret = "vmdk";
>> +        break;
>> +    case QB_FMT_VPC:
>> +        ret = "vpc";
>> +        break;
>> +    default:
>> +        break;
>> +    }
>> +    return ret;
>> +}
>> +
>> +enum QBlockFmtType qb_str2fmttype(const char *fmt_str)
>> +{
>> +    enum QBlockFmtType ret = QB_FMT_NONE;
>> +    if (0 == strcmp(fmt_str, "cow")) {
> 
> This order is not common in QEMU.

How about just changing the whole thing to a table that maps
QBlockFmtType to strings, and then both conversion functions could just
search that table?

> 
>> +        ret = QB_FMT_COW;
>> +    } else if (0 == strcmp(fmt_str, "qed")) {
>> +        ret = QB_FMT_QED;
>> +    } else if (0 == strcmp(fmt_str, "qcow")) {
>> +        ret = QB_FMT_QCOW;
>> +    } else if (0 == strcmp(fmt_str, "qcow2")) {
>> +        ret = QB_FMT_QCOW2;
>> +    } else if (0 == strcmp(fmt_str, "raw")) {
>> +        ret = QB_FMT_RAW;
>> +    } else if (0 == strcmp(fmt_str, "rbd")) {
>> +        ret = QB_FMT_RBD;
>> +    } else if (0 == strcmp(fmt_str, "sheepdog")) {
>> +        ret = QB_FMT_SHEEPDOG;
>> +    } else if (0 == strcmp(fmt_str, "vdi")) {
>> +        ret = QB_FMT_VDI;
>> +    } else if (0 == strcmp(fmt_str, "vmdk")) {
>> +        ret = QB_FMT_VMDK;
>> +    } else if (0 == strcmp(fmt_str, "vpc")) {
>> +        ret = QB_FMT_VPC;
>> +    }
>> +    return ret;
>> +}

Kevin

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

* Re: [Qemu-devel] [PATCH V2 1/6] libqblock API design
  2012-09-12  8:19     ` Kevin Wolf
@ 2012-09-12  9:21       ` Wenchao Xia
  2012-09-14 19:08       ` Blue Swirl
  1 sibling, 0 replies; 34+ messages in thread
From: Wenchao Xia @ 2012-09-12  9:21 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: aliguori, stefanha, qemu-devel, Blue Swirl, pbonzini, eblake

于 2012-9-12 16:19, Kevin Wolf 写道:
> Am 11.09.2012 22:28, schrieb Blue Swirl:
>> On Mon, Sep 10, 2012 at 8:26 AM, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>>    This patch contains the major APIs in the library.
>>> Important APIs:
>>>    1 QBroker. These structure was used to retrieve errors, every thread must
>>> create one first, later maybe thread related staff could be added into it.
>>>    2 QBlockState. It stands for an block image object.
>>>    3 QBlockStaticInfo. It contains static information such as location, backing
>>> file, size.
>>>    4 ABI was kept with reserved members.
>>>    5 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.c | 1077 +++++++++++++++++++++++++++++++++++++++++++++++++
>>>   libqblock/libqblock.h |  292 +++++++++++++
>>>   2 files changed, 1369 insertions(+), 0 deletions(-)
>>>   create mode 100644 libqblock/libqblock.c
>>>   create mode 100644 libqblock/libqblock.h
>>>
>>> diff --git a/libqblock/libqblock.c b/libqblock/libqblock.c
>>> new file mode 100644
>>> index 0000000..133ac0f
>>> --- /dev/null
>>> +++ b/libqblock/libqblock.c
>>> @@ -0,0 +1,1077 @@
>>> +/*
>>> + * 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"
>>> +
>>> +struct LibqblockGlobalData {
>>> +    int init_flag;
>>> +};
>>> +
>>> +struct LibqblockGlobalData g_libqblock_data;
>>> +
>>> +__attribute__((constructor))
>>> +static void libqblock_init(void)
>>> +{
>>> +    if (g_libqblock_data.init_flag == 0) {
>>> +        bdrv_init();
>>> +        qemu_init_main_loop();
>>> +    }
>>> +    g_libqblock_data.init_flag = 1;
>>> +}
>>> +
>>> +const char *qb_fmttype2str(enum QBlockFmtType fmt_type)
>>> +{
>>> +    const char *ret = NULL;
>>> +    switch (fmt_type) {
>>> +    case QB_FMT_COW:
>>> +        ret = "cow";
>>> +        break;
>>> +    case QB_FMT_QED:
>>> +        ret = "qed";
>>> +        break;
>>> +    case QB_FMT_QCOW:
>>> +        ret = "qcow";
>>> +        break;
>>> +    case QB_FMT_QCOW2:
>>> +        ret = "qcow2";
>>> +        break;
>>> +    case QB_FMT_RAW:
>>> +        ret = "raw";
>>> +        break;
>>> +    case QB_FMT_RBD:
>>> +        ret = "rbd";
>>> +        break;
>>> +    case QB_FMT_SHEEPDOG:
>>> +        ret = "sheepdog";
>>> +        break;
>>> +    case QB_FMT_VDI:
>>> +        ret = "vdi";
>>> +        break;
>>> +    case QB_FMT_VMDK:
>>> +        ret = "vmdk";
>>> +        break;
>>> +    case QB_FMT_VPC:
>>> +        ret = "vpc";
>>> +        break;
>>> +    default:
>>> +        break;
>>> +    }
>>> +    return ret;
>>> +}
>>> +
>>> +enum QBlockFmtType qb_str2fmttype(const char *fmt_str)
>>> +{
>>> +    enum QBlockFmtType ret = QB_FMT_NONE;
>>> +    if (0 == strcmp(fmt_str, "cow")) {
>>
>> This order is not common in QEMU.
>
> How about just changing the whole thing to a table that maps
> QBlockFmtType to strings, and then both conversion functions could just
> search that table?
>
   Good idea, will go this way.

>>
>>> +        ret = QB_FMT_COW;
>>> +    } else if (0 == strcmp(fmt_str, "qed")) {
>>> +        ret = QB_FMT_QED;
>>> +    } else if (0 == strcmp(fmt_str, "qcow")) {
>>> +        ret = QB_FMT_QCOW;
>>> +    } else if (0 == strcmp(fmt_str, "qcow2")) {
>>> +        ret = QB_FMT_QCOW2;
>>> +    } else if (0 == strcmp(fmt_str, "raw")) {
>>> +        ret = QB_FMT_RAW;
>>> +    } else if (0 == strcmp(fmt_str, "rbd")) {
>>> +        ret = QB_FMT_RBD;
>>> +    } else if (0 == strcmp(fmt_str, "sheepdog")) {
>>> +        ret = QB_FMT_SHEEPDOG;
>>> +    } else if (0 == strcmp(fmt_str, "vdi")) {
>>> +        ret = QB_FMT_VDI;
>>> +    } else if (0 == strcmp(fmt_str, "vmdk")) {
>>> +        ret = QB_FMT_VMDK;
>>> +    } else if (0 == strcmp(fmt_str, "vpc")) {
>>> +        ret = QB_FMT_VPC;
>>> +    }
>>> +    return ret;
>>> +}
>
> Kevin
>


-- 
Best Regards

Wenchao Xia

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

* Re: [Qemu-devel] [PATCH V2 2/6] libqblock type and structure defines
  2012-09-12  3:05       ` Wenchao Xia
@ 2012-09-12 12:59         ` Eric Blake
  2012-09-13  3:24           ` Wenchao Xia
  0 siblings, 1 reply; 34+ messages in thread
From: Eric Blake @ 2012-09-12 12:59 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: kwolf, aliguori, stefanha, qemu-devel, Blue Swirl, pbonzini

[-- Attachment #1: Type: text/plain, Size: 1030 bytes --]

On 09/11/2012 09:05 PM, Wenchao Xia wrote:
>> Seriously?  We require a C99-compliant compiler, which is required to
>> treat the more compact version identically (all undefined names evaluate
>> to 0 in the preprocessor), and HACKING doesn't mandate that we spell out
>> a defined-ness check first.  Okay, so configure adds -Wundef to the set
>> of CFLAGS, but I fail to see why we want -Wundef (that's an
>> anachronistic warning, only there to help you if you are writing code
>> portable to K&R).
>>
>   So if the preprocessor replaced __GNUC__ to 0, is there difference
> between these two kinds of macoros?
> #if __GNUC__ >= 4
> #if defined(__GNUC__) && __GNUC__ >=4

The only difference is whether -Wundef will warn, and I'm trying to
argue that qemu's current use of -Wundef is pointless, as that warning
exists solely for K&R compatibility, not for modern standard-compliant
code correctness.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]

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

* Re: [Qemu-devel] [PATCH V2 2/6] libqblock type and structure defines
  2012-09-12 12:59         ` Eric Blake
@ 2012-09-13  3:24           ` Wenchao Xia
  2012-09-13  3:33             ` Eric Blake
  0 siblings, 1 reply; 34+ messages in thread
From: Wenchao Xia @ 2012-09-13  3:24 UTC (permalink / raw)
  To: Eric Blake; +Cc: kwolf, aliguori, stefanha, qemu-devel, Blue Swirl, pbonzini

于 2012-9-12 20:59, Eric Blake 写道:
> On 09/11/2012 09:05 PM, Wenchao Xia wrote:
>>> Seriously?  We require a C99-compliant compiler, which is required to
>>> treat the more compact version identically (all undefined names evaluate
>>> to 0 in the preprocessor), and HACKING doesn't mandate that we spell out
>>> a defined-ness check first.  Okay, so configure adds -Wundef to the set
>>> of CFLAGS, but I fail to see why we want -Wundef (that's an
>>> anachronistic warning, only there to help you if you are writing code
>>> portable to K&R).
>>>
>>    So if the preprocessor replaced __GNUC__ to 0, is there difference
>> between these two kinds of macoros?
>> #if __GNUC__ >= 4
>> #if defined(__GNUC__) && __GNUC__ >=4
>
> The only difference is whether -Wundef will warn, and I'm trying to
> argue that qemu's current use of -Wundef is pointless, as that warning
> exists solely for K&R compatibility, not for modern standard-compliant
> code correctness.
>
  OK ,then I think
#if __GNUC__ >= 4
....
#else
   [warn name space pollution may happen]
#endif
would be better.
-- 
Best Regards

Wenchao Xia

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

* Re: [Qemu-devel] [PATCH V2 2/6] libqblock type and structure defines
  2012-09-13  3:24           ` Wenchao Xia
@ 2012-09-13  3:33             ` Eric Blake
  2012-09-13  3:49               ` Eric Blake
  0 siblings, 1 reply; 34+ messages in thread
From: Eric Blake @ 2012-09-13  3:33 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: kwolf, aliguori, stefanha, qemu-devel, Blue Swirl, pbonzini

[-- Attachment #1: Type: text/plain, Size: 1958 bytes --]

On 09/12/2012 09:24 PM, Wenchao Xia wrote:
> 于 2012-9-12 20:59, Eric Blake 写道:
>> On 09/11/2012 09:05 PM, Wenchao Xia wrote:
>>>> Seriously?  We require a C99-compliant compiler, which is required to
>>>> treat the more compact version identically (all undefined names
>>>> evaluate
>>>> to 0 in the preprocessor), and HACKING doesn't mandate that we spell
>>>> out
>>>> a defined-ness check first.  Okay, so configure adds -Wundef to the set
>>>> of CFLAGS, but I fail to see why we want -Wundef (that's an
>>>> anachronistic warning, only there to help you if you are writing code
>>>> portable to K&R).
>>>>
>>>    So if the preprocessor replaced __GNUC__ to 0, is there difference
>>> between these two kinds of macoros?
>>> #if __GNUC__ >= 4
>>> #if defined(__GNUC__) && __GNUC__ >=4
>>
>> The only difference is whether -Wundef will warn, and I'm trying to
>> argue that qemu's current use of -Wundef is pointless, as that warning
>> exists solely for K&R compatibility, not for modern standard-compliant
>> code correctness.
>>
>  OK ,then I think
> #if __GNUC__ >= 4
> ....
> #else
>   [warn name space pollution may happen]
> #endif
> would be better.

It may be shorter, but it is definitely not better, at least not in the
current context of qemu.  Using the short form will fail a -Werror
build, unless you also write a patch to qemu's configure to quit
supplying -Wundef during builds.  But as touching configure has a bigger
impact to the overall qemu project, you're going to need a lot more
buy-in from other developers that -Wundef is not helping qemu gain any
portability, and that it is safe to ditch it (or get enough
counter-arguments from other developers why qemu insists on the
anachronistic style enforced by -Wundef, at which point you must comply
and use the longer form).

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]

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

* Re: [Qemu-devel] [PATCH V2 2/6] libqblock type and structure defines
  2012-09-13  3:33             ` Eric Blake
@ 2012-09-13  3:49               ` Eric Blake
  2012-09-14 18:11                 ` Blue Swirl
  0 siblings, 1 reply; 34+ messages in thread
From: Eric Blake @ 2012-09-13  3:49 UTC (permalink / raw)
  To: Eric Blake
  Cc: kwolf, aliguori, stefanha, qemu-devel, Blue Swirl, pbonzini,
	Wenchao Xia

[-- Attachment #1: Type: text/plain, Size: 1881 bytes --]

On 09/12/2012 09:33 PM, Eric Blake wrote:
>>  OK ,then I think
>> #if __GNUC__ >= 4
>> ....
>> #else
>>   [warn name space pollution may happen]
>> #endif
>> would be better.
> 
> It may be shorter, but it is definitely not better, at least not in the
> current context of qemu.  Using the short form will fail a -Werror
> build, unless you also write a patch to qemu's configure to quit
> supplying -Wundef during builds.  But as touching configure has a bigger
> impact to the overall qemu project, you're going to need a lot more
> buy-in from other developers that -Wundef is not helping qemu gain any
> portability, and that it is safe to ditch it (or get enough
> counter-arguments from other developers why qemu insists on the
> anachronistic style enforced by -Wundef, at which point you must comply
> and use the longer form).

On second thought, this _particular_ usage will never fail a -Wundef
-Werror build, precisely because -Wundef is a gcc warning, which impies
the warning is only ever useful in the same scenarios that the __GNUC__
macro is always defined (that is, __GNUC__ is undefined only on a
non-gcc compiler, but what non-gcc compiler supports -Wundef -Werror?).

But why should this line be the one exemption to the rules?  Either qemu
insists on the -Wundef style of coding (and you should use the long form
to conform to that style, on the off-chance that someone ever wants to
port to a non-gcc compiler, even in this one place where gcc can't warn
you about the violation of that style), or we should change the qemu
style (at which point, the short form is nicer here, but it also implies
the potential for cleaning up lots of other places to also use short
forms and rely on preprocessor 0 computation).

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]

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

* Re: [Qemu-devel] [PATCH V2 1/6] libqblock API design
  2012-09-11  3:16     ` Wenchao Xia
@ 2012-09-14  2:03       ` Wenchao Xia
  0 siblings, 0 replies; 34+ messages in thread
From: Wenchao Xia @ 2012-09-14  2:03 UTC (permalink / raw)
  To: Eric Blake; +Cc: kwolf, aliguori, stefanha, qemu-devel, blauwirbel, pbonzini

Hi,
  about the OOM issue, I plan to drop the OOM request for now in
implemention, and at next step use rpc to wrap these APIs, then
user can select to link directly against the library or the wrapped
API, what do you think about it?
> 于 2012-9-11 5:07, Eric Blake 写道:
>> On 09/10/2012 02:26 AM, Wenchao Xia wrote:
>>>    This patch contains the major APIs in the library.
>>> Important APIs:
>>>    1 QBroker. These structure was used to retrieve errors, every 
>>> thread must
>>> create one first, later maybe thread related staff could be added 
>>> into it.
>>>    2 QBlockState. It stands for an block image object.
>>>    3 QBlockStaticInfo. It contains static information such as 
>>> location, backing
>>> file, size.
>>>    4 ABI was kept with reserved members.
>>>    5 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.c | 1077 
>>> +++++++++++++++++++++++++++++++++++++++++++++++++
>>>   libqblock/libqblock.h |  292 +++++++++++++
>>
>> Two new files, but no build machinery to build them to see if they have
>> blatant errors.  Yes, it is bisectable, but no, the bisection is not
>> useful.  I'd rather see the Makefile patches with stub files at the
>> beginning, then flesh out the stubs, where each patch builds along the 
>> way.
>>
>> In particular,
>>
>    OK, I understand the importance to make each patch workable to
> maintainer, will adjust the patches to make each one works.
> 
>>> +++ b/libqblock/libqblock.c
>>
>>> +#include "libqblock.h"
>>> +#include "libqblock-internal.h"
>>
>> There is no libqblock-internal.h with just this patch applied, making it
>> impossible to validate this patch in order (the fact that 'make' isn't
>> flagging this incomplete patch is because you aren't building
>> libqblock-o yet).  I'm planning on overlooking the .c file and focusing
>> on the user interface (I'd rather leave the detailed review to those
>> more familiar with qemu, while I'm personally worried about how libvirt
>> would ever use libqblock if you ever solved the glib-aborts-on-OOM issue
>> to make the library even worth using).
>>
>    About the OOM issue, I think there are potential 3 ways to solve it:
> 1 modify qemu block layer code, in this way providing libqblock at first
> will help, for that it will encapsulate and isolate all codes needed
> for block layer, and we got test case on the top to validate OOM
> behavior.
> 2 Using glib and forgot OOM now. Then at higher layer, they have two
> choice: if they want no exiting on OOM, wrap the API with xmlrpc or
> something like; otherwise directly use the API.
> 3 switch the implemention of libqblock, do not link qemu block code
> directly, fork and execute qemu-img, qemu-nbd. This require a
> re-implement about AIO in libqblock, with GSource AIO framework I am
> not sure if it would exit on OOM, but I guess better to not involve any
> glib in this case. Additional challenge would be, any more
> functionalities adding require patch for qemu-img, qemu-io, qemu-nbd
> first, such as image information retrieving, allocation detection,
> and libqblock need to parse string output carefully, better to get
> all output in json format. Personally I am worried to handle many
> exception in string parsing.
> 
>    To me, the second way seems most reasonable, it allows qemu block
> remain tightly bind to glib. The first way also make sense, but
> need to loose the tight between qemu and glib. what do you think?
> 
>>> +++ b/libqblock/libqblock.h
>>> @@ -0,0 +1,292 @@
>>> +/*
>>> + * 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"
>>
>> Even worse - you've introduced a public header that I'm supposed to be
>> able to use, but I can't use it because libqblock-types.h and
>> libqblock-error.h don't exist.  I'd much rather review a patch series
>> built incrementally from ground up, with each piece working, rather than
>> reviewing an API that depends on constructs that aren't defined until
>> later patches.
>>
>>> +/**
>>> + * qb_broker_new: allocate a new broker
>>> + *
>>> + * Broker is used to pass operation to libqblock, and get feed back 
>>> from it.
>>
>> s/feed back/feedback/
>>
>>> +/**
>>> + * qb_state_delete: free a QBlockState struct
>>> + *
>>> + * if image was opened, qb_close must be called before delete.
>>
>> And if it wasn't closed, what happens?  Should this function return int,
>> instead of void, to error out in the case that it is called out of order?
>>
>>> +/**
>>> + * qb_prot_info_new: create a new struct QBlockProtInfo.
>>
>> Inconsistent on whether your descriptions end in '.' or not.
>>
>>> +/* sync access */
>>> +/**
>>> + * qb_read: block sync read.
>>> + *
>>> + * return number of bytes read, libqblock negative error value on fail.
>>> + *
>>> + * @broker: operation broker.
>>> + * @qbs: pointer to struct QBlockState.
>>> + * @buf: buffer that receive the content.
>>> + * @len: length to read.
>>> + * @offset: offset in the block data.
>>> + */
>>> +DLL_PUBLIC
>>> +int64_t qb_read(struct QBroker *broker,
>>> +                struct QBlockState *qbs,
>>> +                uint8_t *buf,
>>> +                uint32_t len,
>>> +                uint64_t offset);
>>
>> Seems odd to have 32 bit limit on input, when output handles 64 bit.
>>
>>> +
>>> +/**
>>> + * qb_write: block sync write.
>>> + *
>>> + * return number of bytes written, libqblock negative error value on 
>>> fail.
>>> + *
>>> + * @broker: operation broker.
>>> + * @qbs: pointer to struct QBlockState.
>>> + * @buf: buffer that receive the content.
>>> + * @len: length to write.
>>> + * @offset: offset in the block data.
>>> + */
>>> +DLL_PUBLIC
>>> +int64_t qb_write(struct QBroker *broker,
>>> +                 struct QBlockState *qbs,
>>> +                 const uint8_t *buf,
>>> +                 uint32_t len,
>>> +                 uint64_t offset);
>>
>> and again.
>>
>>> +/* 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.
>>> + *
>>> + * @broker: operation broker.
>>> + * @qbs: pointer to struct QBlockState.
>>> + * @start: start position, unit is byte.
>>> + * @length: length to check, unit is byte.
>>
>> Needs to be at least int32_t to match your read/write; or better yet
>> int64_t to allow probes of more than 2G at a time.
>>
>>> + * @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(struct QBroker *broker,
>>> +                        struct QBlockState *qbs,
>>> +                        uint64_t start,
>>> +                        int length,
>>> +                        int *pstatus,
>>> +                        int *plength);
>>
>> If you change the type of length, then plength needs to match.
>>
>>> +/**
>>> + * qb_fmttype2str: libqblock format enum type to a string.
>>> + *
>>> + * return a pointer to the string, or NULL if type is not supported, 
>>> and
>>> + *  returned pointer do NOT need to free.
>>
>> grammar; I suggest:
>> returned pointer must not be freed
>>
> 
> 


-- 
Best Regards

Wenchao Xia

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

* Re: [Qemu-devel] [PATCH V2 3/6] libqblock error handling
  2012-09-12  2:58     ` Wenchao Xia
@ 2012-09-14 17:09       ` Blue Swirl
  0 siblings, 0 replies; 34+ messages in thread
From: Blue Swirl @ 2012-09-14 17:09 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: kwolf, aliguori, stefanha, qemu-devel, pbonzini, eblake

On Wed, Sep 12, 2012 at 2:58 AM, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
> 于 2012-9-12 4:32, Blue Swirl 写道:
>
>> On Mon, Sep 10, 2012 at 8:26 AM, Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> wrote:
>>>
>>>    This patch contains error handling APIs, which user could call them to
>>> get error details.
>>>
>>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>>> ---
>>>   libqblock/libqblock-error.c |   60
>>> +++++++++++++++++++++++++++++++++++++++++++
>>>   libqblock/libqblock-error.h |   50 +++++++++++++++++++++++++++++++++++
>>>   2 files changed, 110 insertions(+), 0 deletions(-)
>>>   create mode 100644 libqblock/libqblock-error.c
>>>   create mode 100644 libqblock/libqblock-error.h
>>>
>>> diff --git a/libqblock/libqblock-error.c b/libqblock/libqblock-error.c
>>> new file mode 100644
>>> index 0000000..d5ebabf
>>> --- /dev/null
>>> +++ b/libqblock/libqblock-error.c
>>> @@ -0,0 +1,60 @@
>>> +/*
>>> + * 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(struct QBroker *broker,
>>> +                            char *buf, int buf_size)
>>
>>
>> size_t buf_size
>>
>   OK.
>
>
>>> +{
>>> +    const char *err_ret_str;
>>> +    switch (broker->err_ret) {
>>> +    case QB_ERR_MEM_ERR:
>>> +        err_ret_str = "Not enough memory.";
>>> +        break;
>>> +    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 = "Unknow error.";
>>> +        break;
>>> +    }
>>> +    if (broker == NULL) {
>>> +        snprintf(buf, buf_size, "%s", err_ret_str);
>>> +        return;
>>> +    }
>>> +
>>> +    if (broker->err_ret == QB_ERR_INTERNAL_ERR) {
>>> +        snprintf(buf, buf_size, "%s %s errno [%d]. strerror [%s].",
>>> +                     err_ret_str, broker->err_msg,
>>> +                     broker->err_no, strerror(-broker->err_no));
>>> +    } else {
>>> +        snprintf(buf, buf_size, "%s %s",
>>> +                     err_ret_str, broker->err_msg);
>>> +    }
>>
>>
>> Please NUL terminate the string.
>>
>   snprintf will add "\0" automatically, could u explain more about this?

Sorry, I claim temporary insanity.

>
>
>>> +    return;
>>> +}
>>> +
>>> +int qb_error_get_errno(struct QBroker *broker)
>>> +{
>>> +    if (broker->err_ret == QB_ERR_INTERNAL_ERR) {
>>> +        return broker->err_no;
>>> +    }
>>> +    return 0;
>>> +}
>>> diff --git a/libqblock/libqblock-error.h b/libqblock/libqblock-error.h
>>> new file mode 100644
>>> index 0000000..0690cfb
>>> --- /dev/null
>>> +++ b/libqblock/libqblock-error.h
>>> @@ -0,0 +1,50 @@
>>> +/*
>>> + * 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_MEM_ERR (-1)
>>> +#define QB_ERR_INTERNAL_ERR (-2)
>>> +#define QB_ERR_INVALID_PARAM (-3)
>>> +#define QB_ERR_BLOCK_OUT_OF_RANGE (-100)
>>> +
>>> +/* 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.
>>> + *
>>> + * @broker: operation broker, 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(struct QBroker *broker,
>>> +                            char *buf, int buf_size);
>>> +
>>> +/**
>>> + * qb_error_get_errno: get error number, only valid when err_ret is
>>> + *   QB_ERR_INTERNAL_ERR.
>>> + *
>>> + * return negative errno or 0 if last error is not QB_ERR_INTERNAL_ERR.
>>> + *
>>> + * @broker: operation broker.
>>> + */
>>> +DLL_PUBLIC
>>> +int qb_error_get_errno(struct QBroker *broker);
>>> +
>>> +#endif
>>> --
>>> 1.7.1
>>>
>>>
>>
>
>
> --
> Best Regards
>
> Wenchao Xia
>

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

* Re: [Qemu-devel] [PATCH V2 2/6] libqblock type and structure defines
  2012-09-11 22:52     ` Eric Blake
  2012-09-12  3:05       ` Wenchao Xia
@ 2012-09-14 18:02       ` Blue Swirl
  1 sibling, 0 replies; 34+ messages in thread
From: Blue Swirl @ 2012-09-14 18:02 UTC (permalink / raw)
  To: Eric Blake; +Cc: kwolf, aliguori, stefanha, qemu-devel, pbonzini, Wenchao Xia

On Tue, Sep 11, 2012 at 10:52 PM, Eric Blake <eblake@redhat.com> wrote:
> On 09/11/2012 02:31 PM, Blue Swirl wrote:
>> On Mon, Sep 10, 2012 at 8:26 AM, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>>   This patch contains type and defines used in APIs, one file for public usage
>>> by user, one for libqblock internal usage.
>>>
>
>>> +
>>> +#if __GNUC__ >= 4
>>
>> #if defined(__GNUC__) && __GNUC__ ...
>
> Seriously?  We require a C99-compliant compiler, which is required to
> treat the more compact version identically (all undefined names evaluate
> to 0 in the preprocessor), and HACKING doesn't mandate that we spell out
> a defined-ness check first.  Okay, so configure adds -Wundef to the set
> of CFLAGS, but I fail to see why we want -Wundef (that's an
> anachronistic warning, only there to help you if you are writing code
> portable to K&R).

I don't think we require C99 compliance. Removing -Wundef may have
merits of its own even if I fail to see them. Typically a stricter set
of warnings is better.

But this case is different from other QEMU since this is supposed to
be a public header file for a shared library that will be used by
projects external to QEMU. We have absolutely no control what compiler
flags the user project is using, so we should try to be compatible
with any reasonable compilers with any reasonable settings. We could
even try to be compatible with C++.

The above applies only to these headers. Aiming for compatibility for
other compilers for the .c files would make sense if there was a lot
of interest to use only the library directly in another project, but
let's wait for the interested crowds to show up.

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

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

* Re: [Qemu-devel] [PATCH V2 2/6] libqblock type and structure defines
  2012-09-13  3:49               ` Eric Blake
@ 2012-09-14 18:11                 ` Blue Swirl
  2012-09-17  2:23                   ` Wenchao Xia
  0 siblings, 1 reply; 34+ messages in thread
From: Blue Swirl @ 2012-09-14 18:11 UTC (permalink / raw)
  To: Eric Blake; +Cc: kwolf, aliguori, stefanha, qemu-devel, pbonzini, Wenchao Xia

On Thu, Sep 13, 2012 at 3:49 AM, Eric Blake <eblake@redhat.com> wrote:
> On 09/12/2012 09:33 PM, Eric Blake wrote:
>>>  OK ,then I think
>>> #if __GNUC__ >= 4
>>> ....
>>> #else
>>>   [warn name space pollution may happen]
>>> #endif
>>> would be better.
>>
>> It may be shorter, but it is definitely not better, at least not in the
>> current context of qemu.  Using the short form will fail a -Werror
>> build, unless you also write a patch to qemu's configure to quit
>> supplying -Wundef during builds.  But as touching configure has a bigger
>> impact to the overall qemu project, you're going to need a lot more
>> buy-in from other developers that -Wundef is not helping qemu gain any
>> portability, and that it is safe to ditch it (or get enough
>> counter-arguments from other developers why qemu insists on the
>> anachronistic style enforced by -Wundef, at which point you must comply
>> and use the longer form).
>
> On second thought, this _particular_ usage will never fail a -Wundef
> -Werror build, precisely because -Wundef is a gcc warning, which impies
> the warning is only ever useful in the same scenarios that the __GNUC__
> macro is always defined (that is, __GNUC__ is undefined only on a
> non-gcc compiler, but what non-gcc compiler supports -Wundef -Werror?).

The library could be used by a project that does not use GCC or pick
CFLAGS from QEMU configuration. Supporting for example MSVC or C++
users for the library could be interesting one day, even if we didn't
support MSVC or C++ at all for building the rest of QEMU.

>
> But why should this line be the one exemption to the rules?  Either qemu
> insists on the -Wundef style of coding (and you should use the long form
> to conform to that style, on the off-chance that someone ever wants to
> port to a non-gcc compiler, even in this one place where gcc can't warn
> you about the violation of that style), or we should change the qemu
> style (at which point, the short form is nicer here, but it also implies
> the potential for cleaning up lots of other places to also use short
> forms and rely on preprocessor 0 computation).
>
> --
> Eric Blake   eblake@redhat.com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>

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

* Re: [Qemu-devel] [PATCH V2 1/6] libqblock API design
  2012-09-12  8:19     ` Kevin Wolf
  2012-09-12  9:21       ` Wenchao Xia
@ 2012-09-14 19:08       ` Blue Swirl
  1 sibling, 0 replies; 34+ messages in thread
From: Blue Swirl @ 2012-09-14 19:08 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: aliguori, stefanha, qemu-devel, pbonzini, eblake, Wenchao Xia

On Wed, Sep 12, 2012 at 8:19 AM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 11.09.2012 22:28, schrieb Blue Swirl:
>> On Mon, Sep 10, 2012 at 8:26 AM, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>>   This patch contains the major APIs in the library.
>>> Important APIs:
>>>   1 QBroker. These structure was used to retrieve errors, every thread must
>>> create one first, later maybe thread related staff could be added into it.
>>>   2 QBlockState. It stands for an block image object.
>>>   3 QBlockStaticInfo. It contains static information such as location, backing
>>> file, size.
>>>   4 ABI was kept with reserved members.
>>>   5 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.c | 1077 +++++++++++++++++++++++++++++++++++++++++++++++++
>>>  libqblock/libqblock.h |  292 +++++++++++++
>>>  2 files changed, 1369 insertions(+), 0 deletions(-)
>>>  create mode 100644 libqblock/libqblock.c
>>>  create mode 100644 libqblock/libqblock.h
>>>
>>> diff --git a/libqblock/libqblock.c b/libqblock/libqblock.c
>>> new file mode 100644
>>> index 0000000..133ac0f
>>> --- /dev/null
>>> +++ b/libqblock/libqblock.c
>>> @@ -0,0 +1,1077 @@
>>> +/*
>>> + * 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"
>>> +
>>> +struct LibqblockGlobalData {
>>> +    int init_flag;
>>> +};
>>> +
>>> +struct LibqblockGlobalData g_libqblock_data;
>>> +
>>> +__attribute__((constructor))
>>> +static void libqblock_init(void)
>>> +{
>>> +    if (g_libqblock_data.init_flag == 0) {
>>> +        bdrv_init();
>>> +        qemu_init_main_loop();
>>> +    }
>>> +    g_libqblock_data.init_flag = 1;
>>> +}
>>> +
>>> +const char *qb_fmttype2str(enum QBlockFmtType fmt_type)
>>> +{
>>> +    const char *ret = NULL;
>>> +    switch (fmt_type) {
>>> +    case QB_FMT_COW:
>>> +        ret = "cow";
>>> +        break;
>>> +    case QB_FMT_QED:
>>> +        ret = "qed";
>>> +        break;
>>> +    case QB_FMT_QCOW:
>>> +        ret = "qcow";
>>> +        break;
>>> +    case QB_FMT_QCOW2:
>>> +        ret = "qcow2";
>>> +        break;
>>> +    case QB_FMT_RAW:
>>> +        ret = "raw";
>>> +        break;
>>> +    case QB_FMT_RBD:
>>> +        ret = "rbd";
>>> +        break;
>>> +    case QB_FMT_SHEEPDOG:
>>> +        ret = "sheepdog";
>>> +        break;
>>> +    case QB_FMT_VDI:
>>> +        ret = "vdi";
>>> +        break;
>>> +    case QB_FMT_VMDK:
>>> +        ret = "vmdk";
>>> +        break;
>>> +    case QB_FMT_VPC:
>>> +        ret = "vpc";
>>> +        break;
>>> +    default:
>>> +        break;
>>> +    }
>>> +    return ret;
>>> +}
>>> +
>>> +enum QBlockFmtType qb_str2fmttype(const char *fmt_str)
>>> +{
>>> +    enum QBlockFmtType ret = QB_FMT_NONE;
>>> +    if (0 == strcmp(fmt_str, "cow")) {
>>
>> This order is not common in QEMU.
>
> How about just changing the whole thing to a table that maps
> QBlockFmtType to strings, and then both conversion functions could just
> search that table?

Yes, better.

>
>>
>>> +        ret = QB_FMT_COW;
>>> +    } else if (0 == strcmp(fmt_str, "qed")) {
>>> +        ret = QB_FMT_QED;
>>> +    } else if (0 == strcmp(fmt_str, "qcow")) {
>>> +        ret = QB_FMT_QCOW;
>>> +    } else if (0 == strcmp(fmt_str, "qcow2")) {
>>> +        ret = QB_FMT_QCOW2;
>>> +    } else if (0 == strcmp(fmt_str, "raw")) {
>>> +        ret = QB_FMT_RAW;
>>> +    } else if (0 == strcmp(fmt_str, "rbd")) {
>>> +        ret = QB_FMT_RBD;
>>> +    } else if (0 == strcmp(fmt_str, "sheepdog")) {
>>> +        ret = QB_FMT_SHEEPDOG;
>>> +    } else if (0 == strcmp(fmt_str, "vdi")) {
>>> +        ret = QB_FMT_VDI;
>>> +    } else if (0 == strcmp(fmt_str, "vmdk")) {
>>> +        ret = QB_FMT_VMDK;
>>> +    } else if (0 == strcmp(fmt_str, "vpc")) {
>>> +        ret = QB_FMT_VPC;
>>> +    }
>>> +    return ret;
>>> +}
>
> Kevin

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

* Re: [Qemu-devel] [PATCH V2 2/6] libqblock type and structure defines
  2012-09-14 18:11                 ` Blue Swirl
@ 2012-09-17  2:23                   ` Wenchao Xia
  2012-09-17 19:08                     ` Blue Swirl
  0 siblings, 1 reply; 34+ messages in thread
From: Wenchao Xia @ 2012-09-17  2:23 UTC (permalink / raw)
  To: Blue Swirl; +Cc: kwolf, aliguori, stefanha, qemu-devel, pbonzini, Eric Blake

于 2012-9-15 2:11, Blue Swirl 写道:
> On Thu, Sep 13, 2012 at 3:49 AM, Eric Blake <eblake@redhat.com> wrote:
>> On 09/12/2012 09:33 PM, Eric Blake wrote:
>>>>   OK ,then I think
>>>> #if __GNUC__ >= 4
>>>> ....
>>>> #else
>>>>    [warn name space pollution may happen]
>>>> #endif
>>>> would be better.
>>>
>>> It may be shorter, but it is definitely not better, at least not in the
>>> current context of qemu.  Using the short form will fail a -Werror
>>> build, unless you also write a patch to qemu's configure to quit
>>> supplying -Wundef during builds.  But as touching configure has a bigger
>>> impact to the overall qemu project, you're going to need a lot more
>>> buy-in from other developers that -Wundef is not helping qemu gain any
>>> portability, and that it is safe to ditch it (or get enough
>>> counter-arguments from other developers why qemu insists on the
>>> anachronistic style enforced by -Wundef, at which point you must comply
>>> and use the longer form).
>>
>> On second thought, this _particular_ usage will never fail a -Wundef
>> -Werror build, precisely because -Wundef is a gcc warning, which impies
>> the warning is only ever useful in the same scenarios that the __GNUC__
>> macro is always defined (that is, __GNUC__ is undefined only on a
>> non-gcc compiler, but what non-gcc compiler supports -Wundef -Werror?).
>
> The library could be used by a project that does not use GCC or pick
> CFLAGS from QEMU configuration. Supporting for example MSVC or C++
> users for the library could be interesting one day, even if we didn't
> support MSVC or C++ at all for building the rest of QEMU.
>
   Each compiler would have its own predefined macro, so I think now
I can just support gcc and give a warning when gcc not found. If
more compiler is needed, extend the macro in the future.

>>
>> But why should this line be the one exemption to the rules?  Either qemu
>> insists on the -Wundef style of coding (and you should use the long form
>> to conform to that style, on the off-chance that someone ever wants to
>> port to a non-gcc compiler, even in this one place where gcc can't warn
>> you about the violation of that style), or we should change the qemu
>> style (at which point, the short form is nicer here, but it also implies
>> the potential for cleaning up lots of other places to also use short
>> forms and rely on preprocessor 0 computation).
>>
>> --
>> Eric Blake   eblake@redhat.com    +1-919-301-3266
>> Libvirt virtualization library http://libvirt.org
>>
>


-- 
Best Regards

Wenchao Xia

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

* Re: [Qemu-devel] [PATCH V2 2/6] libqblock type and structure defines
  2012-09-17  2:23                   ` Wenchao Xia
@ 2012-09-17 19:08                     ` Blue Swirl
  0 siblings, 0 replies; 34+ messages in thread
From: Blue Swirl @ 2012-09-17 19:08 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: kwolf, aliguori, stefanha, qemu-devel, pbonzini, Eric Blake

On Mon, Sep 17, 2012 at 2:23 AM, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
> 于 2012-9-15 2:11, Blue Swirl 写道:
>
>> On Thu, Sep 13, 2012 at 3:49 AM, Eric Blake <eblake@redhat.com> wrote:
>>>
>>> On 09/12/2012 09:33 PM, Eric Blake wrote:
>>>>>
>>>>>   OK ,then I think
>>>>> #if __GNUC__ >= 4
>>>>> ....
>>>>> #else
>>>>>    [warn name space pollution may happen]
>>>>> #endif
>>>>> would be better.
>>>>
>>>>
>>>> It may be shorter, but it is definitely not better, at least not in the
>>>> current context of qemu.  Using the short form will fail a -Werror
>>>> build, unless you also write a patch to qemu's configure to quit
>>>> supplying -Wundef during builds.  But as touching configure has a bigger
>>>> impact to the overall qemu project, you're going to need a lot more
>>>> buy-in from other developers that -Wundef is not helping qemu gain any
>>>> portability, and that it is safe to ditch it (or get enough
>>>> counter-arguments from other developers why qemu insists on the
>>>> anachronistic style enforced by -Wundef, at which point you must comply
>>>> and use the longer form).
>>>
>>>
>>> On second thought, this _particular_ usage will never fail a -Wundef
>>> -Werror build, precisely because -Wundef is a gcc warning, which impies
>>> the warning is only ever useful in the same scenarios that the __GNUC__
>>> macro is always defined (that is, __GNUC__ is undefined only on a
>>> non-gcc compiler, but what non-gcc compiler supports -Wundef -Werror?).
>>
>>
>> The library could be used by a project that does not use GCC or pick
>> CFLAGS from QEMU configuration. Supporting for example MSVC or C++
>> users for the library could be interesting one day, even if we didn't
>> support MSVC or C++ at all for building the rest of QEMU.
>>
>   Each compiler would have its own predefined macro, so I think now
> I can just support gcc and give a warning when gcc not found. If
> more compiler is needed, extend the macro in the future.

Yes, that's OK for now. It's also possible that any interest for
non-GCC compilation or C++ will never materialize. But please support
-Wundef.

>
>
>>>
>>> But why should this line be the one exemption to the rules?  Either qemu
>>> insists on the -Wundef style of coding (and you should use the long form
>>> to conform to that style, on the off-chance that someone ever wants to
>>> port to a non-gcc compiler, even in this one place where gcc can't warn
>>> you about the violation of that style), or we should change the qemu
>>> style (at which point, the short form is nicer here, but it also implies
>>> the potential for cleaning up lots of other places to also use short
>>> forms and rely on preprocessor 0 computation).
>>>
>>> --
>>> Eric Blake   eblake@redhat.com    +1-919-301-3266
>>> Libvirt virtualization library http://libvirt.org
>>>
>>
>
>
> --
> Best Regards
>
> Wenchao Xia
>

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

end of thread, other threads:[~2012-09-17 19:08 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-10  8:26 [Qemu-devel] [PATCH V2 0/6] libqblock, qemu block layer library Wenchao Xia
2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 1/6] libqblock API design Wenchao Xia
2012-09-10 21:07   ` Eric Blake
2012-09-11  3:16     ` Wenchao Xia
2012-09-14  2:03       ` Wenchao Xia
2012-09-11 20:28   ` Blue Swirl
2012-09-12  2:54     ` Wenchao Xia
2012-09-12  8:19     ` Kevin Wolf
2012-09-12  9:21       ` Wenchao Xia
2012-09-14 19:08       ` Blue Swirl
2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 2/6] libqblock type and structure defines Wenchao Xia
2012-09-10 21:27   ` Eric Blake
2012-09-11  3:26     ` Wenchao Xia
2012-09-11  4:12       ` Eric Blake
2012-09-11 20:31   ` Blue Swirl
2012-09-11 22:52     ` Eric Blake
2012-09-12  3:05       ` Wenchao Xia
2012-09-12 12:59         ` Eric Blake
2012-09-13  3:24           ` Wenchao Xia
2012-09-13  3:33             ` Eric Blake
2012-09-13  3:49               ` Eric Blake
2012-09-14 18:11                 ` Blue Swirl
2012-09-17  2:23                   ` Wenchao Xia
2012-09-17 19:08                     ` Blue Swirl
2012-09-14 18:02       ` Blue Swirl
2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 3/6] libqblock error handling Wenchao Xia
2012-09-10 21:33   ` Eric Blake
2012-09-11  4:36     ` Wenchao Xia
2012-09-11 20:32   ` Blue Swirl
2012-09-12  2:58     ` Wenchao Xia
2012-09-14 17:09       ` Blue Swirl
2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 4/6] libqblock export some qemu block function Wenchao Xia
2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 5/6] libqblock building system Wenchao Xia
2012-09-10  8:26 ` [Qemu-devel] [PATCH V2 6/6] 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).