Libguestfs - Sep 2020 - [libnbd PATCH 2/3] generator: Rename OPT_SET_META_CONTEXT states

I'm posting this now, as I'm at the end of a workday and I got things
working for manual experimentation.

Still to do:
- write interop tests for qemu-nbd and nbdkit (including my proposed
patch addition of qemu-nbd -A to show qemu:allocation-depth)
- figure out if we can make 'nbdinfo --map' use the new API to
automatically select all contexts advertised by the server

Eric Blake (3):
  api: Add get_nr_meta_contexts, clear_meta_contexts
  generator: Rename OPT_SET_META_CONTEXT states
  api: Add nbd_opt_list_meta_context

 lib/internal.h                                |   1 +
 generator/API.ml                              | 162 +++++++++++++++++-
 generator/Makefile.am                         |   4 +-
 generator/state_machine.ml                    |   6 +-
 ...t.c => states-newstyle-opt-meta-context.c} | 106 ++++++++----
 .../states-newstyle-opt-structured-reply.c    |   4 +-
 generator/states-newstyle.c                   |   5 +-
 lib/handle.c                                  |  32 ++++
 lib/opt.c                                     |  73 ++++++++
 tests/meta-base-allocation.c                  |  36 +++-
 TODO                                          |  11 ++
 11 files changed, 394 insertions(+), 46 deletions(-)
 rename generator/{states-newstyle-opt-set-meta-context.c =>
states-newstyle-opt-meta-context.c} (66%)

-- 
2.28.0

An upcoming commit will add the ability to query which meta contexts a
server is advertising; but first we need to be able to present a
different list of meta context requests in response to what we learn
from the server.  Add APIs for reviewing which requests have already
been registered, as well as a way to reset the list of requests.

This adds some testsuite coverage; and more will appear with the
addition of nbd_opt_list_meta_contexts in later patches.
---
 generator/API.ml             | 78 ++++++++++++++++++++++++++++++++++--
 lib/handle.c                 | 32 +++++++++++++++
 tests/meta-base-allocation.c | 36 +++++++++++++++--
 3 files changed, 139 insertions(+), 7 deletions(-)

diff --git a/generator/API.ml b/generator/API.ml
index 0a876c4..938ace4 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -974,7 +974,10 @@ this is whether blocks of data are allocated, zero or
sparse).

 This call adds one metadata context to the list to be negotiated.
 You can call it as many times as needed.  The list is initially
-empty when the handle is created.
+empty when the handle is created; you can check the contents of
+the list with L<nbd_get_nr_meta_contexts(3)> and
+L<nbd_get_meta_context(3)>, or clear it with
+L<nbd_clear_meta_contexts(3)>.

 The NBD protocol limits meta context names to 4096 bytes, but
 servers may not support the full length.  The encoding of meta
@@ -991,9 +994,73 @@ C<LIBNBD_CONTEXT_> for some well-known contexts, but
you are free
 to pass in other contexts.

 Other metadata contexts are server-specific, but include
