From 4cde9d78c4293e294f80376266cfce420f15a6ff Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Tue, 23 Jul 2024 15:28:06 +0100 Subject: [PATCH] server: Take a thread-local copy of the last call to nbdkit_error nbdkit_error has traditionally been a "fancy wrapper around fprintf" (kind of, don't take that literally). It is encouraged that plugins and filters do something like: if (error) { nbdkit_error ("oops, a bad thing happened"); return -1; } but we don't enforce this. Plugins might call nbdkit_error more than once or not at all. The point where we get to sending an error back over the wire to the NBD client is long after the plugin returned above, and after nbdkit_error was called. Therefore in order to send errors back to the NBD client, we must keep the last error message around. This change simply modifies nbdkit_error to make a best-effort attempt to save the last error message in thread-local storage. We also clear the last error when a new request starts, to ensure that we don't leak errors across different callbacks or connections. (cherry picked from commit bfa6d4064cb74f429149d14ab4025b258fc95ec4) --- server/log.c | 21 +++++++++++++++++++++ server/protocol.c | 5 +++++ 2 files changed, 26 insertions(+) diff --git a/server/log.c b/server/log.c index 9c1f667a..acf14d57 100644 --- a/server/log.c +++ b/server/log.c @@ -40,6 +40,25 @@ #include "internal.h" +/* Copy the error message to threadlocal. This is sent to callers + * which are using structured replies, but is for extra information + * only so don't fail if we are unable to copy it. + */ +static void +copy_error_to_threadlocal (int orig_errno, const char *fs, va_list args) +{ + va_list args_copy; + char *msg; + int r; + + va_copy (args_copy, args); + errno = orig_errno; /* must restore in case fs contains %m */ + r = vasprintf (&msg, fs, args_copy); + va_end (args_copy); + if (r != -1 && msg) + threadlocal_set_last_error (msg); /* ownership passed to threadlocal */ +} + /* Call the right log_*_verror function depending on log_sink. * Note: preserves the previous value of errno. */ @@ -48,6 +67,8 @@ log_verror (const char *fs, va_list args) { int orig_errno = errno; + copy_error_to_threadlocal (orig_errno, fs, args); + switch (log_to) { case LOG_TO_DEFAULT: if (forked_into_background) diff --git a/server/protocol.c b/server/protocol.c index 677da05c..d428bfc8 100644 --- a/server/protocol.c +++ b/server/protocol.c @@ -239,6 +239,11 @@ handle_request (uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count, */ threadlocal_set_errno (0); + /* Also clear the last error in this thread so we will only save + * nbdkit_error() from this request. + */ + threadlocal_clear_last_error (); + switch (cmd) { case NBD_CMD_READ: if (backend_pread (c, buf, count, offset, 0, &err) == -1) -- 2.43.0