On Tue, Feb 21, 2017 at 6:54 AM Eric Blake <ebl...@redhat.com> wrote:
> The NBD protocol has several constants defined in various extensions
> that we are about to implement. Expose them to the code, along with
> an easy way to map various constants to strings during diagnostic
> messages.
>
> Doing this points out a debug message in server.c that got
> parameters mixed up.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
>
Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
>
> ---
> v4: new patch
> ---
> include/block/nbd.h | 34 +++++++++++++++++++-------
> nbd/nbd-internal.h | 9 +++----
> nbd/client.c | 56 ++++++++++++++++++++++++++++---------------
> nbd/common.c | 69
> +++++++++++++++++++++++++++++++++++++++++++++++++++++
> nbd/server.c | 17 +++++++------
> 5 files changed, 145 insertions(+), 40 deletions(-)
>
> diff --git a/include/block/nbd.h b/include/block/nbd.h
> index 8cc9cbe..c84e022 100644
> --- a/include/block/nbd.h
> +++ b/include/block/nbd.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (C) 2016 Red Hat, Inc.
> + * Copyright (C) 2016-2017 Red Hat, Inc.
> * Copyright (C) 2005 Anthony Liguori <anth...@codemonkey.ws>
> *
> * Network Block Device
> @@ -83,18 +83,36 @@ typedef struct NBDReply NBDReply;
> #define NBD_FLAG_C_FIXED_NEWSTYLE (1 << 0) /* Fixed newstyle protocol. */
> #define NBD_FLAG_C_NO_ZEROES (1 << 1) /* End handshake without
> zeroes. */
>
> -/* Reply types. */
> +/* Option requests. */
> +#define NBD_OPT_EXPORT_NAME (1)
> +#define NBD_OPT_ABORT (2)
> +#define NBD_OPT_LIST (3)
> +/* #define NBD_OPT_PEEK_EXPORT (4) not in use */
> +#define NBD_OPT_STARTTLS (5)
> +#define NBD_OPT_INFO (6)
> +#define NBD_OPT_GO (7)
> +
> +/* Option reply types. */
> #define NBD_REP_ERR(value) ((UINT32_C(1) << 31) | (value))
>
> #define NBD_REP_ACK (1) /* Data sending finished.
> */
> #define NBD_REP_SERVER (2) /* Export description. */
> +#define NBD_REP_INFO (3) /* NBD_OPT_INFO/GO. */
>
> -#define NBD_REP_ERR_UNSUP NBD_REP_ERR(1) /* Unknown option */
> -#define NBD_REP_ERR_POLICY NBD_REP_ERR(2) /* Server denied */
> -#define NBD_REP_ERR_INVALID NBD_REP_ERR(3) /* Invalid length */
> -#define NBD_REP_ERR_PLATFORM NBD_REP_ERR(4) /* Not compiled in */
> -#define NBD_REP_ERR_TLS_REQD NBD_REP_ERR(5) /* TLS required */
> -#define NBD_REP_ERR_SHUTDOWN NBD_REP_ERR(7) /* Server shutting down */
> +#define NBD_REP_ERR_UNSUP NBD_REP_ERR(1) /* Unknown option */
> +#define NBD_REP_ERR_POLICY NBD_REP_ERR(2) /* Server denied */
> +#define NBD_REP_ERR_INVALID NBD_REP_ERR(3) /* Invalid length */
> +#define NBD_REP_ERR_PLATFORM NBD_REP_ERR(4) /* Not compiled in */
> +#define NBD_REP_ERR_TLS_REQD NBD_REP_ERR(5) /* TLS required */
> +#define NBD_REP_ERR_UNKNOWN NBD_REP_ERR(6) /* Export unknown */
> +#define NBD_REP_ERR_SHUTDOWN NBD_REP_ERR(7) /* Server shutting
> down */
> +#define NBD_REP_ERR_BLOCK_SIZE_REQD NBD_REP_ERR(8) /* Need
> INFO_BLOCK_SIZE */
> +
> +/* Info types, used during NBD_REP_INFO */
> +#define NBD_INFO_EXPORT 0
> +#define NBD_INFO_NAME 1
> +#define NBD_INFO_DESCRIPTION 2
> +#define NBD_INFO_BLOCK_SIZE 3
>
> /* Request flags, sent from client to server during transmission phase */
> #define NBD_CMD_FLAG_FUA (1 << 0) /* 'force unit access' during
> write */
> diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h
> index f43d990..aa5b2fd 100644
> --- a/nbd/nbd-internal.h
> +++ b/nbd/nbd-internal.h
> @@ -76,12 +76,6 @@
> #define NBD_SET_TIMEOUT _IO(0xab, 9)
> #define NBD_SET_FLAGS _IO(0xab, 10)
>
> -#define NBD_OPT_EXPORT_NAME (1)
> -#define NBD_OPT_ABORT (2)
> -#define NBD_OPT_LIST (3)
> -#define NBD_OPT_PEEK_EXPORT (4)
> -#define NBD_OPT_STARTTLS (5)
> -
> /* 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.
> @@ -122,5 +116,8 @@ struct NBDTLSHandshakeData {
>
> void nbd_tls_handshake(QIOTask *task,
> void *opaque);
> +const char *nbd_opt_lookup(uint32_t opt);
> +const char *nbd_rep_lookup(uint32_t rep);
> +const char *nbd_info_lookup(uint16_t info);
>
> #endif
> diff --git a/nbd/client.c b/nbd/client.c
> index 69f0e09..f96539b 100644
> --- a/nbd/client.c
> +++ b/nbd/client.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (C) 2016 Red Hat, Inc.
> + * Copyright (C) 2016-2017 Red Hat, Inc.
> * Copyright (C) 2005 Anthony Liguori <anth...@codemonkey.ws>
> *
> * Network Block Device Client Side
> @@ -130,7 +130,8 @@ static int nbd_send_option_request(QIOChannel *ioc,
> uint32_t opt,
> if (len == -1) {
> req.length = len = strlen(data);
> }
> - TRACE("Sending option request %" PRIu32", len %" PRIu32, opt, len);
> + TRACE("Sending option request %" PRIu32" (%s), len %" PRIu32, opt,
> + nbd_opt_lookup(opt), len);
>
> stq_be_p(&req.magic, NBD_OPTS_MAGIC);
> stl_be_p(&req.option, opt);
> @@ -180,8 +181,10 @@ static int nbd_receive_option_reply(QIOChannel *ioc,
> uint32_t opt,
> be32_to_cpus(&reply->type);
> be32_to_cpus(&reply->length);
>
> - TRACE("Received option reply %" PRIx32", type %" PRIx32", len %"
> PRIu32,
> - reply->option, reply->type, reply->length);
> + TRACE("Received option reply %" PRIx32" (%s), type %" PRIx32
> + " (%s), len %" PRIu32,
> + reply->option, nbd_opt_lookup(reply->option),
> + reply->type, nbd_rep_lookup(reply->type), reply->length);
>
> if (reply->magic != NBD_REP_MAGIC) {
> error_setg(errp, "Unexpected option reply magic");
> @@ -215,12 +218,16 @@ static int nbd_handle_reply_err(QIOChannel *ioc,
> nbd_opt_reply *reply,
>
> if (reply->length) {
> if (reply->length > NBD_MAX_BUFFER_SIZE) {
> - error_setg(errp, "server's error message is too long");
> + error_setg(errp, "server error 0x%" PRIx32
> + " (%s) message is too long",
> + reply->type, nbd_rep_lookup(reply->type));
> goto cleanup;
> }
> msg = g_malloc(reply->length + 1);
> if (read_sync(ioc, msg, reply->length) != reply->length) {
> - error_setg(errp, "failed to read option error message");
> + error_setg(errp, "failed to read option error 0x%" PRIx32
> + " (%s) message",
> + reply->type, nbd_rep_lookup(reply->type));
> goto cleanup;
> }
> msg[reply->length] = '\0';
> @@ -229,38 +236,49 @@ static int nbd_handle_reply_err(QIOChannel *ioc,
> nbd_opt_reply *reply,
> switch (reply->type) {
> case NBD_REP_ERR_UNSUP:
> TRACE("server doesn't understand request %" PRIx32
> - ", attempting fallback", reply->option);
> + " (%s), attempting fallback",
> + reply->option, nbd_opt_lookup(reply->option));
> result = 0;
> goto cleanup;
>
> case NBD_REP_ERR_POLICY:
> - error_setg(errp, "Denied by server for option %" PRIx32,
> - reply->option);
> + error_setg(errp, "Denied by server for option %" PRIx32 " (%s)",
> + reply->option, nbd_opt_lookup(reply->option));
> break;
>
> case NBD_REP_ERR_INVALID:
> - error_setg(errp, "Invalid data length for option %" PRIx32,
> - reply->option);
> + error_setg(errp, "Invalid data length for option %" PRIx32 "
> (%s)",
> + reply->option, nbd_opt_lookup(reply->option));
> break;
>
> case NBD_REP_ERR_PLATFORM:
> - error_setg(errp, "Server lacks support for option %" PRIx32,
> - reply->option);
> + error_setg(errp, "Server lacks support for option %" PRIx32 "
> (%s)",
> + reply->option, nbd_opt_lookup(reply->option));
> break;
>
> case NBD_REP_ERR_TLS_REQD:
> - error_setg(errp, "TLS negotiation required before option %"
> PRIx32,
> - reply->option);
> + error_setg(errp, "TLS negotiation required before option %" PRIx32
> + " (%s)", reply->option, nbd_opt_lookup(reply->option));
> + break;
> +
> + case NBD_REP_ERR_UNKNOWN:
> + error_setg(errp, "Requested export not available for option %"
> PRIx32
> + " (%s)", reply->option, nbd_opt_lookup(reply->option));
> break;
>
> case NBD_REP_ERR_SHUTDOWN:
> - error_setg(errp, "Server shutting down before option %" PRIx32,
> - reply->option);
> + error_setg(errp, "Server shutting down before option %" PRIx32 "
> (%s)",
> + reply->option, nbd_opt_lookup(reply->option));
> + break;
> +
> + case NBD_REP_ERR_BLOCK_SIZE_REQD:
> + error_setg(errp, "Server requires INFO_BLOCK_SIZE for option %"
> PRIx32
> + " (%s)", reply->option, nbd_opt_lookup(reply->option));
> break;
>
> default:
> - error_setg(errp, "Unknown error code when asking for option %"
> PRIx32,
> - reply->option);
> + error_setg(errp, "Unknown error code when asking for option %"
> PRIx32
> + " (%s)", reply->option, nbd_opt_lookup(reply->option));
> break;
> }
>
> diff --git a/nbd/common.c b/nbd/common.c
> index a5f39ea..85622f9 100644
> --- a/nbd/common.c
> +++ b/nbd/common.c
> @@ -89,3 +89,72 @@ void nbd_tls_handshake(QIOTask *task,
> data->complete = true;
> g_main_loop_quit(data->loop);
> }
> +
> +
> +const char *nbd_opt_lookup(uint32_t opt)
> +{
> + switch (opt) {
> + case NBD_OPT_EXPORT_NAME:
> + return "export name";
> + case NBD_OPT_ABORT:
> + return "abort";
> + case NBD_OPT_LIST:
> + return "list";
> + case NBD_OPT_STARTTLS:
> + return "starttls";
> + case NBD_OPT_INFO:
> + return "info";
> + case NBD_OPT_GO:
> + return "go";
> + default:
> + return "<unknown>";
> + }
> +}
> +
> +
> +const char *nbd_rep_lookup(uint32_t rep)
> +{
> + switch (rep) {
> + case NBD_REP_ACK:
> + return "ack";
> + case NBD_REP_SERVER:
> + return "server";
> + case NBD_REP_INFO:
> + return "info";
> + case NBD_REP_ERR_UNSUP:
> + return "unsupported";
> + case NBD_REP_ERR_POLICY:
> + return "denied by policy";
> + case NBD_REP_ERR_INVALID:
> + return "invalid";
> + case NBD_REP_ERR_PLATFORM:
> + return "platform lacks support";
> + case NBD_REP_ERR_TLS_REQD:
> + return "TLS required";
> + case NBD_REP_ERR_UNKNOWN:
> + return "export unknown";
> + case NBD_REP_ERR_SHUTDOWN:
> + return "server shutting down";
> + case NBD_REP_ERR_BLOCK_SIZE_REQD:
> + return "block size required";
> + default:
> + return "<unknown>";
> + }
> +}
> +
> +
> +const char *nbd_info_lookup(uint16_t info)
> +{
> + switch (info) {
> + case NBD_INFO_EXPORT:
> + return "export";
> + case NBD_INFO_NAME:
> + return "name";
> + case NBD_INFO_DESCRIPTION:
> + return "description";
> + case NBD_INFO_BLOCK_SIZE:
> + return "block size";
> + default:
> + return "<unknown>";
> + }
> +}
> diff --git a/nbd/server.c b/nbd/server.c
> index efe5cb8..767ca0f 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (C) 2016 Red Hat, Inc.
> + * Copyright (C) 2016-2017 Red Hat, Inc.
> * Copyright (C) 2005 Anthony Liguori <anth...@codemonkey.ws>
> *
> * Network Block Device Server Side
> @@ -206,8 +206,8 @@ static int nbd_negotiate_send_rep_len(QIOChannel *ioc,
> uint32_t type,
> {
> uint64_t magic;
>
> - TRACE("Reply opt=%" PRIx32 " type=%" PRIx32 " len=%" PRIu32,
> - type, opt, len);
> + TRACE("Reply opt=%" PRIx32 " (%s), type=%" PRIx32 " (%s), len=%"
> PRIu32,
> + opt, nbd_opt_lookup(opt), type, nbd_rep_lookup(type), len);
>
> magic = cpu_to_be64(NBD_REP_MAGIC);
> if (nbd_negotiate_write(ioc, &magic, sizeof(magic)) != sizeof(magic))
> {
> @@ -493,7 +493,8 @@ static int nbd_negotiate_options(NBDClient *client)
> }
> length = be32_to_cpu(length);
>
> - TRACE("Checking option 0x%" PRIx32, clientflags);
> + TRACE("Checking option 0x%" PRIx32 " (%s)", clientflags,
> + nbd_opt_lookup(clientflags));
> if (client->tlscreds &&
> client->ioc == (QIOChannel *)client->sioc) {
> QIOChannel *tioc;
> @@ -581,8 +582,9 @@ static int nbd_negotiate_options(NBDClient *client)
> NBD_REP_ERR_UNSUP,
> clientflags,
> "Unsupported option 0x%"
> - PRIx32,
> - clientflags);
> + PRIx32 " (%s)",
> + clientflags,
> +
> nbd_opt_lookup(clientflags));
> if (ret < 0) {
> return ret;
> }
> @@ -598,7 +600,8 @@ static int nbd_negotiate_options(NBDClient *client)
> return nbd_negotiate_handle_export_name(client, length);
>
> default:
> - TRACE("Unsupported option 0x%" PRIx32, clientflags);
> + TRACE("Unsupported option 0x%" PRIx32 " (%s)",
> clientflags,
> + nbd_opt_lookup(clientflags));
> return -EINVAL;
> }
> }
> --
> 2.9.3
>
>
> --
Marc-André Lureau