-C<\"qemu:dirty-bitmap:...\"> for qemu-nbd
-(see qemu-nbd I<-B> option).";
-    see_also = [Link "block_status"];
+C<\"qemu:dirty-bitmap:...\"> and
C<\"qemu:allocation-depth\"> for
+qemu-nbd (see qemu-nbd I<-B> and I<-A> options).";
+    see_also = [Link "block_status"; Link
"can_meta_context";
+                Link "get_nr_meta_contexts"; Link
"get_meta_context";
+                Link "clear_meta_contexts"];
+  };
+
+  "get_nr_meta_contexts", {
+    default_call with
+    args = []; ret = RInt;
+    shortdesc = "return the current number of requested meta
contexts";
+    longdesc = "\
+During connection libnbd can negotiate zero or more metadata
+contexts with the server.  Metadata contexts are features (such
+as C<\"base:allocation\">) which describe information returned
+by the L<nbd_block_status(3)> command (for
C<\"base:allocation\">
+this is whether blocks of data are allocated, zero or sparse).
+
+This command returns how many meta contexts have been added to
+the list to request from the server via L<nbd_add_meta_context(3)>.
+The server is not obligated to honor all of the requests; to see
+what it actually supports, see L<nbd_can_meta_context(3)>.";
+    see_also = [Link "block_status"; Link
"can_meta_context";
+                Link "add_meta_context"; Link
"get_meta_context";
+                Link "clear_meta_contexts"];
+  };
+
+  "get_meta_context", {
+    default_call with
+    args = [ Int "i" ]; ret = RString;
+    shortdesc = "return the i'th meta context request";
+    longdesc = "\
+During connection libnbd can negotiate zero or more metadata
+contexts with the server.  Metadata contexts are features (such
+as C<\"base:allocation\">) which describe information returned
+by the L<nbd_block_status(3)> command (for
C<\"base:allocation\">
+this is whether blocks of data are allocated, zero or sparse).
+
+This command returns the i'th meta context request, as added by
+L<nbd_add_meta_context(3)>, and bounded by
+L<nbd_get_nr_meta_contexts(3)>.";
+    see_also = [Link "block_status"; Link
"can_meta_context";
+                Link "add_meta_context"; Link
"get_nr_meta_contexts";
+                Link "clear_meta_contexts"];
+  };
+
+  "clear_meta_contexts", {
+    default_call with
+    args = []; ret = RErr;
+    permitted_states = [ Created; Negotiating ];
+    shortdesc = "reset the list of requested meta contexts";
+    longdesc = "\
+During connection libnbd can negotiate zero or more metadata
+contexts with the server.  Metadata contexts are features (such
+as C<\"base:allocation\">) which describe information returned
+by the L<nbd_block_status(3)> command (for
C<\"base:allocation\">
+this is whether blocks of data are allocated, zero or sparse).
+
+This command resets the list of meta contexts to request back to
+an empty list, for re-population by further use of
+L<nbd_add_meta_context(3)>.  It is primarily useful when option
+negotiation mode is selected (see L<nbd_set_opt_mode(3)>), for
+altering the list of attempted contexts between subsequent export
+queries.";
+    see_also = [Link "block_status"; Link
"can_meta_context";
+                Link "add_meta_context"; Link
"get_nr_meta_contexts";
+                Link "get_meta_context"; Link
"set_opt_mode"];
   };

   "set_uri_allow_transports", {
@@ -2814,6 +2881,9 @@ let first_version = [
   (* Added in 1.5.x development cycle, will be stable and supported in 1.6. *)
   "set_strict_mode", (1, 6);
   "get_strict_mode", (1, 6);
+  "get_nr_meta_contexts", (1, 6);
+  "get_meta_context", (1, 6);
+  "clear_meta_contexts", (1, 6);

   (* These calls are proposed for a future version of libnbd, but
    * have not been added to any released version so far.
diff --git a/lib/handle.c b/lib/handle.c
index a6b2172..e0047b7 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -313,6 +313,38 @@ nbd_unlocked_add_meta_context (struct nbd_handle *h, const
char *name)
   return 0;
 }

+int
+nbd_unlocked_get_nr_meta_contexts (struct nbd_handle *h)
+{
+  return nbd_internal_string_list_length (h->request_meta_contexts);
+}
+
+char *
+nbd_unlocked_get_meta_context (struct nbd_handle *h, int i)
+{
+  size_t len = nbd_internal_string_list_length (h->request_meta_contexts);
+  char *ret;
+
+  if (i < 0 || i >= len) {
+    set_error (EINVAL, "meta context request out of range");
+    return NULL;
+  }
+
+  ret = strdup (h->request_meta_contexts[i]);
+  if (ret == NULL)
+    set_error (errno, "strdup");
+
+  return ret;
+}
+
+int
+nbd_unlocked_clear_meta_contexts (struct nbd_handle *h)
+{
+  nbd_internal_free_string_list (h->request_meta_contexts);
+  h->request_meta_contexts = NULL;
+  return 0;
+}
+
 int
 nbd_unlocked_set_request_structured_replies (struct nbd_handle *h,
                                              bool request)
diff --git a/tests/meta-base-allocation.c b/tests/meta-base-allocation.c
index 3de4c34..5ed57d8 100644
--- a/tests/meta-base-allocation.c
+++ b/tests/meta-base-allocation.c
@@ -1,5 +1,5 @@
 /* NBD client library in userspace
- * Copyright (C) 2013-2019 Red Hat Inc.
+ * Copyright (C) 2013-2020 Red Hat Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -30,6 +30,8 @@

 #include <libnbd.h>

+#define BOGUS_CONTEXT "x-libnbd:nosuch"
+
 static int check_extent (void *data,
                          const char *metacontext,
                          uint64_t offset,
@@ -58,6 +60,18 @@ main (int argc, char *argv[])
     exit (EXIT_FAILURE);
   }

+  /* No contexts requested by default */
+  if ((r = nbd_get_nr_meta_contexts (nbd)) != 0) {
+    fprintf (stderr, "unexpected number of contexts: %d\n", r);
+    exit (EXIT_FAILURE);
+  }
+
+  /* Clearing an empty list is not fatal */
+  if (nbd_clear_meta_contexts (nbd) != 0) {
+    fprintf (stderr, "unable to clear requested contexts\n");
+    exit (EXIT_FAILURE);
+  }
+
   /* Negotiate metadata context "base:allocation" with the server.
    * This is supported in nbdkit >= 1.12.
    */
@@ -69,11 +83,27 @@ main (int argc, char *argv[])
   /* Also request negotiation of a bogus context, which should not
    * fail here nor affect block status later.
    */
-  if (nbd_add_meta_context (nbd, "x-libnbd:nosuch") == -1) {
+  if (nbd_add_meta_context (nbd, BOGUS_CONTEXT) == -1) {
     fprintf (stderr, "%s\n", nbd_get_error ());
     exit (EXIT_FAILURE);
   }

+  /* Test that we can read back what we have requested */
+  if ((r = nbd_get_nr_meta_contexts (nbd)) != 2) {
+    fprintf (stderr, "unexpected number of contexts: %d\n", r);
+    exit (EXIT_FAILURE);
+  }
+  s = nbd_get_meta_context (nbd, 1);
+  if (s == NULL) {
+    fprintf (stderr, "unable to read back requested context 1: %s\n",
+             nbd_get_error ());
+    exit (EXIT_FAILURE);
+  }
+  if (strcmp (s, BOGUS_CONTEXT) != 0) {
+    fprintf (stderr, "read back wrong context: %s\n", s);
+    exit (EXIT_FAILURE);
+  }
+
   if (nbd_connect_command (nbd, args) == -1) {
     fprintf (stderr, "%s\n", nbd_get_error ());
     exit (EXIT_FAILURE);
@@ -92,7 +122,7 @@ main (int argc, char *argv[])
     exit (EXIT_FAILURE);
   }

-  switch (nbd_can_meta_context (nbd, "x-libnbd:nosuch")) {
+  switch (nbd_can_meta_context (nbd, BOGUS_CONTEXT)) {
   case -1:
     fprintf (stderr, "%s\n", nbd_get_error ());
     exit (EXIT_FAILURE);
-- 
2.28.0

Drop the SET designation, so that we can reuse these states for
OPT_LIST_META_CONTEXT in the next patch.
---
 generator/Makefile.am                         |  4 +--
 generator/state_machine.ml                    |  6 ++---
 ...t.c => states-newstyle-opt-meta-context.c} | 26 +++++++++----------
 .../states-newstyle-opt-structured-reply.c    |  4 +--
 generator/states-newstyle.c                   |  2 +-
 5 files changed, 21 insertions(+), 21 deletions(-)
 rename generator/{states-newstyle-opt-set-meta-context.c =>
states-newstyle-opt-meta-context.c} (90%)

diff --git a/generator/Makefile.am b/generator/Makefile.am
index d64a953..61f27f4 100644
--- a/generator/Makefile.am
+++ b/generator/Makefile.am
@@ -1,5 +1,5 @@
 # nbd client library in userspace
-# Copyright (C) 2013-2019 Red Hat Inc.
+# Copyright (C) 2013-2020 Red Hat Inc.
 #
 # This library is free software; you can redistribute it and/or
 # modify it under the terms of the GNU Lesser General Public
@@ -32,7 +32,7 @@ states_code = \
 	states-newstyle-opt-export-name.c \
 	states-newstyle-opt-list.c \
 	states-newstyle-opt-go.c \
-	states-newstyle-opt-set-meta-context.c \
+	states-newstyle-opt-meta-context.c \
 	states-newstyle-opt-starttls.c \
 	states-newstyle-opt-structured-reply.c \
 	states-newstyle.c \
diff --git a/generator/state_machine.ml b/generator/state_machine.ml
index c1fb073..10b6983 100644
--- a/generator/state_machine.ml
+++ b/generator/state_machine.ml
@@ -283,7 +283,7 @@ and newstyle_state_machine = [
    *)
   Group ("OPT_STARTTLS", newstyle_opt_starttls_state_machine);
   Group ("OPT_STRUCTURED_REPLY",
newstyle_opt_structured_reply_state_machine);
-  Group ("OPT_SET_META_CONTEXT",
newstyle_opt_set_meta_context_state_machine);
+  Group ("OPT_META_CONTEXT",
newstyle_opt_meta_context_state_machine);
   Group ("OPT_GO", newstyle_opt_go_state_machine);
   Group ("OPT_EXPORT_NAME", newstyle_opt_export_name_state_machine);

@@ -453,8 +453,8 @@ and newstyle_opt_structured_reply_state_machine = [
   };
 ]

-(* Fixed newstyle NBD_OPT_SET_META_CONTEXT option. *)
-and newstyle_opt_set_meta_context_state_machine = [
+(* Fixed newstyle NBD_OPT_SET/LIST_META_CONTEXT option. *)
+and newstyle_opt_meta_context_state_machine = [
   State {
     default_state with
     name = "START";
diff --git a/generator/states-newstyle-opt-set-meta-context.c
b/generator/states-newstyle-opt-meta-context.c
similarity index 90%
rename from generator/states-newstyle-opt-set-meta-context.c
rename to generator/states-newstyle-opt-meta-context.c
index e2541fa..0dc48af 100644
--- a/generator/states-newstyle-opt-set-meta-context.c
+++ b/generator/states-newstyle-opt-meta-context.c
@@ -16,10 +16,10 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */

-/* State machine for negotiating NBD_OPT_SET_META_CONTEXT. */
+/* State machine for negotiating NBD_OPT_SET/LIST_META_CONTEXT. */

 STATE_MACHINE {
- NEWSTYLE.OPT_SET_META_CONTEXT.START:
+ NEWSTYLE.OPT_META_CONTEXT.START:
   size_t i, nr_queries;
   uint32_t len;

@@ -52,7 +52,7 @@ STATE_MACHINE {
   SET_NEXT_STATE (%SEND);
   return 0;

- NEWSTYLE.OPT_SET_META_CONTEXT.SEND:
+ NEWSTYLE.OPT_META_CONTEXT.SEND:
   switch (send_from_wbuf (h)) {
   case -1: SET_NEXT_STATE (%.DEAD); return 0;
   case 0:
@@ -64,7 +64,7 @@ STATE_MACHINE {
   }
   return 0;

- NEWSTYLE.OPT_SET_META_CONTEXT.SEND_EXPORTNAMELEN:
+ NEWSTYLE.OPT_META_CONTEXT.SEND_EXPORTNAMELEN:
   switch (send_from_wbuf (h)) {
   case -1: SET_NEXT_STATE (%.DEAD); return 0;
   case 0:
@@ -75,7 +75,7 @@ STATE_MACHINE {
   }
   return 0;

- NEWSTYLE.OPT_SET_META_CONTEXT.SEND_EXPORTNAME:
+ NEWSTYLE.OPT_META_CONTEXT.SEND_EXPORTNAME:
   switch (send_from_wbuf (h)) {
   case -1: SET_NEXT_STATE (%.DEAD); return 0;
   case 0:
@@ -88,7 +88,7 @@ STATE_MACHINE {
   }
   return 0;

- NEWSTYLE.OPT_SET_META_CONTEXT.SEND_NRQUERIES:
+ NEWSTYLE.OPT_META_CONTEXT.SEND_NRQUERIES:
   switch (send_from_wbuf (h)) {
   case -1: SET_NEXT_STATE (%.DEAD); return 0;
   case 0:
@@ -97,7 +97,7 @@ STATE_MACHINE {
   }
   return 0;

- NEWSTYLE.OPT_SET_META_CONTEXT.PREPARE_NEXT_QUERY:
+ NEWSTYLE.OPT_META_CONTEXT.PREPARE_NEXT_QUERY:
   const char *query = h->request_meta_contexts[h->querynum];

   if (query == NULL) { /* end of list of requested meta contexts */
@@ -112,7 +112,7 @@ STATE_MACHINE {
   SET_NEXT_STATE (%SEND_QUERYLEN);
   return 0;

- NEWSTYLE.OPT_SET_META_CONTEXT.SEND_QUERYLEN:
+ NEWSTYLE.OPT_META_CONTEXT.SEND_QUERYLEN:
   const char *query = h->request_meta_contexts[h->querynum];

   switch (send_from_wbuf (h)) {
@@ -124,7 +124,7 @@ STATE_MACHINE {
   }
   return 0;

- NEWSTYLE.OPT_SET_META_CONTEXT.SEND_QUERY:
+ NEWSTYLE.OPT_META_CONTEXT.SEND_QUERY:
   switch (send_from_wbuf (h)) {
   case -1: SET_NEXT_STATE (%.DEAD); return 0;
   case 0:
@@ -133,13 +133,13 @@ STATE_MACHINE {
   }
   return 0;

- NEWSTYLE.OPT_SET_META_CONTEXT.PREPARE_FOR_REPLY:
+ NEWSTYLE.OPT_META_CONTEXT.PREPARE_FOR_REPLY:
   h->rbuf = &h->sbuf.or.option_reply;
   h->rlen = sizeof h->sbuf.or.option_reply;
   SET_NEXT_STATE (%RECV_REPLY);
   return 0;

- NEWSTYLE.OPT_SET_META_CONTEXT.RECV_REPLY:
+ NEWSTYLE.OPT_META_CONTEXT.RECV_REPLY:
   switch (recv_into_rbuf (h)) {
   case -1: SET_NEXT_STATE (%.DEAD); return 0;
   case 0:
@@ -151,14 +151,14 @@ STATE_MACHINE {
   }
   return 0;

- NEWSTYLE.OPT_SET_META_CONTEXT.RECV_REPLY_PAYLOAD:
+ NEWSTYLE.OPT_META_CONTEXT.RECV_REPLY_PAYLOAD:
   switch (recv_into_rbuf (h)) {
   case -1: SET_NEXT_STATE (%.DEAD); return 0;
   case 0:  SET_NEXT_STATE (%CHECK_REPLY);
   }
   return 0;

- NEWSTYLE.OPT_SET_META_CONTEXT.CHECK_REPLY:
+ NEWSTYLE.OPT_META_CONTEXT.CHECK_REPLY:
   uint32_t reply;
   uint32_t len;
   const size_t maxpayload = sizeof h->sbuf.or.payload.context;
diff --git a/generator/states-newstyle-opt-structured-reply.c
b/generator/states-newstyle-opt-structured-reply.c
index 71a4952..8b689a6 100644
--- a/generator/states-newstyle-opt-structured-reply.c
+++ b/generator/states-newstyle-opt-structured-reply.c
@@ -22,7 +22,7 @@ STATE_MACHINE {
  NEWSTYLE.OPT_STRUCTURED_REPLY.START:
   assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE);
   if (!h->request_sr) {
-    SET_NEXT_STATE (%^OPT_SET_META_CONTEXT.START);
+    SET_NEXT_STATE (%^OPT_META_CONTEXT.START);
     return 0;
   }

@@ -87,7 +87,7 @@ STATE_MACHINE {
   if (h->opt_mode)
     SET_NEXT_STATE (%.NEGOTIATING);
   else
-    SET_NEXT_STATE (%^OPT_SET_META_CONTEXT.START);
+    SET_NEXT_STATE (%^OPT_META_CONTEXT.START);
   return 0;

 } /* END STATE MACHINE */
diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c
index 92cf5c9..a0a5928 100644
--- a/generator/states-newstyle.c
+++ b/generator/states-newstyle.c
@@ -123,7 +123,7 @@ STATE_MACHINE {
       if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0)
         SET_NEXT_STATE (%OPT_EXPORT_NAME.START);
       else
-        SET_NEXT_STATE (%OPT_SET_META_CONTEXT.START);
+        SET_NEXT_STATE (%OPT_META_CONTEXT.START);
       return 0;
     case NBD_OPT_LIST:
       SET_NEXT_STATE (%OPT_LIST.START);
-- 
2.28.0

Right now, we require the user to supply potential metacontext names
in advance, without knowing what the server actually supports until
after the connection or nbd_opt_info call is complete.  But the NBD
protocol also supports a client being able to query what contexts a
server supports, including where a client asks with a prefix and the
server replies back with all answers that match the prefix.

Not to mention this will allow nbdinfo to gain full feature parity
with 'qemu-nbd --list'.
---
 lib/internal.h                               |  1 +
 generator/API.ml                             | 84 +++++++++++++++++++-
 generator/states-newstyle-opt-meta-context.c | 80 +++++++++++++++----
 generator/states-newstyle.c                  |  3 +
 lib/opt.c                                    | 73 +++++++++++++++++
 TODO                                         | 11 +++
 6 files changed, 234 insertions(+), 18 deletions(-)

diff --git a/lib/internal.h b/lib/internal.h
index cde5dcd..ad1eeb9 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -77,6 +77,7 @@ struct command_cb {
     nbd_extent_callback extent;
     nbd_chunk_callback chunk;
     nbd_list_callback list;
+    nbd_context_callback context;
   } fn;
   nbd_completion_callback completion;
 };
diff --git a/generator/API.ml b/generator/API.ml
index 938ace4..358ec38 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -142,8 +142,13 @@ let list_closure = {
   cbname = "list";
   cbargs = [ CBString "name"; CBString "description" ]
 }
+let context_closure = {
+  cbname = "context";
+  cbargs = [ CBString "name" ]
+}
 let all_closures = [ chunk_closure; completion_closure;
-                     debug_closure; extent_closure; list_closure ]
+                     debug_closure; extent_closure; list_closure;
+                     context_closure ]

 (* Enums. *)
 let tls_enum = {
@@ -852,7 +857,7 @@ to end the connection without finishing negotiation.";
     example = Some "examples/list-exports.c";
     see_also = [Link "get_opt_mode"; Link
"aio_is_negotiating";
                 Link "opt_abort"; Link "opt_go"; Link
"opt_list";
-                Link "opt_info"];
+                Link "opt_info"; Link
"opt_list_meta_context"];
   };

   "get_opt_mode", {
@@ -960,6 +965,56 @@ corresponding L<nbd_opt_go(3)> would succeed.";
                 Link "set_export_name"];
   };

+  "opt_list_meta_context", {
+    default_call with
+    args = [ Closure context_closure ]; ret = RInt;
+    permitted_states = [ Negotiating ];
+    shortdesc = "request the server to list available meta contexts";
+    longdesc = "\
+Request that the server list available meta contexts associated with
+the export previously specified by the most recent
+L<nbd_set_export_name(3)> or L<nbd_connect_uri(3)>.  This can only
be
+used if L<nbd_set_opt_mode(3)> enabled option mode.
+
+The NBD protocol allows a client to decide how many queries to ask
+the server.  Rather than taking that list of queries as a parameter
+to this function, libnbd reuses the current list of requested meta
+contexts as set by L<nbd_add_meta_context(3)>; you can use
+L<nbd_clear_meta_contexts(3)> to set up a different list of queries.
+When the list is empty, a server will typically reply with all
+contexts that it supports; when the list is non-empty, the server
+will reply only with supported contexts that match the client's
+request.  Note that a reply by the server might be encoded to
+represent several feasible contexts on one string, rather than an
+actual context name that would actually succeed during
+L<nbd_opt_go(3)>; so it is still necessary to use
+L<nbd_can_meta_context(3)> after connecting to see which contexts
+are actually supported.
+
+The C<context> function is called once per server reply, with any
+C<user_data> passed to this function, and with C<name> supplied by
+the server.  Remember that it is not safe to call
+L<nbd_add_meta_context(3)> from within the context of the
+callback function; rather, your code must copy any C<name> needed for
+later use after this function completes.  At present, the return value
+of the callback is ignored, although a return of -1 should be avoided.
+
+For convenience, when this function succeeds, it returns the number
+of replies returned by the server.
+
+Not all servers understand this request, and even when it is understood,
+the server might intentionally send an empty list because it does not
+support the requested context, or may encounter a failure after
+delivering partial results.  Thus, this function may succeed even when
+no contexts are reported, or may fail but have a non-empty list.  Likewise,
+the NBD protocol does not specify an upper bound for the number of
+replies that might be advertised, so client code should be aware that
+a server may send a lengthy list.";
+    see_also = [Link "set_opt_mode"; Link
"aio_opt_list_meta_context";
+                Link "add_meta_context"; Link
"clear_meta_contexts";
+                Link "opt_go"; Link "set_export_name"];
+  };
+
   "add_meta_context", {
     default_call with
     args = [ String "name" ]; ret = RErr;
@@ -2233,6 +2288,29 @@ callback.";
     see_also = [Link "set_opt_mode"; Link "opt_info"; Link
"is_read_only"];
   };

+  "aio_opt_list_meta_context", {
+    default_call with
+    args = [ Closure context_closure ]; ret = RInt;
+    optargs = [ OClosure completion_closure ];
+    permitted_states = [ Negotiating ];
+    shortdesc = "request the server to list available meta contexts";
+    longdesc = "\
+Request that the server list available meta contexts associated with
+the export previously specified by the most recent
+L<nbd_set_export_name(3)> or L<nbd_connect_uri(3)>.  This can only
be
+used if L<nbd_set_opt_mode(3)> enabled option mode.
+
+To determine when the request completes, wait for
+L<nbd_aio_is_connecting(3)> to return false.  Or supply the optional
+C<completion_callback> which will be invoked as described in
+L<libnbd(3)/Completion callbacks>, except that it is automatically
+retired regardless of return value.  Note that detecting whether the
+server returns an error (as is done by the return value of the
+synchronous counterpart) is only possible with a completion
+callback.";
+    see_also = [Link "set_opt_mode"; Link
"opt_list_meta_context"];
+  };
+
   "aio_pread", {
     default_call with
     args = [ BytesPersistOut ("buf", "count"); UInt64
"offset" ];
@@ -2884,6 +2962,8 @@ let first_version = [
   "get_nr_meta_contexts", (1, 6);
   "get_meta_context", (1, 6);
   "clear_meta_contexts", (1, 6);
+  "opt_list_meta_context", (1, 6);
+  "aio_opt_list_meta_context", (1, 6);

   (* These calls are proposed for a future version of libnbd, but
    * have not been added to any released version so far.
diff --git a/generator/states-newstyle-opt-meta-context.c
b/generator/states-newstyle-opt-meta-context.c
index 0dc48af..ce878ee 100644
--- a/generator/states-newstyle-opt-meta-context.c
+++ b/generator/states-newstyle-opt-meta-context.c
@@ -21,18 +21,28 @@
 STATE_MACHINE {
  NEWSTYLE.OPT_META_CONTEXT.START:
   size_t i, nr_queries;
-  uint32_t len;
+  uint32_t len, opt;

   /* If the server doesn't support SRs then we must skip this group.
    * Also we skip the group if the client didn't request any metadata
-   * contexts.
+   * contexts, when doing SET (but an empty LIST is okay).
    */
   assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE);
   nbd_internal_reset_size_and_flags (h);
-  if (!h->structured_replies ||
-      nbd_internal_string_list_length (h->request_meta_contexts) == 0) {
-    SET_NEXT_STATE (%^OPT_GO.START);
-    return 0;
+  if (h->opt_current == NBD_OPT_LIST_META_CONTEXT) {
+    assert (h->opt_mode);
+    assert (h->structured_replies);
+    assert (CALLBACK_IS_NOT_NULL (h->opt_cb.fn.context));
+    opt = h->opt_current;
+  }
+  else {
+    assert (CALLBACK_IS_NULL (h->opt_cb.fn.context));
+    opt = NBD_OPT_SET_META_CONTEXT;
+    if (!h->structured_replies ||
+        nbd_internal_string_list_length (h->request_meta_contexts) == 0) {
+      SET_NEXT_STATE (%^OPT_GO.START);
+      return 0;
+    }
   }

   assert (h->meta_contexts == NULL);
@@ -44,7 +54,7 @@ STATE_MACHINE {
     len += 4 /* length of query */ + strlen (h->request_meta_contexts[i]);

   h->sbuf.option.version = htobe64 (NBD_NEW_VERSION);
-  h->sbuf.option.option = htobe32 (NBD_OPT_SET_META_CONTEXT);
+  h->sbuf.option.option = htobe32 (opt);
   h->sbuf.option.optlen = htobe32 (len);
   h->wbuf = &h->sbuf;
   h->wlen = sizeof (h->sbuf.option);
@@ -98,7 +108,8 @@ STATE_MACHINE {
   return 0;

  NEWSTYLE.OPT_META_CONTEXT.PREPARE_NEXT_QUERY:
-  const char *query = h->request_meta_contexts[h->querynum];
+  const char *query = !h->request_meta_contexts ? NULL
+    : h->request_meta_contexts[h->querynum];

   if (query == NULL) { /* end of list of requested meta contexts */
     SET_NEXT_STATE (%PREPARE_FOR_REPLY);
@@ -140,10 +151,17 @@ STATE_MACHINE {
   return 0;

  NEWSTYLE.OPT_META_CONTEXT.RECV_REPLY:
+  uint32_t opt;
+
+  if (h->opt_current == NBD_OPT_LIST_META_CONTEXT)
+    opt = h->opt_current;
+  else
+    opt = NBD_OPT_SET_META_CONTEXT;
+
   switch (recv_into_rbuf (h)) {
   case -1: SET_NEXT_STATE (%.DEAD); return 0;
   case 0:
-    if (prepare_for_reply_payload (h, NBD_OPT_SET_META_CONTEXT) == -1) {
+    if (prepare_for_reply_payload (h, opt) == -1) {
       SET_NEXT_STATE (%.DEAD);
       return 0;
     }
@@ -163,12 +181,25 @@ STATE_MACHINE {
   uint32_t len;
   const size_t maxpayload = sizeof h->sbuf.or.payload.context;
   struct meta_context *meta_context;
+  uint32_t opt;
+  int err = 0;
+
+  if (h->opt_current == NBD_OPT_LIST_META_CONTEXT)
+    opt = h->opt_current;
+  else
+    opt = NBD_OPT_SET_META_CONTEXT;

   reply = be32toh (h->sbuf.or.option_reply.reply);
   len = be32toh (h->sbuf.or.option_reply.replylen);
   switch (reply) {
   case NBD_REP_ACK:           /* End of list of replies. */
-    SET_NEXT_STATE (%^OPT_GO.START);
+    if (opt == NBD_OPT_LIST_META_CONTEXT) {
+      SET_NEXT_STATE (%.NEGOTIATING);
+      CALL_CALLBACK (h->opt_cb.completion, &err);
+      nbd_internal_free_option (h);
+    }
+    else
+      SET_NEXT_STATE (%^OPT_GO.START);
     break;
   case NBD_REP_META_CONTEXT:  /* A context. */
     if (len > maxpayload)
@@ -194,21 +225,38 @@ STATE_MACHINE {
       }
       debug (h, "negotiated %s with context ID %" PRIu32,
              meta_context->name, meta_context->context_id);
-      meta_context->next = h->meta_contexts;
-      h->meta_contexts = meta_context;
+      if (opt == NBD_OPT_LIST_META_CONTEXT) {
+        CALL_CALLBACK (h->opt_cb.fn.context, meta_context->name);
+        free (meta_context->name);
+        free (meta_context);
+      }
+      else {
+        meta_context->next = h->meta_contexts;
+        h->meta_contexts = meta_context;
+      }
     }
     SET_NEXT_STATE (%PREPARE_FOR_REPLY);
     break;
   default:
-    /* Anything else is an error, ignore it */
+    /* Anything else is an error, ignore it for SET, report it for LIST */
     if (handle_reply_error (h) == -1) {
       SET_NEXT_STATE (%.DEAD);
       return 0;
     }

-    debug (h, "handshake: unexpected error from "
-           "NBD_OPT_SET_META_CONTEXT (%" PRIu32 ")",
reply);
-    SET_NEXT_STATE (%^OPT_GO.START);
+    if (opt == NBD_OPT_LIST_META_CONTEXT) {
+      err = ENOTSUP;
+      set_error (err, "unexpected response, possibly the server does not
"
+                 "support listing contexts");
+      CALL_CALLBACK (h->opt_cb.completion, &err);
+      nbd_internal_free_option (h);
+      SET_NEXT_STATE (%.NEGOTIATING);
+    }
+    else {
+      debug (h, "handshake: unexpected error from "
+             "NBD_OPT_SET_META_CONTEXT (%" PRIu32 ")",
reply);
+      SET_NEXT_STATE (%^OPT_GO.START);
+    }
     break;
   }
   return 0;
diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c
index a0a5928..6fb7548 100644
--- a/generator/states-newstyle.c
+++ b/generator/states-newstyle.c
@@ -136,6 +136,9 @@ STATE_MACHINE {
       }
       SET_NEXT_STATE (%PREPARE_OPT_ABORT);
       return 0;
+    case NBD_OPT_LIST_META_CONTEXT:
+      SET_NEXT_STATE (%OPT_META_CONTEXT.START);
+      return 0;
     case 0:
       break;
     default:
diff --git a/lib/opt.c b/lib/opt.c
index 6ea8326..2317b72 100644
--- a/lib/opt.c
+++ b/lib/opt.c
@@ -32,6 +32,8 @@ nbd_internal_free_option (struct nbd_handle *h)
 {
   if (h->opt_current == NBD_OPT_LIST)
     FREE_CALLBACK (h->opt_cb.fn.list);
+  else if (h->opt_current == NBD_OPT_LIST_META_CONTEXT)
+    FREE_CALLBACK (h->opt_cb.fn.context);
   FREE_CALLBACK (h->opt_cb.completion);
 }

@@ -166,6 +168,51 @@ nbd_unlocked_opt_list (struct nbd_handle *h,
nbd_list_callback *list)
   return s.count;
 }

+struct context_helper {
+  int count;
+  nbd_context_callback context;
+  int err;
+};
+static int
+context_visitor (void *opaque, const char *name)
+{
+  struct context_helper *h = opaque;
+  if (h->count < INT_MAX)
+    h->count++;
+  CALL_CALLBACK (h->context, name);
+  return 0;
+}
+static int
+context_complete (void *opaque, int *err)
+{
+  struct context_helper *h = opaque;
+  h->err = *err;
+  FREE_CALLBACK (h->context);
+  return 0;
+}
+
+/* Issue NBD_OPT_LIST_META_CONTEXT and wait for the reply. */
+int
+nbd_unlocked_opt_list_meta_context (struct nbd_handle *h,
+                                    nbd_context_callback *context)
+{
+  struct context_helper s = { .context = *context };
+  nbd_context_callback l = { .callback = context_visitor, .user_data = &s
};
+  nbd_completion_callback c = { .callback = context_complete, .user_data =
&s };
+
+  if (nbd_unlocked_aio_opt_list_meta_context (h, &l, &c) == -1)
+    return -1;
+
+  SET_CALLBACK_TO_NULL (*context);
+  if (wait_for_option (h) == -1)
+    return -1;
+  if (s.err) {
+    set_error (s.err, "server replied with error to list meta context
request");
+    return -1;
+  }
+  return s.count;
+}
+
 /* Issue NBD_OPT_GO (or NBD_OPT_EXPORT_NAME) without waiting. */
 int
 nbd_unlocked_aio_opt_go (struct nbd_handle *h,
@@ -230,3 +277,29 @@ nbd_unlocked_aio_opt_list (struct nbd_handle *h,
nbd_list_callback *list,
     debug (h, "option queued, ignoring state machine failure");
   return 0;
 }
+
+/* Issue NBD_OPT_LIST_META_CONTEXT without waiting. */
+int
+nbd_unlocked_aio_opt_list_meta_context (struct nbd_handle *h,
+                                        nbd_context_callback *context,
+                                        nbd_completion_callback *complete)
+{
+  if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) {
+    set_error (ENOTSUP, "server is not using fixed newstyle
protocol");
+    return -1;
+  }
+  if (!h->structured_replies) {
+    set_error (ENOTSUP, "server lacks structured replies");
+    return -1;
+  }
+
+  assert (CALLBACK_IS_NULL (h->opt_cb.fn.context));
+  h->opt_cb.fn.context = *context;
+  SET_CALLBACK_TO_NULL (*context);
+  h->opt_cb.completion = *complete;
+  SET_CALLBACK_TO_NULL (*complete);
+  h->opt_current = NBD_OPT_LIST_META_CONTEXT;
+  if (nbd_internal_run (h, cmd_issue) == -1)
+    debug (h, "option queued, ignoring state machine failure");
+  return 0;
+}
diff --git a/TODO b/TODO
index 4a0cd22..b6c676b 100644
--- a/TODO
+++ b/TODO
@@ -13,6 +13,17 @@ NBD resize extension.

 TLS should properly shut down the session (calling gnutls_bye).

+Potential deadlock with nbd_add_meta_context: if a client sends enough
+requests to the server that it blocks while writing, but the server
+replies to requests as they come in rather than waiting for the end of
+the client request, then the server can likewise block in writing
+replies that libnbd is not yet reading.  Not an issue for existing
+servers that don't have enough contexts to reply with enough data to
+fill buffers, but could be an issue with qemu-nbd if it is taught to
+exports many dirty bitmaps simultaneously.  Revamping the
+states-newstyle-meta-context.c state machine to let libnbd handle
+NBD_REP_META_CONTEXT while still writing queries could be hairy.
+
 Performance: Chart it over various buffer sizes and threads, as that
   should make it easier to identify systematic issues.

-- 
2.28.0

This is not related to the current patch, but generally for libnbd:

(1) Should we be testing interop separately for qemu-storage-daemon
(qsd), or is qsd basically qemu-nbd in a new wrapper so it's not worth
doing it?

(2) I found a bug in the new nbdinfo behaviour:

$ nbdkit -fv file dir=/scratch

(where /scratch is a directory with a lot of files in it)

$ nbdinfo --version
libnbd 1.5.3
$ nbdinfo --list nbd://localhost
[... lots of files shown ...]
nbd_opt_info: recv: Connection reset by peer

nbdkit gives this error:

nbdkit: file[1]: error: client exceeded maximum number of options (32)

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

On Mon, Sep 28, 2020 at 05:05:16PM -0500, Eric Blake
wrote:
> +  "get_meta_context", {
> +    default_call with
> +    args = [ Int "i" ]; ret = RString;
Previously we have only used the Int parameter type for things which
are plausibly integers (timeouts, signal numbers).  This one really
ought to be an unsigned or better still a size_t.  Would it help if I
provided a SizeT type for the generator?

But in general yes this patch is obvious so ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

On Mon, Sep 28, 2020 at 05:05:17PM -0500, Eric Blake
wrote:
> Drop the SET designation, so that we can reuse these states for
> OPT_LIST_META_CONTEXT in the next patch.
I think this is a straight α conversion, so ACK.

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 9/29/20 6:23 AM, Richard W.M. Jones wrote:
> This is not related to the current patch, but generally for libnbd:
> 
> (1) Should we be testing interop separately for qemu-storage-daemon
> (qsd), or is qsd basically qemu-nbd in a new wrapper so it's not worth
> doing it?
> 
> (2) I found a bug in the new nbdinfo behaviour:
> 
> $ nbdkit -fv file dir=/scratch
> 
> (where /scratch is a directory with a lot of files in it)
> 
> $ nbdinfo --version
> libnbd 1.5.3
> $ nbdinfo --list nbd://localhost
> [... lots of files shown ...]
> nbd_opt_info: recv: Connection reset by peer
> 
> nbdkit gives this error:
> 
> nbdkit: file[1]: error: client exceeded maximum number of options (32)
Yeah, we really ought to raise nbdkit's limits when .list_exports 
returns a large list.  We haven't yet released nbdkit 1.24 where the 
file driver actually has a large .list_exports; but it may also be a 
reason to figure out if libnbd should go to the effort of re-connecting 
for the later portion of a list on servers like older nbdkit that limit 
the length of the negotiation phase.

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

On Mon, Sep 28, 2020 at 05:05:18PM -0500, Eric Blake
wrote:
> Right now, we require the user to supply potential metacontext names
> in advance, without knowing what the server actually supports until
> after the connection or nbd_opt_info call is complete.  But the NBD
> protocol also supports a client being able to query what contexts a
> server supports, including where a client asks with a prefix and the
> server replies back with all answers that match the prefix.
> 
> Not to mention this will allow nbdinfo to gain full feature parity
> with 'qemu-nbd --list'.
I think I missed this review, sorry.

This feature looks great.  I think you said in the
cover letter that you wanted to do further work, but I'll
ACK it anyway.

Rich.
>  lib/internal.h                               |  1 +
>  generator/API.ml                             | 84 +++++++++++++++++++-
>  generator/states-newstyle-opt-meta-context.c | 80 +++++++++++++++----
>  generator/states-newstyle.c                  |  3 +
>  lib/opt.c                                    | 73 +++++++++++++++++
>  TODO                                         | 11 +++
>  6 files changed, 234 insertions(+), 18 deletions(-)
> 
> diff --git a/lib/internal.h b/lib/internal.h
> index cde5dcd..ad1eeb9 100644
> --- a/lib/internal.h
> +++ b/lib/internal.h
> @@ -77,6 +77,7 @@ struct command_cb {
>      nbd_extent_callback extent;
>      nbd_chunk_callback chunk;
>      nbd_list_callback list;
> +    nbd_context_callback context;
>    } fn;
>    nbd_completion_callback completion;
>  };
> diff --git a/generator/API.ml b/generator/API.ml
> index 938ace4..358ec38 100644
> --- a/generator/API.ml
> +++ b/generator/API.ml
> @@ -142,8 +142,13 @@ let list_closure = {
>    cbname = "list";
>    cbargs = [ CBString "name"; CBString "description" ]
>  }
> +let context_closure = {
> +  cbname = "context";
> +  cbargs = [ CBString "name" ]
> +}
>  let all_closures = [ chunk_closure; completion_closure;
> -                     debug_closure; extent_closure; list_closure ]
> +                     debug_closure; extent_closure; list_closure;
> +                     context_closure ]
> 
>  (* Enums. *)
>  let tls_enum = {
> @@ -852,7 +857,7 @@ to end the connection without finishing
negotiation.";
>      example = Some "examples/list-exports.c";
>      see_also = [Link "get_opt_mode"; Link
"aio_is_negotiating";
>                  Link "opt_abort"; Link "opt_go"; Link
"opt_list";
> -                Link "opt_info"];
> +                Link "opt_info"; Link
"opt_list_meta_context"];
>    };
> 
>    "get_opt_mode", {
> @@ -960,6 +965,56 @@ corresponding L<nbd_opt_go(3)> would
succeed.";
>                  Link "set_export_name"];
>    };
> 
> +  "opt_list_meta_context", {
> +    default_call with
> +    args = [ Closure context_closure ]; ret = RInt;
> +    permitted_states = [ Negotiating ];
> +    shortdesc = "request the server to list available meta
contexts";
> +    longdesc = "\
> +Request that the server list available meta contexts associated with
> +the export previously specified by the most recent
> +L<nbd_set_export_name(3)> or L<nbd_connect_uri(3)>.  This can
only be
> +used if L<nbd_set_opt_mode(3)> enabled option mode.
> +
> +The NBD protocol allows a client to decide how many queries to ask
> +the server.  Rather than taking that list of queries as a parameter
> +to this function, libnbd reuses the current list of requested meta
> +contexts as set by L<nbd_add_meta_context(3)>; you can use
> +L<nbd_clear_meta_contexts(3)> to set up a different list of queries.
> +When the list is empty, a server will typically reply with all
> +contexts that it supports; when the list is non-empty, the server
> +will reply only with supported contexts that match the client's
> +request.  Note that a reply by the server might be encoded to
> +represent several feasible contexts on one string, rather than an
> +actual context name that would actually succeed during
> +L<nbd_opt_go(3)>; so it is still necessary to use
> +L<nbd_can_meta_context(3)> after connecting to see which contexts
> +are actually supported.
> +
> +The C<context> function is called once per server reply, with any
> +C<user_data> passed to this function, and with C<name>
supplied by
> +the server.  Remember that it is not safe to call
> +L<nbd_add_meta_context(3)> from within the context of the
> +callback function; rather, your code must copy any C<name> needed
for
> +later use after this function completes.  At present, the return value
> +of the callback is ignored, although a return of -1 should be avoided.
> +
> +For convenience, when this function succeeds, it returns the number
> +of replies returned by the server.
> +
> +Not all servers understand this request, and even when it is understood,
> +the server might intentionally send an empty list because it does not
> +support the requested context, or may encounter a failure after
> +delivering partial results.  Thus, this function may succeed even when
> +no contexts are reported, or may fail but have a non-empty list. 
Likewise,
> +the NBD protocol does not specify an upper bound for the number of
> +replies that might be advertised, so client code should be aware that
> +a server may send a lengthy list.";
> +    see_also = [Link "set_opt_mode"; Link
"aio_opt_list_meta_context";
> +                Link "add_meta_context"; Link
"clear_meta_contexts";
> +                Link "opt_go"; Link
"set_export_name"];
> +  };
> +
>    "add_meta_context", {
>      default_call with
>      args = [ String "name" ]; ret = RErr;
> @@ -2233,6 +2288,29 @@ callback.";
>      see_also = [Link "set_opt_mode"; Link "opt_info";
Link "is_read_only"];
>    };
> 
> +  "aio_opt_list_meta_context", {
> +    default_call with
> +    args = [ Closure context_closure ]; ret = RInt;
> +    optargs = [ OClosure completion_closure ];
> +    permitted_states = [ Negotiating ];
> +    shortdesc = "request the server to list available meta
contexts";
> +    longdesc = "\
> +Request that the server list available meta contexts associated with
> +the export previously specified by the most recent
> +L<nbd_set_export_name(3)> or L<nbd_connect_uri(3)>.  This can
only be
> +used if L<nbd_set_opt_mode(3)> enabled option mode.
> +
> +To determine when the request completes, wait for
> +L<nbd_aio_is_connecting(3)> to return false.  Or supply the optional
> +C<completion_callback> which will be invoked as described in
> +L<libnbd(3)/Completion callbacks>, except that it is automatically
> +retired regardless of return value.  Note that detecting whether the
> +server returns an error (as is done by the return value of the
> +synchronous counterpart) is only possible with a completion
> +callback.";
> +    see_also = [Link "set_opt_mode"; Link
"opt_list_meta_context"];
> +  };
> +
>    "aio_pread", {
>      default_call with
>      args = [ BytesPersistOut ("buf", "count"); UInt64
"offset" ];
> @@ -2884,6 +2962,8 @@ let first_version = [
>    "get_nr_meta_contexts", (1, 6);
>    "get_meta_context", (1, 6);
>    "clear_meta_contexts", (1, 6);
> +  "opt_list_meta_context", (1, 6);
> +  "aio_opt_list_meta_context", (1, 6);
> 
>    (* These calls are proposed for a future version of libnbd, but
>     * have not been added to any released version so far.
> diff --git a/generator/states-newstyle-opt-meta-context.c
b/generator/states-newstyle-opt-meta-context.c
> index 0dc48af..ce878ee 100644
> --- a/generator/states-newstyle-opt-meta-context.c
> +++ b/generator/states-newstyle-opt-meta-context.c
> @@ -21,18 +21,28 @@
>  STATE_MACHINE {
>   NEWSTYLE.OPT_META_CONTEXT.START:
>    size_t i, nr_queries;
> -  uint32_t len;
> +  uint32_t len, opt;
> 
>    /* If the server doesn't support SRs then we must skip this group.
>     * Also we skip the group if the client didn't request any metadata
> -   * contexts.
> +   * contexts, when doing SET (but an empty LIST is okay).
>     */
>    assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE);
>    nbd_internal_reset_size_and_flags (h);
> -  if (!h->structured_replies ||
> -      nbd_internal_string_list_length (h->request_meta_contexts) == 0)
{
> -    SET_NEXT_STATE (%^OPT_GO.START);
> -    return 0;
> +  if (h->opt_current == NBD_OPT_LIST_META_CONTEXT) {
> +    assert (h->opt_mode);
> +    assert (h->structured_replies);
> +    assert (CALLBACK_IS_NOT_NULL (h->opt_cb.fn.context));
> +    opt = h->opt_current;
> +  }
> +  else {
> +    assert (CALLBACK_IS_NULL (h->opt_cb.fn.context));
> +    opt = NBD_OPT_SET_META_CONTEXT;
> +    if (!h->structured_replies ||
> +        nbd_internal_string_list_length (h->request_meta_contexts) ==
0) {
> +      SET_NEXT_STATE (%^OPT_GO.START);
> +      return 0;
> +    }
>    }
> 
>    assert (h->meta_contexts == NULL);
> @@ -44,7 +54,7 @@ STATE_MACHINE {
>      len += 4 /* length of query */ + strlen
(h->request_meta_contexts[i]);
> 
>    h->sbuf.option.version = htobe64 (NBD_NEW_VERSION);
> -  h->sbuf.option.option = htobe32 (NBD_OPT_SET_META_CONTEXT);
> +  h->sbuf.option.option = htobe32 (opt);
>    h->sbuf.option.optlen = htobe32 (len);
>    h->wbuf = &h->sbuf;
>    h->wlen = sizeof (h->sbuf.option);
> @@ -98,7 +108,8 @@ STATE_MACHINE {
>    return 0;
> 
>   NEWSTYLE.OPT_META_CONTEXT.PREPARE_NEXT_QUERY:
> -  const char *query = h->request_meta_contexts[h->querynum];
> +  const char *query = !h->request_meta_contexts ? NULL
> +    : h->request_meta_contexts[h->querynum];
> 
>    if (query == NULL) { /* end of list of requested meta contexts */
>      SET_NEXT_STATE (%PREPARE_FOR_REPLY);
> @@ -140,10 +151,17 @@ STATE_MACHINE {
>    return 0;
> 
>   NEWSTYLE.OPT_META_CONTEXT.RECV_REPLY:
> +  uint32_t opt;
> +
> +  if (h->opt_current == NBD_OPT_LIST_META_CONTEXT)
> +    opt = h->opt_current;
> +  else
> +    opt = NBD_OPT_SET_META_CONTEXT;
> +
>    switch (recv_into_rbuf (h)) {
>    case -1: SET_NEXT_STATE (%.DEAD); return 0;
>    case 0:
> -    if (prepare_for_reply_payload (h, NBD_OPT_SET_META_CONTEXT) == -1) {
> +    if (prepare_for_reply_payload (h, opt) == -1) {
>        SET_NEXT_STATE (%.DEAD);
>        return 0;
>      }
> @@ -163,12 +181,25 @@ STATE_MACHINE {
>    uint32_t len;
>    const size_t maxpayload = sizeof h->sbuf.or.payload.context;
>    struct meta_context *meta_context;
> +  uint32_t opt;
> +  int err = 0;
> +
> +  if (h->opt_current == NBD_OPT_LIST_META_CONTEXT)
> +    opt = h->opt_current;
> +  else
> +    opt = NBD_OPT_SET_META_CONTEXT;
> 
>    reply = be32toh (h->sbuf.or.option_reply.reply);
>    len = be32toh (h->sbuf.or.option_reply.replylen);
>    switch (reply) {
>    case NBD_REP_ACK:           /* End of list of replies. */
> -    SET_NEXT_STATE (%^OPT_GO.START);
> +    if (opt == NBD_OPT_LIST_META_CONTEXT) {
> +      SET_NEXT_STATE (%.NEGOTIATING);
> +      CALL_CALLBACK (h->opt_cb.completion, &err);
> +      nbd_internal_free_option (h);
> +    }
> +    else
> +      SET_NEXT_STATE (%^OPT_GO.START);
>      break;
>    case NBD_REP_META_CONTEXT:  /* A context. */
>      if (len > maxpayload)
> @@ -194,21 +225,38 @@ STATE_MACHINE {
>        }
>        debug (h, "negotiated %s with context ID %" PRIu32,
>               meta_context->name, meta_context->context_id);
> -      meta_context->next = h->meta_contexts;
> -      h->meta_contexts = meta_context;
> +      if (opt == NBD_OPT_LIST_META_CONTEXT) {
> +        CALL_CALLBACK (h->opt_cb.fn.context, meta_context->name);
> +        free (meta_context->name);
> +        free (meta_context);
> +      }
> +      else {
> +        meta_context->next = h->meta_contexts;
> +        h->meta_contexts = meta_context;
> +      }
>      }
>      SET_NEXT_STATE (%PREPARE_FOR_REPLY);
>      break;
>    default:
> -    /* Anything else is an error, ignore it */
> +    /* Anything else is an error, ignore it for SET, report it for LIST */
>      if (handle_reply_error (h) == -1) {
>        SET_NEXT_STATE (%.DEAD);
>        return 0;
>      }
> 
> -    debug (h, "handshake: unexpected error from "
> -           "NBD_OPT_SET_META_CONTEXT (%" PRIu32 ")",
reply);
> -    SET_NEXT_STATE (%^OPT_GO.START);
> +    if (opt == NBD_OPT_LIST_META_CONTEXT) {
> +      err = ENOTSUP;
> +      set_error (err, "unexpected response, possibly the server does
not "
> +                 "support listing contexts");
> +      CALL_CALLBACK (h->opt_cb.completion, &err);
> +      nbd_internal_free_option (h);
> +      SET_NEXT_STATE (%.NEGOTIATING);
> +    }
> +    else {
> +      debug (h, "handshake: unexpected error from "
> +             "NBD_OPT_SET_META_CONTEXT (%" PRIu32 ")",
reply);
> +      SET_NEXT_STATE (%^OPT_GO.START);
> +    }
>      break;
>    }
>    return 0;
> diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c
> index a0a5928..6fb7548 100644
> --- a/generator/states-newstyle.c
> +++ b/generator/states-newstyle.c
> @@ -136,6 +136,9 @@ STATE_MACHINE {
>        }
>        SET_NEXT_STATE (%PREPARE_OPT_ABORT);
>        return 0;
> +    case NBD_OPT_LIST_META_CONTEXT:
> +      SET_NEXT_STATE (%OPT_META_CONTEXT.START);
> +      return 0;
>      case 0:
>        break;
>      default:
> diff --git a/lib/opt.c b/lib/opt.c
> index 6ea8326..2317b72 100644
> --- a/lib/opt.c
> +++ b/lib/opt.c
> @@ -32,6 +32,8 @@ nbd_internal_free_option (struct nbd_handle *h)
>  {
>    if (h->opt_current == NBD_OPT_LIST)
>      FREE_CALLBACK (h->opt_cb.fn.list);
> +  else if (h->opt_current == NBD_OPT_LIST_META_CONTEXT)
> +    FREE_CALLBACK (h->opt_cb.fn.context);
>    FREE_CALLBACK (h->opt_cb.completion);
>  }
> 
> @@ -166,6 +168,51 @@ nbd_unlocked_opt_list (struct nbd_handle *h,
nbd_list_callback *list)
>    return s.count;
>  }
> 
> +struct context_helper {
> +  int count;
> +  nbd_context_callback context;
> +  int err;
> +};
> +static int
> +context_visitor (void *opaque, const char *name)
> +{
> +  struct context_helper *h = opaque;
> +  if (h->count < INT_MAX)
> +    h->count++;
> +  CALL_CALLBACK (h->context, name);
> +  return 0;
> +}
> +static int
> +context_complete (void *opaque, int *err)
> +{
> +  struct context_helper *h = opaque;
> +  h->err = *err;
> +  FREE_CALLBACK (h->context);
> +  return 0;
> +}
> +
> +/* Issue NBD_OPT_LIST_META_CONTEXT and wait for the reply. */
> +int
> +nbd_unlocked_opt_list_meta_context (struct nbd_handle *h,
> +                                    nbd_context_callback *context)
> +{
> +  struct context_helper s = { .context = *context };
> +  nbd_context_callback l = { .callback = context_visitor, .user_data =
&s };
> +  nbd_completion_callback c = { .callback = context_complete, .user_data =
&s };
> +
> +  if (nbd_unlocked_aio_opt_list_meta_context (h, &l, &c) == -1)
> +    return -1;
> +
> +  SET_CALLBACK_TO_NULL (*context);
> +  if (wait_for_option (h) == -1)
> +    return -1;
> +  if (s.err) {
> +    set_error (s.err, "server replied with error to list meta context
request");
> +    return -1;
> +  }
> +  return s.count;
> +}
> +
>  /* Issue NBD_OPT_GO (or NBD_OPT_EXPORT_NAME) without waiting. */
>  int
>  nbd_unlocked_aio_opt_go (struct nbd_handle *h,
> @@ -230,3 +277,29 @@ nbd_unlocked_aio_opt_list (struct nbd_handle *h,
nbd_list_callback *list,
>      debug (h, "option queued, ignoring state machine failure");
>    return 0;
>  }
> +
> +/* Issue NBD_OPT_LIST_META_CONTEXT without waiting. */
> +int
> +nbd_unlocked_aio_opt_list_meta_context (struct nbd_handle *h,
> +                                        nbd_context_callback *context,
> +                                        nbd_completion_callback *complete)
> +{
> +  if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) {
> +    set_error (ENOTSUP, "server is not using fixed newstyle
protocol");
> +    return -1;
> +  }
> +  if (!h->structured_replies) {
> +    set_error (ENOTSUP, "server lacks structured replies");
> +    return -1;
> +  }
> +
> +  assert (CALLBACK_IS_NULL (h->opt_cb.fn.context));
> +  h->opt_cb.fn.context = *context;
> +  SET_CALLBACK_TO_NULL (*context);
> +  h->opt_cb.completion = *complete;
> +  SET_CALLBACK_TO_NULL (*complete);
> +  h->opt_current = NBD_OPT_LIST_META_CONTEXT;
> +  if (nbd_internal_run (h, cmd_issue) == -1)
> +    debug (h, "option queued, ignoring state machine failure");
> +  return 0;
> +}
> diff --git a/TODO b/TODO
> index 4a0cd22..b6c676b 100644
> --- a/TODO
> +++ b/TODO
> @@ -13,6 +13,17 @@ NBD resize extension.
> 
>  TLS should properly shut down the session (calling gnutls_bye).
> 
> +Potential deadlock with nbd_add_meta_context: if a client sends enough
> +requests to the server that it blocks while writing, but the server
> +replies to requests as they come in rather than waiting for the end of
> +the client request, then the server can likewise block in writing
> +replies that libnbd is not yet reading.  Not an issue for existing
> +servers that don't have enough contexts to reply with enough data to
> +fill buffers, but could be an issue with qemu-nbd if it is taught to
> +exports many dirty bitmaps simultaneously.  Revamping the
> +states-newstyle-meta-context.c state machine to let libnbd handle
> +NBD_REP_META_CONTEXT while still writing queries could be hairy.
> +
>  Performance: Chart it over various buffer sizes and threads, as that
>    should make it easier to identify systematic issues.
> 
> -- 
> 2.28.0
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top