[Libguestfs] [libnbd PATCH] api: Add way to avoid structured replies

Eric Blake eblake at redhat.com
Wed Sep 4 18:28:08 UTC 2019


We want to default to requesting structured replies, whether or not
that request will be honored (it's essential for efficient sparse file
reads and the DF flag for structured pread, as well as for meta
context support even if we do not request a default meta context).
However, for integration testing, it can be nice to easily request a
client that does not request structured replies.

We can test this by reusing our eflags test.  Note that nbdkit does
not provide a 'can_df' callback in the sh script (so our key=value
override is silently ignored), rather, we control whether nbdkit
advertises df based on whether we request structured replies.
---

I'm open to renaming the API to the shorter 'nbd_set_request_sr' if
the existing name choice seems too long.

This is a counterpart to the recent addition of --no-sr in nbdkit 1.14
for server-side disable; but by doing it in the client side, we can
test lack of structured replies even while still using nbdkit 1.12.x.

 lib/internal.h                                |  1 +
 generator/generator                           | 30 +++++++++++++++++++
 .../states-newstyle-opt-structured-reply.c    |  5 ++++
 lib/handle.c                                  | 15 ++++++++++
 tests/Makefile.am                             | 18 +++++++++++
 tests/eflags.c                                |  6 ++++
 TODO                                          |  2 +-
 7 files changed, 76 insertions(+), 1 deletion(-)

diff --git a/lib/internal.h b/lib/internal.h
index 581777a..a48edff 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -73,6 +73,7 @@ struct nbd_handle {
   char *tls_psk_file;           /* PSK filename, NULL = no PSK */

   /* Desired metadata contexts. */
+  bool request_sr;
   char **request_meta_contexts;

   /* Global flags from the server. */
diff --git a/generator/generator b/generator/generator
index 1cc5c19..5c32afa 100755
--- a/generator/generator
+++ b/generator/generator
@@ -1243,6 +1243,36 @@ Get the current TLS PSK filename.";
   };
 *)

