[V2,3/6] libqblock error handling

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

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.

于 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.

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
>
>

于 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
>>
>>
>

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
>