[PATCH nbdkit 0/3] tests: Test export flags (eflags).
Some feature additions to the shell script plugin allow us to test the export flags field reasonably easily. Rich.
This implements something like a readonly 1MB disk reading as zeroes: nbdkit sh script=- <<'EOF' case "$1" in get_size) echo 1M ;; pread) dd if=/dev/zero count=$3 iflag=count_bytes ;; *) exit 2 ;; esac EOF Use of "-" is analogous to reading passwords from stdin. --- plugins/sh/nbdkit-sh-plugin.pod | 17 ++++++++++ plugins/sh/sh.c | 55 ++++++++++++++++++++++++++++++++- 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/plugins/sh/nbdkit-sh-plugin.pod b/plugins/sh/nbdkit-sh-plugin.pod index 371de5f..0644c36 100644 --- a/plugins/sh/nbdkit-sh-plugin.pod +++ b/plugins/sh/nbdkit-sh-plugin.pod @@ -6,6 +6,10 @@ nbdkit-sh-plugin - nbdkit shell, script or executable plugin nbdkit sh /path/to/script [arguments...] + nbdkit sh script=- [arguments...] <<'EOF' + ... shell script here ... + EOF + =head1 DESCRIPTION C<nbdkit-sh-plugin> allows you to write plugins for L<nbdkit(1)> using @@ -26,6 +30,19 @@ like this: You may have to add further C<key=value> arguments to the command line. +=head2 Inline shell scripts + +It is also possible to write a shell script plugin "inline" using C<-> +as the name of the script, like this: + + nbdkit sh script=- <<'EOF' + case "$1" in + get_size) echo 1M ;; + pread) dd if=/dev/zero count=$3 iflag=count_bytes ;; + *) exit 2 ;; + esac + EOF + =head1 WRITING AN NBDKIT SH PLUGIN For an example plugin written in Bash, see: diff --git a/plugins/sh/sh.c b/plugins/sh/sh.c index 368f1eb..fcd839b 100644 --- a/plugins/sh/sh.c +++ b/plugins/sh/sh.c @@ -39,6 +39,7 @@ #include <string.h> #include <unistd.h> #include <errno.h> +#include <sys/stat.h> #include <nbdkit-plugin.h> @@ -115,6 +116,50 @@ sh_dump_plugin (void) } } +/* This implements the "inline script" feature (script=-). Read stdin + * into a temporary file and return the name of the file which the + * caller must free. For convenience we put the temporary file into + * tmpdir but that's an implementation detail. + */ +static char * +inline_script (void) +{ + const char scriptname[] = "inline-script.sh"; + char *filename = NULL; + char *cmd = NULL; + + if (asprintf (&filename, "%s/%s", tmpdir, scriptname) == -1) { + nbdkit_error ("asprintf: %m"); + goto err; + } + + /* Safe because both the tmpdir and script name are controlled by us + * and don't contain characters that need quoting. + */ + if (asprintf (&cmd, "cat > %s", filename) == -1) { + nbdkit_error ("asprintf: %m"); + goto err; + } + + if (system (cmd) != 0) { + nbdkit_error ("sh: failed to copy inline script to temporary file"); + goto err; + } + + if (chmod (filename, 0500) == -1) { + nbdkit_error ("chmod: %s: %m", filename); + goto err; + } + + free (cmd); + return filename; + + err: + free (filename); + free (cmd); + return NULL; +} + static int sh_config (const char *key, const char *value) { @@ -124,7 +169,15 @@ sh_config (const char *key, const char *value) nbdkit_error ("the first parameter must be script=/path/to/script"); return -1; } - script = nbdkit_realpath (value); + + /* If the script name is not "-" then it's expected to be a + * filename, otherwise it's an inline script which must be read + * into a temporary file. Either way we want an absolute path. + */ + if (strcmp (value, "-") != 0) + script = nbdkit_realpath (value); + else + script = inline_script (); if (script == NULL) return -1; -- 2.19.2
On 12/14/18 4:16 PM, Richard W.M. Jones wrote:
This implements something like a readonly 1MB disk reading as zeroes:
nbdkit sh script=- <<'EOF' case "$1" in get_size) echo 1M ;; pread) dd if=/dev/zero count=$3 iflag=count_bytes ;; *) exit 2 ;; esac EOF
Use of "-" is analogous to reading passwords from stdin. ---
@@ -26,6 +30,19 @@ like this: You may have to add further C<key=value> arguments to the command line.
+=head2 Inline shell scripts + +It is also possible to write a shell script plugin "inline" using C<-> +as the name of the script, like this: + + nbdkit sh script=- <<'EOF' + case "$1" in + get_size) echo 1M ;; + pread) dd if=/dev/zero count=$3 iflag=count_bytes ;; + *) exit 2 ;; + esac + EOF +
Worth a specific mention that 'script=' is required, since otherwise the '-' looks like an option to nbdkit proper rather than a script parameter? Probably not worth a mention that a local file named '-' can still be invoked via [script=]./- (there, script is optional because the leading ./ avoids the leading -).
+static char * +inline_script (void) +{ + const char scriptname[] = "inline-script.sh"; + char *filename = NULL; + char *cmd = NULL; + + if (asprintf (&filename, "%s/%s", tmpdir, scriptname) == -1) { + nbdkit_error ("asprintf: %m"); + goto err; + } + + /* Safe because both the tmpdir and script name are controlled by us + * and don't contain characters that need quoting. + */ + if (asprintf (&cmd, "cat > %s", filename) == -1) {
Useful comment :) And I verified it is true. Looks good. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Fri, Dec 14, 2018 at 04:31:32PM -0600, Eric Blake wrote:
On 12/14/18 4:16 PM, Richard W.M. Jones wrote:
This implements something like a readonly 1MB disk reading as zeroes:
nbdkit sh script=- <<'EOF' case "$1" in get_size) echo 1M ;; pread) dd if=/dev/zero count=$3 iflag=count_bytes ;; *) exit 2 ;; esac EOF
Use of "-" is analogous to reading passwords from stdin. ---
@@ -26,6 +30,19 @@ like this: You may have to add further C<key=value> arguments to the command line. +=head2 Inline shell scripts + +It is also possible to write a shell script plugin "inline" using C<-> +as the name of the script, like this: + + nbdkit sh script=- <<'EOF' + case "$1" in + get_size) echo 1M ;; + pread) dd if=/dev/zero count=$3 iflag=count_bytes ;; + *) exit 2 ;; + esac + EOF +
Worth a specific mention that 'script=' is required, since otherwise the '-' looks like an option to nbdkit proper rather than a script parameter?
Actually I tried using just "-" (for the first time, I didn't try it before) and it works. So the above can be written: nbdkit sh - <<'EOF' case "$1" in get_size) echo 1M ;; pread) dd if=/dev/zero count=$3 iflag=count_bytes ;; *) exit 2 ;; esac EOF At least on Linux. I wonder if that also works on FreeBSD etc and if it depends on an implementation detail of the parser. I'll have to do some more investigations. Rich.
Probably not worth a mention that a local file named '-' can still be invoked via [script=]./- (there, script is optional because the leading ./ avoids the leading -).
+static char * +inline_script (void) +{ + const char scriptname[] = "inline-script.sh"; + char *filename = NULL; + char *cmd = NULL; + + if (asprintf (&filename, "%s/%s", tmpdir, scriptname) == -1) { + nbdkit_error ("asprintf: %m"); + goto err; + } + + /* Safe because both the tmpdir and script name are controlled by us + * and don't contain characters that need quoting. + */ + if (asprintf (&cmd, "cat > %s", filename) == -1) {
Useful comment :) And I verified it is true.
Looks good.
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
On Sat, Dec 15, 2018 at 07:22:41AM +0000, Richard W.M. Jones wrote:
On Fri, Dec 14, 2018 at 04:31:32PM -0600, Eric Blake wrote:
On 12/14/18 4:16 PM, Richard W.M. Jones wrote:
This implements something like a readonly 1MB disk reading as zeroes:
nbdkit sh script=- <<'EOF' case "$1" in get_size) echo 1M ;; pread) dd if=/dev/zero count=$3 iflag=count_bytes ;; *) exit 2 ;; esac EOF
Use of "-" is analogous to reading passwords from stdin. ---
@@ -26,6 +30,19 @@ like this: You may have to add further C<key=value> arguments to the command line. +=head2 Inline shell scripts + +It is also possible to write a shell script plugin "inline" using C<-> +as the name of the script, like this: + + nbdkit sh script=- <<'EOF' + case "$1" in + get_size) echo 1M ;; + pread) dd if=/dev/zero count=$3 iflag=count_bytes ;; + *) exit 2 ;; + esac + EOF +
Worth a specific mention that 'script=' is required, since otherwise the '-' looks like an option to nbdkit proper rather than a script parameter?
Actually I tried using just "-" (for the first time, I didn't try it before) and it works. So the above can be written:
nbdkit sh - <<'EOF' case "$1" in get_size) echo 1M ;; pread) dd if=/dev/zero count=$3 iflag=count_bytes ;; *) exit 2 ;; esac EOF
At least on Linux. I wonder if that also works on FreeBSD etc and if it depends on an implementation detail of the parser. I'll have to do some more investigations.
This works on FreeBSD, and looking at the way that coreutils cat(1) is implemented (as an example of presumably portable code) it also simply assumes that "-" is parsed as a non-option. I'm going to rework this, along with the other review comments you made. Thanks, Rich.
Rich.
Probably not worth a mention that a local file named '-' can still be invoked via [script=]./- (there, script is optional because the leading ./ avoids the leading -).
+static char * +inline_script (void) +{ + const char scriptname[] = "inline-script.sh"; + char *filename = NULL; + char *cmd = NULL; + + if (asprintf (&filename, "%s/%s", tmpdir, scriptname) == -1) { + nbdkit_error ("asprintf: %m"); + goto err; + } + + /* Safe because both the tmpdir and script name are controlled by us + * and don't contain characters that need quoting. + */ + if (asprintf (&cmd, "cat > %s", filename) == -1) {
Useful comment :) And I verified it is true.
Looks good.
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
This allows us to add the following callbacks: - can_zero returns boolean - can_fua should print "none", "emulate" or "native" Furthermore the following callbacks are modified in a backwards compatible manner: - pwrite adding flags parameter - trim adding flags parameter This change is not backwards compatible: - zero may_trim parameter replaced by flags parameter --- plugins/sh/nbdkit-sh-plugin.pod | 41 +++++++++++--- plugins/sh/sh.c | 99 +++++++++++++++++++++++++++------ 2 files changed, 115 insertions(+), 25 deletions(-) diff --git a/plugins/sh/nbdkit-sh-plugin.pod b/plugins/sh/nbdkit-sh-plugin.pod index 0644c36..ebd3cfb 100644 --- a/plugins/sh/nbdkit-sh-plugin.pod +++ b/plugins/sh/nbdkit-sh-plugin.pod @@ -196,17 +196,21 @@ This method is required. =item C<can_trim> +=item C<can_zero> + Unlike in other languages, you B<must> provide the C<can_*> methods otherwise they are assumed to all return false and your C<pwrite>, -C<flush> and C<trim> methods will never be called. The reason for -this is obscure: In other languages we can detect if (eg) a C<pwrite> -method is defined and synthesize an appropriate response if no actual -C<can_write> method is defined. However detecting if methods are -present without running them is not possible with this plugin. +C<flush>, C<trim> and C<zero> methods will never be called. The +reason for this is obscure: In other languages we can detect if (eg) a +C<pwrite> method is defined and synthesize an appropriate response if +no actual C<can_write> method is defined. However detecting if +methods are present without running them is not possible with this +plugin. /path/to/script can_write <handle> /path/to/script can_flush <handle> /path/to/script can_trim <handle> + /path/to/script can_zero <handle> The script should exit with code C<0> for true or code C<3> for false. @@ -216,6 +220,15 @@ The script should exit with code C<0> for true or code C<3> for false. The script should exit with code C<0> for true or code C<3> for false. +=item C<can_fua> + + /path/to/script can_fua <handle> + +This controls Forced Unit Access (FUA) behaviour of the core server. +Unlike the other C<can_*> callbacks, this one is I<not> a boolean. It +must print either "none", "emulate" or "native". The meaning of these +is described in L<nbdkit-plugin(3)>. + =item C<pread> /path/to/script pread <handle> <count> <offset> @@ -227,10 +240,13 @@ This method is required. =item C<pwrite> - /path/to/script pwrite <handle> <count> <offset> + /path/to/script pwrite <handle> <count> <offset> <flags> The script should read the binary data to be written from stdin. +The C<flags> parameter can be an empty string or C<"fua">. In future +a comma-separated list of flags may be present. + Unlike in other languages, if you provide a C<pwrite> method you B<must> also provide a C<can_write> method which exits with code C<0> (true). @@ -245,16 +261,23 @@ B<must> also provide a C<can_flush> method which exits with code C<0> =item C<trim> - /path/to/script trim <handle> <count> <offset> + /path/to/script trim <handle> <count> <offset> <flags> + +The C<flags> parameter can be an empty string or C<"fua">. In future +a comma-separated list of flags may be present. Unlike in other languages, if you provide a C<trim> method you B<must> also provide a C<can_trim> method which exits with code C<0> (true). =item C<zero> - /path/to/script zero <handle> <count> <offset> <may_trim> + /path/to/script zero <handle> <count> <offset> <flags> -C<may_trim> will be either C<true> or C<false>. +The C<flags> parameter can be an empty string or C<"fua">. In future +a comma-separated list of flags may be present. + +Unlike in other languages, if you provide a C<zero> method you B<must> +also provide a C<can_zero> method which exits with code C<0> (true). =back diff --git a/plugins/sh/sh.c b/plugins/sh/sh.c index fcd839b..58693fb 100644 --- a/plugins/sh/sh.c +++ b/plugins/sh/sh.c @@ -41,6 +41,8 @@ #include <errno.h> #include <sys/stat.h> +#define NBDKIT_API_VERSION 2 + #include <nbdkit-plugin.h> #include "call.h" @@ -360,7 +362,8 @@ sh_get_size (void *handle) } static int -sh_pread (void *handle, void *buf, uint32_t count, uint64_t offset) +sh_pread (void *handle, void *buf, uint32_t count, uint64_t offset, + uint32_t flags) { char *h = handle; char cbuf[32], obuf[32]; @@ -404,12 +407,14 @@ sh_pread (void *handle, void *buf, uint32_t count, uint64_t offset) } static int -sh_pwrite (void *handle, const void *buf, - uint32_t count, uint64_t offset) +sh_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset, + uint32_t flags) { char *h = handle; char cbuf[32], obuf[32]; - const char *args[] = { script, "pwrite", h, cbuf, obuf, NULL }; + /* In future, comma-separated list of flags. */ + const char *sflags = flags & NBDKIT_FLAG_FUA ? "fua" : ""; + const char *args[] = { script, "pwrite", h, cbuf, obuf, sflags, NULL }; snprintf (cbuf, sizeof cbuf, "%" PRIu32, count); snprintf (obuf, sizeof obuf, "%" PRIu64, offset); @@ -467,12 +472,6 @@ sh_can_flush (void *handle) return boolean_method (handle, "can_flush"); } -static int -sh_can_trim (void *handle) -{ - return boolean_method (handle, "can_trim"); -} - static int sh_is_rotational (void *handle) { @@ -480,7 +479,70 @@ sh_is_rotational (void *handle) } static int -sh_flush (void *handle) +sh_can_trim (void *handle) +{ + return boolean_method (handle, "can_trim"); +} + +static int +sh_can_zero (void *handle) +{ + return boolean_method (handle, "can_zero"); +} + +static int +sh_can_fua (void *handle) +{ + char *h = handle; + const char *args[] = { script, "can_fua", h, NULL }; + char *s = NULL; + size_t slen; + int r; + + switch (call_read (&s, &slen, args)) { + case OK: + if (slen > 0 && s[slen-1] == '\n') + s[slen-1] = '\0'; + if (strcmp (s, "none") == 0) + r = NBDKIT_FUA_NONE; + else if (strcmp (s, "emulate") == 0) + r = NBDKIT_FUA_EMULATE; + else if (strcmp (s, "native") == 0) + r = NBDKIT_FUA_NATIVE; + else { + nbdkit_error ("%s: could not parse output from can_fua method: %s", + script, s); + free (s); + return -1; + } + free (s); + return r; + + case MISSING: + free (s); + /* NBDKIT_FUA_EMULATE means that nbdkit will call .flush. However + * we cannot know if that callback exists, so the safest default + * is to return NBDKIT_FUA_NONE. + */ + return NBDKIT_FUA_NONE; + + case ERROR: + free (s); + return -1; + + case RET_FALSE: + free (s); + nbdkit_error ("%s: %s method returned unexpected code (3/false)", + script, "can_fua"); + errno = EIO; + return -1; + + default: abort (); + } +} + +static int +sh_flush (void *handle, uint32_t flags) { char *h = handle; const char *args[] = { script, "flush", h, NULL }; @@ -507,11 +569,13 @@ sh_flush (void *handle) } static int -sh_trim (void *handle, uint32_t count, uint64_t offset) +sh_trim (void *handle, uint32_t count, uint64_t offset, uint32_t flags) { char *h = handle; char cbuf[32], obuf[32]; - const char *args[] = { script, "trim", h, cbuf, obuf, NULL }; + /* In future, comma-separated list of flags. */ + const char *sflags = flags & NBDKIT_FLAG_FUA ? "fua" : ""; + const char *args[] = { script, "trim", h, cbuf, obuf, sflags, NULL }; snprintf (cbuf, sizeof cbuf, "%" PRIu32, count); snprintf (obuf, sizeof obuf, "%" PRIu64, offset); @@ -538,12 +602,13 @@ sh_trim (void *handle, uint32_t count, uint64_t offset) } static int -sh_zero (void *handle, uint32_t count, uint64_t offset, int may_trim) +sh_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags) { char *h = handle; char cbuf[32], obuf[32]; - const char *args[] = { script, "zero", h, cbuf, obuf, - may_trim ? "true" : "false", NULL }; + /* In future, comma-separated list of flags. */ + const char *sflags = flags & NBDKIT_FLAG_FUA ? "fua" : ""; + const char *args[] = { script, "zero", h, cbuf, obuf, sflags, NULL }; snprintf (cbuf, sizeof cbuf, "%" PRIu32, count); snprintf (obuf, sizeof obuf, "%" PRIu64, offset); @@ -596,6 +661,8 @@ static struct nbdkit_plugin plugin = { .can_flush = sh_can_flush, .is_rotational = sh_is_rotational, .can_trim = sh_can_trim, + .can_zero = sh_can_zero, + .can_fua = sh_can_fua, .pread = sh_pread, .pwrite = sh_pwrite, -- 2.19.2
On 12/14/18 4:16 PM, Richard W.M. Jones wrote:
This allows us to add the following callbacks:
- can_zero returns boolean - can_fua should print "none", "emulate" or "native"
Furthermore the following callbacks are modified in a backwards compatible manner:
- pwrite adding flags parameter - trim adding flags parameter
This change is not backwards compatible:
- zero may_trim parameter replaced by flags parameter --- plugins/sh/nbdkit-sh-plugin.pod | 41 +++++++++++--- plugins/sh/sh.c | 99 +++++++++++++++++++++++++++------ 2 files changed, 115 insertions(+), 25 deletions(-)
+=item C<can_fua> + + /path/to/script can_fua <handle> + +This controls Forced Unit Access (FUA) behaviour of the core server. +Unlike the other C<can_*> callbacks, this one is I<not> a boolean. It +must print either "none", "emulate" or "native". The meaning of these
Worth adding 'to stdout'?
+is described in L<nbdkit-plugin(3)>. + =item C<pread>
/path/to/script pread <handle> <count> <offset> @@ -227,10 +240,13 @@ This method is required.
=item C<pwrite>
- /path/to/script pwrite <handle> <count> <offset> + /path/to/script pwrite <handle> <count> <offset> <flags>
The script should read the binary data to be written from stdin.
+The C<flags> parameter can be an empty string or C<"fua">. In future
s/In future/In the future,/
+a comma-separated list of flags may be present.
Is the empty string "" always provided as an argument, or will the argument be omitted in that case?
+ Unlike in other languages, if you provide a C<pwrite> method you B<must> also provide a C<can_write> method which exits with code C<0> (true). @@ -245,16 +261,23 @@ B<must> also provide a C<can_flush> method which exits with code C<0>
=item C<trim>
- /path/to/script trim <handle> <count> <offset> + /path/to/script trim <handle> <count> <offset> <flags> + +The C<flags> parameter can be an empty string or C<"fua">. In future
Copy-and-paste "In future"
+a comma-separated list of flags may be present.
Unlike in other languages, if you provide a C<trim> method you B<must> also provide a C<can_trim> method which exits with code C<0> (true).
=item C<zero>
- /path/to/script zero <handle> <count> <offset> <may_trim> + /path/to/script zero <handle> <count> <offset> <flags>
-C<may_trim> will be either C<true> or C<false>. +The C<flags> parameter can be an empty string or C<"fua">. In future +a comma-separated list of flags may be present.
This one is wrong, since you have both fua and may_trim now.
+ + case RET_FALSE: + free (s); + nbdkit_error ("%s: %s method returned unexpected code (3/false)", + script, "can_fua");
Could we be nice and treat this as "none"?
@@ -538,12 +602,13 @@ sh_trim (void *handle, uint32_t count, uint64_t offset) }
static int -sh_zero (void *handle, uint32_t count, uint64_t offset, int may_trim) +sh_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags) { char *h = handle; char cbuf[32], obuf[32]; - const char *args[] = { script, "zero", h, cbuf, obuf, - may_trim ? "true" : "false", NULL }; + /* In future, comma-separated list of flags. */
While I commented about 'in future' being too terse in the .pod, I don't mind it being compressed in the .c file. On the other hand, the comment here is wrong, as sh_zero needs to handle both fua and may_trim flags.
+ const char *sflags = flags & NBDKIT_FLAG_FUA ? "fua" : "";
and thus this computation for sflags is incomplete. Otherwise, looking good. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
--- tests/Makefile.am | 3 + tests/test-eflags.sh | 222 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 225 insertions(+) diff --git a/tests/Makefile.am b/tests/Makefile.am index a01c47c..173d622 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -282,6 +282,9 @@ test_oldstyle_LDADD = libtest.la $(LIBGUESTFS_LIBS) endif HAVE_LIBGUESTFS +# Test export flags. +TESTS += test-eflags.sh + # common disk image shared with several tests if HAVE_GUESTFISH check_DATA += disk diff --git a/tests/test-eflags.sh b/tests/test-eflags.sh new file mode 100755 index 0000000..96ec311 --- /dev/null +++ b/tests/test-eflags.sh @@ -0,0 +1,222 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright (C) 2018 Red Hat Inc. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# * Neither the name of Red Hat nor the names of its contributors may be +# used to endorse or promote products derived from this software without +# specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, +# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A +# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF +# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND +# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT +# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF +# SUCH DAMAGE. + +# Test export flags. +# +# Run nbdkit with various can_* callbacks defined and with or without +# the -r flag, and check that nbdkit constructs the export flags +# controlling READ_ONLY, ROTATIONAL, SEND_TRIM, etc. as expected. +# +# We use the oldstyle (-o) protocol here because it's simpler to read +# out the eflags. We use the shell plugin because it gives maximum +# control over the can_* callbacks (at least, max without having to +# write a C plugin). + +source ./functions.sh +set -e + +if ! socat -h >/dev/null; then + echo "$0: 'socat' command not available" + exit 77 +fi + +# Check 'od' command exists and can do endian swaps (which the FreeBSD +# version cannot). +if ! od --endian=big </dev/null >/dev/null; then + echo "$0: 'od' command not available or doesn't support endian swaps" + exit 77 +fi + +files="eflags.out" +rm -f $files +cleanup_fn rm -f $files + +# The export flags. +# See also src/protocol.h +HAS_FLAGS=$(( 1 << 0 )) +READ_ONLY=$(( 1 << 1 )) +SEND_FLUSH=$(( 1 << 2 )) +SEND_FUA=$(( 1 << 3 )) +ROTATIONAL=$(( 1 << 4 )) +SEND_TRIM=$(( 1 << 5 )) +SEND_WRITE_ZEROES=$(( 1 << 6 )) + +all_flags="HAS_FLAGS READ_ONLY SEND_FLUSH SEND_FUA ROTATIONAL SEND_TRIM SEND_WRITE_ZEROES" + +do_nbdkit () +{ + nbdkit -v -U - -o "$@" sh script=- --run ' + socat -b 28 unix-connect:$unixsocket \ + exec:"dd bs=1 skip=26 count=2 of=eflags.out status=none" + ' + + # By adding 0 we convert the string to an integer (removing + # whitespace). + eflags=$((0 + $(od --endian=big -An -N2 -d < eflags.out))) + printf "eflags 0x%04x" $eflags + for f in $all_flags; do + [ $(( eflags & ${!f} )) -ne 0 ] && echo -n " $f" + done + echo +} + +fail () +{ + echo "$@" + exit 1 +} + +# no -r +# can_write=false + +do_nbdkit <<'EOF' +case "$1" in + get_size) echo 1M ;; + *) exit 2 ;; +esac +EOF + +[ $eflags -eq $(( HAS_FLAGS|READ_ONLY )) ] || + fail "expected HAS_FLAGS|READ_ONLY" + +# -r +# can_write=false + +do_nbdkit -r <<'EOF' +case "$1" in + get_size) echo 1M ;; + *) exit 2 ;; +esac +EOF + +[ $eflags -eq $(( HAS_FLAGS|READ_ONLY )) ] || + fail "expected HAS_FLAGS|READ_ONLY" + +# no -r +# can_write=true +# +# NBD_FLAG_SEND_WRITE_ZEROES is always set on writable connections +# even if can_zero returns false, because nbdkit reckons it can +# emulate zeroing using pwrite. + +do_nbdkit <<'EOF' +case "$1" in + get_size) echo 1M ;; + can_write) exit 0 ;; + *) exit 2 ;; +esac +EOF + +[ $eflags -eq $(( HAS_FLAGS|SEND_WRITE_ZEROES )) ] || + fail "expected HAS_FLAGS|SEND_WRITE_ZEROES" + +# -r +# can_write=true +# +# The -r flag overrides the plugin so this behaves as if can_write is +# false. + +do_nbdkit -r <<'EOF' +case "$1" in + get_size) echo 1M ;; + can_write) exit 0 ;; + *) exit 2 ;; +esac +EOF + +[ $eflags -eq $(( HAS_FLAGS|READ_ONLY )) ] || + fail "expected HAS_FLAGS|READ_ONLY" + +# can_write=false +# can_trim=true +# can_zero=true +# +# If writing is not possible then trim and zero are always disabled. + +do_nbdkit <<'EOF' +case "$1" in + get_size) echo 1M ;; + can_write) exit 3 ;; + can_trim) exit 0 ;; + can_zero) exit 0 ;; + *) exit 2 ;; +esac +EOF + +[ $eflags -eq $(( HAS_FLAGS|READ_ONLY )) ] || + fail "expected HAS_FLAGS|READ_ONLY" + +# can_write=true +# can_trim=true + +do_nbdkit <<'EOF' +case "$1" in + get_size) echo 1M ;; + can_write) exit 0 ;; + can_trim) exit 0 ;; + *) exit 2 ;; +esac +EOF + +[ $eflags -eq $(( HAS_FLAGS|SEND_TRIM|SEND_WRITE_ZEROES )) ] || + fail "expected HAS_FLAGS|SEND_TRIM|SEND_WRITE_ZEROES" + +# can_write=true +# is_rotational=true + +do_nbdkit <<'EOF' +case "$1" in + get_size) echo 1M ;; + can_write) exit 0 ;; + is_rotational) exit 0 ;; + *) exit 2 ;; +esac +EOF + +[ $eflags -eq $(( HAS_FLAGS|ROTATIONAL|SEND_WRITE_ZEROES )) ] || + fail "expected HAS_FLAGS|ROTATIONAL|SEND_WRITE_ZEROES" + +# can_write=true +# can_fua=native + +do_nbdkit <<'EOF' +case "$1" in + get_size) echo 1M ;; + can_write) exit 0 ;; + can_fua) echo "native" ;; + *) exit 2 ;; +esac +EOF + +[ $eflags -eq $(( HAS_FLAGS|SEND_FUA|SEND_WRITE_ZEROES )) ] || + fail "expected HAS_FLAGS|SEND_FUA|SEND_WRITE_ZEROES" -- 2.19.2
On 12/14/18 4:16 PM, Richard W.M. Jones wrote:
--- tests/Makefile.am | 3 + tests/test-eflags.sh | 222 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 225 insertions(+)
+# Test export flags. +# +# Run nbdkit with various can_* callbacks defined and with or without +# the -r flag, and check that nbdkit constructs the export flags +# controlling READ_ONLY, ROTATIONAL, SEND_TRIM, etc. as expected. +# +# We use the oldstyle (-o) protocol here because it's simpler to read +# out the eflags.
Indeed - the client has to talk in newstyle, but not in oldstyle :)
We use the shell plugin because it gives maximum +# control over the can_* callbacks (at least, max without having to +# write a C plugin). + +source ./functions.sh +set -e + +if ! socat -h >/dev/null; then + echo "$0: 'socat' command not available" + exit 77 +fi + +# Check 'od' command exists and can do endian swaps (which the FreeBSD +# version cannot).
Do we need endian swaps, or can we just read one byte at a time and then reconstruct things by hand (after all, we know the bytes are in network order, so we don't have to worry about whether we are running on a big- or little-endian machine to do the integer math to build up the 16-bit value from those two bytes).
+if ! od --endian=big </dev/null >/dev/null; then + echo "$0: 'od' command not available or doesn't support endian swaps" + exit 77 +fi + +files="eflags.out" +rm -f $files +cleanup_fn rm -f $files + +# The export flags. +# See also src/protocol.h
Comment may need tweaking if I finish my patches to move src/proto* to common/proto/. We'll see what lands first :)
+HAS_FLAGS=$(( 1 << 0 )) +READ_ONLY=$(( 1 << 1 )) +SEND_FLUSH=$(( 1 << 2 )) +SEND_FUA=$(( 1 << 3 )) +ROTATIONAL=$(( 1 << 4 )) +SEND_TRIM=$(( 1 << 5 )) +SEND_WRITE_ZEROES=$(( 1 << 6 )) + +all_flags="HAS_FLAGS READ_ONLY SEND_FLUSH SEND_FUA ROTATIONAL SEND_TRIM SEND_WRITE_ZEROES"
Well, that's all the flags we currently drive, although more flags exist (and we may eventually patch nbdkit to drive those, as well).
+ +do_nbdkit () +{ + nbdkit -v -U - -o "$@" sh script=- --run ' + socat -b 28 unix-connect:$unixsocket \ + exec:"dd bs=1 skip=26 count=2 of=eflags.out status=none"
dd can portably do byteswaps, with conv=swab, but then you have to know if you are big- or little-endian to know whether to use it.
+ ' + + # By adding 0 we convert the string to an integer (removing + # whitespace). + eflags=$((0 + $(od --endian=big -An -N2 -d < eflags.out)))
So back to my earlier thought of just reading 1 byte at a time and building up the value ourselves: eflags=$(( ($(od -An -N1 -tu1 eflags.out) << 8) | $(od -An -N1 -j1 -tu1 eflags.out) ))
+ printf "eflags 0x%04x" $eflags + for f in $all_flags; do + [ $(( eflags & ${!f} )) -ne 0 ] && echo -n " $f" + done + echo +} +
+ +# no -r +# can_write=true +# +# NBD_FLAG_SEND_WRITE_ZEROES is always set on writable connections +# even if can_zero returns false, because nbdkit reckons it can +# emulate zeroing using pwrite.
And we have the nozero filter if we want things otherwise (if you want to use that in this test). Otherwise, this looks sane. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
participants (2)
-
Eric Blake -
Richard W.M. Jones