Libguestfs - Jul 2020 - [RFC nbdkit PATCH 4/4] sh, eval: Add .list_exports support

This takes Rich's API proposal and starts fleshing it out with enough
meat that I was able to test 'nbdkit eval' advertising multiple
exports with descriptions paired with 'qemu-nbd --list'.

Eric Blake (3):
  server: Add exports list functions
  server: Prepare to use export list from plugin
  sh, eval: Add .list_exports support

Richard W.M. Jones (1):
  server: Implement list_exports.

 docs/nbdkit-plugin.pod               |  57 +++++++++-
 docs/nbdkit-protocol.pod             |   7 +-
 plugins/eval/nbdkit-eval-plugin.pod  |   2 +
 plugins/sh/nbdkit-sh-plugin.pod      |  31 ++++++
 include/nbdkit-common.h              |   4 +
 include/nbdkit-filter.h              |  12 +++
 include/nbdkit-plugin.h              |   3 +
 server/Makefile.am                   |   2 +
 tests/Makefile.am                    |   2 +
 server/internal.h                    |   6 ++
 common/utils/cleanup.h               |   3 +
 server/backend.c                     |  22 ++++
 server/exports.c                     | 150 +++++++++++++++++++++++++++
 server/filters.c                     |   9 ++
 server/nbdkit.syms                   |   5 +
 server/plugins.c                     |  15 +++
 server/protocol-handshake-newstyle.c |  80 ++++++++------
 server/protocol-handshake.c          |  24 +++++
 common/utils/cleanup-nbdkit.c        |   6 ++
 plugins/sh/methods.h                 |   4 +-
 plugins/eval/eval.c                  |   2 +
 plugins/sh/methods.c                 |  91 ++++++++++++++++
 plugins/sh/sh.c                      |   1 +
 plugins/sh/example.sh                |   8 ++
 tests/test-eval-exports.sh           |  65 ++++++++++++
 25 files changed, 570 insertions(+), 41 deletions(-)
 create mode 100644 server/exports.c
 create mode 100755 tests/test-eval-exports.sh

-- 
2.28.0

Add exports.c for manipulating an exports list, borrowing heavily from
similar code in managing an extents list.  The new functions are
exposed through nbdkit-filter.h, because filters will eventually need
a way to grab the export list from a plugin, but actually wiring that
up will be in a later patch.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 include/nbdkit-common.h       |   4 +
 include/nbdkit-filter.h       |  12 +++
 server/Makefile.am            |   2 +
 common/utils/cleanup.h        |   3 +
 server/exports.c              | 150 ++++++++++++++++++++++++++++++++++
 server/nbdkit.syms            |   5 ++
 common/utils/cleanup-nbdkit.c |   6 ++
 7 files changed, 182 insertions(+)
 create mode 100644 server/exports.c

diff --git a/include/nbdkit-common.h b/include/nbdkit-common.h
index 671cd4a4..d38b37d2 100644
--- a/include/nbdkit-common.h
+++ b/include/nbdkit-common.h
@@ -117,6 +117,10 @@ struct nbdkit_extents;
 extern int nbdkit_add_extent (struct nbdkit_extents *,
                               uint64_t offset, uint64_t length, uint32_t type);

+struct nbdkit_exports;
+extern int nbdkit_add_export (struct nbdkit_exports *,
+                              const char *name, const char *description);
+
 /* A static non-NULL pointer which can be used when you don't need a
  * per-connection handle.
  */
diff --git a/include/nbdkit-filter.h b/include/nbdkit-filter.h
index cec12db7..01ff3dce 100644
--- a/include/nbdkit-filter.h
+++ b/include/nbdkit-filter.h
@@ -123,6 +123,18 @@ extern int nbdkit_extents_aligned (struct nbdkit_next_ops
*next_ops,
                                    uint32_t flags, uint32_t align,
                                    struct nbdkit_extents *extents, int *err);

