[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 1/5] qerror: add error codes for fopen failur
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] [PATCH v3 1/5] qerror: add error codes for fopen failure |
Date: |
Fri, 23 Mar 2012 11:21:25 -0300 |
On Sun, 18 Mar 2012 19:29:09 +0100
Alon Levy <address@hidden> wrote:
> Added:
>
> QERR_EINTR
> QERR_EACCES
> QERR_EEXIST
> QERR_OPEN_FILE_EMFILE
> QERR_ENOSPC
> QERR_EPERM
> QERR_READ_ONLY
> QERR_ENOTDIR
> QERR_EFBIG
>
> Signed-off-by: Alon Levy <address@hidden>
> ---
> qerror.c | 36 ++++++++++++++++++++++++++++++++++++
> qerror.h | 27 +++++++++++++++++++++++++++
> 2 files changed, 63 insertions(+)
>
> diff --git a/qerror.c b/qerror.c
> index f55d435..4915939 100644
> --- a/qerror.c
> +++ b/qerror.c
> @@ -213,6 +213,42 @@ static const QErrorStringTable qerror_table[] = {
> .desc = "Could not open '%(filename)'",
> },
> {
> + .error_fmt = QERR_EINTR,
> + .desc = "Interrupted open of '%(filename)'",
> + },
There are two problems here. First, the QERR_INTR macro doesn't have the
filename argument, so this will crash.
The second problem is that, I've talked to Anthony and EINTR should not
be returned to clients, it should be handled intead. So, please handle it
during write().
> + {
> + .error_fmt = QERR_EACCES,
> + .desc = "Cannot access file'",
> + },
We already have QERR_PERMISSION_DENIED.
> + {
> + .error_fmt = QERR_EEXIST,
> + .desc = "File already exists'",
> + },
Can you use the description provided by man 3 errno? This is valid for all
errors you're adding.
> + {
> + .error_fmt = QERR_OPEN_FILE_EMFILE,
> + .desc = "Max open files when opening file'",
> + },
> + {
> + .error_fmt = QERR_ENOSPC,
> + .desc = "No space left opening file'",
> + },
> + {
> + .error_fmt = QERR_EPERM,
> + .desc = "Permission denied (EPERM) opening file'",
> + },
> + {
> + .error_fmt = QERR_READ_ONLY,
> + .desc = "Read only filesystem opening file'",
> + },
> + {
> + .error_fmt = QERR_ENOTDIR,
> + .desc = "Directory related error opening file'",
> + },
> + {
> + .error_fmt = QERR_EFBIG,
> + .desc = "File too big opening'",
> + },
> + {
> .error_fmt = QERR_PERMISSION_DENIED,
> .desc = "Insufficient permission to perform this operation",
> },
> diff --git a/qerror.h b/qerror.h
> index e26c635..ddc04e8 100644
> --- a/qerror.h
> +++ b/qerror.h
> @@ -181,6 +181,33 @@ QError *qobject_to_qerror(const QObject *obj);
> #define QERR_OPEN_FILE_FAILED \
> "{ 'class': 'OpenFileFailed', 'data': { 'filename': %s } }"
>
> +#define QERR_OPEN_FILE_EMFILE \
> + "{ 'class': 'OpenFileEMFILE', 'data': {} }"
> +
> +#define QERR_EINTR \
> + "{ 'class': 'EINTR', 'data': {} }"
> +
> +#define QERR_EACCES \
> + "{ 'class': 'EACCES', 'data': {} }"
> +
> +#define QERR_EEXIST \
> + "{ 'class': 'EEXIST', 'data': {} }"
We use more descriptive names instead of using the errno name. Like
AlreadyExists.
Please fix it for errors you adding.
> +
> +#define QERR_ENOSPC \
> + "{ 'class': 'ENOSPC', 'data': {} }"
> +
> +#define QERR_EPERM \
> + "{ 'class': 'EPERM', 'data': {} }"
> +
> +#define QERR_READ_ONLY \
> + "{ 'class': 'ReadOnly', 'data': {} }"
> +
> +#define QERR_ENOTDIR \
> + "{ 'class': 'ENOTDIR', 'data': {} }"
> +
> +#define QERR_EFBIG \
> + "{ 'class': 'EFBIG', 'data': {} }"
> +
> #define QERR_PERMISSION_DENIED \
> "{ 'class': 'PermissionDenied', 'data': {} }"
>
- Re: [Qemu-devel] [PATCH v2 1/5] error: add error_set_file_open_failed, (continued)
[Qemu-devel] [PATCH v3 0/5] screendump qapi convertion, Alon Levy, 2012/03/18
[Qemu-devel] [PATCH v3 5/5] vga: ppm_save(): Return error on failure, Alon Levy, 2012/03/18
[Qemu-devel] [PATCH v3 1/5] qerror: add error codes for fopen failure, Alon Levy, 2012/03/18
Re: [Qemu-devel] [PATCH v3 1/5] qerror: add error codes for fopen failure, Alon Levy, 2012/03/26
Re: [Qemu-devel] [PATCH v3 1/5] qerror: add error codes for fopen failure, Alon Levy, 2012/03/26
[Qemu-devel] [PATCH v3 3/5] vga_hw_screen_dump: add Error** param, Alon Levy, 2012/03/18
[Qemu-devel] [PATCH v3 4/5] qapi: convert screendump, Alon Levy, 2012/03/18