[V18,08/10] libqblock: libqblock API implement

This patch contains implemention for APIs. Basically it is a layer
above qemu block general layer now.
  qb_image_new() will try do init for this library.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 libqblock/libqblock-error.c |   49 +++
 libqblock/libqblock.c       |  991 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 1040 insertions(+), 0 deletions(-)

On Sat, Feb 9, 2013 at 7:42 AM, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>   This patch contains implemention for APIs. Basically it is a layer
> above qemu block general layer now.
>   qb_image_new() will try do init for this library.
>
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  libqblock/libqblock-error.c |   49 +++
>  libqblock/libqblock.c       |  991 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 1040 insertions(+), 0 deletions(-)
>
> diff --git a/libqblock/libqblock-error.c b/libqblock/libqblock-error.c
> index e69de29..2367ab4 100644
> --- a/libqblock/libqblock-error.c
> +++ b/libqblock/libqblock-error.c
> @@ -0,0 +1,49 @@
> +/*
> + * QEMU block layer library
> + *
> + * Copyright IBM, Corp. 2013
> + *
> + * 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"
> +
> +char *qb_error_get_human_str(QBlockImage *image)
> +{
> +    char *ret = NULL;
> +    QBlockContext *ctx = image->ctx;
> +    const char *str;
> +    switch (ctx->err_ret) {
> +    case QB_ERR_INTERNAL_ERR:
> +        str = "Internal error.";
> +        break;
> +    case QB_ERR_FATAL_ERR:
> +        str = "Fatal error.";
> +        break;
> +    case QB_ERR_INVALID_PARAM:
> +        str = "Invalid param.";
> +        break;
> +    case QB_ERR_BLOCK_OUT_OF_RANGE:
> +        str = "request is out of image's range.";
> +        break;
> +    default:
> +        str = "Unknown error.";
> +        break;
> +    }
> +
> +    if (ctx->err_ret == QB_ERR_INTERNAL_ERR) {
> +        ret = g_strdup_printf("%s %s errno [%d]. strerror [%s].",
> +                     str, ctx->g_error->message,
> +                     ctx->err_no, strerror(-ctx->err_no));
> +    } else {
> +        ret = g_strdup_printf("%s %s",
> +                     str, ctx->g_error->message);
> +    }
> +    return ret;
> +}
> diff --git a/libqblock/libqblock.c b/libqblock/libqblock.c
> index e69de29..d7b6ee5 100644
> --- a/libqblock/libqblock.c
> +++ b/libqblock/libqblock.c
> @@ -0,0 +1,991 @@
> +/*
> + * QEMU block layer library
> + *
> + * Copyright IBM, Corp. 2013
> + *
> + * 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 <pthread.h>
> +
> +#include "libqblock.h"
> +#include "libqblock-internal.h"
> +
> +#include "block/block_int.h"
> +
> +#define LIBQB_FILENAME_MAX 4096
> +
> +typedef struct LibqblockGlobalData {
> +    int init_flag;
> +    pthread_mutex_t mutex;
> +} LibqblockGlobalData;
> +
> +LibqblockGlobalData libqb_global_data;
> +
> +typedef struct LibqbFormatStrMapping {
> +    const char *fmt_str;
> +    QBlockFormat fmt_type;
> +} LibqbFormatStrMapping;
> +
> +LibqbFormatStrMapping libqb_formatstr_table[] = {

static const

> +    {"cow", QB_FORMAT_COW},
> +    {"qed", QB_FORMAT_QED},
> +    {"qcow", QB_FORMAT_QCOW},
> +    {"qcow2", QB_FORMAT_QCOW2},
> +    {"raw", QB_FORMAT_RAW},
> +    {"rbd", QB_FORMAT_RBD},
> +    {"sheepdog", QB_FORMAT_SHEEPDOG},
> +    {"vdi", QB_FORMAT_VDI},
> +    {"vmdk", QB_FORMAT_VMDK},
> +    {"vpc", QB_FORMAT_VPC},
> +    {NULL, 0},

You can avoid this NULL entry and save space by using ARRAY_SIZE() in
the loop. Compiler could also unroll the loop.

> +};
> +
> +__attribute__((constructor))
> +static void libqblock_init(void)
> +{
> +    /* Todo: add an assertion about the ABI. */
> +    libqb_global_data.init_flag = 0;
> +    pthread_mutex_init(&libqb_global_data.mutex, NULL);
> +}
> +
> +const char *qb_formattype2str(QBlockFormat fmt_type)
> +{
> +    int i = 0;
> +    LibqbFormatStrMapping *tb = libqb_formatstr_table;

This does not seem to be useful, you could use libqb_formatstr_table directly.

> +
> +    if ((fmt_type <= QB_FORMAT_NONE) || (fmt_type >= QB_FORMAT_MAX)) {
> +        return NULL;
> +    }
> +    while (tb[i].fmt_str != NULL) {

for (i = 0; i < ARRAY_SIZE(libqb_formatstr_table); i++) {

> +        if (tb[i].fmt_type == fmt_type) {
> +            return tb[i].fmt_str;
> +        }
> +        i++;
> +    }
> +    return NULL;
> +}
> +
> +QBlockFormat qb_str2fmttype(const char *fmt_str)
> +{
> +    int i = 0;
> +    LibqbFormatStrMapping *tb = libqb_formatstr_table;
> +
> +    if (fmt_str == NULL) {
> +        return QB_FORMAT_NONE;
> +    }
> +    while (tb[i].fmt_str != NULL) {
> +        if ((strcmp(fmt_str, tb[i].fmt_str) == 0)) {
> +            return tb[i].fmt_type;
> +        }
> +        i++;
> +    }
> +    return QB_FORMAT_NONE;
> +}
> +
> +static void set_context_err(QBlockContext *context, int err_ret,
> +                            const char *fmt, ...)

GCC_FMT_ATTR()?

> +{
> +    va_list ap;
> +
> +    if (context->g_error != NULL) {
> +        g_error_free(context->g_error);
> +    }
> +
> +    va_start(ap, fmt);
> +    context->g_error = g_error_new_valist(qb_error_quark(), err_ret, fmt, ap);
> +    va_end(ap);
> +
> +    context->err_ret = err_ret;
> +    if (err_ret == QB_ERR_INTERNAL_ERR) {
> +        context->err_no = -errno;
> +    } else {
> +        context->err_no = 0;
> +    }
> +}
> +
> +static int create_context(QBlockContext **p_context)
> +{
> +    /*
> +     * library init comes here, to make sure block_init is called before with
> +     * flag __attribute__((constructor)).
> +     */
> +    if (pthread_mutex_lock(&libqb_global_data.mutex)) {
> +        return QB_ERR_FATAL_ERR;
> +    }
> +    if (libqb_global_data.init_flag == 0) {
> +        qemu_init_main_loop();
> +        bdrv_init();
> +        libqb_global_data.init_flag = 1;
> +    }
> +    if (pthread_mutex_unlock(&libqb_global_data.mutex)) {
> +        return QB_ERR_FATAL_ERR;
> +    }
> +
> +    *p_context = g_malloc0(sizeof(QBlockContext));
> +    /* AIO code could comes here later. */
> +    return 0;
> +}
> +
> +static void delete_context(QBlockContext **p_context)
> +{
> +    if ((*p_context)->g_error != NULL) {
> +        g_error_free((*p_context)->g_error);
> +    }
> +    g_free(*p_context);
> +    *p_context = NULL;
> +    return;

Useless return, please remove.

> +}
> +
> +int qb_image_new(QBlockImage **p_qbi)
> +{
> +    int ret;
> +    *p_qbi = g_malloc0_n(1, sizeof(QBlockImage));
> +    ret = create_context(&(*p_qbi)->ctx);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    (*p_qbi)->bdrvs = bdrv_new("hda");
> +    if ((*p_qbi)->bdrvs == NULL) {
> +        set_context_err((*p_qbi)->ctx, QB_ERR_INTERNAL_ERR,
> +                       "failed to create the driver.");
> +        return QB_ERR_INTERNAL_ERR;
> +    }
> +    (*p_qbi)->ref_count = 1;
> +    return 0;
> +}
> +
> +void qb_image_ref(QBlockImage *qbi)
> +{
> +    qbi->ref_count++;
> +}
> +
> +void qb_image_unref(QBlockImage **p_qbi)
> +{
> +    (*p_qbi)->ref_count--;
> +    if ((*p_qbi)->ref_count > 0) {
> +        return;
> +    }
> +    delete_context(&(*p_qbi)->ctx);
> +    if ((*p_qbi)->filename != NULL) {
> +        qb_close(*p_qbi);
> +    }
> +    if ((*p_qbi)->bdrvs != NULL) {
> +        bdrv_delete((*p_qbi)->bdrvs);
> +        (*p_qbi)->bdrvs = NULL;
> +    }
> +    g_free(*p_qbi);
> +    *p_qbi = NULL;
> +    return;

Ditto.

> +}
> +
> +/* return 0 if every thing is fine */
> +static int loc_check_params(QBlockContext *context,
> +                            QBlockLocationInfo *loc)
> +{
> +    context->err_ret = 0;
> +
> +    switch (loc->prot_type) {
> +    case QB_PROTO_FILE:
> +        if (loc->o_file.filename == NULL) {
> +            set_context_err(context, QB_ERR_INVALID_PARAM,
> +                           "Filename was not set.");
> +            goto out;
> +        }
> +        if (path_has_protocol(loc->o_file.filename) > 0) {
> +            set_context_err(context, QB_ERR_INVALID_PARAM,
> +                           "filename [%s] had protocol.",
> +                           loc->o_file.filename);
> +            goto out;
> +        }
> +        break;
> +    default:
> +        set_context_err(context, QB_ERR_INVALID_PARAM,
> +                       "Protocol type [%d] was not valid.",
> +                       loc->prot_type);
> +        break;
> +    }
> +
> + out:
> +    return context->err_ret;
> +}
> +
> +/* translate loc structure to internal filename, returned char* need free,
> + * assuming filename is not NULL. *filename would be set to NULL if no valid
> + * filename found. *filename must be freed later.
> + * return 0 if no error with *filename set.
> + */
> +static int loc2filename(QBlockContext *context,
> +                        QBlockLocationInfo *loc,
> +                        char **p_filename)
> +{
> +    context->err_ret = 0;
> +
> +    if (*p_filename != NULL) {
> +        g_free(*p_filename);
> +        *p_filename = NULL;
> +    }
> +    switch (loc->prot_type) {
> +    case QB_PROTO_FILE:
> +        *p_filename = g_strdup(loc->o_file.filename);
> +        break;
> +    default:
> +        set_context_err(context, QB_ERR_INVALID_PARAM,
> +                 "protocol type [%d] is not supported.",
> +                  loc->prot_type);
> +        break;
> +    }
> +
> +    return context->err_ret;
> +}
> +
> +/* translate filename to location, loc->prot_type = NONE if fail, filename
> +   must be valid. loc internal char pointer must be freed later.
> + * return 0 if no error.
> + */
> +static int filename2loc(QBlockContext *context,
> +                        QBlockLocationInfo *loc,
> +                        const char *filename)
> +{
> +    context->err_ret = 0;
> +
> +    if (path_has_protocol(filename) > 0) {
> +        set_context_err(context, QB_ERR_INVALID_PARAM,
> +                     "Filename [%s] had protocol, not supported now.",
> +                     filename);
> +        goto out;
> +    }
> +
> +    loc->prot_type = QB_PROTO_FILE;
> +    switch (loc->prot_type) {
> +    case QB_PROTO_FILE:
> +        loc->o_file.filename = g_strdup(filename);
> +        break;
> +    default:
> +        break;
> +    }
> +
> + out:
> +    return context->err_ret;
> +}
> +
> +/* return 0 if OK, or qblock error number */
> +static int set_backing_file_options(QBlockContext *context,
> +                                    QEMUOptionParameter *param,
> +                                    QBlockLocationInfo *loc,
> +                                    QBlockFormat *fmt)
> +{
> +    char *backing_filename = NULL;
> +    const char *fmtstr_backing = NULL;
> +    int ret = 0;
> +
> +    if (loc == NULL) {
> +        goto out;
> +    }
> +
> +    /* ret can < 0 if loc have not been set, mean user did not specify backing
> +       file, so no need to check return value */
> +    loc2filename(context, loc, &backing_filename);
> +
> +    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_formattype2str(*fmt);
> +        if (fmtstr_backing) {
> +            ret = set_option_parameter(param,
> +                                BLOCK_OPT_BACKING_FMT, fmtstr_backing);
> +            assert(ret == 0);
> +        }
> +    }
> +
> + out:
> +    g_free(backing_filename);
> +    return ret;
> +}
> +
> +int qb_create(QBlockImage *qbi,
> +              QBlockLocationInfo *loc,
> +              QBlockFormatInfo *fmt,
> +              int flag)
> +{
> +    int ret = 0, bd_ret;
> +    char *filename = NULL;
> +    BlockDriverState *bs = NULL;
> +    BlockDriver *drv = NULL, *backing_drv = NULL;
> +    bool tmp_bool;
> +    QBlockContext *context = qbi->ctx;
> +
> +    const char *fmtstr = NULL, *tmp = NULL;
> +    QEMUOptionParameter *param = NULL, *create_options = NULL;
> +    QEMUOptionParameter *backing_fmt, *backing_file, *size;
> +    QBlockFormatOptionsCOW *o_cow = NULL;
> +    QBlockFormatOptionsQED *o_qed = NULL;
> +    QBlockFormatOptionsQCOW *o_qcow = NULL;
> +    QBlockFormatOptionsQCOW2 *o_qcow2 = NULL;
> +    QBlockFormatOptionsRBD *o_rbd = NULL;
> +    QBlockFormatOptionsSD *o_sd = NULL;
> +    QBlockFormatOptionsVDI *o_vdi = NULL;
> +    QBlockFormatOptionsVMDK *o_vmdk = NULL;
> +    QBlockFormatOptionsVPC *o_vpc = NULL;
> +
> +
> +    /* check parameters */
> +    if (flag & (~LIBQBLOCK_O_VALID_MASK)) {
> +        set_context_err(context, QB_ERR_INVALID_PARAM,
> +                           "invalid flag was set.");
> +        ret = context->err_ret;
> +        goto out;
> +    }
> +
> +    if ((loc == NULL) || (qbi == NULL) || (fmt == NULL)) {
> +        set_context_err(context, QB_ERR_INVALID_PARAM,
> +                          "Got unexpected NULL pointer in parameters.");
> +        ret = context->err_ret;
> +        goto out;
> +    }
> +
> +    ret = loc_check_params(context, loc);
> +    if (ret != 0) {
> +        goto out;
> +    }
> +
> +    /* internal translate */
> +    ret = loc2filename(context, loc, &filename);
> +    if (ret != 0) {
> +        goto out;
> +    }
> +
> +    fmtstr = qb_formattype2str(fmt->fmt_type);
> +    if (fmtstr == NULL) {
> +        set_context_err(context, QB_ERR_INVALID_PARAM,
> +                 "Got unexpected NULL pointer in parameters.");
> +        ret = context->err_ret;
> +        goto out;
> +    }
> +
> +    drv = bdrv_find_format(fmtstr);
> +    assert(drv != NULL);
> +
> +    create_options = append_option_parameters(create_options,
> +                                              drv->create_options);
> +    param = parse_option_parameters("", create_options, param);
> +
> +    bd_ret = set_option_parameter_int(param,
> +                                BLOCK_OPT_SIZE, fmt->virt_size);
> +    assert(bd_ret == 0);
> +
> +    switch (fmt->fmt_type) {
> +    case QB_FORMAT_COW:
> +        o_cow = &fmt->o_cow;
> +        /* do not need to check loc, it may be not set */
> +        ret = set_backing_file_options(context, param,
> +                                       &o_cow->backing_loc, NULL);
> +        if (ret != 0) {
> +            goto out;
> +        }
> +        break;
> +    case QB_FORMAT_QED:
> +        o_qed = &fmt->o_qed;
> +        ret = set_backing_file_options(context, param,
> +                                 &o_qed->backing_loc, &o_qed->backing_fmt);
> +        if (ret != 0) {
> +            goto out;
> +        }
> +        bd_ret = set_option_parameter_int(param,
> +                                BLOCK_OPT_CLUSTER_SIZE, o_qed->cluster_size);
> +        assert(bd_ret == 0);
> +        bd_ret = set_option_parameter_int(param,
> +                                BLOCK_OPT_TABLE_SIZE, o_qed->table_size);
> +        assert(bd_ret == 0);
> +        break;
> +    case QB_FORMAT_QCOW:
> +        o_qcow = &fmt->o_qcow;
> +        ret = set_backing_file_options(context, param,
> +                                       &o_qcow->backing_loc, NULL);
> +        if (ret != 0) {
> +            goto out;
> +        }
> +        tmp = o_qcow->encrypt ? "on" : "off";
> +        bd_ret = set_option_parameter(param, BLOCK_OPT_ENCRYPT, tmp);
> +        assert(bd_ret == 0);
> +        break;
> +    case QB_FORMAT_QCOW2:
> +        o_qcow2 = &fmt->o_qcow2;
> +        ret = set_backing_file_options(context, param,
> +                              &o_qcow2->backing_loc, &o_qcow2->backing_fmt);
> +        if (ret != 0) {
> +            goto out;
> +        }
> +        tmp = o_qcow2->encrypt ? "on" : "off";
> +        bd_ret = set_option_parameter(param, BLOCK_OPT_ENCRYPT, tmp);
> +        assert(bd_ret == 0);
> +        bd_ret = set_option_parameter_int(param,
> +                              BLOCK_OPT_CLUSTER_SIZE, o_qcow2->cluster_size);
> +        assert(bd_ret == 0);
> +
> +        if (o_qcow2->cpt_lv != QB_FORMAT_QCOW2_COMPAT_DEFAULT) {
> +            tmp = o_qcow2->cpt_lv == QB_FORMAT_QCOW2_COMPAT_V0_10 ?
> +                                     "0.10" : "1.1";
> +            bd_ret = set_option_parameter(param,
> +                              BLOCK_OPT_COMPAT_LEVEL, tmp);
> +            assert(bd_ret == 0);
> +        }
> +
> +        if (o_qcow2->pre_mode != QB_FORMAT_QCOW2_PREALLOC_DEFAULT) {
> +            tmp = o_qcow2->pre_mode == QB_FORMAT_QCOW2_PREALLOC_OFF ?
> +                                       "off" : "metadata";
> +            bd_ret = set_option_parameter(param,
> +                              BLOCK_OPT_PREALLOC, tmp);
> +            assert(bd_ret == 0);
> +        }
> +        break;
> +
> +    case QB_FORMAT_RAW:
> +        /* do nothing now. */
> +        break;
> +    case QB_FORMAT_RBD:
> +        o_rbd = &fmt->o_rbd;
> +        bd_ret = set_option_parameter_int(param,
> +                              BLOCK_OPT_CLUSTER_SIZE, o_rbd->cluster_size);
> +        assert(bd_ret == 0);
> +        break;
> +    case QB_FORMAT_SHEEPDOG:
> +        o_sd = &fmt->o_sd;
> +        ret = set_backing_file_options(context, param,
> +                                       &o_sd->backing_loc, NULL);
> +        if (ret != 0) {
> +            goto out;
> +        }
> +        if (o_sd->pre_mode != QB_FORMAT_SD_PREALLOC_DEFAULT) {
> +            tmp = o_sd->pre_mode == QB_FORMAT_SD_PREALLOC_OFF ? "off" : "full";
> +            bd_ret = set_option_parameter(param,
> +                              BLOCK_OPT_PREALLOC, tmp);
> +            assert(bd_ret == 0);
> +        }
> +        break;
> +    case QB_FORMAT_VDI:
> +        o_vdi = &fmt->o_vdi;
> +        /* following option is not always valid depends on configuration */
> +        set_option_parameter_int(param,
> +                              BLOCK_OPT_CLUSTER_SIZE, o_vdi->cluster_size);
> +        if (o_vdi->pre_mode != QB_FORMAT_VDI_PREALLOC_DEFAULT) {
> +            tmp_bool = o_vdi->pre_mode == QB_FORMAT_VDI_PREALLOC_METADATA ?
> +                                                     true : false;
> +            set_option_parameter_int(param, "static", tmp_bool);
> +        }
> +        break;
> +    case QB_FORMAT_VMDK:
> +        o_vmdk = &fmt->o_vmdk;
> +        ret = set_backing_file_options(context, param,
> +                                       &o_vmdk->backing_loc, NULL);
> +        if (ret != 0) {
> +            goto out;
> +        }
> +
> +        if (o_vmdk->cpt_lv != QB_FORMAT_VMDK_COMPAT_DEFAULT) {
> +            tmp_bool = o_vmdk->cpt_lv == QB_FORMAT_VMDK_COMPAT_VMDKV6_TRUE ?
> +                                                     true : false;
> +            bd_ret = set_option_parameter_int(param, BLOCK_OPT_COMPAT6,
> +                                                     tmp_bool);
> +            assert(bd_ret == 0);
> +        }
> +        if (o_vmdk->subfmt != QB_FORMAT_VMDK_SUBFMT_DEFAULT) {
> +            switch (o_vmdk->subfmt) {
> +            case QB_FORMAT_VMDK_SUBFMT_MONOLITHIC_SPARSE:
> +                tmp = "monolithicSparse";
> +                break;
> +            case QB_FORMAT_VMDK_SUBFMT_MONOLITHIC_FLAT:
> +                tmp = "monolithicFlat";
> +                break;
> +            case QB_FORMAT_VMDK_SUBFMT_TWOGBMAX_EXTENT_SPARSE:
> +                tmp = "twoGbMaxExtentSparse";
> +                break;
> +            case QB_FORMAT_VMDK_SUBFMT_TWOGBMAX_EXTENT_FLAT:
> +                tmp = "twoGbMaxExtentFlat";
> +                break;
> +            case QB_FORMAT_VMDK_SUBFMT_STREAM_OPTIMIZED:
> +                tmp = "streamOptimized";
> +                break;
> +            default:
> +                set_context_err(context, QB_ERR_INVALID_PARAM,
> +                    "invalid VMDK sumfmt type %d was set.", o_vmdk->subfmt);
> +                ret = context->err_ret;
> +                goto out;
> +                break;
> +            }
> +            bd_ret = set_option_parameter(param,
> +                              BLOCK_OPT_SUBFMT, tmp);
> +            assert(bd_ret == 0);
> +        }
> +        break;
> +    case QB_FORMAT_VPC:
> +        o_vpc = &fmt->o_vpc;
> +        if (o_vpc->subfmt != QB_FORMAT_VPC_SUBFMT_DEFAULT) {
> +            tmp = o_vpc->subfmt == QB_FORMAT_VPC_SUBFMT_DYNAMIC ?
> +                                               "dynamic" : "fixed";
> +            bd_ret = set_option_parameter(param,
> +                               BLOCK_OPT_SUBFMT, tmp);
> +            assert(bd_ret == 0);
> +        }
> +        break;
> +    default:
> +        set_context_err(context, QB_ERR_INVALID_PARAM,
> +                        "invalid format type %d was set.", fmt->fmt_type);
> +        ret = context->err_ret;
> +        goto out;
> +        break;
> +    }
> +
> +    backing_file = get_option_parameter(param, BLOCK_OPT_BACKING_FILE);
> +    if (backing_file && backing_file->value.s) {
> +        if (!strcmp(filename, backing_file->value.s)) {
> +            set_context_err(context, QB_ERR_INVALID_PARAM,
> +                          "Backing file is the same with new file.");
> +            ret = context->err_ret;
> +            goto out;
> +        }
> +    }
> +
> +    backing_fmt = get_option_parameter(param, BLOCK_OPT_BACKING_FMT);
> +    if (backing_fmt && backing_fmt->value.s) {
> +        backing_drv = bdrv_find_format(backing_fmt->value.s);
> +        assert(backing_drv != NULL);
> +    }
> +
> +    size = get_option_parameter(param, BLOCK_OPT_SIZE);
> +    if (size && size->value.n <= 0) {
> +        if (backing_file && backing_file->value.s) {
> +            uint64_t size;
> +            char buf[32];
> +            int back_flags;
> +
> +            /* backing files always opened read-only */
> +            back_flags =
> +                flag &
> +                ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
> +
> +            bs = bdrv_new("");
> +
> +            ret = bdrv_open(bs, backing_file->value.s,
> +                                back_flags, backing_drv);
> +            if (ret < 0) {
> +                set_context_err(context, QB_ERR_INVALID_PARAM,
> +                               "Failed to open the backing file.");
> +                ret = context->err_ret;
> +                goto out;
> +            }
> +            bdrv_get_geometry(bs, &size);
> +            size *= BDRV_SECTOR_SIZE;
> +
> +            snprintf(buf, sizeof(buf), "%" PRId64, size);
> +            set_option_parameter(param, BLOCK_OPT_SIZE, buf);
> +        } else {
> +            set_context_err(context, QB_ERR_INTERNAL_ERR,
> +                           "Neither size or backing file was not set.");
> +            ret = context->err_ret;
> +            goto out;
> +        }
> +    }
> +
> +    bd_ret = bdrv_create(drv, filename, param);
> +
> +
> +    if (bd_ret < 0) {
> +        const char *errstr;
> +        if (bd_ret == -ENOTSUP) {
> +            errstr = "formatting option not supported.";
> +        } else if (bd_ret == -EFBIG) {
> +            errstr = "The image size is too large.";
> +        } else {
> +            errstr = "Error in creating the image.";
> +        }
> +        set_context_err(context, QB_ERR_INTERNAL_ERR, errstr);
> +        ret = context->err_ret;
> +    }
> +
> +out:
> +    free_option_parameters(create_options);
> +    free_option_parameters(param);
> +    g_free(filename);
> +    if (bs) {
> +        bdrv_delete(bs);
> +    }
> +
> +    return ret;
> +}
> +
> +int qb_open(QBlockImage *qbi,
> +            QBlockLocationInfo *loc,
> +            QBlockFormatInfo *fmt,
> +            int flag)
> +{
> +    int ret = 0, bd_ret;
> +    BlockDriverState *bs;
> +    BlockDriver *bd;
> +    const char *fmtstr;
> +    char *filename = NULL;
> +    QBlockContext *context = qbi->ctx;
> +
> +    /* take care of user settings */
> +    /* do nothing now */
> +
> +    /* check parameters */
> +    if (flag & (~LIBQBLOCK_O_VALID_MASK)) {
> +        set_context_err(context, QB_ERR_INVALID_PARAM,
> +                      "Invalid flag was set.");
> +        ret = context->err_ret;
> +        goto out;
> +    }
> +
> +    if ((loc == NULL) || (qbi == NULL)) {
> +        set_context_err(context, QB_ERR_INVALID_PARAM,
> +                      "Got unexpected NULL pointer in parameters.");
> +        ret = context->err_ret;
> +        goto out;
> +    }
> +
> +    ret = loc_check_params(context, loc);
> +    if (ret != 0) {
> +        goto out;
> +    }
> +
> +    /* internal translate */
> +    ret = loc2filename(context, loc, &filename);
> +    if (ret != 0) {
> +        goto out;
> +    }
> +
> +    fmtstr = NULL;
> +    bd = NULL;
> +    if (fmt != NULL) {
> +        fmtstr = qb_formattype2str(fmt->fmt_type);
> +    }
> +
> +    if (fmtstr != NULL) {
> +        bd = bdrv_find_format(fmtstr);
> +        assert(bd != NULL);
> +    }
> +
> +    /* do real opening */
> +    bs = qbi->bdrvs;
> +    bd_ret = bdrv_open(bs, filename, flag, bd);
> +    if (bd_ret < 0) {
> +        set_context_err(context, QB_ERR_INTERNAL_ERR,
> +                      "Failed in opening with driver, bd_ret is %d.", bd_ret);
> +        ret = context->err_ret;
> +        goto out;
> +    }
> +
> +    if (qbi->filename != NULL) {
> +        g_free(qbi->filename);
> +    }
> +    qbi->filename = g_strdup(filename);
> +
> + out:
> +    g_free(filename);
> +    return ret;
> +}
> +
> +void qb_close(QBlockImage *qbi)
> +{
> +    BlockDriverState *bs;
> +
> +    bs = qbi->bdrvs;
> +
> +    if (qbi->filename != NULL) {
> +        g_free(qbi->filename);
> +        qbi->filename = NULL;
> +        bdrv_close(bs);
> +    }
> +    return;
> +}
> +
> +int32_t qb_read(QBlockImage *qbi,
> +                uint8_t *buf,
> +                uint32_t len,
> +                uint64_t offset)
> +{
> +    int bd_ret;
> +    BlockDriverState *bs;
> +    QBlockContext *context = qbi->ctx;
> +
> +    context->err_ret = 0;
> +    bs = qbi->bdrvs;
> +
> +    if (len <= 0) {
> +        set_context_err(context, QB_ERR_INVALID_PARAM,
> +                      "Param len is less or equal to zero.");
> +        return context->err_ret;
> +    }
> +
> +    bd_ret = bdrv_pread(bs, offset, buf, len);
> +    if (bd_ret < 0) {
> +        set_context_err(context, QB_ERR_INTERNAL_ERR,
> +                       "I/O errors.");
> +        context->err_no = bd_ret;
> +        return context->err_ret;
> +    }
> +
> +    return 0;
> +}
> +
> +int32_t qb_write(QBlockImage *qbi,
> +                 const uint8_t *buf,
> +                 uint32_t len,
> +                 uint64_t offset)
> +{
> +    int bd_ret;
> +    BlockDriverState *bs;
> +    QBlockContext *context = qbi->ctx;
> +
> +    context->err_ret = 0;
> +    bs = qbi->bdrvs;
> +
> +    if (len <= 0) {
> +        set_context_err(context, QB_ERR_INVALID_PARAM,
> +                      "Param len is less or equal to zero.");
> +        return context->err_ret;
> +    }
> +
> +    bd_ret = bdrv_pwrite(bs, offset, buf, len);
> +    if (bd_ret < 0) {
> +        set_context_err(context, QB_ERR_INTERNAL_ERR,
> +                       "I/O errors.");
> +        context->err_no = bd_ret;
> +        return context->err_ret;
> +    }
> +
> +    return 0;
> +}
> +
> +int qb_flush(QBlockImage *qbi)
> +{
> +    int bd_ret;
> +    BlockDriverState *bs;
> +    QBlockContext *context = qbi->ctx;
> +
> +    context->err_ret = 0;
> +    bs = qbi->bdrvs;
> +    bd_ret = bdrv_flush(bs);
> +    if (bd_ret < 0) {
> +        set_context_err(context, QB_ERR_INTERNAL_ERR,
> +                       "Internal error.");
> +    }
> +    return context->err_ret;
> +}
> +
> +int qb_check_allocation(QBlockImage *qbi,
> +                        uint64_t start,
> +                        uint64_t length,
> +                        int *pstatus,
> +                        uint64_t *plength)
> +{
> +    int ret;
> +    int sector_start, sector_num, num;
> +    BlockDriverState *bs;
> +    unsigned int real_len, ret_len;
> +    QBlockContext *context = qbi->ctx;
> +
> +    context->err_ret = 0;
> +    bs = qbi->bdrvs;
> +
> +    /* Now bdrv_is_allocated take nb_sectors as int with unit sector, but this
> +    API take length as int64_t with unit bytes, so a check is needed to ensure
> +    no silent error will happen in translating bytes to sector later if length
> +    is too big.
> +       max_sectors is set to INT_MAX - 1 in case of that real_len is bigger
> +    than length.
> +       Gcc issue? a = (0x7ffffffe << 9) was compiled to 0xfffffffffffffc00, it
> +    should be 0xfffffffc00, so use variable instead of direct macro
> +    calculation. */
> +    if (sizeof(int) < (sizeof(int64_t) - BDRV_SECTOR_BITS/8)) {
> +        uint64_t max_sectors = INT_MAX - 1;
> +        uint64_t max_bytes = max_sectors << BDRV_SECTOR_BITS;
> +
> +        if (length > max_bytes) {
> +            set_context_err(context, QB_ERR_INVALID_PARAM,
> +                           "Length is too big.");
> +            goto out;
> +        }
> +    }
> +
> +    if (length <= 0) {
> +        set_context_err(context, QB_ERR_INVALID_PARAM,
> +                       "Length is not valid.");
> +        goto out;
> +    }
> +
> +    if (qbi->filename == NULL) {
> +        set_context_err(context, QB_ERR_INVALID_PARAM,
> +                       "Image was not opened first.");
> +        goto out;
> +    }
> +
> +    /* translate to sector */
> +    sector_start = start >> BDRV_SECTOR_BITS;
> +    real_len = (start & (~BDRV_SECTOR_MASK)) + length;
> +    sector_num = real_len >> BDRV_SECTOR_BITS;
> +    if ((real_len & (~BDRV_SECTOR_MASK)) != 0) {
> +        sector_num++;
> +    }
> +
> +    ret = bdrv_is_allocated(bs, sector_start, sector_num, &num);
> +    if ((ret == 0) && (num == 0)) {
> +        set_context_err(context, QB_ERR_BLOCK_OUT_OF_RANGE,
> +                       "Start position was bigger than the image's size.");
> +        goto out;
> +    }
> +
> +    *pstatus = ret;
> +    ret_len = (num << BDRV_SECTOR_BITS) - (start & (~BDRV_SECTOR_MASK));
> +    if (ret_len > length) {
> +        ret_len = length;
> +    }
> +    *plength = ret_len;
> +
> + out:
> +    return context->err_ret;
> +}
> +
> +static void qb_setup_info_addr(const QBlockStaticInfo *info,
> +                               QBlockStaticInfoAddr *info_addr)
> +{
> +    const QBlockLocationInfo *backing_loc = NULL;
> +    const bool *encrypt = NULL;
> +    const QBlockFormatInfo *fmt = &info->fmt;
> +
> +    memset(info_addr, 0, sizeof(QBlockStaticInfoAddr));
> +
> +    switch (fmt->fmt_type) {
> +    case QB_FORMAT_COW:
> +        backing_loc = &fmt->o_cow.backing_loc;
> +        break;
> +    case QB_FORMAT_QED:
> +        backing_loc = &fmt->o_qed.backing_loc;
> +        break;
> +    case QB_FORMAT_QCOW:
> +        backing_loc = &fmt->o_qcow.backing_loc;
> +        encrypt = &fmt->o_qcow.encrypt;
> +        break;
> +    case QB_FORMAT_QCOW2:
> +        backing_loc = &fmt->o_qcow2.backing_loc;
> +        encrypt = &fmt->o_qcow2.encrypt;
> +        break;
> +    case QB_FORMAT_RAW:
> +        break;
> +    case QB_FORMAT_RBD:
> +        break;
> +    case QB_FORMAT_SHEEPDOG:
> +        backing_loc = &fmt->o_sd.backing_loc;
> +        break;
> +    case QB_FORMAT_VDI:
> +        break;
> +    case QB_FORMAT_VMDK:
> +        backing_loc = &fmt->o_vmdk.backing_loc;
> +        break;
> +    case QB_FORMAT_VPC:
> +        break;
> +    default:
> +        break;
> +    }
> +
> +    info_addr->backing_loc = (QBlockLocationInfo *)backing_loc;
> +    info_addr->encrypt = (bool *)encrypt;
> +    return;
> +}
> +
> +const uint64_t *qb_get_virt_size(const QBlockStaticInfo *info)
> +{
> +    return &info->fmt.virt_size;
> +}
> +
> +const QBlockLocationInfo *qb_get_backing_loc(const QBlockStaticInfo *info)
> +{
> +    QBlockStaticInfoAddr addr;
> +    qb_setup_info_addr(info, &addr);
> +    return addr.backing_loc;
> +}
> +
> +const bool *qb_get_encrypt(const QBlockStaticInfo *info)
> +{
> +    QBlockStaticInfoAddr addr;
> +    qb_setup_info_addr(info, &addr);
> +    return addr.encrypt;
> +}
> +
> +int qb_info_image_static_get(QBlockImage *qbi,
> +                             QBlockStaticInfo *info)
> +{
> +    int ret = 0;
> +    BlockDriverState *bs;
> +    QBlockStaticInfoAddr addr;
> +    const char *fmt_str;
> +    uint64_t total_sectors;
> +    char backing_filename[LIBQB_FILENAME_MAX];
> +    QBlockContext *context = qbi->ctx;
> +
> +    if (qbi->filename == NULL) {
> +        set_context_err(context, QB_ERR_INVALID_PARAM,
> +                       "Block Image was not opened.");
> +        ret = context->err_ret;
> +        goto out;
> +    }
> +
> +    bs = qbi->bdrvs;
> +
> +    ret = filename2loc(context,
> +                       &info->loc,
> +                       qbi->filename);
> +    if (ret < 0) {
> +        goto out;
> +    }
> +
> +    fmt_str = bdrv_get_format_name(bs);
> +    info->fmt.fmt_type = qb_str2fmttype(fmt_str);
> +    if (info->fmt.fmt_type == QB_FORMAT_NONE) {
> +        set_context_err(context, QB_ERR_INVALID_PARAM,
> +                       "Image format %s not valid.", fmt_str);
> +        ret = context->err_ret;
> +        goto out;
> +    }
> +
> +    /* we got the format type and basic location info now, setup the struct
> +    pointer to the internal members */
> +    qb_setup_info_addr(info, &addr);
> +
> +    bdrv_get_geometry(bs, &total_sectors);
> +    info->fmt.virt_size = total_sectors * BDRV_SECTOR_SIZE;
> +
> +    if (addr.encrypt != NULL) {
> +        *(addr.encrypt) = bdrv_is_encrypted(bs);
> +    }
> +
> +    bdrv_get_full_backing_filename(bs, backing_filename,
> +                                   sizeof(backing_filename));
> +    if (backing_filename[0] != '\0') {
> +        assert(addr.backing_loc != NULL);
> +        ret = filename2loc(context,
> +                           addr.backing_loc,
> +                           backing_filename);
> +        if (ret < 0) {
> +            goto out;
> +        }
> +    }
> +
> +    info->sector_size = BDRV_SECTOR_SIZE;
> +
> + out:
> +    return ret;
> +}
> --
> 1.7.1
>
>
>

