While we are unlikely to change our decision that -e should not control our response to NBD_OPT_LIST (because we intend to add a new callback .extents_list for that), it turns out that it is a lot easier to write: nbdkit -U - -e foo info --run 'nbdsh -u "$uri" -c "print(h.pread(3, 0))"' than it is to write the equivalent: nbdkit -U - info --run 'nbdsh -u nbd+unix:///foo\?socket=$unixsocket \ -c "print(h.pread(3, 0))"' and our test-long-name.sh was evidence of that fact. Let's revert that part of the patch, but improve the documentation to mention that -e is now useful _only_ with --run, and only if the client uses it. This partially reverts commit e71178e9b71403, although for -Wshadow reasons, we now have to name the global variable export_name due to other code in the meantime using parameters named exportname. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- docs/nbdkit-captive.pod | 12 +++++++++++- docs/nbdkit-protocol.pod | 2 +- docs/nbdkit.pod | 18 ++++++++++++++++++ docs/synopsis.txt | 3 ++- server/internal.h | 1 + server/captive.c | 20 +++++++++++++++++--- server/main.c | 3 ++- tests/test-long-name.sh | 10 +++++----- 8 files changed, 57 insertions(+), 12 deletions(-) diff --git a/docs/nbdkit-captive.pod b/docs/nbdkit-captive.pod index f1ac9fc4..94e79c59 100644 --- a/docs/nbdkit-captive.pod +++ b/docs/nbdkit-captive.pod @@ -53,7 +53,9 @@ The following shell variables are available in the I<--run> argument: A URI that refers to the nbdkit port or socket in the preferred form documented by the NBD project. As this variable may contain a bare C<?> for Unix sockets, it is safest to use $uri within double quotes -to avoid unintentional globbing. +to avoid unintentional globbing. For plugins that support distinct +data based on export names, the B<-e> option to nbdkit controls which +export name will be set in the URI. =item C<$nbd> @@ -72,6 +74,14 @@ 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 export name (which may be "") that the process should use when +connecting to nbdkit, as set by the B<-e> (B<--exportname>) command +line option of nbdkit. This only matters to plugins that +differentiate what they serve based on the export name requested by +the client. + =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 e0e4042b..8fe8c67e 100644 --- a/docs/nbdkit-protocol.pod +++ b/docs/nbdkit-protocol.pod @@ -102,7 +102,7 @@ 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. Versions of nbdkit before 1.16 could advertise a single export name to -clients, specified through the now deprecated I<-e> option. In nbdkit +clients, via a now deprecated side effect of the 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. diff --git a/docs/nbdkit.pod b/docs/nbdkit.pod index b204709b..e23cc90f 100644 --- a/docs/nbdkit.pod +++ b/docs/nbdkit.pod @@ -214,6 +214,24 @@ 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 a preferred exportname to expose in the shell environment created +during B<--run>. The use of this option without B<--run> has no +effect. This option does I<not> change what nbdkit advertises as a +server, but can aid in writing a captive client that wants to access +particular content from a plugin that differentiates content based on +the client's choice of export name. + +If not set, the B<--run> environment is set to access the default +exportname C<""> (empty string). + =item B<-f> =item B<--foreground> diff --git a/docs/synopsis.txt b/docs/synopsis.txt index caf49922..07b9dcff 100644 --- a/docs/synopsis.txt +++ b/docs/synopsis.txt @@ -1,4 +1,5 @@ -nbdkit [-D|--debug PLUGIN|FILTER|nbdkit.FLAG=N] [--exit-with-parent] +nbdkit [-D|--debug PLUGIN|FILTER|nbdkit.FLAG=N] + [-e|--exportname EXPORTNAME] [--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 4911a49b..1acbbb06 100644 --- a/server/internal.h +++ b/server/internal.h @@ -108,6 +108,7 @@ enum log_to { }; extern struct debug_flag *debug_flags; +extern const char *export_name; extern bool foreground; extern const char *ipaddr; extern enum log_to log_to; diff --git a/server/captive.c b/server/captive.c index 53b82782..a8947d7c 100644 --- a/server/captive.c +++ b/server/captive.c @@ -61,6 +61,9 @@ run_command (void) if (!run) return; + if (!export_name) + export_name = ""; + fp = open_memstream (&cmd, &len); if (fp == NULL) { perror ("open_memstream"); @@ -72,15 +75,26 @@ run_command (void) if (port) { fprintf (fp, "nbd://localhost:"); shell_quote (port, fp); + if (strcmp (export_name, "") != 0) { + putc ('/', fp); + uri_quote (export_name, fp); + } } else if (unixsocket) { - fprintf (fp, "nbd+unix://\\?socket="); + fprintf (fp, "nbd+unix://"); + if (strcmp (export_name, "") != 0) { + putc ('/', fp); + uri_quote (export_name, fp); + } + fprintf (fp, "\\?socket="); uri_quote (unixsocket, fp); } putc ('\n', fp); - /* $exportname is for backwards compatibility. */ - fprintf (fp, "exportname=\n"); + /* Expose $exportname. */ + fprintf (fp, "exportname="); + shell_quote (export_name, 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 d815fe12..2f0aaf07 100644 --- a/server/main.c +++ b/server/main.c @@ -79,6 +79,7 @@ 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 *export_name; /* -e */ bool foreground; /* -f */ const char *ipaddr; /* -i */ enum log_to log_to = LOG_TO_DEFAULT; /* --log */ @@ -344,7 +345,7 @@ main (int argc, char *argv[]) break; case 'e': - /* Does nothing, ignored for compatibility with older nbdkit. */ + export_name = optarg; break; case 'f': diff --git a/tests/test-long-name.sh b/tests/test-long-name.sh index 1da2515a..897f8e94 100755 --- a/tests/test-long-name.sh +++ b/tests/test-long-name.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash # nbdkit -# Copyright (C) 2019 Red Hat Inc. +# Copyright (C) 2019-2020 Red Hat Inc. # # Redistribution and use in source and binary forms, with or without # modification, are permitted provided that the following conditions are @@ -53,10 +53,6 @@ name1k=$name256$name256$name256$name256 name4k=$name1k$name1k$name1k$name1k almost4k=${name4k%8$name16} -# Test behavior of -e: accept 4k max, reject longer -nbdkit -U - -e $name4k null --run true || fail=1 -nbdkit -U - -e a$name4k null --run true && fail=1 - # Test that $exportname and $uri reflect the name out=$(nbdkit -U - -e $name4k null --run 'echo $exportname') if test "$name4k" != "$out"; then @@ -110,6 +106,9 @@ assert h.get_size() == 0 EOF ' +# TODO when we add .list_extents, we should test a plugin advertising +# a name near 4k in length. nbdkit -e no longer does this +if false; then # The rest of this test uses the ‘qemu-nbd --list’ option added in qemu 4.0. if ! qemu-nbd --help | grep -sq -- --list; then echo "$0: skipping because qemu-nbd does not support the --list option" @@ -118,5 +117,6 @@ fi # Test response to NBD_OPT_LIST nbdkit -U - -e $name4k null --run 'qemu-nbd --list -k $unixsocket' || fail=1 +fi exit $fail -- 2.27.0