+  "set_request_structured_replies", {
+    default_call with
+    args = [Bool "request"]; ret = RErr;
+    permitted_states = [ Created ];
+    first_version = (1, 2);
+    shortdesc = "control use of structured replies";
+    longdesc = "\
+By default, libnbd tries to negotiate structured replies with the
+server, as this protocol extension must be in use before
+C<nbd_can_meta_context> or C<nbd_can_df> can return true.  However,
+for integration testing, it can be useful to clear this flag
+rather than find a way to alter the server to fail the negotiation
+request.";
+    see_also = ["L<nbd_get_request_structured_replies(3)>";
+                "L<nbd_can_meta_context(3)>"; "L<nbd_can_df(3)>"];
+  };
+
+  "get_request_structured_replies", {
+    default_call with
+    args = []; ret = RBool;
+    first_version = (1, 2);
+    shortdesc = "see if structured replies are attempted";
+    longdesc = "\
+Return the state of the request structured replies flag on this
+handle.  Note that this only reports whether the client attempts
+to negotiate structured replies, and not whether the server was
+able to honor that request";
+    see_also = ["L<nbd_set_request_structured_replies(3)>"];
+  };
+
   "add_meta_context", {
     default_call with
     args = [ String "name" ]; ret = RErr;
diff --git a/generator/states-newstyle-opt-structured-reply.c b/generator/states-newstyle-opt-structured-reply.c
index d932248..415f7e0 100644
--- a/generator/states-newstyle-opt-structured-reply.c
+++ b/generator/states-newstyle-opt-structured-reply.c
@@ -20,6 +20,11 @@

 /* STATE MACHINE */ {
  NEWSTYLE.OPT_STRUCTURED_REPLY.START:
+  if (!h->request_sr) {
+    SET_NEXT_STATE (%^OPT_SET_META_CONTEXT.START);
+    return 0;
+  }
+
   h->sbuf.option.version = htobe64 (NBD_NEW_VERSION);
   h->sbuf.option.option = htobe32 (NBD_OPT_STRUCTURED_REPLY);
   h->sbuf.option.optlen = htobe32 (0);
diff --git a/lib/handle.c b/lib/handle.c
index f8cc83a..c23ef01 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -63,6 +63,7 @@ nbd_create (void)

   h->unique = 1;
   h->tls_verify_peer = true;
+  h->request_sr = true;

   s = getenv ("LIBNBD_DEBUG");
   h->debug = s && strcmp (s, "1") == 0;
@@ -242,6 +243,20 @@ nbd_unlocked_add_meta_context (struct nbd_handle *h, const char *name)
   return 0;
 }

+int
+nbd_unlocked_set_request_structured_replies (struct nbd_handle *h,
+                                             bool request)
+{
+  h->request_sr = request;
+  return 0;
+}
+
+int
+nbd_unlocked_get_request_structured_replies (struct nbd_handle *h)
+{
+  return h->request_sr;
+}
+
 const char *
 nbd_unlocked_get_package_name (struct nbd_handle *h)
 {
diff --git a/tests/Makefile.am b/tests/Makefile.am
index a7bc1b5..41b620d 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -147,6 +147,8 @@ check_PROGRAMS += \
 	can-not-zero-flag \
 	can-fast-zero-flag \
 	can-not-fast-zero-flag \
+	can-df-flag \
+	can-not-df-flag \
 	can-multi-conn-flag \
 	can-not-multi-conn-flag \
 	can-cache-flag \
@@ -181,6 +183,8 @@ TESTS += \
 	can-not-zero-flag \
 	can-fast-zero-flag \
 	can-not-fast-zero-flag \
+	can-df-flag \
+	can-not-df-flag \
 	can-multi-conn-flag \
 	can-not-multi-conn-flag \
 	can-cache-flag \
@@ -309,6 +313,20 @@ can_not_fast_zero_flag_CPPFLAGS = \
 can_not_fast_zero_flag_CFLAGS = $(WARNINGS_CFLAGS)
 can_not_fast_zero_flag_LDADD = $(top_builddir)/lib/libnbd.la

+can_df_flag_SOURCES = eflags.c
+can_df_flag_CPPFLAGS = \
+	-I$(top_srcdir)/include -Dflag=can_df \
+	$(NULL)
+can_df_flag_CFLAGS = $(WARNINGS_CFLAGS)
+can_df_flag_LDADD = $(top_builddir)/lib/libnbd.la
+
+can_not_df_flag_SOURCES = eflags.c
+can_not_df_flag_CPPFLAGS = \
+	-I$(top_srcdir)/include -Dflag=can_df -Dvalue=false -Dno_sr \
+	$(NULL)
+can_not_df_flag_CFLAGS = $(WARNINGS_CFLAGS)
+can_not_df_flag_LDADD = $(top_builddir)/lib/libnbd.la
+
 can_multi_conn_flag_SOURCES = eflags.c
 can_multi_conn_flag_CPPFLAGS = \
 	-I$(top_srcdir)/include -Dflag=can_multi_conn \
diff --git a/tests/eflags.c b/tests/eflags.c
index 675802a..afff3a9 100644
--- a/tests/eflags.c
+++ b/tests/eflags.c
@@ -84,6 +84,12 @@ main (int argc, char *argv[])
     fprintf (stderr, "%s\n", nbd_get_error ());
     exit (EXIT_FAILURE);
   }
+#ifdef no_sr
+  if (nbd_set_request_structured_replies (nbd, false) == -1) {
+    fprintf (stderr, "%s\n", nbd_get_error ());
+    exit (EXIT_FAILURE);
+  }
+#endif
   if (nbd_connect_command (nbd, args) == -1) {
     fprintf (stderr, "%s\n", nbd_get_error ());
     exit (EXIT_FAILURE);
diff --git a/TODO b/TODO
index 34bb983..642d39f 100644
--- a/TODO
+++ b/TODO
@@ -48,7 +48,7 @@ Suggested API improvements:
   - support PKCS11 URIs (RFC 7512)

   Easier server implementation testing:
-  - a way to disable requesting structured replies
+  - a way to force NBD_OPT_EXPORT_NAME over NBD_OPT_GO
   - a way to forcefully violate protocol (such as allowing writes to a
     readonly connection, or sending a request that exceeds bounds) for
     testing server reactions
-- 
2.21.0




More information about the Libguestfs mailing list