[Libguestfs] [nbdkit PATCH v2 3/8] server: Respond to NBD_INFO_NAME request

Eric Blake
2020-08-27T02:16:47Z

The NBD spec says that additional information in response to
NBD_OPT_GO or NBD_OPT_INFO, such as NBD_INFO_NAME, is optional, but
also recommends that if a client hints about wanting a particular
piece of information, that it is better for the server to reply to
those known hints.  This is also the the only way for a client to
learn the canonical name that is used when connecting to the default
export name.

libnbd 1.4 is adding new APIs to request the client hint, and to track
what the server replies with, making this easy enough to test.
However, at present, the ondemand plugin is the only one to advertise
an alternate name for the default export, so the test will be further
enhanced when .default_export is plumbed through the sh plugin.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 tests/Makefile.am                    |   4 +-
 server/protocol-handshake-newstyle.c |  59 +++++++++++++-
 tests/test-export-info.sh            | 112 +++++++++++++++++++++++++++
 3 files changed, 169 insertions(+), 6 deletions(-)
 create mode 100755 tests/test-export-info.sh

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 2db56ded..29c4811b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -491,8 +491,8 @@ TESTS += test-eflags.sh
 EXTRA_DIST += test-eflags.sh

 # Test export name.
-TESTS += test-export-name.sh
-EXTRA_DIST += test-export-name.sh
+TESTS += test-export-name.sh test-export-info.sh
+EXTRA_DIST += test-export-name.sh test-export-info.sh

 # cdi plugin test.
 TESTS += test-cdi.sh
diff --git a/server/protocol-handshake-newstyle.c
b/server/protocol-handshake-newstyle.c
index 6e5f3822..1777219f 100644
--- a/server/protocol-handshake-newstyle.c
+++ b/server/protocol-handshake-newstyle.c
@@ -156,6 +156,36 @@ send_newstyle_option_reply_info_export (uint32_t option,
uint32_t reply,
   return 0;
 }

+/* Can be used for NBD_INFO_NAME and NBD_INFO_DESCRIPTION. */
+static int
+send_newstyle_option_reply_info_str (uint32_t option, uint32_t reply,
+                                     uint16_t info, const char *str,
+                                     size_t len)
+{
+  GET_CONN;
+  struct nbd_fixed_new_option_reply fixed_new_option_reply;
+  struct nbd_fixed_new_option_reply_info_name_or_desc name;
+
+  if (len == -1)
+    len = strlen (str);
+
+  fixed_new_option_reply.magic = htobe64 (NBD_REP_MAGIC);
+  fixed_new_option_reply.option = htobe32 (option);
+  fixed_new_option_reply.reply = htobe32 (reply);
+  fixed_new_option_reply.replylen = htobe32 (sizeof info + len);
+  name.info = htobe16 (info);
+
+  if (conn->send (&fixed_new_option_reply,
+                  sizeof fixed_new_option_reply, SEND_MORE) == -1 ||
+      conn->send (&name, sizeof name, SEND_MORE) == -1 ||
+      conn->send (str, len, 0) == -1) {
+    nbdkit_error ("write: %s: %m", name_of_nbd_opt (option));
+    return -1;
+  }
+
+  return 0;
+}
+
 static int
 send_newstyle_option_reply_meta_context (uint32_t option, uint32_t reply,
                                          uint32_t context_id,
@@ -533,16 +563,37 @@ negotiate_handshake_newstyle_options (void)
                                                     exportsize) == -1)
           return -1;

-        /* For now we ignore all other info requests (but we must
-         * ignore NBD_INFO_EXPORT if it was requested, because we
-         * replied already above).  Therefore this loop doesn't do
-         * much at the moment.
+        /* For now we send NBD_INFO_NAME if requested, and ignore all
+         * other info requests (including NBD_INFO_EXPORT if it was
+         * requested, because we replied already above).
+         * XXX NBD_INFO_DESCRIPTION is easy once we add .export_description.
          */
         for (i = 0; i < nrinfos; ++i) {
           memcpy (&info, &data[4 + exportnamelen + 2 + i*2], 2);
           info = be16toh (info);
           switch (info) {
           case NBD_INFO_EXPORT: /* ignore - reply sent above */ break;
+          case NBD_INFO_NAME:
+            {
+              const char *name = &data[4];
+              size_t namelen = exportnamelen;
+
+              if (exportnamelen == 0) {
+                name = backend_default_export (top, read_only);
+                if (!name) {
+                  debug ("newstyle negotiation: %s: "
+                         "NBD_INFO_NAME: no name to send", optname);
+                  break;
+                }
+                namelen = -1;
+              }
+              if (send_newstyle_option_reply_info_str (option,
+                                                       NBD_REP_INFO,
+                                                       NBD_INFO_NAME,
+                                                       name, namelen) == -1)
+                return -1;
+            }
+            break;
           default:
             debug ("newstyle negotiation: %s: "
                    "ignoring NBD_INFO_* request %u (%s)",
diff --git a/tests/test-export-info.sh b/tests/test-export-info.sh
new file mode 100755
index 00000000..b0dd4ce8
--- /dev/null
+++ b/tests/test-export-info.sh
@@ -0,0 +1,112 @@
+#!/usr/bin/env bash
+# nbdkit
+# Copyright (C) 2019-2020 Red Hat Inc.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution.
+#
+# * Neither the name of Red Hat nor the names of its contributors may be
+# used to endorse or promote products derived from this software without
+# specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+
+source ./functions.sh
+set -e
+set -x
+
+requires_plugin sh
+requires nbdsh -c 'print (h.set_full_info)'
+
+export sock=`mktemp -u`
+files="$sock export-info.pid"
+rm -f $files
+cleanup_fn rm -f $files
+
+# Create an nbdkit sh plugin for checking NBD_INFO replies to NBD_OPT_GO.
+# XXX Update when .default_export and .export_description are implemented in sh
+start_nbdkit -P export-info.pid -U $sock \
+             sh - <<'EOF'
+case "$1" in
+    default_export) echo hello ;;
+    open) echo "$3" ;;
+    export_description) echo "$2 world" ;;
+    get_size) echo 0 ;;
+    *) exit 2 ;;
+esac
+EOF
+
+# Without client request, nothing is advertised
+nbdsh -c '
+import os
+
+h.set_opt_mode (True)
+h.connect_unix (os.environ["sock"])
+
+h.set_export_name ("a")
+h.opt_info ()
+try:
+  h.get_canonical_export_name ()
+  assert False
+except nbd.Error as ex:
+  pass
+
+h.set_export_name ("")
+h.opt_info ()
+try:
+  h.get_canonical_export_name ()
+  assert False
+except nbd.Error as ex:
+  pass
+
+h.opt_go ()
+try:
+  h.get_canonical_export_name ()
+  assert False
+except nbd.Error as ex:
+  pass
+
+h.shutdown ()
+'
+
+# With client request, reflect the export name back
+nbdsh -c '
+import os
+
+h.set_opt_mode (True)
+h.set_full_info (True)
+h.connect_unix (os.environ["sock"])
+
+h.set_export_name ("a")
+h.opt_info ()
+assert h.get_canonical_export_name () == "a"
+
+h.set_export_name ("")
+h.opt_info ()
+# XXX Adjust once default export works
+assert h.get_canonical_export_name () == ""
+
+h.opt_go ()
+assert h.get_canonical_export_name () == ""
+
+h.shutdown ()
+'
--
2.28.0