+/* Export functions. */
+struct nbdkit_export {
+  char *name;
+  char *description;
+};
+
+extern struct nbdkit_exports *nbdkit_exports_new (int default_only);
+extern void nbdkit_exports_free (struct nbdkit_exports *);
+extern size_t nbdkit_exports_count (const struct nbdkit_exports *);
+extern const struct nbdkit_export nbdkit_get_export (const struct
nbdkit_exports *,
+                                                     size_t);
+
 /* Filter struct. */
 struct nbdkit_filter {
   /* Do not set these fields directly; use NBDKIT_REGISTER_FILTER.
diff --git a/server/Makefile.am b/server/Makefile.am
index 4c789934..58b22341 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -43,6 +43,7 @@ nbdkit_SOURCES = \
 	crypto.c \
 	debug.c \
 	debug-flags.c \
+	exports.c \
 	extents.c \
 	filters.c \
 	internal.h \
@@ -139,6 +140,7 @@ check_PROGRAMS = test-public
 test_public_SOURCES = \
 	test-public.c \
 	public.c \
+	exports.c \
 	extents.c \
 	$(NULL)
 test_public_CPPFLAGS = \
diff --git a/common/utils/cleanup.h b/common/utils/cleanup.h
index bcb65f7b..6b59556b 100644
--- a/common/utils/cleanup.h
+++ b/common/utils/cleanup.h
@@ -76,5 +76,8 @@ extern void cleanup_rwlock_unlock (pthread_rwlock_t **ptr);
 struct nbdkit_extents;
 extern void cleanup_extents_free (struct nbdkit_extents **ptr);
 #define CLEANUP_EXTENTS_FREE __attribute__((cleanup (cleanup_extents_free)))
+struct nbdkit_exports;
+extern void cleanup_exports_free (struct nbdkit_exports **ptr);
+#define CLEANUP_EXPORTS_FREE __attribute__((cleanup (cleanup_exports_free)))

 #endif /* NBDKIT_CLEANUP_H */
diff --git a/server/exports.c b/server/exports.c
new file mode 100644
index 00000000..75c3fb58
--- /dev/null
+++ b/server/exports.c
@@ -0,0 +1,150 @@
+/* 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.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <string.h>
+#include <inttypes.h>
+#include <errno.h>
+#include <assert.h>
+
+#include "cleanup.h"
+#include "isaligned.h"
+#include "minmax.h"
+#include "rounding.h"
+#include "vector.h"
+
+#include "internal.h"
+
+/* Cap nr_exports to avoid sending over-large replies to the client,
+ * and to avoid a plugin with large list consuming too much memory.
+ */
+#define MAX_EXPORTS 10000
+
+/* Appendable list of exports. */
+DEFINE_VECTOR_TYPE(exports, struct nbdkit_export);
+
+struct nbdkit_exports {
+  exports exports;
+
+  bool default_only;
+};
+
+struct nbdkit_exports *
+nbdkit_exports_new (int default_only)
+{
+  struct nbdkit_exports *r;
+
+  r = malloc (sizeof *r);
+  if (r == NULL) {
+    nbdkit_error ("nbdkit_exports_new: malloc: %m");
+    return NULL;
+  }
+  r->exports = (exports) empty_vector;
+  r->default_only = default_only != 0;
+  return r;
+}
+
+static void
+nbdkit_export_clear (struct nbdkit_export exp)
+{
+  free (exp.name);
+  free (exp.description);
+}
+
+void
+nbdkit_exports_free (struct nbdkit_exports *exps)
+{
+  if (exps) {
+    exports_iter (&exps->exports, nbdkit_export_clear);
+    free (exps->exports.ptr);
+    free (exps);
+  }
+}
+
+size_t
+nbdkit_exports_count (const struct nbdkit_exports *exps)
+{
+  return exps->exports.size;
+}
+
+const struct nbdkit_export
+nbdkit_get_export (const struct nbdkit_exports *exps, size_t i)
+{
+  assert (i < exps->exports.size);
+  return exps->exports.ptr[i];
+}
+
+int
+nbdkit_add_export (struct nbdkit_exports *exps,
+                   const char *name, const char *description)
+{
+  struct nbdkit_export e = { NULL, NULL };
+
+  if (exps->default_only && exps->exports.size == 1)
+    return 0;
+
+  if (exps->exports.size == MAX_EXPORTS) {
+    nbdkit_error ("nbdkit_add_export: too many exports");
+    errno = EINVAL;
+    return -1;
+  }
+
+  e.name = strdup (name);
+  if (e.name == NULL) {
+    nbdkit_error ("nbdkit_add_export: strdup: %m");
+    return -1;
+  }
+  if (description) {
+    e.description = strdup (description);
+    if (e.description == NULL) {
+      nbdkit_error ("nbdkit_add_export: strdup: %m");
+      free (e.name);
+      errno = ENOMEM;
+      return -1;
+    }
+  }
+
+  if (exports_append (&exps->exports, e) == -1) {
+    nbdkit_error ("nbdkit_add_export: realloc: %m");
+    free (e.name);
+    free (e.description);
+    errno = ENOMEM;
+    return -1;
+  }
+
+  return 0;
+}
diff --git a/server/nbdkit.syms b/server/nbdkit.syms
index d62ad484..6cc6ed32 100644
--- a/server/nbdkit.syms
+++ b/server/nbdkit.syms
@@ -39,10 +39,15 @@
   # The functions we want plugins and filters to call.
   global:
     nbdkit_absolute_path;
+    nbdkit_add_export;
     nbdkit_add_extent;
     nbdkit_debug;
     nbdkit_error;
     nbdkit_export_name;
+    nbdkit_exports_count;
+    nbdkit_exports_free;
+    nbdkit_exports_new;
+    nbdkit_get_export;
     nbdkit_extents_aligned;
     nbdkit_extents_count;
     nbdkit_extents_free;
diff --git a/common/utils/cleanup-nbdkit.c b/common/utils/cleanup-nbdkit.c
index aaaf14a0..e7553c73 100644
--- a/common/utils/cleanup-nbdkit.c
+++ b/common/utils/cleanup-nbdkit.c
@@ -43,3 +43,9 @@ cleanup_extents_free (struct nbdkit_extents **ptr)
 {
   nbdkit_extents_free (*ptr);
 }
+
+void
+cleanup_exports_free (struct nbdkit_exports **ptr)
+{
+  nbdkit_exports_free (*ptr);
+}
-- 
2.28.0

Wire up everything in the server to query the export list from the
plugin.  Actual patches to let plugins and filters return a
non-default list will come later, so for now, the behavior is
unchanged: NBD_OPT_LIST still lists just "", and a client requesting
"" still lets the plugin see an export name of "".

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 server/internal.h                    |  6 +++
 server/backend.c                     | 22 ++++++++
 server/filters.c                     |  9 ++++
 server/plugins.c                     |  9 ++++
 server/protocol-handshake-newstyle.c | 80 ++++++++++++++++------------
 server/protocol-handshake.c          | 24 +++++++++
 6 files changed, 116 insertions(+), 34 deletions(-)

diff --git a/server/internal.h b/server/internal.h
index 1acbbb06..32388d07 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -361,6 +361,8 @@ struct backend {
   void (*get_ready) (struct backend *);
   void (*after_fork) (struct backend *);
   int (*preconnect) (struct backend *, int readonly);
+  int (*list_exports) (struct backend *, int readonly, int default_only,
+                       struct nbdkit_exports *exports);
   void *(*open) (struct backend *, int readonly, const char *exportname);
   int (*prepare) (struct backend *, void *handle, int readonly);
   int (*finalize) (struct backend *, void *handle);
@@ -405,6 +407,10 @@ extern void backend_load (struct backend *b, const char
*name,
 extern void backend_unload (struct backend *b, void (*unload) (void))
   __attribute__((__nonnull__ (1)));

+extern int backend_list_exports (struct backend *b, int readonly,
+                                 int default_only,
+                                 struct nbdkit_exports *exports)
+  __attribute__((__nonnull__ (1, 4)));
 /* exportname is only valid for this call and almost certainly will be
  * freed on return of this function, so backends must save the
  * exportname if they need to refer to it later.
diff --git a/server/backend.c b/server/backend.c
index d39fdeaf..315bd8d2 100644
--- a/server/backend.c
+++ b/server/backend.c
@@ -151,6 +151,28 @@ backend_unload (struct backend *b, void (*unload) (void))
   free (b->name);
 }

+int
+backend_list_exports (struct backend *b, int readonly, int default_only,
+                      struct nbdkit_exports *exports)
+{
+  GET_CONN;
+  struct handle *h = get_handle (conn, b->i);
+  int r;
+
+  controlpath_debug ("%s: list_exports readonly=%d default_only=%d",
+                     b->name, readonly, default_only);
+
+  assert (h->handle == NULL);
+  assert ((h->state & HANDLE_OPEN) == 0);
+  r = b->list_exports (b, readonly, default_only, exports);
+  if (r == -1)
+    controlpath_debug ("%s: list_exports failed", b->name);
+  else
+    controlpath_debug ("%s: list_exports returned %zu names",
b->name,
+                       nbdkit_exports_count (exports));
+  return r;
+}
+
 int
 backend_open (struct backend *b, int readonly, const char *exportname)
 {
diff --git a/server/filters.c b/server/filters.c
index 7d268096..e5b5b860 100644
--- a/server/filters.c
+++ b/server/filters.c
@@ -237,6 +237,14 @@ plugin_magic_config_key (struct backend *b)
   return b->next->magic_config_key (b->next);
 }

+static int
+filter_list_exports (struct backend *b, int readonly, int default_only,
+                     struct nbdkit_exports *exports)
+{
+  /* XXX No filter override yet... */
+  return backend_list_exports (b->next, readonly, default_only, exports);
+}
+
 static void *
 filter_open (struct backend *b, int readonly, const char *exportname)
 {
@@ -540,6 +548,7 @@ static struct backend filter_functions = {
   .get_ready = filter_get_ready,
   .after_fork = filter_after_fork,
   .preconnect = filter_preconnect,
+  .list_exports = filter_list_exports,
   .open = filter_open,
   .prepare = filter_prepare,
   .finalize = filter_finalize,
diff --git a/server/plugins.c b/server/plugins.c
index 8449b1d9..8020046b 100644
--- a/server/plugins.c
+++ b/server/plugins.c
@@ -277,6 +277,14 @@ plugin_preconnect (struct backend *b, int readonly)
   return p->plugin.preconnect (readonly);
 }

+static int
+plugin_list_exports (struct backend *b, int readonly, int default_only,
+                     struct nbdkit_exports *exports)
+{
+  /* XXX No plugin support yet, so for now just advertise "" */
+  return nbdkit_add_export (exports, "", NULL);
+}
+
 static void *
 plugin_open (struct backend *b, int readonly, const char *exportname)
 {
@@ -730,6 +738,7 @@ static struct backend plugin_functions = {
   .get_ready = plugin_get_ready,
   .after_fork = plugin_after_fork,
   .preconnect = plugin_preconnect,
+  .list_exports = plugin_list_exports,
   .open = plugin_open,
   .prepare = plugin_prepare,
   .finalize = plugin_finalize,
diff --git a/server/protocol-handshake-newstyle.c
b/server/protocol-handshake-newstyle.c
index 4025a645..8f41c7a8 100644
--- a/server/protocol-handshake-newstyle.c
+++ b/server/protocol-handshake-newstyle.c
@@ -75,44 +75,59 @@ send_newstyle_option_reply (uint32_t option, uint32_t reply)
   return 0;
 }

-/* Reply to NBD_OPT_LIST with a single empty export name.
- * TODO: Ask the plugin for the list of exports.
+/* Reply to NBD_OPT_LIST with the plugin's list of export names.
  */
 static int
-send_newstyle_option_reply_exportname (uint32_t option, uint32_t reply)
+send_newstyle_option_reply_exportnames (uint32_t option)
 {
   GET_CONN;
   struct nbd_fixed_new_option_reply fixed_new_option_reply;
-  const size_t name_len = 0;    /* length of export name */
-  uint32_t len;
+  size_t i;
+  CLEANUP_EXPORTS_FREE struct nbdkit_exports *exps = NULL;

-  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 (name_len + sizeof (len));
+  exps = nbdkit_exports_new (false);
+  if (exps == NULL)
+    return send_newstyle_option_reply (option, NBD_REP_ERR_TOO_BIG);
+  if (backend_list_exports (top, read_only, false, exps) == -1)
+    return send_newstyle_option_reply (option, NBD_REP_ERR_PLATFORM);

-  if (conn->send (&fixed_new_option_reply,
-                  sizeof fixed_new_option_reply, SEND_MORE) == -1) {
-    nbdkit_error ("write: %s: %m", name_of_nbd_opt (option));
-    return -1;
-  }
+  for (i = 0; i < nbdkit_exports_count (exps); i++) {
+    const struct nbdkit_export export = nbdkit_get_export (exps, i);
+    size_t name_len = strlen (export.name);
+    size_t desc_len = export.description ? strlen (export.description) : 0;
+    uint32_t len;

-  len = htobe32 (name_len);
-  if (conn->send (&len, sizeof len, SEND_MORE) == -1) {
-    nbdkit_error ("write: %s: %s: %m",
-                  name_of_nbd_opt (option), "sending length");
-    return -1;
-  }
-#if 0
-  /* If we were sending a non-"" export name, this is what we'd
use. */
-  if (conn->send (exportname, name_len, 0) == -1) {
-    nbdkit_error ("write: %s: %s: %m",
-                  name_of_nbd_opt (option), "sending export name");
-    return -1;
+    fixed_new_option_reply.magic = htobe64 (NBD_REP_MAGIC);
+    fixed_new_option_reply.option = htobe32 (option);
+    fixed_new_option_reply.reply = htobe32 (NBD_REP_SERVER);
+    fixed_new_option_reply.replylen = htobe32 (name_len + sizeof (len) +
+                                               desc_len);
+
+    if (conn->send (&fixed_new_option_reply,
+                    sizeof fixed_new_option_reply, SEND_MORE) == -1) {
+      nbdkit_error ("write: %s: %m", name_of_nbd_opt (option));
+      return -1;
+    }
+
+    len = htobe32 (name_len);
+    if (conn->send (&len, sizeof len, SEND_MORE) == -1) {
+      nbdkit_error ("write: %s: %s: %m",
+                    name_of_nbd_opt (option), "sending length");
+      return -1;
+    }
+    if (conn->send (export.name, name_len, SEND_MORE) == -1) {
+      nbdkit_error ("write: %s: %s: %m",
+                    name_of_nbd_opt (option), "sending export name");
+      return -1;
+    }
+    if (conn->send (export.description, desc_len, 0) == -1) {
+      nbdkit_error ("write: %s: %s: %m",
+                    name_of_nbd_opt (option), "sending export
description");
+      return -1;
+    }
   }
-#endif

-  return 0;
+  return send_newstyle_option_reply (option, NBD_REP_ACK);
 }

 static int
@@ -384,13 +399,10 @@ negotiate_handshake_newstyle_options (void)
         continue;
       }

-      /* Send back the exportname. */
-      debug ("newstyle negotiation: %s: advertising export
\"\"",
+      /* Send back the exportname list. */
+      debug ("newstyle negotiation: %s: advertising exports",
              name_of_nbd_opt (option));
-      if (send_newstyle_option_reply_exportname (option, NBD_REP_SERVER) == -1)
-        return -1;
-
-      if (send_newstyle_option_reply (option, NBD_REP_ACK) == -1)
+      if (send_newstyle_option_reply_exportnames (option) == -1)
         return -1;
       break;

diff --git a/server/protocol-handshake.c b/server/protocol-handshake.c
index 80233713..afc954d2 100644
--- a/server/protocol-handshake.c
+++ b/server/protocol-handshake.c
@@ -59,6 +59,8 @@ protocol_handshake ()
 }

 /* Common code used by oldstyle and newstyle protocols to:
+ *
+ * - canonicalize default export name of ""
  *
  * - call the backend .open method
  *
@@ -79,6 +81,28 @@ protocol_common_open (uint64_t *exportsize, uint16_t *flags,
   int64_t size;
   uint16_t eflags = NBD_FLAG_HAS_FLAGS;
   int fl;
+  CLEANUP_FREE char *default_export = NULL;
+
+  if (!*exportname) {
+    /* Map client's request for the default export ("") to
plugin's
+     * canonical name.  This is only a best-effort attempt, other
+     * than detection of malloc failure.
+     */
+    CLEANUP_EXPORTS_FREE struct nbdkit_exports *exps = NULL;
+
+    exps = nbdkit_exports_new (true);
+    if (exps == NULL)
+      return -1;
+    if (backend_list_exports (top, read_only, true, exps) == 0 &&
+        nbdkit_exports_count (exps)) {
+      default_export = strdup (nbdkit_get_export (exps, 0).name);
+      if (default_export == NULL) {
+        nbdkit_error ("strdup: %m");
+        return -1;
+      }
+      exportname = default_export;
+    }
+  }

   if (backend_open (top, read_only, exportname) == -1)
     return -1;
-- 
2.28.0

From: "Richard W.M. Jones" <rjones@redhat.com>

See also:
https://www.redhat.com/archives/libguestfs/2020-July/msg00090.html
Message-Id: <20200722124201.1823468-2-rjones@redhat.com>

For now, this implementation is only for plugins; adding filter
support will come later (and probably requires further edits, for the
ability for any filter to open up an independent connection to the
plugin without being handed next_ops on entry).
---
 docs/nbdkit-plugin.pod   | 57 +++++++++++++++++++++++++++++++++++++---
 docs/nbdkit-protocol.pod |  7 +++--
 include/nbdkit-plugin.h  |  3 +++
 server/plugins.c         | 10 +++++--
 4 files changed, 69 insertions(+), 8 deletions(-)

diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
index f8e9962a..7ce89e9e 100644
--- a/docs/nbdkit-plugin.pod
+++ b/docs/nbdkit-plugin.pod
@@ -152,6 +152,9 @@ the plugin:
  │ preconnect │                  │
  └──────┬─────┘                  │
  ┌──────┴─────┐                  │
+ │list_exports│                  │
+ └──────┬─────┘                  │
+ ┌──────┴─────┐                  │
  │ open       │                  │
  └──────┬─────┘                  │
  ┌──────┴─────┐  NBD option      │
@@ -160,10 +163,10 @@ the plugin:
  ┌──────┴─────┐           ┌──────┴─────┐ client #2
  │ get_size   │           │ preconnect │
  └──────┬─────┘           └──────┬─────┘
- ┌──────┴─────┐ data      ┌──────┴─────┐
- │ pread      │ serving   │ open       │
- └──────┬─────┘↺          └──────┬─────┘
- ┌──────┴─────┐                 ...
+ ┌──────┴─────┐ data
+ │ pread      │ serving
+ └──────┬─────┘↺                ...
+ ┌──────┴─────┐
  │ pwrite     │
  └──────┬─────┘↺          ┌──────┴─────┐
  ┌──────┴─────┐           │ close      │
@@ -236,6 +239,12 @@ L<getpid(2)>.
 Called when a TCP connection has been made to the server.  This
 happens early, before NBD or TLS negotiation.

+=item C<.list_exports>
+
+Early in option negotiation the client may try to list the exports
+served by the plugin, and plugins can optionally implement this
+callback to answer the client.  See L</EXPORT NAME> below.
+
 =item C<.open>

 A new client has connected and finished the NBD handshake.  TLS
@@ -652,6 +661,46 @@ Returning C<0> will allow the connection to continue.
If there is an
 error or you want to deny the connection, call C<nbdkit_error> with an
 error message and return C<-1>.

+=head2 C<.list_exports>
+
+ int list_exports (int readonly, int default_only,
+                   struct nbdkit_exports *exports);
+
+This optional callback is called if the client tries to list the
+exports served by the plugin (using C<NBD_OPT_LIST>).  If the plugin
+does not supply this callback then a single export called C<"">
is
+returned.  The NBD protocol defines C<""> as the default
export, so
+this is suitable for plugins which ignore the export name and always
+serve the same content.  See also L</EXPORT NAME> below.
+
+The C<exports> parameter is an opaque object for collecting the list
+of exports.  Call C<nbdkit_add_export> to add a single export to the
+list.  If the plugin has a concept of a default export (usually but
+not always called C<"">) then it should return that first in
the list.
+
+ int nbdkit_add_export (struct nbdkit_export *exports,
+                        const char *name, const char *description);
+
+The C<name> must be a non-NULL, UTF-8 string between 0 and 4096 bytes
+in length.  Export names must be unique.  C<description> is an
+optional description of the export which some clients can display but
+which is otherwise unused (if you don't want a description, you can
+pass this parameter as C<NULL>).  The string(s) are copied into the
+exports list so you may free them immediately after calling this
+function.
+
+If the C<default_only> flag then the client is querying for the name
+of the default export, and the plugin may add only a single export to
+the returned list (the default export name, usually C<"">). 
The
+plugin can ignore this flag and return all exports if it wants.
+
+The C<readonly> flag informs the plugin that the server was started
+with the I<-r> flag on the command line.
+
+Returning C<0> will send the list of exports back to the client.  If
+there is an error, C<.list_exports> should call C<nbdkit_error>
with
+an error message and return C<-1>.
+
 =head2 C<.open>

  void *open (int readonly);
diff --git a/docs/nbdkit-protocol.pod b/docs/nbdkit-protocol.pod
index 8fe8c67e..b1e182c1 100644
--- a/docs/nbdkit-protocol.pod
+++ b/docs/nbdkit-protocol.pod
@@ -99,12 +99,15 @@ Supported in nbdkit E<ge> 1.1.12, and the default in
nbdkit E<ge> 1.3.
 =item export names

 Partially supported in nbdkit E<ge> 1.1.12.  Support for plugins to
-read the client export name added in nbdkit E<ge> 1.15.2.
+read the client export name added in nbdkit E<ge> 1.15.2.  Support for
+C<NBD_OPT_LIST> was added in nbdkit E<ge> 1.21.20.

 Versions of nbdkit before 1.16 could advertise a single export name to
 clients, via a now deprecated side effect of the I<-e> option.  In nbdkit
 1.15.2, plugins could read the client requested export name using
-C<nbdkit_export_name()> and serve different content.
+C<nbdkit_export_name()> and serve different content.  In nbdkit
+1.21.20, plugins could implement C<.list_exports> to answer
+C<NBD_OPT_LIST> queries.

 =item C<NBD_FLAG_NO_ZEROES>

diff --git a/include/nbdkit-plugin.h b/include/nbdkit-plugin.h
index c6c1256c..86b8565d 100644
--- a/include/nbdkit-plugin.h
+++ b/include/nbdkit-plugin.h
@@ -139,6 +139,9 @@ struct nbdkit_plugin {

   int (*get_ready) (void);
   int (*after_fork) (void);
+
+  int (*list_exports) (int readonly, int default_only,
+                       struct nbdkit_exports *exports);
 };

 extern void nbdkit_set_error (int err);
diff --git a/server/plugins.c b/server/plugins.c
index 8020046b..d4364cd2 100644
--- a/server/plugins.c
+++ b/server/plugins.c
@@ -161,6 +161,7 @@ plugin_dump_fields (struct backend *b)
   HAS (get_ready);
   HAS (after_fork);
   HAS (preconnect);
+  HAS (list_exports);
   HAS (open);
   HAS (close);
   HAS (get_size);
@@ -281,8 +282,13 @@ static int
 plugin_list_exports (struct backend *b, int readonly, int default_only,
                      struct nbdkit_exports *exports)
 {
-  /* XXX No plugin support yet, so for now just advertise "" */
-  return nbdkit_add_export (exports, "", NULL);
+  GET_CONN;
+  struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
+
+  if (!p->plugin.list_exports)
+    return nbdkit_add_export (exports, "", NULL);
+
+  return p->plugin.list_exports (readonly, default_only, exports);
 }

 static void *
-- 
2.28.0

Exposing .list_exports through to shell scripts makes testing export
listing a lot more feasible.  The design chosen here is amenable to
'ls -1' or 'find' output provided there are no newlines or
whitespace
in the files being listed (the user gives up descriptions in that
case), while also being flexible enough to support names or
descriptions with arbitrary content.  One idea that might be worth
considering for future patches is parsing a magic header line
describing how the rest of the input would be parsed, where the first
line being unrecognized uses a default format, but other magic words
can introduce alternative formats that might be easier to produce
within scripts.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

RFC because the test is incomplete (in part because of libnbd
bugs that I discovered which need fixing first).

 plugins/eval/nbdkit-eval-plugin.pod |  2 +
 plugins/sh/nbdkit-sh-plugin.pod     | 31 ++++++++++
 tests/Makefile.am                   |  2 +
 plugins/sh/methods.h                |  4 +-
 plugins/eval/eval.c                 |  2 +
 plugins/sh/methods.c                | 91 +++++++++++++++++++++++++++++
 plugins/sh/sh.c                     |  1 +
 plugins/sh/example.sh               |  8 +++
 tests/test-eval-exports.sh          | 65 +++++++++++++++++++++
 9 files changed, 205 insertions(+), 1 deletion(-)
 create mode 100755 tests/test-eval-exports.sh

diff --git a/plugins/eval/nbdkit-eval-plugin.pod
b/plugins/eval/nbdkit-eval-plugin.pod
index 7e25a01f..7126c6de 100644
--- a/plugins/eval/nbdkit-eval-plugin.pod
+++ b/plugins/eval/nbdkit-eval-plugin.pod
@@ -108,6 +108,8 @@ features):

 =item B<is_rotational=>SCRIPT

+=item B<list_exports=>SCRIPT
+
 =item B<open=>SCRIPT

 =item B<pread=>SCRIPT
diff --git a/plugins/sh/nbdkit-sh-plugin.pod b/plugins/sh/nbdkit-sh-plugin.pod
index 771c6bc0..303e96a7 100644
--- a/plugins/sh/nbdkit-sh-plugin.pod
+++ b/plugins/sh/nbdkit-sh-plugin.pod
@@ -266,6 +266,37 @@ with status C<1>; unrecognized output is ignored.

  /path/to/script preconnect <readonly> <exportname>

+=item C<list_exports>
+
+ /path/to/script list_exports <readonly> <default_only>
+
+The C<readonly> parameter will be C<true> or C<false>.  The
+C<default_only> parameter will be C<true> if the caller is only
+interested in the canonical name of the default export, or C<false> to
+get a full list of export names; the script may safely ignore this
+parameter and always provide a full list if desired.
+
+This must print, one per line on stdout, a list of zero or more export
+declarations in the format:
+
+ name [description...]
+
+which correspond to the inputs of the C C<nbdkit_add_export> function
+(see L<nbdkit-plugin(3)>).  Within the line, the sequence C<\n> is
an
+escape for a literal newline, and all other uses of backslash escape
+the next character.  The C<name> field is mandatory; if it begins with
+a single quote C<'>, it ends on the first matching unescaped single
+quote (with the quotes not part of the name), otherwise the name ends
+at the first unescaped whitespace.  Any text remaining on the line
+after C<name> is treated as the C<description>, with unescaped
leading
+and trailing whitespace stripped.  Note that if none of the files in a
+directory contain backslash, single quote, or whitespace in their
+names, then the output of C<ls> will list names without descriptions.
+
+This method is I<not> required; if it is absent, the list of exports
+advertised by nbdkit will be the single export with the empty string
+as a name and no description.
+
 =item C<open>

  /path/to/script open <readonly> <exportname>
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 129e0647..e3c42219 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -614,10 +614,12 @@ test_data_LDADD = libtest.la $(LIBGUESTFS_LIBS)
 TESTS += \
 	test-eval.sh \
 	test-eval-file.sh \
+	test-eval-exports.sh \
 	$(NULL)
 EXTRA_DIST += \
 	test-eval.sh \
 	test-eval-file.sh \
+	test-eval-exports.sh \
 	$(NULL)

 # file plugin test.
diff --git a/plugins/sh/methods.h b/plugins/sh/methods.h
index 08a5ed17..69017fa4 100644
--- a/plugins/sh/methods.h
+++ b/plugins/sh/methods.h
@@ -1,5 +1,5 @@
 /* nbdkit
- * Copyright (C) 2018 Red Hat Inc.
+ * Copyright (C) 2018-2020 Red Hat Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
@@ -44,6 +44,8 @@ extern int sh_thread_model (void);
 extern int sh_get_ready (void);
 extern int sh_after_fork (void);
 extern int sh_preconnect (int readonly);
+extern int sh_list_exports (int readonly, int default_only,
+                            struct nbdkit_exports *exports);
 extern void *sh_open (int readonly);
 extern void sh_close (void *handle);
 extern int64_t sh_get_size (void *handle);
diff --git a/plugins/eval/eval.c b/plugins/eval/eval.c
index 54c5029e..2bd5e79f 100644
--- a/plugins/eval/eval.c
+++ b/plugins/eval/eval.c
@@ -74,6 +74,7 @@ static const char *known_methods[] = {
   "get_ready",
   "get_size",
   "is_rotational",
+  "list_exports",
   "missing",
   "open",
   "pread",
@@ -393,6 +394,7 @@ static struct nbdkit_plugin plugin = {
   .after_fork        = sh_after_fork,

   .preconnect        = sh_preconnect,
+  .list_exports      = sh_list_exports,
   .open              = sh_open,
   .close             = sh_close,

diff --git a/plugins/sh/methods.c b/plugins/sh/methods.c
index 8257103e..a4422238 100644
--- a/plugins/sh/methods.c
+++ b/plugins/sh/methods.c
@@ -225,6 +225,97 @@ struct sh_handle {
   int can_zero;
 };

+static int
+parse_exports (const char *script,
+               const char *s, size_t slen, struct nbdkit_exports *exports)
+{
+  FILE *fp = NULL;
+  CLEANUP_FREE char *line = NULL;
+  size_t linelen = 0;
+  ssize_t len;
+  int ret = -1;
+
+  fp = fmemopen ((void *) s, slen, "r");
+  if (!fp) {
+    nbdkit_error ("%s: fmemopen: %m", script);
+    goto out;
+  }
+
+  while ((len = getline (&line, &linelen, fp)) != -1) {
+    bool quoted = *line == '\'';
+    char *p = line, *q = p + quoted, *desc;
+
+    /* Modify line in-place with our parse of name */
+    while (q - line < len && (quoted ? *q != '\'' :
!ascii_isspace (*q))) {
+      if (*q == '\\') {
+        q++;
+        if (*q == 'n')
+          *q = '\n';
+      }
+      *p++ = *q++;
+    }
+    /* Transition from name to description */
+    *p++ = '\0';
+    q++;
+    while (ascii_isspace (*q))
+      q++;
+    p = desc = q;
+    while (q - line < len) {
+      if (*q == '\\') {
+        q++;
+        if (*q == 'n')
+          *q = '\n';
+      }
+      *p++ = *q++;
+    }
+    while (p > desc && ascii_isspace (p[-1]))
+      p--;
+    *p = '\0';
+
+    nbdkit_debug ("%s: adding export '%s' '%s'",
script,  line, desc);
+    if (nbdkit_add_export (exports, line, desc) == -1)
+      goto out;
+  }
+
+  ret = 0;
+
+ out:
+  if (fp)
+    fclose (fp);
+  return ret;
+}
+
+int
+sh_list_exports (int readonly, int default_only,
+                 struct nbdkit_exports *exports)
+{
+  const char *method = "list_exports";
+  const char *script = get_script (method);
+  const char *args[] = { script, method, readonly ? "true" :
"false",
+                         default_only ? "true" : "false",
NULL };
+  CLEANUP_FREE char *s = NULL;
+  size_t slen;
+
+  switch (call_read (&s, &slen, args)) {
+  case OK:
+    return parse_exports (script, s, slen, exports);
+
+  case MISSING:
+    return nbdkit_add_export (exports, "", NULL);
+
+  case ERROR:
+    return -1;
+
+  case RET_FALSE:
+    nbdkit_error ("%s: %s method returned unexpected code (3/false)",
+                  script, method);
+    errno = EIO;
+    return -1;
+
+  default: abort ();
+  }
+}
+
 void *
 sh_open (int readonly)
 {
diff --git a/plugins/sh/sh.c b/plugins/sh/sh.c
index 9e484823..374888a4 100644
--- a/plugins/sh/sh.c
+++ b/plugins/sh/sh.c
@@ -300,6 +300,7 @@ static struct nbdkit_plugin plugin = {
   .after_fork        = sh_after_fork,

   .preconnect        = sh_preconnect,
+  .list_exports      = sh_list_exports,
   .open              = sh_open,
   .close             = sh_close,

diff --git a/plugins/sh/example.sh b/plugins/sh/example.sh
index 99e4e890..a6550055 100755
--- a/plugins/sh/example.sh
+++ b/plugins/sh/example.sh
@@ -85,6 +85,14 @@ case "$1" in
         echo parallel
         ;;

+    list_exports)
+	# The following lists the names of all files in the current
+	# directory that do not contain whitespace, backslash, or single
+	# quotes.  No description accompanies the export names.
+	# The first file listed is used when a client requests export ''.
+	find . -type f \! -name "*['\\\\[:space:]]*"
+	;;
+
     open)
         # Open a new client connection.

diff --git a/tests/test-eval-exports.sh b/tests/test-eval-exports.sh
new file mode 100755
index 00000000..e9f99565
--- /dev/null
+++ b/tests/test-eval-exports.sh
@@ -0,0 +1,65 @@
+#!/usr/bin/env bash
+# nbdkit
+# Copyright (C) 2018-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.
+
+# This is an example from the nbdkit-eval-plugin(1) manual page.
+# Check here that it doesn't regress.
+
+source ./functions.sh
+set -e
+set -x
+
+requires nbdsh -c 'print (h.get_list_export_description)'
+requires nbdinfo --help
+requires jq --version
+
+files="eval-exports.list eval-exports.out"
+rm -f $files
+cleanup_fn rm -f $files
+
+# Control case: no list_exports
+nbdkit -U - -v eval get_size='echo 0' \
+       --run 'nbdinfo --list --json "$uri"' >
eval-exports.out
+cat eval-exports.out
+diff -u <(jq -c '[.exports[] | [."export-name",
.description]]' \
+	     eval-exports.out) - <<EOF
+[["",null]]
+EOF
+
+# Empty exports list produces 0 exports
+: >eval-exports.list
+nbdkit -U - -v eval list_exports='cat eval-exports.list'
get_size='echo 0' \
+       --run 'nbdinfo --list --json "$uri"' >
eval-exports.out
+cat eval-exports.out
+diff -u <(jq -c '[.exports[] | [."export-name",
.description]]' \
+	     eval-exports.out) - <<EOF
+[]
+EOF
-- 
2.28.0

