[Libguestfs] [PATCH nbdkit v2 1/3] server: Add nbdkit_export_name() to allow export name to be read.

Richard W.M. Jones rjones at redhat.com
Thu Sep 12 21:01:03 UTC 2019


This allows plugins (or filters) to read the export name which was
passed to the server from the client.
---
 TODO                                 |  8 +++++++
 docs/nbdkit-plugin.pod               | 29 ++++++++++++++++++++++
 include/nbdkit-common.h              |  1 +
 server/connections.c                 | 36 ++++++++++++++++++----------
 server/internal.h                    |  1 +
 server/nbdkit.syms                   |  1 +
 server/protocol-handshake-newstyle.c | 31 ++++++++++++++----------
 server/public.c                      | 13 ++++++++++
 8 files changed, 95 insertions(+), 25 deletions(-)

diff --git a/TODO b/TODO
index 49b60b1..2468d74 100644
--- a/TODO
+++ b/TODO
@@ -62,6 +62,12 @@ General ideas for improvements
   and also look at the implementation of the -swap option in
   nbd-client.
 
+* Clients should be able to list export names supported by plugins.
+  Current behaviour is not really correct: We only list the -e
+  parameter from the command line, which is different from the export
+  name(s) that a plugin might want to support.  Probably we should
+  deprecate the -e option entirely since it does nothing useful.
+
 Suggestions for plugins
 -----------------------
 
@@ -190,3 +196,5 @@ using ‘#define NBDKIT_API_VERSION <version>’.
   value) strings.  nbdkit should know the possible keys for the plugin
   and filters, and the type of the values, and both check and parse
   them for the plugin.
+
+* Modify open() API so it takes an export name parameter.
diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
index bb162e4..219906e 100644
--- a/docs/nbdkit-plugin.pod
+++ b/docs/nbdkit-plugin.pod
@@ -362,6 +362,32 @@ requested, or -1 after calling C<nbdkit_error> if there is no point in
 continuing the current command.  Attempts to sleep more than
 C<INT_MAX> seconds are treated as an error.
 
+=head1 EXPORT NAME
+
+If the client negotiated an NBD export name with nbdkit then plugins
+may read this from any connected callbacks.  Nbdkit's normal behaviour
+is to accept any export name passed by the client, log it in debug
+output, but otherwise ignore it.  By using C<nbdkit_export_name>
+plugins may choose to filter by export name or serve different
+content.
+
+=head2 C<nbdkit_export_name>
+
+ const char *nbdkit_export_name (void);
+
+Return the optional NBD export name if one was negotiated with the
+current client (this uses thread-local magic so no parameter is
+required).  The returned string is only valid while the client is
+connected, so if you need to store it in the plugin you must copy it.
+
+The export name is a free-form text string, it is not necessarily a
+path or filename and it does not need to begin with a C<'/'>
+character.  The NBD protocol describes the empty string (C<"">) as a
+representing a "default export" or to be used in cases where the
+export name does not make sense.
+
+On error, nbdkit_error is called and the call returns C<NULL>.
+
 =head1 CALLBACKS
 
 =head2 C<.name>
@@ -709,6 +735,9 @@ request or setting C<NBDKIT_FLAG_FUA> on a request must be visible
 across all connections to the plugin before the plugin replies to that
 request.
 
+Properly working clients should send the same export name for each of
+these connections.
+
 If you use Linux L<nbd-client(8)> option S<I<-C num>> with
 S<num E<gt> 1> then Linux checks this flag and will refuse to connect
 if C<.can_multi_conn> is false.
diff --git a/include/nbdkit-common.h b/include/nbdkit-common.h
index 42d94a1..acf0abd 100644
--- a/include/nbdkit-common.h
+++ b/include/nbdkit-common.h
@@ -86,6 +86,7 @@ extern int nbdkit_parse_bool (const char *str);
 extern int nbdkit_read_password (const char *value, char **password);
 extern char *nbdkit_realpath (const char *path);
 extern int nbdkit_nanosleep (unsigned sec, unsigned nsec);
+extern const char *nbdkit_export_name (void);
 
 struct nbdkit_extents;
 extern int nbdkit_add_extent (struct nbdkit_extents *,
diff --git a/server/connections.c b/server/connections.c
index 8ef5e57..819f7b8 100644
--- a/server/connections.c
+++ b/server/connections.c
@@ -255,11 +255,18 @@ new_connection (int sockin, int sockout, int nworkers)
     perror ("malloc");
     return NULL;
   }
+
+  conn->status_pipe[0] = conn->status_pipe[1] = -1;
+
+  conn->exportname = strdup ("");
+  if (conn->exportname == NULL) {
+    perror ("strdup");
+    goto error;
+  }
   conn->handles = calloc (backend->i + 1, sizeof *conn->handles);
   if (conn->handles == NULL) {
     perror ("malloc");
-    free (conn);
-    return NULL;
+    goto error;
   }
   conn->nr_handles = backend->i + 1;
   memset (conn->handles, -1, conn->nr_handles * sizeof *conn->handles);
