10:19 a.m.
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 the connection struct, and provides
a way to access that last error.
---
server/internal.h | 4 ++++
server/connections.c | 49 ++++++++++++++++++++++++++++++++++++++++++++
server/log.c | 25 ++++++++++++++++++++++
3 files changed, 78 insertions(+)
diff --git a/server/internal.h b/server/internal.h
index 1a783e3fc0..fa74ea22af 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -274,6 +274,7 @@ struct connection {
int status_pipe[2]; /* track status changes via poll when nworkers > 1 */
void *crypto_session;
int nworkers;
+ char *last_error;
struct context *top_context; /* The context tied to 'top'. */
char **default_exportname; /* One per plugin and filter. */
@@ -300,6 +301,9 @@ struct connection {
extern void handle_single_connection (int sockin, int sockout);
extern conn_status connection_get_status (void);
extern bool connection_set_status (conn_status value);
+extern void connection_set_last_error (char *msg);
+extern void connection_clear_last_error (void);
+extern char *connection_get_last_error (void);
/* protocol-handshake.c */
extern int protocol_handshake (void);
diff --git a/server/connections.c b/server/connections.c
index 13b67f3f96..fe534da934 100644
--- a/server/connections.c
+++ b/server/connections.c
@@ -111,6 +111,54 @@ connection_set_status (conn_status value)
return ret;
}
+/* Set the last_error field in the connection. The ownership of the
+ * 'msg' string is passed to the connection and will be freed here.
+ */
+void
+connection_set_last_error (char *msg)
+{
+ GET_CONN;
+
+ if (conn->nworkers &&
+ pthread_mutex_lock (&conn->status_lock))
+ abort ();
+ free (conn->last_error);
+ conn->last_error = msg;
+ if (conn->nworkers &&
+ pthread_mutex_unlock (&conn->status_lock))
+ abort ();
+}
+
+/* Clear the last_error field in the connection handle. */
+void
+connection_clear_last_error (void)
+{
+ connection_set_last_error (NULL);
+}
+
+/* Get a copy of the last_error field in the connection handle. If
+ * successful, this returns a non-NULL string that the caller must
+ * free. Returning NULL is not necessarily an error. The last_error
+ * is informational and may not be available.
+ */
+char *
+connection_get_last_error (void)
+{
+ GET_CONN;
+ char *r = NULL;
+
+ if (conn->nworkers &&
+ pthread_mutex_lock (&conn->status_lock))
+ abort ();
+ if (conn->last_error)
+ r = strdup (conn->last_error);
+ if (conn->nworkers &&
+ pthread_mutex_unlock (&conn->status_lock))
+ abort ();
+
+ return r;
+}
+
struct worker_data {
struct connection *conn;
char *name;
@@ -391,6 +439,7 @@ free_connection (struct connection *conn)
for_each_backend (b)
free (conn->default_exportname[b->i]);
free (conn->default_exportname);
+ free (conn->last_error);
free (conn);
threadlocal_set_conn (NULL);
diff --git a/server/log.c b/server/log.c
index 9c1f667a9b..37aa2c3494 100644
--- a/server/log.c
+++ b/server/log.c
@@ -40,6 +40,29 @@
#include "internal.h"
+/* Copy the error message to the connection handle, if there is one.
+ * 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_connection (int orig_errno, const char *fs, va_list args)
+{
+ struct connection *conn = threadlocal_get_conn ();
+ va_list args_copy;
+ char *msg;
+ int r;
+
+ if (!conn) return;
+
+ 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)
+ connection_set_last_error (msg); /* ownership passed to connection */
+}
+
/* Call the right log_*_verror function depending on log_sink.
* Note: preserves the previous value of errno.
*/
@@ -48,6 +71,8 @@ log_verror (const char *fs, va_list args)
{
int orig_errno = errno;
+ copy_error_to_connection (orig_errno, fs, args);
+
switch (log_to) {
case LOG_TO_DEFAULT:
if (forked_into_background)
--
2.44.0