On 8 Apr 2016, at 23:05, Eric Blake <ebl...@redhat.com> wrote:
> NBD_OPT_EXPORT_NAME is lousy: it requires us to close the connection
> rather than report an error. Upstream NBD recently added NBD_OPT_GO
> as the improved version of the option that does what we want, along
> with NBD_OPT_INFO that returns the same information but does not
> transition to transmission phase.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
Reviewed-by: Alex Bligh <a...@alex.org.uk>
> ---
> nbd/server.c | 122 ++++++++++++++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 109 insertions(+), 13 deletions(-)
>
> diff --git a/nbd/server.c b/nbd/server.c
> index 379df8c..e68e83c 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -233,17 +233,19 @@ static int nbd_negotiate_send_rep(QIOChannel *ioc,
> uint32_t type, uint32_t opt)
> return nbd_negotiate_send_rep_len(ioc, type, opt, 0);
> }
>
> -/* Send an NBD_REP_SERVER reply to NBD_OPT_LIST, including payload.
> +/* Send the common part of an NBD_REP_SERVER reply for the given option,
> + * and include extra_len in the advertised payload.
> * Return -errno to kill connection, 0 to continue negotiation */
> -static int nbd_negotiate_send_rep_list(QIOChannel *ioc, NBDExport *exp)
> +static int nbd_negotiate_send_rep_server(QIOChannel *ioc, NBDExport *exp,
> + uint32_t opt, uint32_t extra_len)
> {
> uint32_t len;
> int rc;
>
> TRACE("Advertising export name '%s'", exp->name ? exp->name : "");
> len = strlen(exp->name);
> - rc = nbd_negotiate_send_rep_len(ioc, NBD_REP_SERVER, NBD_OPT_LIST,
> - len + sizeof(len));
> + rc = nbd_negotiate_send_rep_len(ioc, NBD_REP_SERVER, opt,
> + len + sizeof(len) + extra_len);
> if (rc < 0) {
> return rc;
> }
> @@ -261,6 +263,15 @@ static int nbd_negotiate_send_rep_list(QIOChannel *ioc,
> NBDExport *exp)
> return 0;
> }
>
> +/* Send an NBD_REP_SERVER reply to NBD_OPT_LIST, including payload.
> + * Return -errno to kill connection, 0 to continue negotiation. */
> +static int nbd_negotiate_send_rep_list(QIOChannel *ioc, NBDExport *exp)
> +{
> + return nbd_negotiate_send_rep_server(ioc, exp, NBD_OPT_LIST, 0);
> +}
> +
> +/* Send a sequence of replies to NBD_OPT_LIST.
> + * Return -errno to kill connection, 0 to continue negotiation. */
> static int nbd_negotiate_handle_list(NBDClient *client, uint32_t length)
> {
> NBDExport *exp;
> @@ -283,6 +294,8 @@ static int nbd_negotiate_handle_list(NBDClient *client,
> uint32_t length)
> return nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, NBD_OPT_LIST);
> }
>
> +/* Send a reply to NBD_OPT_EXPORT_NAME.
> + * Return -errno to kill connection, 0 to end negotiation. */
> static int nbd_negotiate_handle_export_name(NBDClient *client, uint32_t
> length)
> {
> int rc = -EINVAL;
> @@ -318,6 +331,73 @@ fail:
> }
>
>
> +/* Handle NBD_OPT_INFO and NBD_OPT_GO.
> + * Return -errno to kill connection, 0 if ready for next option, and 1
> + * to move into transmission phase. */
> +static int nbd_negotiate_handle_info(NBDClient *client, uint32_t length,
> + uint32_t opt, uint16_t myflags)
> +{
> + int rc;
> + char name[NBD_MAX_NAME_SIZE + 1];
> + NBDExport *exp;
> + uint64_t size;
> + uint16_t flags;
> +
> + /* Client sends:
> + [20 .. xx] export name (length bytes)
> + */
> + TRACE("Checking length");
> + if (length >= sizeof(name)) {
> + if (nbd_negotiate_drop_sync(client->ioc, length) != length) {
> + return -EIO;
> + }
> + return nbd_negotiate_send_rep(client->ioc, NBD_REP_ERR_INVALID, opt);
> + }
> + if (nbd_negotiate_read(client->ioc, name, length) != length) {
> + LOG("read failed");
> + return -EIO;
> + }
> + name[length] = '\0';
> +
> + TRACE("Client requested info on export '%s'", name);
> +
> + exp = nbd_export_find(name);
> + if (!exp) {
> + return nbd_negotiate_send_rep(client->ioc, NBD_REP_ERR_UNKNOWN, opt);
> + }
> +
> + QEMU_BUILD_BUG_ON(NBD_FINAL_REPLY_SIZE != sizeof(size) + sizeof(flags));
> + rc = nbd_negotiate_send_rep_server(client->ioc, exp, opt,
> + NBD_FINAL_REPLY_SIZE);
> + if (rc < 0) {
> + return rc;
> + }
> +
> + assert((exp->nbdflags & ~65535) == 0);
> + size = cpu_to_be64(exp->size);
> + flags = cpu_to_be16(exp->nbdflags | myflags);
> +
> + if (nbd_negotiate_write(client->ioc, &size, sizeof(size)) !=
> + sizeof(size)) {
> + LOG("write failed");
> + return -EIO;
> + }
> + if (nbd_negotiate_write(client->ioc, &flags, sizeof(flags)) !=
> + sizeof(flags)) {
> + LOG("write failed");
> + return -EIO;
> + }
> +
> + if (opt == NBD_OPT_GO) {
> + client->exp = exp;
> + QTAILQ_INSERT_TAIL(&client->exp->clients, client, next);
> + nbd_export_get(client->exp);
> + rc = 1;
> + }
> + return rc;
> +}
> +
> +
> static QIOChannel *nbd_negotiate_handle_starttls(NBDClient *client,
> uint32_t length)
> {
> @@ -366,7 +446,10 @@ static QIOChannel
> *nbd_negotiate_handle_starttls(NBDClient *client,
> }
>
>
> -static int nbd_negotiate_options(NBDClient *client)
> +/* Loop over all client options, during fixed newstyle negotiation.
> + * Return -errno to kill connection, 0 on successful NBD_OPT_EXPORT_NAME,
> + * 1 on successful NBD_OPT_GO. */
> +static int nbd_negotiate_options(NBDClient *client, uint16_t myflags)
> {
> uint32_t flags;
> bool fixedNewstyle = false;
> @@ -480,6 +563,16 @@ static int nbd_negotiate_options(NBDClient *client)
> case NBD_OPT_EXPORT_NAME:
> return nbd_negotiate_handle_export_name(client, length);
>
> + case NBD_OPT_INFO:
> + case NBD_OPT_GO:
> + ret = nbd_negotiate_handle_info(client, length, clientflags,
> + myflags);
> + if (ret) {
> + assert(ret < 0 || clientflags == NBD_OPT_GO);
> + return ret;
> + }
> + break;
> +
> case NBD_OPT_STARTTLS:
> if (nbd_negotiate_drop_sync(client->ioc, length) != length) {
> return -EIO;
> @@ -584,18 +677,21 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData
> *data)
> LOG("write failed");
> goto fail;
> }
> - rc = nbd_negotiate_options(client);
> - if (rc != 0) {
> + rc = nbd_negotiate_options(client, myflags);
> + if (rc < 0) {
> LOG("option negotiation failed");
> goto fail;
> }
>
> - stq_be_p(buf + 18, client->exp->size);
> - stw_be_p(buf + 26, client->exp->nbdflags | myflags);
> - len = client->no_zeroes ? 10 : sizeof(buf) - 18;
> - if (nbd_negotiate_write(client->ioc, buf + 18, len) != len) {
> - LOG("write failed");
> - goto fail;
> + if (!rc) {
> + /* If options ended with NBD_OPT_GO, we already sent this. */
> + stq_be_p(buf + 18, client->exp->size);
> + stw_be_p(buf + 26, client->exp->nbdflags | myflags);
> + len = client->no_zeroes ? 10 : sizeof(buf) - 18;
> + if (nbd_negotiate_write(client->ioc, buf + 18, len) != len) {
> + LOG("write failed");
> + goto fail;
> + }
> }
> }
>
> --
> 2.5.5
>
>
--
Alex Bligh
