Make it possible for the sh and eval plugins to disconnect a client or
shut down the entire nbdkit server by use of special return values.
Prior to this patch we had reserved 4-7 for future use; this defines
4-6, and extends the set of reserved return values to 7-15. We figure
it is unlikely that anyone is using status 8-15 with the intent that
it behaves identically to status 1.
For the testsuite, I only covered the eval plugin; but since it shares
common code with the sh plugin, both styles should work.
---
Finally got the testsuite additions for this in a state that I like.
plugins/sh/nbdkit-sh-plugin.pod | 37 ++++++-
tests/Makefile.am | 2 +
plugins/sh/call.h | 9 +-
plugins/sh/call.c | 85 +++++++--------
tests/test-eval-disconnect.sh | 185 ++++++++++++++++++++++++++++++++
5 files changed, 268 insertions(+), 50 deletions(-)
create mode 100755 tests/test-eval-disconnect.sh
diff --git a/plugins/sh/nbdkit-sh-plugin.pod b/plugins/sh/nbdkit-sh-plugin.pod
index 2a55fdc9..37139e1b 100644
--- a/plugins/sh/nbdkit-sh-plugin.pod
+++ b/plugins/sh/nbdkit-sh-plugin.pod
@@ -96,4 +96,4 @@ The script should exit with specific exit codes:
The method was executed successfully.
-=item 1 and 8-127
+=item 1 and 16-255
There was an error. The script may print on stderr an errno name,
optionally followed by whitespace and a message, for example:
@@ -123,9 +123,38 @@ The requested method is not supported by the script.
For methods which return booleans, this code indicates false.
-=item 4, 5, 6, 7
+=item S<4>
-These exit codes are reserved for future use.
+Triggers a call to the C function C<nbdkit_shutdown>, which requests
+an asynchronous exit of the nbdkit server (disconnecting all clients).
+Since the client may get a response before shutdown is complete,
+nbdkit then treats empty stderr as success for the current callback,
+and parses non-empty stderr as if the script had exited with code
+S<1>.
+
+=item S<5>
+
+Triggers a call to the C function C<nbdkit_disconnect> with C<force>
+set to true, which requests an abrupt disconnect of the current
+client. The contents of stderr are irrelevant with this status, since
+the client will not get a response.
+
+=item S<6>
+
+Triggers a call to the C function C<nbdkit_disconnect> with C<force>
+set to false, which requests a soft disconnect of the current client
+(future client requests are rejected with C<ESHUTDOWN> without calling
+into the plugin, but current requests may complete). Since the client
+will likely get the response to this command, nbdkit then treats empty
+stderr as success for the current callback, and parses non-empty
+stderr as if the script had exited with code S<1>.
+
+=item 7-15
+
+These exit codes are reserved for future use. Note that versions of
+nbdkit E<lt> 1.34 documented that codes S<8> through S<15> behaved
+like code S<1>; although it is unlikely that many scripts relied on
+this similarity in practice.
=back
@@ -578,4 +607,4 @@ Richard W.M. Jones
=head1 COPYRIGHT
-Copyright (C) 2018-2020 Red Hat Inc.
+Copyright (C) 2018-2022 Red Hat Inc.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index d59b797c..3994fc6a 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -768,12 +768,14 @@ TESTS += \
test-eval-file.sh \
test-eval-exports.sh \
test-eval-cache.sh \
+ test-eval-disconnect.sh \
$(NULL)
EXTRA_DIST += \
test-eval.sh \
test-eval-file.sh \
test-eval-exports.sh \
test-eval-cache.sh \
+ test-eval-disconnect.sh \
$(NULL)
# file plugin test.
diff --git a/plugins/sh/call.h b/plugins/sh/call.h
index 76de23a3..44767e81 100644
--- a/plugins/sh/call.h
+++ b/plugins/sh/call.h
@@ -1,5 +1,5 @@
/* nbdkit
- * Copyright (C) 2018 Red Hat Inc.
+ * Copyright (C) 2018-2022 Red Hat Inc.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -51,7 +51,12 @@ typedef enum exit_code {
OK = 0,
ERROR = 1, /* all script error codes are mapped to this */
MISSING = 2, /* method missing */
- RET_FALSE = 3 /* script exited with code 3 meaning false */
+ RET_FALSE = 3, /* script exited with code 3 meaning false */
+ SHUTDOWN = 4, /* call nbdkit_shutdown() before parsing stderr */
+ DISC_FORCE = 5, /* call nbdkit_disconnect(true) */
+ DISC_SOFT = 6, /* call nbdkit_disconnect(false) */
+ /* 7 is reserved since 1.8; handle like ERROR for now */
+ /* 8-15 are reserved since 1.34; handle like ERROR for now */
} exit_code;
extern exit_code call (const char **argv)
diff --git a/plugins/sh/call.c b/plugins/sh/call.c
index 2fa94d88..7d0f2b16 100644
--- a/plugins/sh/call.c
+++ b/plugins/sh/call.c
@@ -1,5 +1,5 @@
/* nbdkit
- * Copyright (C) 2018-2020 Red Hat Inc.
+ * Copyright (C) 2018-2022 Red Hat Inc.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -397,17 +397,47 @@ call3 (const char *wbuf, size_t wbuflen, /* sent to stdin (can be NULL) */
return ret;
}
-static void
-handle_script_error (const char *argv0, string *ebuf)
+/* Normalize return codes and parse error string. */
+static exit_code
+handle_script_error (const char *argv0, string *ebuf, exit_code code)
{
int err;
size_t skip = 0;
char *p;
- if (ebuf->len == 0) {
+ switch (code) {
+ case OK:
+ case MISSING:
+ case RET_FALSE:
+ /* Script successful. */
+ return code;
+
+ case SHUTDOWN:
+ /* Use trigger, then handle empty ebuf specially below */
+ nbdkit_shutdown ();
+ break;
+
+ case DISC_FORCE:
+ case DISC_SOFT:
+ /* Use trigger, then handle empty ebuf specially below */
+ nbdkit_disconnect (code == DISC_FORCE);
+ break;
+
+ case ERROR:
+ default:
+ /* Error even if ebuf is empty */
err = EIO;
+ goto parse;
+ }
+
+ assert (code >= SHUTDOWN && code <= DISC_SOFT);
+ if (ebuf->len == 0)
+ return OK;
+ err = ESHUTDOWN;
+
+ parse:
+ if (ebuf->len == 0)
goto no_error_message;
- }
/* Recognize the errno values that match NBD protocol errors */
if (ascii_strncasecmp (ebuf->ptr, "EPERM", 5) == 0) {
@@ -502,6 +532,7 @@ handle_script_error (const char *argv0, string *ebuf)
/* Set errno. */
errno = err;
+ return ERROR;
}
/* Call the script with parameters. Don't write to stdin or read from
@@ -516,19 +547,7 @@ call (const char **argv)
CLEANUP_FREE_STRING string ebuf = empty_vector;
r = call3 (NULL, 0, &rbuf, &ebuf, argv);
- switch (r) {
- case OK:
- case MISSING:
- case RET_FALSE:
- /* Script successful. */
- return r;
-
- case ERROR:
- default:
- /* Error case. */
- handle_script_error (argv[0], &ebuf);
- return ERROR;
- }
+ return handle_script_error (argv[0], &ebuf, r);
}
/* Call the script with parameters. Read from stdout and return the
@@ -541,20 +560,10 @@ call_read (string *rbuf, const char **argv)
CLEANUP_FREE_STRING string ebuf = empty_vector;
r = call3 (NULL, 0, rbuf, &ebuf, argv);
- switch (r) {
- case OK:
- case MISSING:
- case RET_FALSE:
- /* Script successful. */
- return r;
-
- case ERROR:
- default:
- /* Error case. */
+ r = handle_script_error (argv[0], &ebuf, r);
+ if (r == ERROR)
string_reset (rbuf);
- handle_script_error (argv[0], &ebuf);
- return ERROR;
- }
+ return r;
}
/* Call the script with parameters. Write to stdin of the script.
@@ -568,17 +577,5 @@ call_write (const char *wbuf, size_t wbuflen, const char **argv)
CLEANUP_FREE_STRING string ebuf = empty_vector;
r = call3 (wbuf, wbuflen, &rbuf, &ebuf, argv);
- switch (r) {
- case OK:
- case MISSING:
- case RET_FALSE:
- /* Script successful. */
- return r;
-
- case ERROR:
- default:
- /* Error case. */
- handle_script_error (argv[0], &ebuf);
- return ERROR;
- }
+ return handle_script_error (argv[0], &ebuf, r);
}
diff --git a/tests/test-eval-disconnect.sh b/tests/test-eval-disconnect.sh
new file mode 100755
index 00000000..8427e6fd
--- /dev/null
+++ b/tests/test-eval-disconnect.sh
@@ -0,0 +1,185 @@
+#!/usr/bin/env bash
+# nbdkit
+# Copyright (C) 2022 Red Hat Inc.
+#
+# 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.
+
+# Check shutdown/disconnect behavior triggered by special status values
+
+source ./functions.sh
+set -e
+set -x
+
+requires_plugin eval
+requires_nbdsh_uri
+
+sock=$(mktemp -u /tmp/nbdkit-test-sock.XXXXXX)
+files="eval-disconnect.pid $sock"
+rm -f $files
+cleanup_fn rm -f $files
+
+# Start nbdkit with a plugin that fails writes according to the export
+# name which must be numeric: a positive value leaves stderr empty, a
+# non-positive one outputs EPERM first. Serve multiple clients.
+serve()
+{
+ rm -f $files
+ start_nbdkit -P eval-disconnect.pid -U $sock eval \
+ get_size=' echo 1024 ' \
+ open=' if test $3 -le 0; then \
+ echo EPERM > $tmpdir/err; echo $((-$3)); \
+ else \
+ : > $tmpdir/err; echo $3; \
+ fi ' \
+ flush=' exit 0 ' \
+ pwrite=' cat >/dev/null; cat $tmpdir/err >&2; exit $2 '
+}
+serve
+
+# Noisy status 0 succeeds, despite text to stderr
+nbdsh -u "nbd+unix:///0?socket=$sock" -c - <<\EOF
+h.pwrite(b"1", 0)
+h.flush()
+h.shutdown()
+EOF
+
+# Silent status 1 fails; nbdkit turns lack of error into EIO
+nbdsh -u "nbd+unix:///1?socket=$sock" -c - <<\EOF
+import errno
+try:
+ h.pwrite(b"1", 0)
+ assert False
+except nbd.Error as ex:
+ assert ex.errnum == errno.EIO
+h.flush()
+h.shutdown()
+EOF
+
+# Noisy status 1 fails with supplied EPERM
+nbdsh -u "nbd+unix:///-1?socket=$sock" -c - <<\EOF
+import errno
+try:
+ h.pwrite(b"1", 0)
+ assert False
+except nbd.Error as ex:
+ assert ex.errnum == errno.EPERM
+h.flush()
+h.shutdown()
+EOF
+
+# Silent status 5 kills socket; libnbd detects dead socket as ENOTCONN
+nbdsh -u "nbd+unix:///5?socket=$sock" -c - <<\EOF
+import errno
+try:
+ h.pwrite(b"1", 0)
+ assert False
+except nbd.Error as ex:
+ assert ex.errnum == errno.ENOTCONN
+assert h.aio_is_ready() is False
+EOF
+
+# Noisy status 5 kills socket; libnbd detects dead socket as ENOTCONN
+nbdsh -u "nbd+unix:///-5?socket=$sock" -c - <<\EOF
+import errno
+try:
+ h.pwrite(b"1", 0)
+ assert False
+except nbd.Error as ex:
+ assert ex.errnum == errno.ENOTCONN
+assert h.aio_is_ready() is False
+EOF
+
+# Silent status 6 succeeds, but next command fails with ESHUTDOWN
+nbdsh -u "nbd+unix:///6?socket=$sock" -c - <<\EOF
+import errno
+h.pwrite(b"1", 0)
+try:
+ h.flush()
+ assert False
+except nbd.Error as ex:
+ assert ex.errnum == errno.ESHUTDOWN
+h.shutdown()
+EOF
+
+# Noisy status 6 fails with supplied EPERM, next command fails with ESHUTDOWN
+nbdsh -u "nbd+unix:///-6?socket=$sock" -c - <<\EOF
+import errno
+try:
+ h.pwrite(b"1", 0)
+ assert False
+except nbd.Error as ex:
+ assert ex.errnum == errno.EPERM
+try:
+ h.flush()
+ assert False
+except nbd.Error as ex:
+ assert ex.errnum == errno.ESHUTDOWN
+h.shutdown()
+EOF
+
+# Silent status 4 kills the server. It is racy whether client sees a reply
+# of success or sees the connection killed with libnbd giving ENOTCONN
+nbdsh -u "nbd+unix:///4?socket=$sock" -c - <<\EOF
+import errno
+try:
+ h.pwrite(b"1", 0)
+except nbd.Error as ex:
+ assert ex.errnum == errno.ENOTCONN
+EOF
+
+# Server should die shortly, if not already dead at this point.
+for (( i=0; i < 5; i++ )); do
+ kill -s 0 "$(cat eval-disconnect.pid)" || break
+ sleep 1
+done
+if [ $i = 5 ]; then
+ echo "nbdkit didn't exit fast enough"
+ exit 1
+fi
+
+# Restart server; noisy status 4 races between EPERM or ENOTCONN
+serve
+nbdsh -u "nbd+unix:///-4?socket=$sock" -c - <<\EOF
+import errno
+try:
+ h.pwrite(b"1", 0)
+ assert False
+except nbd.Error as ex:
+ assert ex.errnum == errno.EPERM or ex.errnum == errno.ENOTCONN
+EOF
+
+# Server should die shortly, if not already dead at this point.
+for (( i=0; i < 5; i++ )); do
+ kill -s 0 "$(cat eval-disconnect.pid)" || break
+ sleep 1
+done
+if [ $i = 5 ]; then
+ echo "nbdkit didn't exit fast enough"
+ exit 1
+fi
--
2.38.1
