5:21 p.m.
I hit another transient failure in libnbd CI when a poorly-written
eval script did not consume all of stdin during .pwrite. As behaving
as a data sink can be a somewhat reasonable feature of a
quickly-written sh or eval plugin, we should not be so insistent as
treating an EPIPE failure as an immediate return of EIO to the client.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
I probably need to add unit test coverage of this before committing
(although proving that I win the data race on a client process exiting
faster than the parent can write enough data to hit EPIPE is hard).
plugins/sh/nbdkit-sh-plugin.pod | 8 +++++++
plugins/sh/call.c | 38 ++++++++++++++++++++++-----------
2 files changed, 34 insertions(+), 12 deletions(-)
diff --git a/plugins/sh/nbdkit-sh-plugin.pod b/plugins/sh/nbdkit-sh-plugin.pod
index b2c946a0..8b83a5b3 100644
--- a/plugins/sh/nbdkit-sh-plugin.pod
+++ b/plugins/sh/nbdkit-sh-plugin.pod
@@ -505,6 +505,14 @@ 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).
+With nbdkit E<ge> 1.36, this method may return C<0> without consuming
+any data from stdin, and without producing any output, in order to
+behave as an intentional data sink. But in older versions, nbdkit
+would treat any C<EPIPE> failure in writing to your script as an error
+condition even if your script returns success; to avoid unintended
+failures, you may want to include C<"cat >/dev/null"> in a script
+intending to ignore the client's write requests.
+
=item C<flush>
/path/to/script flush <handle>
diff --git a/plugins/sh/call.c b/plugins/sh/call.c
index 888c6459..79c67a04 100644
--- a/plugins/sh/call.c
+++ b/plugins/sh/call.c
@@ -34,6 +34,7 @@
#include <assert.h>
#include <fcntl.h>
+#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <inttypes.h>
@@ -130,6 +131,7 @@ debug_call (const char **argv)
*/
static int
call3 (const char *wbuf, size_t wbuflen, /* sent to stdin (can be NULL) */
+ bool *pipe_full, /* set if wbuf not fully written */
string *rbuf, /* read from stdout */
string *ebuf, /* read from stderr */
const char **argv) /* script + parameters */
@@ -275,15 +277,8 @@ call3 (const char *wbuf, size_t wbuflen, /* sent to stdin (can be
NULL) */
r = write (pfds[0].fd, wbuf, wbuflen);
if (r == -1) {
if (errno == EPIPE) {
- /* We tried to write to the script but it didn't consume
- * the data. Probably the script exited without reading
- * from stdin. This is an error in the script.
- */
- nbdkit_error ("%s: write to script failed because of a broken pipe:
"
- "this can happen if the script exits without "
- "consuming stdin, which usually indicates a bug "
- "in the script",
- argv0);
+ *pipe_full = true;
+ r = wbuflen;
}
else
nbdkit_error ("%s: write: %m", argv0);
@@ -555,7 +550,7 @@ call (const char **argv)
CLEANUP_FREE_STRING string rbuf = empty_vector;
CLEANUP_FREE_STRING string ebuf = empty_vector;
- r = call3 (NULL, 0, &rbuf, &ebuf, argv);
+ r = call3 (NULL, 0, NULL, &rbuf, &ebuf, argv);
return handle_script_error (argv[0], &ebuf, r);
}
@@ -568,7 +563,7 @@ call_read (string *rbuf, const char **argv)
int r;
CLEANUP_FREE_STRING string ebuf = empty_vector;
- r = call3 (NULL, 0, rbuf, &ebuf, argv);
+ r = call3 (NULL, 0, NULL, rbuf, &ebuf, argv);
r = handle_script_error (argv[0], &ebuf, r);
if (r == ERROR)
string_reset (rbuf);
@@ -584,7 +579,26 @@ call_write (const char *wbuf, size_t wbuflen, const char **argv)
int r;
CLEANUP_FREE_STRING string rbuf = empty_vector;
CLEANUP_FREE_STRING string ebuf = empty_vector;
+ bool pipe_full = false;
- r = call3 (wbuf, wbuflen, &rbuf, &ebuf, argv);
+ r = call3 (wbuf, wbuflen, &pipe_full, &rbuf, &ebuf, argv);
+ if (pipe_full && r == OK) {
+ /* We allow scripts to intentionally ignore data, but they must
+ * have no output when doing so.
+ */
+ if (rbuf.len > 0 || ebuf.len > 0) {
+ nbdkit_error ("%s: write to script failed because of a broken pipe: "
+ "this can happen if the script exits without "
+ "consuming stdin, which usually indicates a bug "
+ "in the script",
+ argv[0]);
+ r = ERROR;
+ }
+ else
+ nbdkit_debug ("%s: write to script failed because of a broken pipe; "
+ "assuming this was an intentional data sink, although it "
+ "may indicate a bug in the script",
+ argv[0]);
+ }
return handle_script_error (argv[0], &ebuf, r);
}
--
2.41.0