[PATCH nbdkit v2 0/6] plugins: Implement magic config key.
v1 was here: https://www.redhat.com/archives/libguestfs/2018-September/msg00024.html v2: - As discussed in the patch review, tighten up the characters permitted in keys. - Update documentation to note that relative paths can be made safe by prefixing with ./ and absolute paths do not need any extra steps. - I pushed patch 1/6 from the v1 series since it was just a trivial comment update. Rich.
This extends the concept of the magic script parameter. In existing nbdkit plugins if the first parameter is "bare" (does not contain '='), then it is parsed magically to allow scripting languages to work: nbdkit perl foo.pl is parsed as: nbdkit perl script=foo.pl This generalises the concept, allowing a plugin.magic_config_key to be defined. If this is non-NULL then any bare parameters on the command line: nbdkit file foo where file_plugin.magic_config_key = "file" is parsed as: nbdkit file file=foo If magic_config_key == NULL then the previous behaviour is used. There is a small (but hopefully not noticable) change to the behaviour of ‘--dump-plugin’. It is now handled after all command line parameters have been parsed but before the .config_complete method is called (whereas previously it would be called after the first command line parameter was parsed). Note this change only applies to plugin parameters. --- docs/nbdkit-plugin.pod | 17 +++++++++++ docs/nbdkit.pod | 8 +++--- include/nbdkit-plugin.h | 2 ++ src/filters.c | 12 ++++++++ src/internal.h | 1 + src/main.c | 62 ++++++++++++++++++++++++----------------- src/plugins.c | 9 ++++++ 7 files changed, 81 insertions(+), 30 deletions(-) diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index 515c032..570a142 100644 --- a/docs/nbdkit-plugin.pod +++ b/docs/nbdkit-plugin.pod @@ -396,6 +396,23 @@ with an error. If there is an error, C<.config> should call C<nbdkit_error> with an error message and return C<-1>. +=head2 C<.magic_config_key> + + const char *magic_config_key; + +This optional string can be used to set a "magic" key used when +parsing plugin parameters. It affects how "bare parameters" (those +which do not contain an C<=> character) are parsed on the command +line. + +If C<magic_config_key != NULL> then any bare parameters are passed to +the C<.config> method as: S<C<config (magic_config_key, argv[i]);>>. + +If C<magic_config_key> is not set then we behave as in nbdkit E<lt> +1.7: If the first parameter on the command line is bare then it is +passed to the C<.config> method as: S<C<config ("script", value);>>. +Any other bare parameters give errors. + =head2 C<.config_complete> int config_complete (void); diff --git a/docs/nbdkit.pod b/docs/nbdkit.pod index 981e571..cf1c5ca 100644 --- a/docs/nbdkit.pod +++ b/docs/nbdkit.pod @@ -404,11 +404,11 @@ To dump information about a plugin, do: =head2 Magic script parameter -As a special case, if the first plugin argument does not contain an -C<'='> character then it is assumed to be C<script=value>. +For some plugins, especially those supporting scripting languages like +Perl, if the first plugin argument does not contain an C<'='> +character then it is assumed to be C<script=value>. -That allows scripting language plugins like L<nbdkit-perl-plugin(1)> -to do: +That allows scripting language plugins to do: nbdkit perl foo.pl [args...] diff --git a/include/nbdkit-plugin.h b/include/nbdkit-plugin.h index c95e26c..31aa6c7 100644 --- a/include/nbdkit-plugin.h +++ b/include/nbdkit-plugin.h @@ -121,6 +121,8 @@ struct nbdkit_plugin { int (*trim) (void *handle, uint32_t count, uint64_t offset, uint32_t flags); int (*zero) (void *handle, uint32_t count, uint64_t offset, uint32_t flags); #endif + + const char *magic_config_key; }; extern void nbdkit_set_error (int err); diff --git a/src/filters.c b/src/filters.c index e6826f2..3626742 100644 --- a/src/filters.c +++ b/src/filters.c @@ -205,6 +205,17 @@ filter_config_complete (struct backend *b) f->backend.next->config_complete (f->backend.next); } +/* magic_config_key only applies to plugins, so this passes the + * request through to the plugin (hence the name). + */ +static const char * +plugin_magic_config_key (struct backend *b) +{ + struct backend_filter *f = container_of (b, struct backend_filter, backend); + + return f->backend.next->magic_config_key (f->backend.next); +} + static int next_open (void *nxdata, int readonly) { @@ -619,6 +630,7 @@ static struct backend filter_functions = { .dump_fields = filter_dump_fields, .config = filter_config, .config_complete = filter_config_complete, + .magic_config_key = plugin_magic_config_key, .open = filter_open, .prepare = filter_prepare, .finalize = filter_finalize, diff --git a/src/internal.h b/src/internal.h index 1c5aa72..d4fc534 100644 --- a/src/internal.h +++ b/src/internal.h @@ -171,6 +171,7 @@ struct backend { void (*dump_fields) (struct backend *); void (*config) (struct backend *, const char *key, const char *value); void (*config_complete) (struct backend *); + const char *(*magic_config_key) (struct backend *); int (*open) (struct backend *, struct connection *conn, int readonly); int (*prepare) (struct backend *, struct connection *conn); int (*finalize) (struct backend *, struct connection *conn); diff --git a/src/main.c b/src/main.c index 8a83a32..9c18d6f 100644 --- a/src/main.c +++ b/src/main.c @@ -231,6 +231,7 @@ main (int argc, char *argv[]) const char *filename; } *filter_filenames = NULL; size_t i; + const char *magic_config_key; threadlocal_init (); @@ -680,37 +681,46 @@ main (int argc, char *argv[]) exit (EXIT_SUCCESS); } - /* Find key=value configuration parameters for this plugin. - * The first one is magical in that if it doesn't contain '=' then - * we assume it is 'script=...'. + /* Call config and config_complete to parse the parameters. + * + * If the plugin provides magic_config_key then any "bare" values + * (ones not containing "=") are prefixed with this key. + * + * For backwards compatibility with old plugins, and to support + * scripting languages, if magic_config_key == NULL then if the + * first parameter is bare it is prefixed with the key "script", and + * any other bare parameters are errors. */ - if (optind < argc && (p = strchr (argv[optind], '=')) == NULL) { - backend->config (backend, "script", argv[optind]); - ++optind; - } - - /* This must run after parsing the possible script parameter so that - * the script can be loaded for scripting languages. Note that all - * scripting languages load the script as soon as they see the - * script=... parameter (and do not wait for config_complete). - */ - if (dump_plugin) { - backend->dump_fields (backend); - exit (EXIT_SUCCESS); - } - - while (optind < argc) { - if ((p = strchr (argv[optind], '=')) != NULL) { + magic_config_key = backend->magic_config_key (backend); + for (i = 0; optind < argc; ++i, ++optind) { + p = strchr (argv[optind], '='); + if (p) { /* key=value */ *p = '\0'; backend->config (backend, argv[optind], p+1); - ++optind; } - else { - fprintf (stderr, - "%s: expecting key=value on the command line but got: %s\n", - program_name, argv[optind]); - exit (EXIT_FAILURE); + else if (magic_config_key == NULL) { + if (i == 0) /* magic script parameter */ + backend->config (backend, "script", argv[optind]); + else { + fprintf (stderr, + "%s: expecting key=value on the command line but got: %s\n", + program_name, argv[optind]); + exit (EXIT_FAILURE); + } } + else { /* magic config key */ + backend->config (backend, magic_config_key, argv[optind]); + } + } + + /* This must run after parsing the parameters so that the script can + * be loaded for scripting languages. But it must be called before + * config_complete so that the plugin doesn't check for missing + * parameters. + */ + if (dump_plugin) { + backend->dump_fields (backend); + exit (EXIT_SUCCESS); } backend->config_complete (backend); diff --git a/src/plugins.c b/src/plugins.c index 780bbd9..2bea6ac 100644 --- a/src/plugins.c +++ b/src/plugins.c @@ -232,6 +232,14 @@ plugin_config_complete (struct backend *b) exit (EXIT_FAILURE); } +static const char * +plugin_magic_config_key (struct backend *b) +{ + struct backend_plugin *p = container_of (b, struct backend_plugin, backend); + + return p->plugin.magic_config_key; +} + static int plugin_open (struct backend *b, struct connection *conn, int readonly) { @@ -630,6 +638,7 @@ static struct backend plugin_functions = { .dump_fields = plugin_dump_fields, .config = plugin_config, .config_complete = plugin_config_complete, + .magic_config_key = plugin_magic_config_key, .open = plugin_open, .prepare = plugin_prepare, .finalize = plugin_finalize, -- 2.18.0
Previously key=value on the command line allowed the key to be pretty much anything that didn't contain an '=' character. Even empty strings were permitted. This tightens up the permitted keys so they must contain only ASCII alphanumeric, period, underscore or dash characters, and must not be an empty string. --- docs/nbdkit-plugin.pod | 18 ++++++++++-------- src/main.c | 32 +++++++++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index 570a142..b37cac1 100644 --- a/docs/nbdkit-plugin.pod +++ b/docs/nbdkit-plugin.pod @@ -376,15 +376,17 @@ optional list of C<key=value> arguments. These are passed to the plugin through this callback when the plugin is first loaded and before any connections are accepted. -This callback may be called zero or more times. Both C<key> and -C<value> parameters will be non-NULL, but it is possible for either to -be empty strings. The strings are owned by nbdkit but will remain -valid for the lifetime of the plugin, so the plugin does not need to -copy them. +This callback may be called zero or more times. -The format of the C<key> accepted by plugins is up to the plugin, but -you should probably look at other plugins and follow the same -conventions. +Both C<key> and C<value> parameters will be non-NULL. They key will +be a non-empty string containing only ASCII alphanumeric plus period, +underscore or dash characters (C<A-Z> C<a-z> C<0-9> C<.> C<_> C<->). +The value may be an arbitrary string, including an empty string. The +strings are owned by nbdkit but will remain valid for the lifetime of +the plugin, so the plugin does not need to copy them. + +The names of C<key>s accepted by plugins is up to the plugin, but you +should probably look at other plugins and follow the same conventions. If the value is a relative path, then note that the server changes directory when it starts up. See L</FILENAMES AND PATHS> above. diff --git a/src/main.c b/src/main.c index 9c18d6f..c523854 100644 --- a/src/main.c +++ b/src/main.c @@ -72,6 +72,7 @@ static void fork_into_background (void); static uid_t parseuser (const char *); static gid_t parsegroup (const char *); static unsigned int get_socket_activation (void); +static int is_config_key (const char *key, size_t len); struct debug_flag *debug_flags; /* -D */ int exit_with_parent; /* --exit-with-parent */ @@ -694,7 +695,7 @@ main (int argc, char *argv[]) magic_config_key = backend->magic_config_key (backend); for (i = 0; optind < argc; ++i, ++optind) { p = strchr (argv[optind], '='); - if (p) { /* key=value */ + if (p && is_config_key (argv[optind], p - argv[optind])) { /* key=value */ *p = '\0'; backend->config (backend, argv[optind], p+1); } @@ -1281,3 +1282,32 @@ get_socket_activation (void) return nr_fds; } + +/* When parsing plugin and filter config key=value from the command + * line, check that the key is a simple alphanumeric with period, + * underscore or dash. + * + * Note this doesn't return an error. If the key is not valid then we + * return false and the parsing code will assume that this is a bare + * value instead. + */ +static int +is_config_key (const char *key, size_t len) +{ + const char allowed[] = + "abcdefghijklmnopqrstuvwxyz" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "0123456789" + "._-"; + size_t i; + + if (key[0] == '\0') + return 0; + + for (i = 0; i < len; ++i) { + if (strchr (allowed, key[i]) == NULL) + return 0; + } + + return 1; +} -- 2.18.0
On 09/08/2018 03:15 AM, Richard W.M. Jones wrote:
Previously key=value on the command line allowed the key to be pretty much anything that didn't contain an '=' character. Even empty strings were permitted.
This tightens up the permitted keys so they must contain only ASCII alphanumeric, period, underscore or dash characters, and must not be an empty string.
Do we want to further restrict things to start with a letter or underscore (and not a dot, digit, or dash)?
--- docs/nbdkit-plugin.pod | 18 ++++++++++-------- src/main.c | 32 +++++++++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 9 deletions(-)
+static int +is_config_key (const char *key, size_t len) +{ + const char allowed[] = + "abcdefghijklmnopqrstuvwxyz" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "0123456789" + "._-"; + size_t i; + + if (key[0] == '\0') + return 0; + + for (i = 0; i < len; ++i) { + if (strchr (allowed, key[i]) == NULL)
Why not use strspn and checking against the length, instead of rolling an O(n^2) algorithm yourself? The libc version might have optimizations to run faster, although the speed of this loop is probably in the noise. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Sat, Sep 08, 2018 at 04:05:45PM -0500, Eric Blake wrote:
On 09/08/2018 03:15 AM, Richard W.M. Jones wrote:
Previously key=value on the command line allowed the key to be pretty much anything that didn't contain an '=' character. Even empty strings were permitted.
This tightens up the permitted keys so they must contain only ASCII alphanumeric, period, underscore or dash characters, and must not be an empty string.
Do we want to further restrict things to start with a letter or underscore (and not a dot, digit, or dash)?
--- docs/nbdkit-plugin.pod | 18 ++++++++++-------- src/main.c | 32 +++++++++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 9 deletions(-)
+static int +is_config_key (const char *key, size_t len) +{ + const char allowed[] = + "abcdefghijklmnopqrstuvwxyz" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "0123456789" + "._-"; + size_t i; + + if (key[0] == '\0')
Also this bit is wrong :-(
+ return 0; + + for (i = 0; i < len; ++i) { + if (strchr (allowed, key[i]) == NULL)
Why not use strspn and checking against the length, instead of rolling an O(n^2) algorithm yourself? The libc version might have optimizations to run faster, although the speed of this loop is probably in the noise.
Will post v3 in a moment. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
--- docs/nbdkit-captive.pod | 4 ++-- docs/nbdkit-plugin.pod | 2 +- docs/nbdkit-service.pod | 2 +- docs/nbdkit-tls.pod | 2 +- filters/cow/nbdkit-cow-filter.pod | 4 ++-- filters/delay/nbdkit-delay-filter.pod | 2 +- filters/error/nbdkit-error-filter.pod | 6 +++--- filters/fua/nbdkit-fua-filter.pod | 8 ++++---- filters/log/nbdkit-log-filter.pod | 4 ++-- filters/nozero/nbdkit-nozero-filter.pod | 4 ++-- filters/offset/nbdkit-offset-filter.pod | 4 ++-- filters/partition/nbdkit-partition-filter.pod | 2 +- plugins/file/file.c | 1 + plugins/file/nbdkit-file-plugin.pod | 7 ++++++- tests/test-blocksize.sh | 4 ++-- tests/test-cache.sh | 2 +- tests/test-cow.sh | 2 +- tests/test-file-block.c | 3 +-- tests/test-file.c | 2 +- tests/test-fua.sh | 8 ++++---- tests/test-log.sh | 2 +- tests/test-nbd.c | 2 +- tests/test-newstyle.c | 2 +- tests/test-nozero.sh | 10 +++++----- tests/test-offset.c | 2 +- tests/test-oldstyle.c | 2 +- tests/test-parallel-file.sh | 4 ++-- tests/test-parallel-nbd.sh | 2 +- tests/test-partition.c | 2 +- tests/test-single.sh | 2 +- 30 files changed, 54 insertions(+), 49 deletions(-) diff --git a/docs/nbdkit-captive.pod b/docs/nbdkit-captive.pod index 6db9520..340374f 100644 --- a/docs/nbdkit-captive.pod +++ b/docs/nbdkit-captive.pod @@ -26,7 +26,7 @@ Some examples should make this clear. To run nbdkit captive under qemu: - nbdkit file file=disk.img --run 'qemu -drive file=$nbd,if=virtio' + nbdkit file disk.img --run 'qemu -drive file=$nbd,if=virtio' On the qemu command line, C<$nbd> is substituted automatically with the right NBD path so it can connect to nbdkit. When qemu exits, @@ -34,7 +34,7 @@ nbdkit is killed and cleaned up automatically. Running nbdkit captive under guestfish: - nbdkit file file=disk.img --run 'guestfish --format=raw -a $nbd -i' + nbdkit file disk.img --run 'guestfish --format=raw -a $nbd -i' When guestfish exits, nbdkit is killed. diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index b37cac1..665f7e2 100644 --- a/docs/nbdkit-plugin.pod +++ b/docs/nbdkit-plugin.pod @@ -267,7 +267,7 @@ an indication of failure. It has the following prototype: The server usually (not always) changes directory to C</> before it starts serving connections. This means that any relative paths passed during configuration will not work when the server is running -(example: S<C<nbdkit plugin.so file=disk.img>>). +(example: S<C<nbdkit plugin.so disk.img>>). To avoid problems, prepend relative paths with the current directory before storing them in the handle. Or open files and store the file diff --git a/docs/nbdkit-service.pod b/docs/nbdkit-service.pod index 956b4ab..c0d275f 100644 --- a/docs/nbdkit-service.pod +++ b/docs/nbdkit-service.pod @@ -45,7 +45,7 @@ Also create a service unit (eg. C</etc/systemd/system/nbdkit.service>) containing: [Service] - ExecStart=/usr/sbin/nbdkit file file=/path/to/serve + ExecStart=/usr/sbin/nbdkit file /path/to/serve For more information on systemd and socket activation, see L<http://0pointer.de/blog/projects/socket-activation.html> diff --git a/docs/nbdkit-tls.pod b/docs/nbdkit-tls.pod index 2e70739..ecca87a 100644 --- a/docs/nbdkit-tls.pod +++ b/docs/nbdkit-tls.pod @@ -194,7 +194,7 @@ client which can read this file will be able to connect to the server. Use the nbdkit I<--tls-psk> option to start the server: - nbdkit --tls=require --tls-psk=/tmp/keys/keys.psk -e / file file=disk.img + nbdkit --tls=require --tls-psk=/tmp/keys/keys.psk -e / file disk.img This option overrides X.509 certificate authentication. diff --git a/filters/cow/nbdkit-cow-filter.pod b/filters/cow/nbdkit-cow-filter.pod index ed48f2b..80100e7 100644 --- a/filters/cow/nbdkit-cow-filter.pod +++ b/filters/cow/nbdkit-cow-filter.pod @@ -59,7 +59,7 @@ normal way. Serve the file F<disk.img>, allowing writes, but do not save any changes into the file: - nbdkit --filter=cow file file=disk.img + nbdkit --filter=cow file disk.img L<nbdkit-xz-plugin(1)> only supports read access, but you can provide temporary write access by doing (although this does B<not> save @@ -76,7 +76,7 @@ otherwise all changes will be lost>. Run nbdkit: - nbdkit --filter=cow file file=disk.img + nbdkit --filter=cow file disk.img and then connect with a client and make whatever changes you need. At the end, disconnect the client. diff --git a/filters/delay/nbdkit-delay-filter.pod b/filters/delay/nbdkit-delay-filter.pod index 3bfb039..62c0093 100644 --- a/filters/delay/nbdkit-delay-filter.pod +++ b/filters/delay/nbdkit-delay-filter.pod @@ -16,7 +16,7 @@ remote server, or to test certain kinds of race conditions in Linux. =head1 EXAMPLE - nbdkit --filter=delay file file=disk.img rdelay=100ms wdelay=100ms + nbdkit --filter=delay file disk.img rdelay=100ms wdelay=100ms =head1 PARAMETERS diff --git a/filters/error/nbdkit-error-filter.pod b/filters/error/nbdkit-error-filter.pod index 0f02282..76003c7 100644 --- a/filters/error/nbdkit-error-filter.pod +++ b/filters/error/nbdkit-error-filter.pod @@ -27,12 +27,12 @@ B<otherwise this filter will do nothing>. Inject a low rate of errors randomly into the connection: - nbdkit --filter=error file file=disk.img error-rate=1% + nbdkit --filter=error file disk.img error-rate=1% Reading and trimming requests will be successful, but all writes and zeroing will return "No space left on device": - nbdkit --filter=error file file=disk.img \ + nbdkit --filter=error file disk.img \ error=ENOSPC \ error-pwrite-rate=100% \ error-zero-rate=100% @@ -41,7 +41,7 @@ To make all connections fail hard 60 seconds after the server is started, use: rm -f /tmp/inject - nbdkit --filter=error file file=disk.img \ + nbdkit --filter=error file disk.img \ error-rate=100% \ error-file=/tmp/inject sleep 60; touch /tmp/inject diff --git a/filters/fua/nbdkit-fua-filter.pod b/filters/fua/nbdkit-fua-filter.pod index 8822a45..e0b3c4d 100644 --- a/filters/fua/nbdkit-fua-filter.pod +++ b/filters/fua/nbdkit-fua-filter.pod @@ -42,23 +42,23 @@ C<NBDKIT_FUA_NONE>). Serve the file F<disk.img>, but force the client to submit explicit flush requests instead of using C<NBD_CMD_FLAG_FUA>: - nbdkit --filter=fua file file=disk.img + nbdkit --filter=fua file disk.img Observe that the blocksize filter optimizes its handling of the FUA flag based on whether it knows nbdkit will be emulating FUA with a flush, by comparing the log filter output on top of different fua filter modes: - nbdkit --filter=blocksize --filter=log --filter=fua file file=disk.img \ + nbdkit --filter=blocksize --filter=log --filter=fua file disk.img \ maxlen=4k logfile=fua_emulated fuamode=emulate - nbdkit --filter=blocksize --filter=log --filter=fua file file=disk.img \ + nbdkit --filter=blocksize --filter=log --filter=fua file disk.img \ maxlen=4k logfile=fua_native fuamode=native Serve the file F<disk.img> in write-through mode, where all writes from the client are immediately flushed to disk as if the client had always requested FUA: - nbdkit --filter=fua file fuamode=force file=disk.img + nbdkit --filter=fua file fuamode=force disk.img =head1 SEE ALSO diff --git a/filters/log/nbdkit-log-filter.pod b/filters/log/nbdkit-log-filter.pod index 89ea551..0903329 100644 --- a/filters/log/nbdkit-log-filter.pod +++ b/filters/log/nbdkit-log-filter.pod @@ -29,12 +29,12 @@ already exists, it will be truncated. Serve the file F<disk.img>, and log each client transaction in the file F<disk.log>: - nbdkit --filter=log file file=disk.img logfile=disk.log + nbdkit --filter=log file disk.img logfile=disk.log Repeat the task, but with the cow (copy-on-write) filter to perform local caching of data served from the original plugin: - nbdkit --filter=cow --filter=log file file=disk.img logfile=disk.log2 + nbdkit --filter=cow --filter=log file disk.img logfile=disk.log2 After running a client that performs the same operations under each of the two servers, you can compare F<disk.log> and F<disk.log2> to see diff --git a/filters/nozero/nbdkit-nozero-filter.pod b/filters/nozero/nbdkit-nozero-filter.pod index 1301432..715605f 100644 --- a/filters/nozero/nbdkit-nozero-filter.pod +++ b/filters/nozero/nbdkit-nozero-filter.pod @@ -34,13 +34,13 @@ way to write zeros. Serve the file F<disk.img>, but force the client to write zeroes explicitly rather than with C<NBD_CMD_WRITE_ZEROES>: - nbdkit --filter=nozero file file=disk.img + nbdkit --filter=nozero file disk.img Serve the file F<disk.img>, allowing the client to take advantage of less network traffic via C<NBD_CMD_WRITE_ZEROES>, but still forcing the data to be written explicitly rather than punching any holes: - nbdkit --filter=nozero file zeromode=emulate file=disk.img + nbdkit --filter=nozero file zeromode=emulate disk.img =head1 SEE ALSO diff --git a/filters/offset/nbdkit-offset-filter.pod b/filters/offset/nbdkit-offset-filter.pod index 61496d2..a232e46 100644 --- a/filters/offset/nbdkit-offset-filter.pod +++ b/filters/offset/nbdkit-offset-filter.pod @@ -44,7 +44,7 @@ Using L<nbdkit-file-plugin(1)>, serve the file C<disk.img> starting at offset C<1M>. The total length served is C<100M> (the underlying file must therefore be at least C<101M> in length): - nbdkit --filter=offset file file=disk.img offset=1M range=100M + nbdkit --filter=offset file disk.img offset=1M range=100M =head2 Serve a single partition @@ -60,7 +60,7 @@ and length of the partition, eg using: You can then serve the partition only using: - nbdkit --filter=offset file file=disk.img offset=65536 range=104727040 + nbdkit --filter=offset file disk.img offset=65536 range=104727040 =head1 SEE ALSO diff --git a/filters/partition/nbdkit-partition-filter.pod b/filters/partition/nbdkit-partition-filter.pod index 0256e52..ae72f3f 100644 --- a/filters/partition/nbdkit-partition-filter.pod +++ b/filters/partition/nbdkit-partition-filter.pod @@ -38,7 +38,7 @@ This parameter is required. F<disk.img> is a partitioned disk image (eg. a virtual machine disk image). To serve the first partition only use: - nbdkit --filter=partition file file=disk.img partition=1 + nbdkit --filter=partition file disk.img partition=1 =head1 SEE ALSO diff --git a/plugins/file/file.c b/plugins/file/file.c index e7cbff6..9d03f18 100644 --- a/plugins/file/file.c +++ b/plugins/file/file.c @@ -476,6 +476,7 @@ static struct nbdkit_plugin plugin = { .config = file_config, .config_complete = file_config_complete, .config_help = file_config_help, + .magic_config_key = "file", .dump_plugin = file_dump_plugin, .open = file_open, .close = file_close, diff --git a/plugins/file/nbdkit-file-plugin.pod b/plugins/file/nbdkit-file-plugin.pod index b2c25d1..f8fe74c 100644 --- a/plugins/file/nbdkit-file-plugin.pod +++ b/plugins/file/nbdkit-file-plugin.pod @@ -4,7 +4,7 @@ nbdkit-file-plugin - nbdkit file plugin =head1 SYNOPSIS - nbdkit file file=FILENAME + nbdkit file FILENAME =head1 DESCRIPTION @@ -26,6 +26,11 @@ be used here. This parameter is required. +In nbdkit E<ge> 1.7, C<file=> may be omitted. To ensure that the +filename does not end up being parsed accidentally as C<key=value>, +prefix relative paths with C<./> (absolute paths do not need +modification). + =item B<rdelay> =item B<wdelay> diff --git a/tests/test-blocksize.sh b/tests/test-blocksize.sh index d239c9b..cb9b09f 100755 --- a/tests/test-blocksize.sh +++ b/tests/test-blocksize.sh @@ -73,9 +73,9 @@ trap cleanup INT QUIT TERM EXIT ERR # Run two parallel nbdkit; to compare the logs and see what changes. nbdkit -P blocksize1.pid -U blocksize1.sock \ - --filter=log file logfile=blocksize1.log file=blocksize1.img + --filter=log file logfile=blocksize1.log blocksize1.img nbdkit -P blocksize2.pid -U blocksize2.sock --filter=blocksize \ - --filter=log file logfile=blocksize2.log file=blocksize2.img \ + --filter=log file logfile=blocksize2.log blocksize2.img \ minblock=1024 maxdata=512k maxlen=1M # We may have to wait a short time for the pid files to appear. diff --git a/tests/test-cache.sh b/tests/test-cache.sh index 6f058b8..215bab1 100755 --- a/tests/test-cache.sh +++ b/tests/test-cache.sh @@ -41,7 +41,7 @@ rm -f $files truncate -s 1G cache.img # Run nbdkit with the caching filter. -nbdkit -P cache.pid -U cache.sock --filter=cache file file=cache.img +nbdkit -P cache.pid -U cache.sock --filter=cache file cache.img # We may have to wait a short time for the pid file to appear. for i in `seq 1 10`; do diff --git a/tests/test-cow.sh b/tests/test-cow.sh index 5bde829..f9b0649 100755 --- a/tests/test-cow.sh +++ b/tests/test-cow.sh @@ -42,7 +42,7 @@ guestfish -N cow-base.img=fs exit lastmod="$(stat -c "%y" cow-base.img)" # Run nbdkit with a COW overlay. -nbdkit -P cow.pid -U cow.sock --filter=cow file file=cow-base.img +nbdkit -P cow.pid -U cow.sock --filter=cow file cow-base.img # We may have to wait a short time for the pid file to appear. for i in `seq 1 10`; do diff --git a/tests/test-file-block.c b/tests/test-file-block.c index f053242..e2ea068 100644 --- a/tests/test-file-block.c +++ b/tests/test-file-block.c @@ -124,9 +124,8 @@ main (int argc, char *argv[]) atexit (detach_loopdev); /* Start nbdkit. */ - snprintf (buf, sizeof buf, "file=%s", loopdev); if (test_start_nbdkit ("-D", "file.zero=1", - "file", buf, NULL) == -1) + "file", loopdev, NULL) == -1) exit (EXIT_FAILURE); g = guestfs_create (); diff --git a/tests/test-file.c b/tests/test-file.c index 65a2568..9382ed2 100644 --- a/tests/test-file.c +++ b/tests/test-file.c @@ -52,7 +52,7 @@ main (int argc, char *argv[]) char *data; size_t i, size; - if (test_start_nbdkit ("file", "file=file-data", NULL) == -1) + if (test_start_nbdkit ("file", "file-data", NULL) == -1) exit (EXIT_FAILURE); g = guestfs_create (); diff --git a/tests/test-fua.sh b/tests/test-fua.sh index 98b1e84..f95aa18 100755 --- a/tests/test-fua.sh +++ b/tests/test-fua.sh @@ -84,13 +84,13 @@ trap cleanup INT QUIT TERM EXIT ERR # 3: fuamode=native: log shows that blocksize preserves fua # 4: fuamode=force: log shows that fua is always enabled nbdkit -P fua1.pid -U fua1.sock --filter=log --filter=fua \ - file logfile=fua1.log file=fua.img + file logfile=fua1.log fua.img nbdkit -P fua2.pid -U fua2.sock --filter=blocksize --filter=log --filter=fua \ - file logfile=fua2.log file=fua.img fuamode=emulate maxdata=4k maxlen=4k + file logfile=fua2.log fua.img fuamode=emulate maxdata=4k maxlen=4k nbdkit -P fua3.pid -U fua3.sock --filter=blocksize --filter=log --filter=fua \ - file logfile=fua3.log file=fua.img fuamode=native maxdata=4k maxlen=4k + file logfile=fua3.log fua.img fuamode=native maxdata=4k maxlen=4k nbdkit -P fua4.pid -U fua4.sock --filter=fua --filter=log \ - file logfile=fua4.log file=fua.img fuamode=force + file logfile=fua4.log fua.img fuamode=force # We may have to wait a short time for the pid files to appear. for i in `seq 1 10`; do diff --git a/tests/test-log.sh b/tests/test-log.sh index 877f9cc..f811de4 100755 --- a/tests/test-log.sh +++ b/tests/test-log.sh @@ -44,7 +44,7 @@ if ! qemu-io -f raw -c 'w 1M 2M' log.img; then fi # Run nbdkit with logging enabled to file. -nbdkit -P log.pid -U log.sock --filter=log file file=log.img logfile=log.log +nbdkit -P log.pid -U log.sock --filter=log file log.img logfile=log.log # We may have to wait a short time for the pid file to appear. for i in `seq 1 10`; do diff --git a/tests/test-nbd.c b/tests/test-nbd.c index 646c0c3..a09a0d5 100644 --- a/tests/test-nbd.c +++ b/tests/test-nbd.c @@ -55,7 +55,7 @@ main (int argc, char *argv[]) /* If wrapping once is good, why not do it twice! Shows that we can * convert between either style of server options. */ - if (test_start_nbdkit ("-o", "file", "file=file-data", NULL) == -1) + if (test_start_nbdkit ("-o", "file", "file-data", NULL) == -1) exit (EXIT_FAILURE); if (asprintf (&sockarg, "socket=%s", sock) < 0) { diff --git a/tests/test-newstyle.c b/tests/test-newstyle.c index cd0ba72..cadccec 100644 --- a/tests/test-newstyle.c +++ b/tests/test-newstyle.c @@ -55,7 +55,7 @@ main (int argc, char *argv[]) size_t i, size; if (test_start_nbdkit ("-e", EXPORTNAME, - "-n", "file", "file=file-data", NULL) == -1) + "-n", "file", "file-data", NULL) == -1) exit (EXIT_FAILURE); g = guestfs_create (); diff --git a/tests/test-nozero.sh b/tests/test-nozero.sh index 781b196..904d822 100755 --- a/tests/test-nozero.sh +++ b/tests/test-nozero.sh @@ -102,15 +102,15 @@ trap cleanup INT QUIT TERM EXIT ERR # 5a/b: both sides of nbd plugin: even though server side does not advertise # ZERO, the client side still exposes it, and just skips calling nbd's .zero nbdkit -P nozero1.pid -U nozero1.sock --filter=log \ - file logfile=nozero1.log file=nozero1.img + file logfile=nozero1.log nozero1.img nbdkit -P nozero2.pid -U nozero2.sock --filter=log --filter=nozero \ - file logfile=nozero2.log file=nozero2.img + file logfile=nozero2.log nozero2.img nbdkit -P nozero3.pid -U nozero3.sock --filter=log --filter=nozero \ - file logfile=nozero3.log file=nozero3.img zeromode=emulate + file logfile=nozero3.log nozero3.img zeromode=emulate nbdkit -P nozero4.pid -U nozero4.sock --filter=nozero --filter=log \ - file logfile=nozero4.log file=nozero4.img zeromode=emulate + file logfile=nozero4.log nozero4.img zeromode=emulate nbdkit -P nozero5a.pid -U nozero5a.sock --filter=log --filter=nozero \ - file logfile=nozero5a.log file=nozero5.img + file logfile=nozero5a.log nozero5.img nbdkit -P nozero5b.pid -U nozero5b.sock --filter=log \ nbd logfile=nozero5b.log socket=nozero5a.sock diff --git a/tests/test-offset.c b/tests/test-offset.c index d47066f..a7f467f 100644 --- a/tests/test-offset.c +++ b/tests/test-offset.c @@ -78,7 +78,7 @@ main (int argc, char *argv[]) * has not been overwritten. */ if (test_start_nbdkit ("--filter", "offset", - "file", "file=offset-data", + "file", "offset-data", "offset=1M", "range=8M", NULL) == -1) exit (EXIT_FAILURE); diff --git a/tests/test-oldstyle.c b/tests/test-oldstyle.c index bcffbe4..b3f3c4e 100644 --- a/tests/test-oldstyle.c +++ b/tests/test-oldstyle.c @@ -52,7 +52,7 @@ main (int argc, char *argv[]) char *data; size_t i, size; - if (test_start_nbdkit ("-o", "file", "file=file-data", NULL) == -1) + if (test_start_nbdkit ("-o", "file", "file-data", NULL) == -1) exit (EXIT_FAILURE); g = guestfs_create (); diff --git a/tests/test-parallel-file.sh b/tests/test-parallel-file.sh index fdc8a1b..fd88faa 100755 --- a/tests/test-parallel-file.sh +++ b/tests/test-parallel-file.sh @@ -56,7 +56,7 @@ qemu-io -f raw -c "aio_write -P 1 0 512" -c "aio_write -P 2 512 512" \ # tuning the delays may help. # With --threads=1, the write should complete first because it was issued first -nbdkit -v -t 1 -U - --filter=delay file file=test-parallel-file.data \ +nbdkit -v -t 1 -U - --filter=delay file test-parallel-file.data \ wdelay=2 rdelay=1 --run 'qemu-io -f raw -c "aio_write -P 2 512 512" \ -c "aio_read -P 1 0 512" -c aio_flush $nbd' | tee test-parallel-file.out @@ -67,7 +67,7 @@ read 512/512 bytes at offset 0"; then fi # With default --threads, the faster read should complete first -nbdkit -v -U - --filter=delay file file=test-parallel-file.data \ +nbdkit -v -U - --filter=delay file test-parallel-file.data \ wdelay=2 rdelay=1 --run 'qemu-io -f raw -c "aio_write -P 2 512 512" \ -c "aio_read -P 1 0 512" -c aio_flush $nbd' | tee test-parallel-file.out diff --git a/tests/test-parallel-nbd.sh b/tests/test-parallel-nbd.sh index e793c11..cb70f46 100755 --- a/tests/test-parallel-nbd.sh +++ b/tests/test-parallel-nbd.sh @@ -64,7 +64,7 @@ qemu-io -f raw -c "aio_write -P 1 0 512" -c "aio_write -P 2 512 512" \ nbdkit --exit-with-parent -v \ -U test-parallel-nbd.sock -P test-parallel-nbd.pid \ --filter=delay \ - file file=test-parallel-nbd.data wdelay=2 rdelay=1 & + file test-parallel-nbd.data wdelay=2 rdelay=1 & # We may have to wait a short time for the pid file to appear. for i in `seq 1 10`; do if test -f test-parallel-nbd.pid; then diff --git a/tests/test-partition.c b/tests/test-partition.c index 0087548..1064293 100644 --- a/tests/test-partition.c +++ b/tests/test-partition.c @@ -53,7 +53,7 @@ main (int argc, char *argv[]) if (test_start_nbdkit ("-r", "--filter", "partition", - "file", "file=disk", + "file", "disk", "partition=1", NULL) == -1) exit (EXIT_FAILURE); diff --git a/tests/test-single.sh b/tests/test-single.sh index dc6ce34..35017c8 100755 --- a/tests/test-single.sh +++ b/tests/test-single.sh @@ -51,7 +51,7 @@ rm -f $files truncate -s 1G single.disk socat unix-listen:single.sock,reuseaddr,fork \ - exec:'nbdkit -r -s file file=single.disk' & + exec:'nbdkit -r -s file single.disk' & pid=$! cleanup () -- 2.18.0
On 09/08/2018 03:15 AM, Richard W.M. Jones wrote:
---
+++ b/plugins/file/nbdkit-file-plugin.pod @@ -4,7 +4,7 @@ nbdkit-file-plugin - nbdkit file plugin
=head1 SYNOPSIS
- nbdkit file file=FILENAME + nbdkit file FILENAME
=head1 DESCRIPTION
@@ -26,6 +26,11 @@ be used here.
This parameter is required.
+In nbdkit E<ge> 1.7, C<file=> may be omitted. To ensure that the
Since you released 1.7.0 without this series, you'll need to tweak this wording.
+filename does not end up being parsed accidentally as C<key=value>, +prefix relative paths with C<./> (absolute paths do not need +modification). + =item B<rdelay>
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Sat, Sep 08, 2018 at 04:10:30PM -0500, Eric Blake wrote:
On 09/08/2018 03:15 AM, Richard W.M. Jones wrote:
---
+++ b/plugins/file/nbdkit-file-plugin.pod @@ -4,7 +4,7 @@ nbdkit-file-plugin - nbdkit file plugin =head1 SYNOPSIS - nbdkit file file=FILENAME + nbdkit file FILENAME =head1 DESCRIPTION @@ -26,6 +26,11 @@ be used here. This parameter is required. +In nbdkit E<ge> 1.7, C<file=> may be omitted. To ensure that the
Since you released 1.7.0 without this series, you'll need to tweak this wording.
Thanks Eric. I think in v3 I'll change this to >= 1.8 since we only really want to guarantee / talk about these kinds of changes in the context of the next stable release. Will post v3 shortly. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
--- plugins/split/nbdkit-split-plugin.pod | 9 +++++++-- plugins/split/split.c | 3 ++- tests/test-split.c | 3 ++- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/plugins/split/nbdkit-split-plugin.pod b/plugins/split/nbdkit-split-plugin.pod index 6a6a01a..bcbbf8f 100644 --- a/plugins/split/nbdkit-split-plugin.pod +++ b/plugins/split/nbdkit-split-plugin.pod @@ -4,12 +4,12 @@ nbdkit-split-plugin - nbdkit plugin to concatenate split files into one disk =head1 SYNOPSIS - nbdkit split file=file1 [file=file2 file=file3 ...] + nbdkit split file1 [file2 file3 ...] =head1 DESCRIPTION C<nbdkit-split-plugin> is a file plugin for L<nbdkit(1)>. One or more -filenames may be given using the C<file=FILENAME> parameter. These +filenames may be given using the C<FILENAME> parameter. These files are logically concatenated into a single disk image. =head2 Differences from nbdkit-file-plugin @@ -62,6 +62,11 @@ the order they appear on the command line. This parameter must appear at least once. +In nbdkit E<ge> 1.7, C<file=> may be omitted. To ensure that the +filename does not end up being parsed accidentally as C<key=value>, +prefix relative paths with C<./> (absolute paths do not need +modification). + =back =head1 SEE ALSO diff --git a/plugins/split/split.c b/plugins/split/split.c index bdcdcf7..7572316 100644 --- a/plugins/split/split.c +++ b/plugins/split/split.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2017 Red Hat Inc. + * Copyright (C) 2017-2018 Red Hat Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -286,6 +286,7 @@ static struct nbdkit_plugin plugin = { .unload = split_unload, .config = split_config, .config_help = split_config_help, + .magic_config_key = "file", .open = split_open, .close = split_close, .get_size = split_get_size, diff --git a/tests/test-split.c b/tests/test-split.c index f20e64b..c4f2a74 100644 --- a/tests/test-split.c +++ b/tests/test-split.c @@ -53,7 +53,8 @@ main (int argc, char *argv[]) size_t i, size; if (test_start_nbdkit ("split", - "file=split1", "file=split2", "file=split3", + "split1", "split2", + "file=split3" /* leave file= to test */, NULL) == -1) exit (EXIT_FAILURE); -- 2.18.0
--- plugins/gzip/gzip.c | 3 ++- plugins/gzip/nbdkit-gzip-plugin.pod | 19 ++++++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/plugins/gzip/gzip.c b/plugins/gzip/gzip.c index 09dd629..26d5e3c 100644 --- a/plugins/gzip/gzip.c +++ b/plugins/gzip/gzip.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2013 Red Hat Inc. + * Copyright (C) 2013-2018 Red Hat Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -217,6 +217,7 @@ static struct nbdkit_plugin plugin = { .config = gzip_config, .config_complete = gzip_config_complete, .config_help = gzip_config_help, + .magic_config_key = "file", .open = gzip_open, .close = gzip_close, .get_size = gzip_get_size, diff --git a/plugins/gzip/nbdkit-gzip-plugin.pod b/plugins/gzip/nbdkit-gzip-plugin.pod index a5f6b79..68dbf45 100644 --- a/plugins/gzip/nbdkit-gzip-plugin.pod +++ b/plugins/gzip/nbdkit-gzip-plugin.pod @@ -4,7 +4,7 @@ nbdkit-gzip-plugin - nbdkit gzip plugin =head1 SYNOPSIS - nbdkit gzip file=FILENAME.gz + nbdkit gzip FILENAME.gz =head1 DESCRIPTION @@ -18,6 +18,23 @@ files because seeking to a position in the gzip file involves uncompressing lots of data. A more practical method to compress large disk images is to use the L<xz(1)> format and L<nbdkit-xz-plugin(1)>. +=head1 PARAMETERS + +=over 4 + +=item B<file=>FILENAME.gz + +Serve the compressed file named C<FILENAME.gz>. + +This parameter is required. + +In nbdkit E<ge> 1.7, C<file=> may be omitted. To ensure that the +filename does not end up being parsed accidentally as C<key=value>, +prefix relative paths with C<./> (absolute paths do not need +modification). + +=back + =head1 SEE ALSO L<nbdkit-xz-plugin(1)>, -- 2.18.0
--- docs/nbdkit.pod | 8 ++++---- filters/cow/nbdkit-cow-filter.pod | 2 +- plugins/xz/nbdkit-xz-plugin.pod | 5 +++++ plugins/xz/xz.c | 3 ++- tests/test-xz.c | 2 +- 5 files changed, 13 insertions(+), 7 deletions(-) diff --git a/docs/nbdkit.pod b/docs/nbdkit.pod index cf1c5ca..d650283 100644 --- a/docs/nbdkit.pod +++ b/docs/nbdkit.pod @@ -75,7 +75,7 @@ Serve only the first partition from compressed disk image F<disk.img.xz>, combining L<nbdkit-xz-plugin(1)> and L<nbdkit-partition-filter(1)>: - nbdkit --filter=partition xz file=disk.img.xz partition=1 + nbdkit --filter=partition xz disk.img.xz partition=1 To understand this command line: @@ -83,9 +83,9 @@ To understand this command line: │ ┌─────┴────┐ │ │ - nbdkit --filter=partition xz file=disk.img.xz partition=1 - │ │ - └───────────────┬───────────────────┘ + nbdkit --filter=partition xz disk.img.xz partition=1 + │ │ + └─────────────┬─────────────────┘ │ filter name and filter parameter diff --git a/filters/cow/nbdkit-cow-filter.pod b/filters/cow/nbdkit-cow-filter.pod index 80100e7..d5cd735 100644 --- a/filters/cow/nbdkit-cow-filter.pod +++ b/filters/cow/nbdkit-cow-filter.pod @@ -65,7 +65,7 @@ L<nbdkit-xz-plugin(1)> only supports read access, but you can provide temporary write access by doing (although this does B<not> save changes to the file): - nbdkit --filter=cow xz file=disk.xz + nbdkit --filter=cow xz disk.xz =head1 CREATING A DIFF WITH QEMU-IMG diff --git a/plugins/xz/nbdkit-xz-plugin.pod b/plugins/xz/nbdkit-xz-plugin.pod index 2c9ab6e..59b2bd7 100644 --- a/plugins/xz/nbdkit-xz-plugin.pod +++ b/plugins/xz/nbdkit-xz-plugin.pod @@ -59,6 +59,11 @@ Serve the file named C<FILENAME.xz>. This parameter is required. +In nbdkit E<ge> 1.7, C<file=> may be omitted. To ensure that the +filename does not end up being parsed accidentally as C<key=value>, +prefix relative paths with C<./> (absolute paths do not need +modification). + =item B<maxblock=>SIZE The maximum block size that the plugin will read. The plugin will diff --git a/plugins/xz/xz.c b/plugins/xz/xz.c index f45e489..a88d713 100644 --- a/plugins/xz/xz.c +++ b/plugins/xz/xz.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2013 Red Hat Inc. + * Copyright (C) 2013-2018 Red Hat Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -250,6 +250,7 @@ static struct nbdkit_plugin plugin = { .config = xz_config, .config_complete = xz_config_complete, .config_help = xz_config_help, + .magic_config_key = "file", .open = xz_open, .close = xz_close, .get_size = xz_get_size, diff --git a/tests/test-xz.c b/tests/test-xz.c index ef58d59..46b1d04 100644 --- a/tests/test-xz.c +++ b/tests/test-xz.c @@ -51,7 +51,7 @@ main (int argc, char *argv[]) int r; char *data; - if (test_start_nbdkit ("xz", "-r", "file=disk.xz", NULL) == -1) + if (test_start_nbdkit ("xz", "-r", "disk.xz", NULL) == -1) exit (EXIT_FAILURE); g = guestfs_create (); -- 2.18.0
participants (2)
-
Eric Blake -
Richard W.M. Jones