[Libguestfs] [PATCH nbdkit] server: Deprecate the -e/--exportname parameter.
Richard W.M. Jones
rjones at redhat.com
Tue Jul 21 15:10:13 UTC 2020
This parameter provided a name for the "default export" -- ie. the one
and only export returned to the client by NBD_OPT_LIST. But nbdkit
traditionally didn't care what export name the client requested.
Since 1.16 plugins have been able to serve different content per
export name (and return errors for unknown exports), but the -e option
didn't reflect that and only created confusion.
What we actually have to do is ask the plugin what exports it provides
and return that list to the client. This is for future work.
This commit deprecates and hides the parameter. If used the option
now does nothing at all.
There are some visible but very minor changes resulting from this
commit:
(1) NBD_OPT_LIST advertises a single export called "" (instead of the
-e parameter). We intend to replace this implementation soon with a
correct one.
(2) The --run option no longer sets $exportname (to -e) nor puts the
export name into the $uri. However this was always the wrong
thing to do since export names are per connection, not per server.
Existing --run scripts will see $exportname expand to "" which is
most likely what they saw before and overwhelmingly more likely to
be correct than if the -e option had been used.
(3) The -e option had the side-effect of forcing the newstyle
protocol, so weirdly this worked and made the server use newstyle:
nbdkit -o -e '' ...
but this would fail with an error at start-up:
nbdkit -e '' -o ...
I thought it best to remove this odd behaviour completely.
(4) I have temporarily disabled tests/test-long-name.sh. This is
still a valid test, but we will have to rewrite it to use
(probably) sh or eval plugins once we have an implementation of
list_exports.
---
docs/nbdkit-captive.pod | 12 ++----------
docs/nbdkit-protocol.pod | 15 +++++++--------
docs/nbdkit.pod | 13 -------------
docs/synopsis.txt | 3 +--
server/internal.h | 1 -
server/captive.c | 18 +-----------------
server/main.c | 21 +--------------------
server/protocol-handshake-newstyle.c | 12 +++++++++---
tests/test-data-7E.sh | 2 +-
tests/test-data-raw.sh | 3 +--
tests/test-long-name.sh | 4 ++++
11 files changed, 27 insertions(+), 77 deletions(-)
diff --git a/docs/nbdkit-captive.pod b/docs/nbdkit-captive.pod
index 09628367..66c5c81c 100644
--- a/docs/nbdkit-captive.pod
+++ b/docs/nbdkit-captive.pod
@@ -47,14 +47,12 @@ The following shell variables are available in the I<--run> argument:
=item C<$uri>
A URI that refers to the nbdkit port or socket in the preferred form
-documented by the NBD project. If nbdkit was started with the B<-e>
-option to set the preferred export name, this is included in the URI.
+documented by the NBD project.
=item C<$nbd>
An older URL that refers to the nbdkit port or socket in a manner
-specific to certain tools. This form does not include an export name,
-even if B<-e> was used.
+specific to certain tools.
Note there is some magic here, since qemu and guestfish URLs have a
different format, so nbdkit tries to guess which you are running. If
@@ -68,12 +66,6 @@ If E<ne> "", the port number that nbdkit is listening on.
If E<ne> "", the Unix domain socket that nbdkit is listening on.
-=item C<$exportname>
-
-The default export name (which may be "") that nbdkit will advertise
-in response to NBD_OPT_LIST. This comes from the B<-e>
-(B<--exportname>) command line option.
-
=back
I<--run> implies I<--foreground>. It is not possible, and probably
diff --git a/docs/nbdkit-protocol.pod b/docs/nbdkit-protocol.pod
index d90993fb..172d1326 100644
--- a/docs/nbdkit-protocol.pod
+++ b/docs/nbdkit-protocol.pod
@@ -5,7 +5,7 @@ nbdkit - which parts of the NBD protocol nbdkit supports
=head1 SYNOPSIS
nbdkit [-n|--newstyle] [--mask-handshake MASK] [--no-sr] [-o|--oldstyle]
- [-e|--exportname EXPORTNAME] [...]
+ [...]
=head1 DESCRIPTION
@@ -29,9 +29,6 @@ can support (in some cases, this can be limited by what callbacks the
plugin handles), even if the client does not negotiate to use all
advertised features.
-Use the I<-e> or I<--exportname> flag to set the optional exportname
-for the newstyle protocol.
-
Nbdkit also includes some options that are useful mainly when
performing integration tests, for proving whether clients have sane
fallback behavior when dealing various older servers permitted by the
@@ -101,11 +98,13 @@ Supported in nbdkit E<ge> 1.1.12, and the default in nbdkit E<ge> 1.3.
=item export names
-Supported in nbdkit E<ge> 1.1.12.
+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.
-nbdkit can advertise an export name, but ignores any export name sent
-by the client. nbdkit does I<not> support serving different data on
-different export names.
+Versions of nbdkit before 1.16 could advertize a single export name to
+clients, specified through the now deprecated 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.
=item C<NBD_FLAG_NO_ZEROES>
diff --git a/docs/nbdkit.pod b/docs/nbdkit.pod
index a6cf988c..b204709b 100644
--- a/docs/nbdkit.pod
+++ b/docs/nbdkit.pod
@@ -214,19 +214,6 @@ An alternative to this is L<nbdkit-captive(1)/CAPTIVE NBDKIT>.
This option implies I<--foreground>.
-=item B<-e> EXPORTNAME
-
-=item B<--export> EXPORTNAME
-
-=item B<--export-name> EXPORTNAME
-
-=item B<--exportname> EXPORTNAME
-
-Set the exportname.
-
-If not set, exportname C<""> (empty string) is used. Exportnames are
-not allowed with the oldstyle protocol.
-
=item B<-f>
=item B<--foreground>
diff --git a/docs/synopsis.txt b/docs/synopsis.txt
index 07b9dcff..caf49922 100644
--- a/docs/synopsis.txt
+++ b/docs/synopsis.txt
@@ -1,5 +1,4 @@
-nbdkit [-D|--debug PLUGIN|FILTER|nbdkit.FLAG=N]
- [-e|--exportname EXPORTNAME] [--exit-with-parent]
+nbdkit [-D|--debug PLUGIN|FILTER|nbdkit.FLAG=N] [--exit-with-parent]
[--filter FILTER ...] [-f|--foreground]
[-g|--group GROUP] [-i|--ipaddr IPADDR]
[--log stderr|syslog|null]
diff --git a/server/internal.h b/server/internal.h
index 68c53366..ebc63a52 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -108,7 +108,6 @@ enum log_to {
};
extern struct debug_flag *debug_flags;
-extern const char *exportname;
extern bool foreground;
extern const char *ipaddr;
extern enum log_to log_to;
diff --git a/server/captive.c b/server/captive.c
index f8107604..aa717870 100644
--- a/server/captive.c
+++ b/server/captive.c
@@ -61,8 +61,6 @@ run_command (void)
if (!run)
return;
- assert (exportname != NULL);
-
fp = open_memstream (&cmd, &len);
if (fp == NULL) {
perror ("open_memstream");
@@ -74,27 +72,13 @@ run_command (void)
if (port) {
fprintf (fp, "nbd://localhost:");
shell_quote (port, fp);
- if (strcmp (exportname, "") != 0) {
- putc ('/', fp);
- uri_quote (exportname, fp);
- }
}
else if (unixsocket) {
- fprintf (fp, "nbd+unix://");
- if (strcmp (exportname, "") != 0) {
- putc ('/', fp);
- uri_quote (exportname, fp);
- }
- fprintf (fp, "\\?socket=");
+ fprintf (fp, "nbd+unix://\\?socket=");
uri_quote (unixsocket, fp);
}
putc ('\n', fp);
- /* Expose $exportname. */
- fprintf (fp, "exportname=");
- shell_quote (exportname, fp);
- putc ('\n', fp);
-
/* Construct older $nbd "URL". Unfortunately guestfish and qemu take
* different syntax, so try to guess which one we need.
*/
diff --git a/server/main.c b/server/main.c
index c432f5bd..d815fe12 100644
--- a/server/main.c
+++ b/server/main.c
@@ -79,7 +79,6 @@ static bool is_config_key (const char *key, size_t len);
struct debug_flag *debug_flags; /* -D */
bool exit_with_parent; /* --exit-with-parent */
-const char *exportname; /* -e */
bool foreground; /* -f */
const char *ipaddr; /* -i */
enum log_to log_to = LOG_TO_DEFAULT; /* --log */
@@ -345,13 +344,7 @@ main (int argc, char *argv[])
break;
case 'e':
- exportname = optarg;
- if (strnlen (exportname, NBD_MAX_STRING + 1) > NBD_MAX_STRING) {
- nbdkit_error ("export name too long");
- exit (EXIT_FAILURE);
- }
- /* TODO: Check that name is valid UTF-8? */
- newstyle = true;
+ /* Does nothing, ignored for compatibility with older nbdkit. */
break;
case 'f':
@@ -487,18 +480,6 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
- /* Oldstyle protocol + exportname not allowed. */
- if (!newstyle && exportname != NULL) {
- fprintf (stderr,
- "%s: cannot use oldstyle protocol (-o) and exportname (-e)\n",
- program_name);
- exit (EXIT_FAILURE);
- }
-
- /* If exportname was not set on the command line, use "". */
- if (exportname == NULL)
- exportname = "";
-
/* --tls=require and oldstyle won't work. */
if (tls == 2 && !newstyle) {
fprintf (stderr,
diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c
index 192d00f3..fa44f253 100644
--- a/server/protocol-handshake-newstyle.c
+++ b/server/protocol-handshake-newstyle.c
@@ -75,12 +75,15 @@ 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.
+ */
static int
send_newstyle_option_reply_exportname (uint32_t option, uint32_t reply)
{
GET_CONN;
struct nbd_fixed_new_option_reply fixed_new_option_reply;
- size_t name_len = strlen (exportname);
+ const size_t name_len = 0; /* length of export name */
uint32_t len;
fixed_new_option_reply.magic = htobe64 (NBD_REP_MAGIC);
@@ -100,11 +103,14 @@ send_newstyle_option_reply_exportname (uint32_t option, uint32_t reply)
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;
}
+#endif
return 0;
}
@@ -364,8 +370,8 @@ negotiate_handshake_newstyle_options (void)
}
/* Send back the exportname. */
- debug ("newstyle negotiation: %s: advertising export '%s'",
- name_of_nbd_opt (option), exportname);
+ debug ("newstyle negotiation: %s: advertising export \"\"",
+ name_of_nbd_opt (option));
if (send_newstyle_option_reply_exportname (option, NBD_REP_SERVER) == -1)
return -1;
diff --git a/tests/test-data-7E.sh b/tests/test-data-7E.sh
index e06d3c7e..c4fa86b1 100755
--- a/tests/test-data-7E.sh
+++ b/tests/test-data-7E.sh
@@ -45,7 +45,7 @@ rm -f $files
cleanup_fn rm -f $files
# Run nbdkit.
-start_nbdkit -P data-7E.pid -U $sock --export= \
+start_nbdkit -P data-7E.pid -U $sock \
--filter=partition \
data size=7E partition=1 \
data="
diff --git a/tests/test-data-raw.sh b/tests/test-data-raw.sh
index 36ee8028..446e2093 100755
--- a/tests/test-data-raw.sh
+++ b/tests/test-data-raw.sh
@@ -44,8 +44,7 @@ rm -f $files
cleanup_fn rm -f $files
# Run nbdkit.
-start_nbdkit -P data-raw.pid -U $sock --export "" \
- data raw=123 size=512
+start_nbdkit -P data-raw.pid -U $sock data raw=123 size=512
nbdsh --connect "nbd+unix://?socket=$sock" \
-c '
diff --git a/tests/test-long-name.sh b/tests/test-long-name.sh
index 6a512603..f70f492d 100755
--- a/tests/test-long-name.sh
+++ b/tests/test-long-name.sh
@@ -38,6 +38,10 @@ fail=0
# Test handling of NBD maximum string length of 4k.
+echo "$0: This test needs to be rewritten to use alternate method to"
+echo "send back the export name to the client."
+exit 77
+
requires qemu-io --version
requires qemu-nbd --version
requires nbdsh -c 'exit(not h.supports_uri())'
--
2.27.0
More information about the Libguestfs
mailing list