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-8, and extends the set of reserved return values to 9-15. We figure
it is unlikely that anyone is using status 4-15 with the intent that
it behaves identically to status 1; more importantly, the choice of
soft disconnect with error for status 8 is the one least likely to
break such an existing sh or eval script.
For the testsuite, I only covered the eval plugin; but since it shares
common code with the sh plugin, both styles should work.
A note about the pod documentation: when the argument to =item would
otherwise appear to be a single number, the use of S<> to mask it is
necessary.
---
plugins/sh/nbdkit-sh-plugin.pod | 41 +++++-
tests/Makefile.am | 2 +
plugins/sh/call.h | 7 +-
plugins/sh/call.c | 92 ++++++-------
plugins/sh/methods.c | 2 +-
tests/test-eval-disconnect.sh | 236 ++++++++++++++++++++++++++++++++
6 files changed, 324 insertions(+), 56 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 1c539599..2905eced 100644
--- a/plugins/sh/nbdkit-sh-plugin.pod
+++ b/plugins/sh/nbdkit-sh-plugin.pod
@@ -96,7 +96,7 @@ 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,42 @@ The requested method is not supported by the script.
For methods which return booleans, this code indicates false.
-=item 4, 5, 6, 7
+=item 4 and 5
-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).
+The client will usually get a response before shutdown is complete
+(although this is racy); so once the shutdown is requested, code 4
+then behaves like code 0 (stderr is ignored, and the server tries to
+return success), and code 5 behaves like code 1 (the server tries to
+return an error to the client parsed from stderr, although a missing
+error defaults to C<ESHUTDOWN> instead of C<EIO>).
+
+=item S<6>
+
+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 7 and 8
+
+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, code 7 then behaves like
+code 0 (stderr is ignored, and the server tries to return success),
+and code 8 behaves like code 1 (the server tries to return an error to
+the client parsed from stderr, although a missing error defaults to
+C<ESHUTDOWN> instead of C<EIO>).
+
+=item 9-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
@@ -583,4 +616,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 a4e93686..b58d2d65 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -769,6 +769,7 @@ TESTS += \
test-eval-exports.sh \
test-eval-cache.sh \
test-eval-dump-plugin.sh \
+ test-eval-disconnect.sh \
$(NULL)
EXTRA_DIST += \
test-eval.sh \
@@ -776,6 +777,7 @@ EXTRA_DIST += \
test-eval-exports.sh \
test-eval-cache.sh \
test-eval-dump-plugin.sh \
+ test-eval-disconnect.sh \
$(NULL)
# file plugin test.
diff --git a/plugins/sh/call.h b/plugins/sh/call.h
index fab77a3c..57da7e98 100644
--- a/plugins/sh/call.h
+++ b/plugins/sh/call.h
@@ -52,8 +52,13 @@ typedef enum exit_code {
ERROR = 1, /* all script error codes are mapped to this */
MISSING = 2, /* method missing */
RET_FALSE = 3, /* script exited with code 3 meaning false */
+ SHUTDOWN_OK = 4, /* call nbdkit_shutdown(), then return OK */
+ SHUTDOWN_ERR = 5, /* call nbdkit_shutdown(), then return ERROR */
+ DISC_FORCE = 6, /* call nbdkit_disconnect(true); return irrelevant */
+ DISC_SOFT_OK = 7, /* call nbdkit_disconnect(false), return OK */
+ DISC_SOFT_ERR = 8, /* call nbdkit_disconnect(false), return ERROR */
/* Adjust methods.c:sh_dump_plugin when defining new codes */
- /* 4-7 is reserved since 1.8; handle like ERROR for now */
+ /* 9-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..64f29079 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
@@ -44,6 +44,7 @@
#include <poll.h>
#include <sys/types.h>
#include <sys/wait.h>
+#include <stdbool.h>
#include <nbdkit-plugin.h>
@@ -397,18 +398,48 @@ 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_OK:
+ nbdkit_shutdown ();
+ return OK;
+
+ case SHUTDOWN_ERR:
+ nbdkit_shutdown ();
+ err = ESHUTDOWN;
+ break;
+
+ case DISC_SOFT_OK:
+ nbdkit_disconnect (false);
+ return OK;
+
+ case DISC_FORCE:
+ case DISC_SOFT_ERR:
+ nbdkit_disconnect (code == DISC_FORCE);
+ err = ESHUTDOWN;
+ break;
+
+ case ERROR:
+ default:
err = EIO;
- goto no_error_message;
+ break;
}
+ assert (ebuf->ptr); /* Even if empty, ebuf was NUL-terminated in call3 */
+
/* Recognize the errno values that match NBD protocol errors */
if (ascii_strncasecmp (ebuf->ptr, "EPERM", 5) == 0) {
err = EPERM;
@@ -463,11 +494,6 @@ handle_script_error (const char *argv0, string *ebuf)
err = EFBIG;
skip = 5;
}
- /* Default to EIO. */
- else {
- err = EIO;
- skip = 0;
- }
if (skip && ebuf->ptr[skip]) {
if (!ascii_isspace (ebuf->ptr[skip])) {
@@ -495,13 +521,13 @@ handle_script_error (const char *argv0, string *ebuf)
nbdkit_error ("%s: %s", argv0, &ebuf->ptr[skip]);
}
else {
- no_error_message:
nbdkit_error ("%s: script exited with error, "
"but did not print an error message on stderr", argv0);
}
/* Set errno. */
errno = err;
+ return ERROR;
}
/* Call the script with parameters. Don't write to stdin or read from
@@ -516,19 +542,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 +555,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 +572,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/plugins/sh/methods.c b/plugins/sh/methods.c
index 4a75a3a0..834393b2 100644
--- a/plugins/sh/methods.c
+++ b/plugins/sh/methods.c
@@ -59,7 +59,7 @@ sh_dump_plugin (void)
CLEANUP_FREE_STRING string o = empty_vector;
/* Dump information about the sh/eval features */
- printf ("max_known_status=%d\n", RET_FALSE);
+ printf ("max_known_status=%d\n", DISC_SOFT_ERR);
/* Dump any additional information from the script */
if (script) {
diff --git a/tests/test-eval-disconnect.sh b/tests/test-eval-disconnect.sh
new file mode 100755
index 00000000..e7853f31
--- /dev/null
+++ b/tests/test-eval-disconnect.sh
@@ -0,0 +1,236 @@
+#!/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 '
+}
+check_dead()
+{
+ # 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
+}
+serve
+
+# Noisy status 0 (OK) 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 (ERROR) 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 (ERROR) 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 4 (SHUTDOWN_OK) kills the server. It is racy whether client
+# sees success or if the connection is 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
+check_dead
+serve
+
+# Noisy status 4 (SHUTDOWN_OK) kills the server. It is racy whether client
+# sees success or if the connection is 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
+check_dead
+serve
+
+# Silent status 5 (SHUTDOWN_ERR) kills the server. It is racy whether client
+# sees implied ESHUTDOWN or if the connection dies with libnbd giving 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.ESHUTDOWN or ex.errnum == errno.ENOTCONN
+EOF
+check_dead
+serve
+
+# Noisy status 5 (SHUTDOWN_ERR) kills the server. It is racy whether client
+# sees EPERM or if the connection is killed with libnbd giving 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.EPERM or ex.errnum == errno.ENOTCONN
+EOF
+check_dead
+serve
+
+# Silent status 6 (DISC_FORCE) kills socket; libnbd detects as ENOTCONN
+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.ENOTCONN
+assert h.aio_is_ready() is False
+EOF
+
+# Noisy status 6 (DISC_FORCE) kills socket; libnbd detects as ENOTCONN
+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.ENOTCONN
+assert h.aio_is_ready() is False
+EOF
+
+# Silent status 7 (DISC_SOFT_OK) succeeds, but next command gives ESHUTDOWN
+nbdsh -u "nbd+unix:///7?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 7 (DISC_SOFT_OK) succeeds, but next command gives ESHUTDOWN
+nbdsh -u "nbd+unix:///-7?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
+
+# Silent status 8 (DISC_SOFT_ERR) fails with implied ESHUTDOWN, then next
+# command gives ESHUTDOWN
+nbdsh -u "nbd+unix:///8?socket=$sock" -c - <<\EOF
+import errno
+try:
+ h.pwrite(b"1", 0)
+ assert False
+except nbd.Error as ex:
+ assert ex.errnum == errno.ESHUTDOWN
+try:
+ h.flush()
+ assert False
+except nbd.Error as ex:
+ assert ex.errnum == errno.ESHUTDOWN
+h.shutdown()
+EOF
+
+# Noisy status 8 (DISC_SOFT_ERR) fails with explicit EPERM, then next
+# command gives ESHUTDOWN
+nbdsh -u "nbd+unix:///-8?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
--
2.38.1
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
