[Libguestfs] [libnbd PATCH 7/7] states: Capture NBD_REP_ERR message

[Eric Blake]

Similar to the earlier patch for structured reply errors, it's worth
logging (at least through debug) any error messages that the server
sends alongside NBD_REP_ERR.  And given our earlier changes, it
doesn't really impact the size of struct nbd_handle.
---
 generator/states-newstyle-opt-go.c            |  5 ++--
 .../states-newstyle-opt-set-meta-context.c    |  6 +++-
 generator/states-newstyle-opt-starttls.c      |  8 ++---
 .../states-newstyle-opt-structured-reply.c    |  6 +++-
 generator/states-newstyle.c                   | 29 +++++++++++++++++++
 lib/internal.h                                |  1 +
 6 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/generator/states-newstyle-opt-go.c b/generator/states-newstyle-opt-go.c
index 3458f09..91a85ef 100644
--- a/generator/states-newstyle-opt-go.c
+++ b/generator/states-newstyle-opt-go.c
@@ -147,9 +147,10 @@
     SET_NEXT_STATE (%^OPT_EXPORT_NAME.START);
     return 0;
   default:
+    if (handle_reply_error (h) == 0)
+      set_error (0, "handshake: unknown reply from NBD_OPT_GO: 0x%" PRIx32,
+                 reply);
     SET_NEXT_STATE (%.DEAD);
-    set_error (0, "handshake: unknown reply from NBD_OPT_GO: 0x%" PRIx32,
-               reply);
     return -1;
   }

diff --git a/generator/states-newstyle-opt-set-meta-context.c b/generator/states-newstyle-opt-set-meta-context.c
index 94120e2..a00a411 100644
--- a/generator/states-newstyle-opt-set-meta-context.c
+++ b/generator/states-newstyle-opt-set-meta-context.c
@@ -200,7 +200,11 @@
     break;
   default:
     /* Anything else is an error, ignore it */
-    /* XXX display any error message if NBD_REP_ERR_? */
+    if (handle_reply_error (h) == -1) {
+      SET_NEXT_STATE (%.DEAD);
+      return -1;
+    }
+
     debug (h, "handshake: unexpected error from "
            "NBD_OPT_SET_META_CONTEXT (%" PRIu32 ")", reply);
     SET_NEXT_STATE (%^OPT_GO.START);
diff --git a/generator/states-newstyle-opt-starttls.c b/generator/states-newstyle-opt-starttls.c
index e994ffd..61f254f 100644
--- a/generator/states-newstyle-opt-starttls.c
+++ b/generator/states-newstyle-opt-starttls.c
@@ -83,9 +83,10 @@
     return 0;

   default:
-    if (!NBD_REP_IS_ERR (reply))
-      debug (h,
-             "server is confused by NBD_OPT_STARTTLS, continuing anyway");
+    if (handle_reply_error (h) == -1) {
+      SET_NEXT_STATE (%.DEAD);
+      return -1;
+    }

     /* Server refused to upgrade to TLS.  If h->tls is not require (2)
      * then we can continue unencrypted.
@@ -100,7 +101,6 @@
     debug (h,
            "server refused TLS (%s), continuing with unencrypted connection",
            reply == NBD_REP_ERR_POLICY ? "policy" : "not supported");
-    /* XXX: capture instead of skip server's payload to NBD_REP_ERR*? */
     SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START);
     return 0;
   }
diff --git a/generator/states-newstyle-opt-structured-reply.c b/generator/states-newstyle-opt-structured-reply.c
index d755411..65d5958 100644
--- a/generator/states-newstyle-opt-structured-reply.c
+++ b/generator/states-newstyle-opt-structured-reply.c
@@ -67,7 +67,11 @@
     h->structured_replies = true;
     break;
   default:
-    /* XXX: capture instead of skip server's payload to NBD_REP_ERR*? */
+    if (handle_reply_error (h) == -1) {
+      SET_NEXT_STATE (%.DEAD);
+      return -1;
+    }
+
     debug (h, "structured replies are not supported by this server");
     h->structured_replies = false;
     break;
diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c
index 369df0f..8a4eec6 100644
--- a/generator/states-newstyle.c
+++ b/generator/states-newstyle.c
@@ -77,6 +77,35 @@ prepare_for_reply_payload (struct nbd_handle *h, uint32_t opt)
   return 0;
 }

+/* Check an unexpected server reply. If it is an error, log any
+ * message from the server and return 0; otherwise, return -1.
+ */
+static int
+handle_reply_error (struct nbd_handle *h)
+{
+  uint32_t len;
+  uint32_t reply;
+
+  len = be32toh (h->sbuf.or.option_reply.replylen);
+  reply = be32toh (h->sbuf.or.option_reply.reply);
+  if (!NBD_REP_IS_ERR (reply)) {
+    set_error (0, "handshake: unexpected option reply type %d", reply);
+    return -1;
+  }
+
+  assert (NBD_MAX_STRING < sizeof h->sbuf.or.payload);
+  if (len > NBD_MAX_STRING) {
+    set_error (0, "handshake: option error string too long");
+    return -1;
+  }
+
+  if (len > 0)
+    debug (h, "handshake: server error message: %.*s", (int) len,
+           h->sbuf.or.payload.err_msg);
+
+  return 0;
+}
+
 /* State machine for parsing the fixed newstyle handshake. */

 /* STATE MACHINE */ {
diff --git a/lib/internal.h b/lib/internal.h
index 5f19279..9acc1b4 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -127,6 +127,7 @@ struct nbd_handle {
           struct nbd_fixed_new_option_reply_meta_context context;
           char str[NBD_MAX_STRING];
         }  __attribute__((packed)) context;
+        char err_msg[NBD_MAX_STRING];
       } payload;
     }  __attribute__((packed)) or;
     struct nbd_export_name_option_reply export_name_reply;
-- 
2.20.1