@@ -272,8 +279,7 @@ new_connection (int sockin, int sockout, int nworkers)
 #ifdef HAVE_PIPE2
     if (pipe2 (conn->status_pipe, O_NONBLOCK | O_CLOEXEC)) {
       perror ("pipe2");
-      free (conn);
-      return NULL;
+      goto error;
     }
 #else
     /* If we were fully parallel, then this function could be
@@ -289,29 +295,24 @@ new_connection (int sockin, int sockout, int nworkers)
     lock_request (NULL);
     if (pipe (conn->status_pipe)) {
       perror ("pipe");
-      free (conn);
       unlock_request (NULL);
-      return NULL;
+      goto error;
     }
     if (set_nonblock (set_cloexec (conn->status_pipe[0])) == -1) {
       perror ("fcntl");
       close (conn->status_pipe[1]);
-      free (conn);
       unlock_request (NULL);
-      return NULL;
+      goto error;
     }
     if (set_nonblock (set_cloexec (conn->status_pipe[1])) == -1) {
       perror ("fcntl");
       close (conn->status_pipe[0]);
-      free (conn);
       unlock_request (NULL);
-      return NULL;
+      goto error;
     }
     unlock_request (NULL);
 #endif
   }
-  else
-    conn->status_pipe[0] = conn->status_pipe[1] = -1;
   conn->sockin = sockin;
   conn->sockout = sockout;
   pthread_mutex_init (&conn->request_lock, NULL);
@@ -329,6 +330,16 @@ new_connection (int sockin, int sockout, int nworkers)
   threadlocal_set_conn (conn);
 
   return conn;
+
+ error:
+  if (conn->status_pipe[0] >= 0)
+    close (conn->status_pipe[0]);
+  if (conn->status_pipe[1] >= 0)
+    close (conn->status_pipe[1]);
+  free (conn->exportname);
+  free (conn->handles);
+  free (conn);
+  return NULL;
 }
 
 static void
@@ -373,6 +384,7 @@ free_connection (struct connection *conn)
   pthread_mutex_destroy (&conn->status_lock);
 
   free (conn->handles);
+  free (conn->exportname);
   free (conn);
 }
 
diff --git a/server/internal.h b/server/internal.h
index ac5b894..1f72b01 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -181,6 +181,7 @@ struct connection {
   struct b_conn_handle *handles;
   size_t nr_handles;
 
+  char *exportname;
   uint32_t cflags;
   uint16_t eflags;
   bool using_tls;
diff --git a/server/nbdkit.syms b/server/nbdkit.syms
index 2a024ed..1fb1315 100644
--- a/server/nbdkit.syms
+++ b/server/nbdkit.syms
@@ -42,6 +42,7 @@
     nbdkit_add_extent;
     nbdkit_debug;
     nbdkit_error;
+    nbdkit_export_name;
     nbdkit_extents_count;
     nbdkit_extents_free;
     nbdkit_extents_new;
diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c
index 75465b7..87e0bcd 100644
--- a/server/protocol-handshake-newstyle.c
+++ b/server/protocol-handshake-newstyle.c
@@ -272,11 +272,17 @@ negotiate_handshake_newstyle_options (struct connection *conn)
       if (conn_recv_full (conn, data, optlen,
                           "read: %s: %m", name_of_nbd_opt (option)) == -1)
         return -1;
-      /* Apart from printing it, ignore the export name. */
+      /* Print the export name and save it in the connection. */
       data[optlen] = '\0';
-      debug ("newstyle negotiation: %s: "
-             "client requested export '%s' (ignored)",
+      debug ("newstyle negotiation: %s: client requested export '%s'",
              name_of_nbd_opt (option), data);
+      free (conn->exportname);
+      conn->exportname = malloc (optlen+1);
+      if (conn->exportname == NULL) {
+        nbdkit_error ("malloc: %m");
+        return -1;
+      }
+      strcpy (conn->exportname, data);
 
       /* We have to finish the handshake by sending handshake_finish. */
       if (finish_newstyle_options (conn, &exportsize) == -1)
@@ -388,7 +394,6 @@ negotiate_handshake_newstyle_options (struct connection *conn)
         uint16_t nrinfos;
         uint16_t info;
         size_t i;
-        CLEANUP_FREE char *requested_exportname = NULL;
 
         /* Validate the name length and number of INFO requests. */
         memcpy (&exportnamelen, &data[0], 4);
@@ -411,19 +416,19 @@ negotiate_handshake_newstyle_options (struct connection *conn)
           continue;
         }
 
-        /* As with NBD_OPT_EXPORT_NAME we print the export name and then
-         * ignore it.
+        /* As with NBD_OPT_EXPORT_NAME we print the export name and
+         * save it in the connection.
          */
-        requested_exportname = malloc (exportnamelen+1);
-        if (requested_exportname == NULL) {
+        free (conn->exportname);
+        conn->exportname = malloc (exportnamelen+1);
+        if (conn->exportname == NULL) {
           nbdkit_error ("malloc: %m");
           return -1;
         }
-        memcpy (requested_exportname, &data[4], exportnamelen);
-        requested_exportname[exportnamelen] = '\0';
-        debug ("newstyle negotiation: %s: "
-               "client requested export '%s' (ignored)",
-               optname, requested_exportname);
+        memcpy (conn->exportname, &data[4], exportnamelen);
+        conn->exportname[exportnamelen] = '\0';
+        debug ("newstyle negotiation: %s: client requested export '%s'",
+               optname, conn->exportname);
 
         /* The spec is confusing, but it is required that we send back
          * NBD_INFO_EXPORT, even if the client did not request it!
diff --git a/server/public.c b/server/public.c
index 630de9b..96ab353 100644
--- a/server/public.c
+++ b/server/public.c
@@ -379,3 +379,16 @@ nbdkit_nanosleep (unsigned sec, unsigned nsec)
   return 0;
 #endif
 }
+
+const char *
+nbdkit_export_name (void)
+{
+  struct connection *conn = threadlocal_get_conn ();
+
+  if (!conn) {
+    nbdkit_error ("no connection in this thread");
+    return NULL;
+  }
+
+  return conn->exportname;
+}
-- 
2.23.0




More information about the Libguestfs mailing list