8:42 p.m.
The plugin interface was impliticly using the last errno value
as the source for any error code sent over the wire to the client.
This is okay for C, but in other language bindings, it gets
rather awkward when you can't guarantee what errno will even be
set to by the time the binding glue has finished executing.
The solution is to expose an explicit utility function for setting
the preferred error value, while still falling back to errno for
backwards compatibility. The saved error code has to be
thread-safe for use in multithreaded plugins.
Note that setting an error value but then returning success from
the plugin has no impact; the error value is only consulted when
the plugin reports failure. Also, plugin_zero() has to be a bit
careful about using EOPNOTSUPP as its trigger to fall back to
.pwrite, so that the fallback does not inherit an accidental
improper error code.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
docs/nbdkit-plugin.pod | 16 +++++++++++++---
include/nbdkit-plugin.h | 3 ++-
src/connections.c | 32 +++++++++++++++++++++++++-------
src/errors.c | 6 ++++++
src/internal.h | 2 ++
src/plugins.c | 10 ++++++++--
src/tls.c | 28 ++++++++++++++++++++++++++--
7 files changed, 82 insertions(+), 15 deletions(-)
diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
index ee6bbd5..ab26a6a 100644
--- a/docs/nbdkit-plugin.pod
+++ b/docs/nbdkit-plugin.pod
@@ -143,14 +143,23 @@ is called once just before the plugin is unloaded from memory.
=head1 ERROR HANDLING
If there is an error in the plugin, the plugin should call
-C<nbdkit_error> with the error message, and then return an error
-indication from the callback, eg. NULL or -1.
+C<nbdkit_set_error> to influence the error code that will be set to
+the client, then C<nbdkit_error> to report an error message; the
+plugin should then return an error indication from the callback,
+eg. NULL or -1. If the call to C<nbdkit_set_error> is omitted, then
+the value of C<errno> will be used instead.
+
+C<nbdkit_set_error> has the following prototype:
+
+ void nbdkit_set_error (int);
C<nbdkit_error> has the following prototype and works like
L<printf(3)>:
void nbdkit_error (const char *fs, ...);
+For convenience, C<nbdkit_error> preserves the value of C<errno>.
+
=head1 FILENAMES AND PATHS
The server usually (not always) changes directory to C</> before it
@@ -525,7 +534,8 @@ L<printf(3)>:
void nbdkit_debug (const char *fs, ...);
Note that C<nbdkit_debug> only prints things when the server is in
-verbose mode (I<-v> option).
+verbose mode (I<-v> option). For convenience, it preserves the value
+of C<errno>.
=head1 INSTALLING THE PLUGIN
diff --git a/include/nbdkit-plugin.h b/include/nbdkit-plugin.h
index 3d25642..2d401bf 100644
--- a/include/nbdkit-plugin.h
+++ b/include/nbdkit-plugin.h
@@ -1,5 +1,5 @@
/* nbdkit
- * Copyright (C) 2013-2016 Red Hat Inc.
+ * Copyright (C) 2013-2017 Red Hat Inc.
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -82,6 +82,7 @@ struct nbdkit_plugin {
/* int (*set_exportname) (void *handle, const char *exportname); */
};
+extern void nbdkit_set_error (int err);
extern void nbdkit_error (const char *msg, ...)
__attribute__((format (printf, 1, 2)));
extern void nbdkit_verror (const char *msg, va_list args);
diff --git a/src/connections.c b/src/connections.c
index c0f0567..83c863e 100644
--- a/src/connections.c
+++ b/src/connections.c
@@ -602,6 +602,19 @@ validate_request (struct connection *conn,
return 1; /* Commands validates. */
}
+/* Grab the appropriate error value.
+ */
+static int
+_get_error (void)
+{
+ int err = errno;
+ int ret = tls_get_error ();
+
+ if (!ret)
+ ret = err ? err : EIO;
+ return ret;
+}
+
/* This is called with the request lock held to actually execute the
* request (by calling the plugin). Note that the request fields have
* been validated already in 'validate_request' so we don't have to
@@ -611,7 +624,8 @@ validate_request (struct connection *conn,
* Only returns -1 if there is a fatal error and the connection cannot
* continue.
*
- * On read/write errors, sets *error to errno (or EIO if errno is not
+ * On read/write errors, sets *error to the value stored in
+ * nbdkit_set_error(), falling back to errno (or EIO if errno is not
* set) and returns 0.
*/
static int
@@ -628,11 +642,15 @@ _handle_request (struct connection *conn,
if (!conn->can_flush || conn->readonly)
flush_after_command = false;
+ /* The plugin should call nbdkit_set_error() to request a particular
+ error, otherwise we fallback to errno or EIO. */
+ tls_set_error (0);
+
switch (cmd) {
case NBD_CMD_READ:
r = plugin_pread (conn, buf, count, offset);
if (r == -1) {
- *error = errno ? errno : EIO;
+ *error = _get_error ();
return 0;
}
break;
@@ -640,7 +658,7 @@ _handle_request (struct connection *conn,
case NBD_CMD_WRITE:
r = plugin_pwrite (conn, buf, count, offset);
if (r == -1) {
- *error = errno ? errno : EIO;
+ *error = _get_error ();
return 0;
}
break;
@@ -648,7 +666,7 @@ _handle_request (struct connection *conn,
case NBD_CMD_FLUSH:
r = plugin_flush (conn);
if (r == -1) {
- *error = errno ? errno : EIO;
+ *error = _get_error ();
return 0;
}
break;
@@ -656,7 +674,7 @@ _handle_request (struct connection *conn,
case NBD_CMD_TRIM:
r = plugin_trim (conn, count, offset);
if (r == -1) {
- *error = errno ? errno : EIO;
+ *error = _get_error ();
return 0;
}
break;
@@ -664,7 +682,7 @@ _handle_request (struct connection *conn,
case NBD_CMD_WRITE_ZEROES:
r = plugin_zero (conn, count, offset, !(flags & NBD_CMD_FLAG_NO_HOLE));
if (r == -1) {
- *error = errno ? errno : EIO;
+ *error = _get_error ();
return 0;
}
break;
@@ -676,7 +694,7 @@ _handle_request (struct connection *conn,
if (flush_after_command) {
r = plugin_flush (conn);
if (r == -1) {
- *error = errno ? errno : EIO;
+ *error = _get_error ();
return 0;
}
}
diff --git a/src/errors.c b/src/errors.c
index adb2db7..4abab5a 100644
--- a/src/errors.c
+++ b/src/errors.c
@@ -131,3 +131,9 @@ nbdkit_error (const char *fs, ...)
errno = err;
}
+
+void
+nbdkit_set_error (int err)
+{
+ tls_set_error (err);
+}
diff --git a/src/internal.h b/src/internal.h
index cb15bab..f0c7768 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -123,6 +123,8 @@ extern void tls_set_instance_num (size_t instance_num);
extern void tls_set_sockaddr (struct sockaddr *addr, socklen_t addrlen);
extern const char *tls_get_name (void);
extern size_t tls_get_instance_num (void);
+extern void tls_set_error (int err);
+extern int tls_get_error (void);
/*extern void tls_get_sockaddr ();*/
/* utils.c */
diff --git a/src/plugins.c b/src/plugins.c
index 574be03..ff7bf48 100644
--- a/src/plugins.c
+++ b/src/plugins.c
@@ -520,7 +520,7 @@ plugin_zero (struct connection *conn,
char *buf;
uint32_t limit;
int result;
- int err;
+ int err = 0;
debug ("zero count=%" PRIu32 " offset=%" PRIu64 "
may_trim=%d",
count, offset, may_trim);
@@ -530,11 +530,17 @@ plugin_zero (struct connection *conn,
if (plugin.zero) {
errno = 0;
result = plugin.zero (conn->handle, count, offset, may_trim);
- if (result == 0 || errno != EOPNOTSUPP)
+ if (result == -1) {
+ err = tls_get_error ();
+ if (!err)
+ err = errno;
+ }
+ if (result == 0 || err != EOPNOTSUPP)
return result;
}
assert (plugin.pwrite);
+ tls_set_error (0);
limit = count < MAX_REQUEST_SIZE ? count : MAX_REQUEST_SIZE;
buf = calloc (limit, 1);
if (!buf) {
diff --git a/src/tls.c b/src/tls.c
index 390b03e..e80e925 100644
--- a/src/tls.c
+++ b/src/tls.c
@@ -44,8 +44,10 @@
#include "nbdkit-plugin.h"
#include "internal.h"
-/* Note currently all thread-local storage data is informational.
- * It's mainly used for smart error and debug messages.
+/* Note that most thread-local storage data is informational, used for
+ * smart error and debug messages on the server side. However, error
+ * tracking can be used to influence which error is sent to the client
+ * in a reply.
*
* The main thread does not have any associated TLS, *unless* it is
* serving a request (the '-s' option).
@@ -56,6 +58,7 @@ struct tls {
size_t instance_num; /* Can be 0. */
struct sockaddr *addr;
socklen_t addrlen;
+ int err;
};
static pthread_key_t tls_key;
@@ -150,3 +153,24 @@ tls_get_instance_num (void)
return tls->instance_num;
}
+
+void
+tls_set_error (int err)
+{
+ struct tls *tls = pthread_getspecific (tls_key);
+
+ if (tls)
+ tls->err = err;
+ else
+ errno = err;
+}
+
+int
+tls_get_error (void)
+{
+ int err = errno;
+ struct tls *tls = pthread_getspecific (tls_key);
+
+ errno = err;
+ return tls ? tls->err : 0;
+}
--
2.9.3