Re: [Qemu-devel] [PATCH V2 1/6] libqblock API design

[Kevin Wolf <kwolf@redhat.com>]

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