All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: qemu-devel@nongnu.org
Subject: [Qemu-devel] [PULL 17/27] nbd: Let server know when client gives up negotiation
Date: Mon, 31 Oct 2016 15:37:33 +0100	[thread overview]
Message-ID: <1477924663-30950-18-git-send-email-pbonzini@redhat.com> (raw)
In-Reply-To: <1477924663-30950-1-git-send-email-pbonzini@redhat.com>

From: Eric Blake <eblake@redhat.com>

The NBD spec says that a client should send NBD_OPT_ABORT
rather than just dropping the connection, if the client doesn't
like something the server sent during option negotiation.  This
is a best-effort attempt only, and can only be done in places
where we know the server is still in sync with what we've sent,
whether or not we've read everything the server has sent.
Technically, the server then has to reply with NBD_REP_ACK, but
it's not worth complicating the client to wait around for that
reply.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1476469998-28592-10-git-send-email-eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd/client.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/nbd/client.c b/nbd/client.c
index e78000a..651e08c 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -111,6 +111,19 @@ static int nbd_send_option_request(QIOChannel *ioc, uint32_t opt,
     return 0;
 }
 
+/* Send NBD_OPT_ABORT as a courtesy to let the server know that we are
+ * not going to attempt further negotiation. */
+static void nbd_send_opt_abort(QIOChannel *ioc)
+{
+    /* Technically, a compliant server is supposed to reply to us; but
+     * older servers disconnected instead. At any rate, we're allowed
+     * to disconnect without waiting for the server reply, so we don't
+     * even care if the request makes it to the server, let alone
+     * waiting around for whether the server replies. */
+    nbd_send_option_request(ioc, NBD_OPT_ABORT, 0, NULL, NULL);
+}
+
+
 /* Receive the header of an option reply, which should match the given
  * opt.  Read through the length field, but NOT the length bytes of
  * payload. Return 0 if successful, -1 with errp set if it is
@@ -121,6 +134,7 @@ static int nbd_receive_option_reply(QIOChannel *ioc, uint32_t opt,
     QEMU_BUILD_BUG_ON(sizeof(*reply) != 20);
     if (read_sync(ioc, reply, sizeof(*reply)) != sizeof(*reply)) {
         error_setg(errp, "failed to read option reply");
+        nbd_send_opt_abort(ioc);
         return -1;
     }
     be64_to_cpus(&reply->magic);
@@ -133,11 +147,13 @@ static int nbd_receive_option_reply(QIOChannel *ioc, uint32_t opt,
 
     if (reply->magic != NBD_REP_MAGIC) {
         error_setg(errp, "Unexpected option reply magic");
+        nbd_send_opt_abort(ioc);
         return -1;
     }
     if (reply->option != opt) {
         error_setg(errp, "Unexpected option type %x expected %x",
                    reply->option, opt);
+        nbd_send_opt_abort(ioc);
         return -1;
     }
     return 0;
@@ -206,6 +222,9 @@ static int nbd_handle_reply_err(QIOChannel *ioc, nbd_opt_reply *reply,
 
  cleanup:
     g_free(msg);
+    if (result < 0) {
+        nbd_send_opt_abort(ioc);
+    }
     return result;
 }
 
@@ -229,25 +248,30 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp)
     if (reply.type == NBD_REP_ACK) {
         if (len != 0) {
             error_setg(errp, "length too long for option end");
+            nbd_send_opt_abort(ioc);
             return -1;
         }
     } else if (reply.type == NBD_REP_SERVER) {
         if (len < sizeof(namelen) || len > NBD_MAX_BUFFER_SIZE) {
             error_setg(errp, "incorrect option length %" PRIu32, len);
+            nbd_send_opt_abort(ioc);
             return -1;
         }
         if (read_sync(ioc, &namelen, sizeof(namelen)) != sizeof(namelen)) {
             error_setg(errp, "failed to read option name length");
+            nbd_send_opt_abort(ioc);
             return -1;
         }
         namelen = be32_to_cpu(namelen);
         len -= sizeof(namelen);
         if (len < namelen) {
             error_setg(errp, "incorrect option name length");
+            nbd_send_opt_abort(ioc);
             return -1;
         }
         if (namelen > NBD_MAX_NAME_SIZE) {
             error_setg(errp, "export name length too long %" PRIu32, namelen);
+            nbd_send_opt_abort(ioc);
             return -1;
         }
 
@@ -256,6 +280,7 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp)
             error_setg(errp, "failed to read export name");
             g_free(*name);
             *name = NULL;
+            nbd_send_opt_abort(ioc);
             return -1;
         }
         (*name)[namelen] = '\0';
@@ -267,6 +292,7 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp)
                 g_free(*name);
                 g_free(buf);
                 *name = NULL;
+                nbd_send_opt_abort(ioc);
                 return -1;
             }
             buf[len] = '\0';
@@ -276,6 +302,7 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp)
     } else {
         error_setg(errp, "Unexpected reply type %" PRIx32 " expected %x",
                    reply.type, NBD_REP_SERVER);
+        nbd_send_opt_abort(ioc);
         return -1;
     }
     return 1;
@@ -325,6 +352,7 @@ static int nbd_receive_query_exports(QIOChannel *ioc,
 
     if (!foundExport) {
         error_setg(errp, "No export with name '%s' available", wantname);
+        nbd_send_opt_abort(ioc);
         return -1;
     }
 
@@ -352,12 +380,14 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc,
     if (reply.type != NBD_REP_ACK) {
         error_setg(errp, "Server rejected request to start TLS %" PRIx32,
                    reply.type);
+        nbd_send_opt_abort(ioc);
         return NULL;
     }
 
     if (reply.length != 0) {
         error_setg(errp, "Start TLS response was not zero %" PRIu32,
                    reply.length);
+        nbd_send_opt_abort(ioc);
         return NULL;
     }
 
-- 
2.7.4

  parent reply	other threads:[~2016-10-31 14:38 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-31 14:37 [Qemu-devel] [PULL 00/27] Misc patches for 2016-10-31 Paolo Bonzini
2016-10-31 14:37 ` [Qemu-devel] [PULL 01/27] checkpatch: tweak "struct should normally be const" warning Paolo Bonzini
2016-10-31 14:37 ` [Qemu-devel] [PULL 02/27] nbd: Use CoQueue for free_sema instead of CoMutex Paolo Bonzini
2016-10-31 14:37 ` [Qemu-devel] [PULL 03/27] qemu-error: remove dependency of stubs on monitor Paolo Bonzini
2016-10-31 14:37 ` [Qemu-devel] [PULL 04/27] tests: send error_report to test log Paolo Bonzini
2016-10-31 14:37 ` [Qemu-devel] [PULL 05/27] exec.c: ensure all AddressSpaceDispatch updates under RCU Paolo Bonzini
2016-10-31 14:37 ` [Qemu-devel] [PULL 06/27] exec.c: do not truncate non-empty memory backend file Paolo Bonzini
2016-10-31 14:37 ` [Qemu-devel] [PULL 07/27] exec.c: check memory backend file size with 'size' option Paolo Bonzini
2016-10-31 14:37 ` [Qemu-devel] [PULL 08/27] hostmem-file: make option 'size' optional Paolo Bonzini
2016-10-31 18:20   ` Eduardo Habkost
2016-10-31 19:47     ` Paolo Bonzini
2016-10-31 22:22       ` Eduardo Habkost
2016-11-01  9:32         ` Haozhong Zhang
2016-11-01 14:16           ` Eduardo Habkost
2016-11-02  1:27             ` Haozhong Zhang
2016-10-31 14:37 ` [Qemu-devel] [PULL 09/27] nbd: Add qemu-nbd -D for human-readable description Paolo Bonzini
2016-10-31 14:37 ` [Qemu-devel] [PULL 10/27] nbd: Treat flags vs. command type as separate fields Paolo Bonzini
2016-10-31 14:37 ` [Qemu-devel] [PULL 11/27] nbd: Rename NBDRequest to NBDRequestData Paolo Bonzini
2016-10-31 14:37 ` [Qemu-devel] [PULL 12/27] nbd: Rename NbdClientSession to NBDClientSession Paolo Bonzini
2016-10-31 14:37 ` [Qemu-devel] [PULL 13/27] nbd: Rename struct nbd_request and nbd_reply Paolo Bonzini
2016-10-31 14:37 ` [Qemu-devel] [PULL 14/27] nbd: Share common reply-sending code in server Paolo Bonzini
2016-10-31 14:37 ` [Qemu-devel] [PULL 15/27] nbd: Send message along with server NBD_REP_ERR errors Paolo Bonzini
2016-10-31 14:37 ` [Qemu-devel] [PULL 16/27] nbd: Share common option-sending code in client Paolo Bonzini
2016-10-31 14:37 ` Paolo Bonzini [this message]
2016-10-31 14:37 ` [Qemu-devel] [PULL 18/27] nbd: Let client skip portions of server reply Paolo Bonzini
2016-10-31 14:37 ` [Qemu-devel] [PULL 19/27] nbd: Less allocation during NBD_OPT_LIST Paolo Bonzini
2016-10-31 14:37 ` [Qemu-devel] [PULL 20/27] nbd: Support shorter handshake Paolo Bonzini
2016-10-31 14:37 ` [Qemu-devel] [PULL 21/27] nbd: Refactor conversion to errno to silence checkpatch Paolo Bonzini
2016-10-31 14:37 ` [Qemu-devel] [PULL 22/27] nbd: Improve server handling of shutdown requests Paolo Bonzini
2016-10-31 18:05   ` Eric Blake
2016-10-31 14:37 ` [Qemu-devel] [PULL 23/27] nbd: Implement NBD_CMD_WRITE_ZEROES on server Paolo Bonzini
2016-10-31 14:37 ` [Qemu-devel] [PULL 24/27] nbd: Implement NBD_CMD_WRITE_ZEROES on client Paolo Bonzini
2016-11-15 22:59   ` Eric Blake
2016-10-31 14:37 ` [Qemu-devel] [PULL 25/27] qemu-char: do not forward events through the mux until QEMU has started Paolo Bonzini
2016-10-31 14:37 ` [Qemu-devel] [PULL 26/27] slirp: fix CharDriver breakage Paolo Bonzini
2016-10-31 14:37 ` [Qemu-devel] [PULL 27/27] x86: add AVX512_4VNNIW and AVX512_4FMAPS features Paolo Bonzini
2016-10-31 16:21 ` [Qemu-devel] [PULL 00/27] Misc patches for 2016-10-31 Peter Maydell
2016-10-31 17:18   ` Alex Bennée
2016-10-31 17:20     ` Peter Maydell
2016-10-31 17:57       ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1477924663-30950-18-git-send-email-pbonzini@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.