Re: [Qemu-devel] [PATCH 12/15] nbd: implement TLS support in the protocol negotiation

[Wouter Verhelst <w@uter.be>]

Minor nitpick:

On Fri, Nov 27, 2015 at 12:20:50PM +0000, Daniel P. Berrange wrote:
[...]
> @@ -563,6 +659,14 @@ static int nbd_receive_options(NBDClient *client)
>              case NBD_OPT_EXPORT_NAME:
>                  return nbd_handle_export_name(client, length);
>  
> +            case NBD_OPT_STARTTLS:
> +                if (client->tlscreds) {
> +                    TRACE("TLS already enabled");
> +                } else {
> +                    TRACE("TLS not configured");
> +                }
> +                nbd_send_rep(client->ioc, NBD_REP_ERR_UNSUP, clientflags);

NBD_REP_ERR_UNSUP is supposed to be reserved as the default reply for
replies unknown to a server implementation (i.e., it's "this request is
not supported by this server"). Trying to negotiate TLS in a TLS channel
would be NBD_REP_ERR_INVALID ("invalid request"). Trying to negotiate
TLS when no TLS configuration is available server-side would be
NBD_REP_ERR_POLICY ("request not allowed by server-side policy").

Keeping to these error codes would allow a client to provide more useful
information to a user beyond "haha it fail"; but I suppose there can be
arguments for not doing so, too.

Beyond this and the default export that I talked about earlier, no
comments.

-- 
It is easy to love a country that is famous for chocolate and beer

  -- Barack Obama, speaking in Brussels, Belgium, 2014-03-26