> On Sat, Feb 9, 2013 at 7:42 AM, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>    This patch contains implemention for APIs. Basically it is a layer
>> above qemu block general layer now.
>>    qb_image_new() will try do init for this library.
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>>   libqblock/libqblock-error.c |   49 +++
>>   libqblock/libqblock.c       |  991 +++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 1040 insertions(+), 0 deletions(-)
>>
>> diff --git a/libqblock/libqblock-error.c b/libqblock/libqblock-error.c
>> index e69de29..2367ab4 100644
>> --- a/libqblock/libqblock-error.c
>> +++ b/libqblock/libqblock-error.c
>> @@ -0,0 +1,49 @@

>> +
>> +typedef struct LibqbFormatStrMapping {
>> +    const char *fmt_str;
>> +    QBlockFormat fmt_type;
>> +} LibqbFormatStrMapping;
>> +
>> +LibqbFormatStrMapping libqb_formatstr_table[] = {
>
> static const
>
   OK.

>> +    {"cow", QB_FORMAT_COW},
>> +    {"qed", QB_FORMAT_QED},
>> +    {"qcow", QB_FORMAT_QCOW},
>> +    {"qcow2", QB_FORMAT_QCOW2},
>> +    {"raw", QB_FORMAT_RAW},
>> +    {"rbd", QB_FORMAT_RBD},
>> +    {"sheepdog", QB_FORMAT_SHEEPDOG},
>> +    {"vdi", QB_FORMAT_VDI},
>> +    {"vmdk", QB_FORMAT_VMDK},
>> +    {"vpc", QB_FORMAT_VPC},
>> +    {NULL, 0},
>
> You can avoid this NULL entry and save space by using ARRAY_SIZE() in
> the loop. Compiler could also unroll the loop.
>
   OK.

>> +};
>> +
>> +__attribute__((constructor))
>> +static void libqblock_init(void)
>> +{
>> +    /* Todo: add an assertion about the ABI. */
>> +    libqb_global_data.init_flag = 0;
>> +    pthread_mutex_init(&libqb_global_data.mutex, NULL);
>> +}
>> +
>> +const char *qb_formattype2str(QBlockFormat fmt_type)
>> +{
>> +    int i = 0;
>> +    LibqbFormatStrMapping *tb = libqb_formatstr_table;
>
> This does not seem to be useful, you could use libqb_formatstr_table directly.
>
   OK.

>> +
>> +    if ((fmt_type <= QB_FORMAT_NONE) || (fmt_type >= QB_FORMAT_MAX)) {
>> +        return NULL;
>> +    }
>> +    while (tb[i].fmt_str != NULL) {
>
> for (i = 0; i < ARRAY_SIZE(libqb_formatstr_table); i++) {
>
   OK.

>> +        if (tb[i].fmt_type == fmt_type) {
>> +            return tb[i].fmt_str;
>> +        }
>> +        i++;
>> +    }
>> +    return NULL;
>> +}
>> +
>> +QBlockFormat qb_str2fmttype(const char *fmt_str)
>> +{
>> +    int i = 0;
>> +    LibqbFormatStrMapping *tb = libqb_formatstr_table;
>> +
>> +    if (fmt_str == NULL) {
>> +        return QB_FORMAT_NONE;
>> +    }
>> +    while (tb[i].fmt_str != NULL) {
>> +        if ((strcmp(fmt_str, tb[i].fmt_str) == 0)) {
>> +            return tb[i].fmt_type;
>> +        }
>> +        i++;
>> +    }
>> +    return QB_FORMAT_NONE;
>> +}
>> +
>> +static void set_context_err(QBlockContext *context, int err_ret,
>> +                            const char *fmt, ...)
>
> GCC_FMT_ATTR()?
>
   Do you mean this declaration is needed here?

>> +{
>> +    va_list ap;
>> +
>> +    if (context->g_error != NULL) {
>> +        g_error_free(context->g_error);
>> +    }
>> +
>> +    va_start(ap, fmt);
>> +    context->g_error = g_error_new_valist(qb_error_quark(), err_ret, fmt, ap);
>> +    va_end(ap);
>> +
>> +    context->err_ret = err_ret;
>> +    if (err_ret == QB_ERR_INTERNAL_ERR) {
>> +        context->err_no = -errno;
>> +    } else {
>> +        context->err_no = 0;
>> +    }
>> +}
>> +

>> +static void delete_context(QBlockContext **p_context)
>> +{
>> +    if ((*p_context)->g_error != NULL) {
>> +        g_error_free((*p_context)->g_error);
>> +    }
>> +    g_free(*p_context);
>> +    *p_context = NULL;
>> +    return;
>
> Useless return, please remove.
>
   OK.


>> --
>> 1.7.1
>>
>>
>>
>

On Wed, Feb 13, 2013 at 2:08 AM, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>
>> On Sat, Feb 9, 2013 at 7:42 AM, Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> wrote:
>>>
>>>    This patch contains implemention for APIs. Basically it is a layer
>>> above qemu block general layer now.
>>>    qb_image_new() will try do init for this library.
>>>
>>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>>> ---
>>>   libqblock/libqblock-error.c |   49 +++
>>>   libqblock/libqblock.c       |  991
>>> +++++++++++++++++++++++++++++++++++++++++++
>>>   2 files changed, 1040 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/libqblock/libqblock-error.c b/libqblock/libqblock-error.c
>>> index e69de29..2367ab4 100644
>>> --- a/libqblock/libqblock-error.c
>>> +++ b/libqblock/libqblock-error.c
>>> @@ -0,0 +1,49 @@
>
>
>>> +
>>> +typedef struct LibqbFormatStrMapping {
>>> +    const char *fmt_str;
>>> +    QBlockFormat fmt_type;
>>> +} LibqbFormatStrMapping;
>>> +
>>> +LibqbFormatStrMapping libqb_formatstr_table[] = {
>>
>>
>> static const
>>
>   OK.
>
>
>>> +    {"cow", QB_FORMAT_COW},
>>> +    {"qed", QB_FORMAT_QED},
>>> +    {"qcow", QB_FORMAT_QCOW},
>>> +    {"qcow2", QB_FORMAT_QCOW2},
>>> +    {"raw", QB_FORMAT_RAW},
>>> +    {"rbd", QB_FORMAT_RBD},
>>> +    {"sheepdog", QB_FORMAT_SHEEPDOG},
>>> +    {"vdi", QB_FORMAT_VDI},
>>> +    {"vmdk", QB_FORMAT_VMDK},
>>> +    {"vpc", QB_FORMAT_VPC},
>>> +    {NULL, 0},
>>
>>
>> You can avoid this NULL entry and save space by using ARRAY_SIZE() in
>> the loop. Compiler could also unroll the loop.
>>
>   OK.
>
>
>>> +};
>>> +
>>> +__attribute__((constructor))
>>> +static void libqblock_init(void)
>>> +{
>>> +    /* Todo: add an assertion about the ABI. */
>>> +    libqb_global_data.init_flag = 0;
>>> +    pthread_mutex_init(&libqb_global_data.mutex, NULL);
>>> +}
>>> +
>>> +const char *qb_formattype2str(QBlockFormat fmt_type)
>>> +{
>>> +    int i = 0;
>>> +    LibqbFormatStrMapping *tb = libqb_formatstr_table;
>>
>>
>> This does not seem to be useful, you could use libqb_formatstr_table
>> directly.
>>
>   OK.
>
>
>>> +
>>> +    if ((fmt_type <= QB_FORMAT_NONE) || (fmt_type >= QB_FORMAT_MAX)) {
>>> +        return NULL;
>>> +    }
>>> +    while (tb[i].fmt_str != NULL) {
>>
>>
>> for (i = 0; i < ARRAY_SIZE(libqb_formatstr_table); i++) {
>>
>   OK.
>
>
>>> +        if (tb[i].fmt_type == fmt_type) {
>>> +            return tb[i].fmt_str;
>>> +        }
>>> +        i++;
>>> +    }
>>> +    return NULL;
>>> +}
>>> +
>>> +QBlockFormat qb_str2fmttype(const char *fmt_str)
>>> +{
>>> +    int i = 0;
>>> +    LibqbFormatStrMapping *tb = libqb_formatstr_table;
>>> +
>>> +    if (fmt_str == NULL) {
>>> +        return QB_FORMAT_NONE;
>>> +    }
>>> +    while (tb[i].fmt_str != NULL) {
>>> +        if ((strcmp(fmt_str, tb[i].fmt_str) == 0)) {
>>> +            return tb[i].fmt_type;
>>> +        }
>>> +        i++;
>>> +    }
>>> +    return QB_FORMAT_NONE;
>>> +}
>>> +
>>> +static void set_context_err(QBlockContext *context, int err_ret,
>>> +                            const char *fmt, ...)
>>
>>
>> GCC_FMT_ATTR()?
>>
>   Do you mean this declaration is needed here?

It declares to GCC that this function uses printf like format strings,
so it can perform additional checks. This helps catching bugs by
callers supplying incorrect arguments.

>
>
>>> +{
>>> +    va_list ap;
>>> +
>>> +    if (context->g_error != NULL) {
>>> +        g_error_free(context->g_error);
>>> +    }
>>> +
>>> +    va_start(ap, fmt);
>>> +    context->g_error = g_error_new_valist(qb_error_quark(), err_ret,
>>> fmt, ap);
>>> +    va_end(ap);
>>> +
>>> +    context->err_ret = err_ret;
>>> +    if (err_ret == QB_ERR_INTERNAL_ERR) {
>>> +        context->err_no = -errno;
>>> +    } else {
>>> +        context->err_no = 0;
>>> +    }
>>> +}
>>> +
>
>
>>> +static void delete_context(QBlockContext **p_context)
>>> +{
>>> +    if ((*p_context)->g_error != NULL) {
>>> +        g_error_free((*p_context)->g_error);
>>> +    }
>>> +    g_free(*p_context);
>>> +    *p_context = NULL;
>>> +    return;
>>
>>
>> Useless return, please remove.
>>
>   OK.
>
>
>>> --
>>> 1.7.1
>>>
>>>
>>>
>>
>
>
> --
> Best Regards
>
> Wenchao Xia
>