Re: [Qemu-block] [Qemu-devel] [PATCH v3 02/44] nbd: Quit server after any write error

[Alex Bligh]

On 23 Apr 2016, at 00:40, Eric Blake <address@hidden> wrote:

> We should never ignore failure from nbd_negotiate_send_rep(); if
> we are unable to write to the client, then it is not worth trying
> to continue the negotiation.  Fortunately, the problem is not
> too severe - chances are that the errors being ignored here (mainly
> inability to write the reply to the client) are indications of
> a closed connection or something similar, which will also affect
> the next attempt to interact with the client and eventually reach
> a point where the errors are detected to end the loop.
> 
> Signed-off-by: Eric Blake <address@hidden>

Reviewed-by: Alex Bligh <address@hidden>

> ---
> nbd/server.c | 32 +++++++++++++++++++++++---------
> 1 file changed, 23 insertions(+), 9 deletions(-)
> 
> diff --git a/nbd/server.c b/nbd/server.c
> index fc36f4d..0a003e4 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -334,7 +334,10 @@ static QIOChannel 
> *nbd_negotiate_handle_starttls(NBDClient *client,
>         return NULL;
>     }
> 
> -    nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, NBD_OPT_STARTTLS);
> +    if (nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK,
> +                               NBD_OPT_STARTTLS) < 0) {
> +        return NULL;
> +    }
> 
>     tioc = qio_channel_tls_new_server(ioc,
>                                       client->tlscreds,
> @@ -460,8 +463,11 @@ static int nbd_negotiate_options(NBDClient *client)
>                 if (nbd_negotiate_drop_sync(client->ioc, length) != length) {
>                     return -EIO;
>                 }
> -                nbd_negotiate_send_rep(client->ioc, NBD_REP_ERR_TLS_REQD,
> -                                       clientflags);
> +                ret = nbd_negotiate_send_rep(client->ioc, 
> NBD_REP_ERR_TLS_REQD,
> +                                             clientflags);
> +                if (ret < 0) {
> +                    return ret;
> +                }
>                 break;
>             }
>         } else if (fixedNewstyle) {
> @@ -485,12 +491,17 @@ static int nbd_negotiate_options(NBDClient *client)
>                 }
>                 if (client->tlscreds) {
>                     TRACE("TLS already enabled");
> -                    nbd_negotiate_send_rep(client->ioc, NBD_REP_ERR_INVALID,
> -                                           clientflags);
> +                    ret = nbd_negotiate_send_rep(client->ioc,
> +                                                 NBD_REP_ERR_INVALID,
> +                                                 clientflags);
>                 } else {
>                     TRACE("TLS not configured");
> -                    nbd_negotiate_send_rep(client->ioc, NBD_REP_ERR_POLICY,
> -                                           clientflags);
> +                    ret = nbd_negotiate_send_rep(client->ioc,
> +                                                 NBD_REP_ERR_POLICY,
> +                                                 clientflags);
> +                }
> +                if (ret < 0) {
> +                    return ret;
>                 }
>                 break;
>             default:
> @@ -498,8 +509,11 @@ static int nbd_negotiate_options(NBDClient *client)
>                 if (nbd_negotiate_drop_sync(client->ioc, length) != length) {
>                     return -EIO;
>                 }
> -                nbd_negotiate_send_rep(client->ioc, NBD_REP_ERR_UNSUP,
> -                                       clientflags);
> +                ret = nbd_negotiate_send_rep(client->ioc, NBD_REP_ERR_UNSUP,
> +                                             clientflags);
> +                if (ret < 0) {
> +                    return ret;
> +                }
>                 break;
>             }
>         } else {
> -- 
> 2.5.5
> 
> 

-- 
Alex Bligh