Patches 1-3 all look quite reasonable to me.

As I think we discussed on IRC after this was posted(?) it might be
better to consider the sh plugin printing something like:

  NAMES
  name1
  name2
  ...

or perhaps

  NAMES <number-of-names>
  name1
  name2
  ...

with an alternate NAMES+DESCRIPTIONS format which interleaves each
name and description.

I wouldn't bother with parsing anything more complicated than line by
line (which means you can't have a name containing \n, but who cares).
And the format above would be extensible in future.

Weird characters in file names make ‘ls -1’ problematic, although I
don't think it's actually a security problem, so perhaps most scripts
can ignore the problem.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

On 7/31/20 5:22 PM, Eric Blake wrote:
> From: "Richard W.M. Jones" <rjones@redhat.com>
> 
> See also:
> https://www.redhat.com/archives/libguestfs/2020-July/msg00090.html
> Message-Id: <20200722124201.1823468-2-rjones@redhat.com>
> 
> For now, this implementation is only for plugins; adding filter
> support will come later (and probably requires further edits, for the
> ability for any filter to open up an independent connection to the
> plugin without being handed next_ops on entry).
> ---
> @@ -652,6 +661,46 @@ Returning C<0> will allow the connection to
continue.  If there is an
>   error or you want to deny the connection, call C<nbdkit_error> with
an
>   error message and return C<-1>.
> 
> +=head2 C<.list_exports>
> +
> + int list_exports (int readonly, int default_only,
> +                   struct nbdkit_exports *exports);
I'm trying to figure out a situation where knowing whether we are 
running under 'nbdkit -r' (and thus all exports will be read-only) will 
affect the list advertised by the plugin.  But then again, we _do_ pass 
the readonly flag to both .preconnect and .open, which are bookends 
around this call.  That said, now's the time to decide whether to keep 
it or to simplify the interface by dropping it, before we bake in a 
stable release with the API.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org