Re: [Qemu-devel] [PATCH] qemu-nbd: only send a limited number of errno codes on the wire

[Max Reitz <mreitz@redhat.com>]

On 07.05.2015 17:26, Paolo Bonzini wrote:
> Right now, NBD includes potentially platform-specific error values in
> the wire protocol.
>
> Luckily, most common error values are more or less universal: in
> particular, of all errno values <= 34 (up to ERANGE), they are all
> the same on supported platforms except for 11 (which is EAGAIN on
> Windows and Linux, but EDEADLK on Darwin and the *BSDs).
>
> So, in order to guarantee some portability, only keep a dozen
> possible error codes and squash everything else to EINVAL.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   nbd.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 47 insertions(+)
>
> diff --git a/nbd.c b/nbd.c
> index eea8c51..1ad5b66 100644
> --- a/nbd.c
> +++ b/nbd.c
> @@ -86,6 +86,37 @@
>   #define NBD_OPT_ABORT           (2)
>   #define NBD_OPT_LIST            (3)
>   
> +/* NBD errors are based on errno numbers, so there is a 1:1 mapping,
> + * but only a limited set of errno values is specified in the protocol.
> + * Everything else is squashed to EINVAL.
> + */
> +static int system_errno_to_nbd_errno(int err)
> +{
> +    switch (err) {
> +    case EPERM:
> +        return 1;
> +    case EIO:
> +        return 5;
> +    case ENXIO:
> +        return 6;
> +    case E2BIG:
> +        return 7;
> +    case ENOMEM:
> +        return 12;
> +    case EACCES:
> +        return 13;
> +    case EFBIG:
> +        return 27;
> +    case ENOSPC:
> +        return 28;
> +    case EROFS:
> +        return 30;
> +    case EINVAL:
> +    default:
> +        return 22;
> +    }
> +}
> +
>   /* Definitions for opaque data types */
>   
>   typedef struct NBDRequest NBDRequest;
> @@ -856,6 +887,20 @@ ssize_t nbd_receive_reply(int csock, struct nbd_reply *reply)
>       reply->error  = be32_to_cpup((uint32_t*)(buf + 4));
>       reply->handle = be64_to_cpup((uint64_t*)(buf + 8));
>   
> +    /* NBD errors should be universally equal to the corresponding
> +     * errno values, check it here.
> +     */
> +    QEMU_BUILD_BUG_ON(EPERM != 1);
> +    QEMU_BUILD_BUG_ON(EIO != 5);
> +    QEMU_BUILD_BUG_ON(ENXIO != 6);
> +    QEMU_BUILD_BUG_ON(E2BIG != 7);
> +    QEMU_BUILD_BUG_ON(ENOMEM != 12);
> +    QEMU_BUILD_BUG_ON(EACCES != 13);
> +    QEMU_BUILD_BUG_ON(EINVAL != 22);
> +    QEMU_BUILD_BUG_ON(EFBIG != 27);
> +    QEMU_BUILD_BUG_ON(ENOSPC != 28);
> +    QEMU_BUILD_BUG_ON(EROFS != 30);
> +

Why no nbd_errno_to_system_errno() function?

Max

>       TRACE("Got reply: "
>             "{ magic = 0x%x, .error = %d, handle = %" PRIu64" }",
>             magic, reply->error, reply->handle);
> @@ -872,6 +917,8 @@ static ssize_t nbd_send_reply(int csock, struct nbd_reply *reply)
>       uint8_t buf[NBD_REPLY_SIZE];
>       ssize_t ret;
>   
> +    reply->error = system_errno_to_nbd_errno(reply->error);
> +
>       /* Reply
>          [ 0 ..  3]    magic   (NBD_REPLY_MAGIC)
>          [ 4 ..  7]    error   (0 == no error)