[Libguestfs] [PATCH nbdkit] server: Deprecate the -e/--exportname parameter.

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