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

[Blue Swirl]

On Wed, Sep 12, 2012 at 8:19 AM, Kevin Wolf <address@hidden> wrote:
> Am 11.09.2012 22:28, schrieb Blue Swirl:
>> On Mon, Sep 10, 2012 at 8:26 AM, Wenchao Xia <address@hidden> 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 <address@hidden>
>>> ---
>>>  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   <address@hidden>
>>> + *
>>> + * This work is licensed under the terms of the GNU LGPL, version 2 or 
>>> later.
>>> + * See the COPYING.LIB file in the top-level directory.
>>> + *
>>> + */
>>> +
>>> +#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