It is very unlikely that the value of errno after completing the glue code in between the completion of the user's callback and the point in time where we construct the client's reply is going to be untouched, which means that we are likely to send the wrong error code across the wire. Add a new callback which determines whether we can trust errno; defaulting to true for C plugins, where we can trace all code in between to ensure errno is not clobbered, and always false for other language bindings. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- docs/nbdkit-plugin.pod | 31 +++++++++++++++++++++++++++++-- include/nbdkit-plugin.h | 13 ++++++++++++- plugins/ocaml/nbdkit-ocaml-plugin.pod | 11 +++++++++++ plugins/ocaml/ocaml.c | 12 ++++++++++++ plugins/perl/nbdkit-perl-plugin.pod | 5 +++++ plugins/perl/perl.c | 12 ++++++++++++ plugins/python/nbdkit-python-plugin.pod | 5 +++++ plugins/python/python.c | 12 ++++++++++++ plugins/ruby/nbdkit-ruby-plugin.pod | 5 +++++ plugins/ruby/ruby.c | 12 ++++++++++++ src/connections.c | 32 +++++++++++++++++++++++--------- src/internal.h | 2 ++ src/plugins.c | 15 +++++++++++++++ 13 files changed, 155 insertions(+), 12 deletions(-) diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index ee6bbd5..5790f9e 100644 --- a/docs/nbdkit-plugin.pod +++ b/docs/nbdkit-plugin.pod @@ -144,12 +144,19 @@ is called once just before the plugin is unloaded from memory. 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. +indication from the callback, eg. NULL or -1. When a callback fails, +the NBD protocol sends an error code to the client; the +C<.errno_is_reliable> callback controls whether this error code +will default to something based on C<errno>, or if it will just +be hard-coded to C<EIO>. C<nbdkit_error> has the following prototype and works like L<printf(3)>: void nbdkit_error (const char *fs, ...); + void nbdkit_verror (const char *fs, va_list args); + +For convenience, C<nbdkit_error> preserves the value of C<errno>. =head1 FILENAMES AND PATHS @@ -371,6 +378,24 @@ error message and return C<-1>. This callback is not required. If omitted, then we return true iff a C<.trim> callback has been defined. +=head2 C<.errno_is_reliable> + + int errno_is_reliable (void *handle); + +This is called during the option negotiation phase to find out if the +handle wants to reflect C<errno> to the client on failures, or to just +hard-code C<EIO>. Typically, a plugin written in C will want to use +C<errno>, while a plugin written in any other language generally +cannot control the value of C<errno> after the glue code of the +language binding wrappers; therefore, this callback is not needed in +the bindings of other languages. + +If there is an error, C<.errno_is_reliable> should call C<nbdkit_error> +with an error message and return C<-1>. + +This callback is not required. If omitted, then we return true for +a plugin written in C, and false for plugins in other languages. + =head2 C<.pread> int pread (void *handle, void *buf, uint32_t count, uint64_t offset); @@ -440,7 +465,7 @@ During the data serving phase, this callback is used to write C<count> bytes of zeroes at C<offset> in the backing store. If C<may_trim> is non-zero, the operation can punch a hole instead of writing actual zero bytes, but only if subsequent reads from the hole read as zeroes. -If this callback is omitted, or if it fails with errno set to +If this callback is omitted, or if it fails with C<errno> set to EOPNOTSUPP, then C<.pwrite> will be used instead. The callback must write the whole C<count> bytes if it can. The NBD @@ -523,7 +548,9 @@ C<nbdkit_debug>, which has the following prototype and works like L<printf(3)>: void nbdkit_debug (const char *fs, ...); + void nbdkit_vdebug (const char *fs, va_list args); +For convenience, C<nbdkit_debug> preserves the value of C<errno>. Note that C<nbdkit_debug> only prints things when the server is in verbose mode (I<-v> option). diff --git a/include/nbdkit-plugin.h b/include/nbdkit-plugin.h index 3d25642..8cb33a0 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 @@ -47,10 +47,19 @@ #define NBDKIT_API_VERSION 1 struct nbdkit_plugin { + /* Do not set these fields directly; use NBDKIT_REGISTER_PLUGIN. + * They exist so that we can support plugins compiled against + * one version of the header with a runtime compiled against a + * different version with more (or fewer) fields. + */ uint64_t _struct_size; int _api_version; int _thread_model; + /* Plugins are responsible for these fields; see the documentation + * for semantics, and which fields are optional. New fields will + * only be added at the end of the struct. + */ const char *name; const char *longname; const char *version; @@ -79,6 +88,8 @@ struct nbdkit_plugin { int (*trim) (void *handle, uint32_t count, uint64_t offset); int (*zero) (void *handle, uint32_t count, uint64_t offset, int may_trim); + int (*errno_is_reliable) (void *handle); + /* int (*set_exportname) (void *handle, const char *exportname); */ }; diff --git a/plugins/ocaml/nbdkit-ocaml-plugin.pod b/plugins/ocaml/nbdkit-ocaml-plugin.pod index 8f67d9b..250c1da 100644 --- a/plugins/ocaml/nbdkit-ocaml-plugin.pod +++ b/plugins/ocaml/nbdkit-ocaml-plugin.pod @@ -89,6 +89,17 @@ handle struct in your plugin: printf "handle ID = %d\n" handle.h_id; (* ... *) +=head2 MISSING CALLBACKS + +=over 4 + +=item Missing: C<errno_is_reliable> + +This is not needed because the process of gluing OCaml code into C cannot +reliably use C<errno>. + +=back + =head2 THREADS The first parameter of C<NBDKit.register_plugin> is the thread model, diff --git a/plugins/ocaml/ocaml.c b/plugins/ocaml/ocaml.c index e2b433e..5d7aeeb 100644 --- a/plugins/ocaml/ocaml.c +++ b/plugins/ocaml/ocaml.c @@ -499,6 +499,17 @@ SET(pwrite) SET(flush) SET(trim) +/* We can't guarantee that errno is stable across language binding + * glue code, so this callback is implemented in C only, and not + * exposed in OCaml. + */ +static int +plugin_ocaml_errno_is_reliable (void *handle) +{ + return 0; +} + +/* We can't directly use NBDKIT_REGISTER_PLUGIN(). */ struct nbdkit_plugin * plugin_init (void) { @@ -506,5 +517,6 @@ plugin_init (void) fprintf (stderr, "error: OCaml code did not call NBDKit.register_plugin\n"); exit (EXIT_FAILURE); } + plugin.errno_is_reliable = plugin_ocaml_errno_is_reliable; return &plugin; } diff --git a/plugins/perl/nbdkit-perl-plugin.pod b/plugins/perl/nbdkit-perl-plugin.pod index 3d0ce14..c02f8ef 100644 --- a/plugins/perl/nbdkit-perl-plugin.pod +++ b/plugins/perl/nbdkit-perl-plugin.pod @@ -282,6 +282,11 @@ If there is an error, the function should call C<die>. These are not needed because you can just use regular Perl C<BEGIN> and C<END> constructs. +=item Missing: C<errno_is_reliable> + +This is not needed because the process of gluing Perl code into C cannot +reliably use C<errno>. + =item Missing: C<name>, C<version>, C<longname>, C<description>, C<config_help> These are not yet supported. diff --git a/plugins/perl/perl.c b/plugins/perl/perl.c index 935e1ba..ec82ecb 100644 --- a/plugins/perl/perl.c +++ b/plugins/perl/perl.c @@ -584,6 +584,16 @@ perl_trim (void *handle, uint32_t count, uint64_t offset) return 0; } +/* We can't guarantee that errno is stable across language binding + * glue code, so this callback is implemented in C only, and not + * exposed in Perl. + */ +static int +perl_errno_is_reliable (void *handle) +{ + return 0; +} + #define perl_config_help \ "script=<FILENAME> (required) The Perl plugin to run.\n" \ "[other arguments may be used by the plugin that you load]" @@ -614,6 +624,8 @@ static struct nbdkit_plugin plugin = { .pwrite = perl_pwrite, .flush = perl_flush, .trim = perl_trim, + + .errno_is_reliable = perl_errno_is_reliable, }; NBDKIT_REGISTER_PLUGIN(plugin) diff --git a/plugins/python/nbdkit-python-plugin.pod b/plugins/python/nbdkit-python-plugin.pod index 9b0f0ef..63e5f96 100644 --- a/plugins/python/nbdkit-python-plugin.pod +++ b/plugins/python/nbdkit-python-plugin.pod @@ -207,6 +207,11 @@ backing store. These are not needed because you can just use ordinary Python constructs. +=item Missing: C<errno_is_reliable> + +This is not needed because the process of gluing Python code into C cannot +reliably use C<errno>. + =item Missing: C<name>, C<version>, C<longname>, C<description>, C<config_help> These are not yet supported. diff --git a/plugins/python/python.c b/plugins/python/python.c index 0504715..7e9fc1e 100644 --- a/plugins/python/python.c +++ b/plugins/python/python.c @@ -549,6 +549,16 @@ py_can_trim (void *handle) return 0; } +/* We can't guarantee that errno is stable across language binding + * glue code, so this callback is implemented in C only, and not + * exposed in Python. + */ +static int +py_errno_is_reliable (void *handle) +{ + return 0; +} + #define py_config_help \ "script=<FILENAME> (required) The Python plugin to run.\n" \ "[other arguments may be used by the plugin that you load]" @@ -579,6 +589,8 @@ static struct nbdkit_plugin plugin = { .pwrite = py_pwrite, .flush = py_flush, .trim = py_trim, + + .errno_is_reliable = py_errno_is_reliable, }; NBDKIT_REGISTER_PLUGIN(plugin) diff --git a/plugins/ruby/nbdkit-ruby-plugin.pod b/plugins/ruby/nbdkit-ruby-plugin.pod index 8511479..3f49cb4 100644 --- a/plugins/ruby/nbdkit-ruby-plugin.pod +++ b/plugins/ruby/nbdkit-ruby-plugin.pod @@ -208,6 +208,11 @@ backing store. These are not needed because you can just use ordinary Ruby constructs. +=item Missing: C<errno_is_reliable> + +This is not needed because the process of gluing Ruby code into C cannot +reliably use C<errno>. + =item Missing: C<name>, C<version>, C<longname>, C<description>, C<config_help> These are not yet supported. diff --git a/plugins/ruby/ruby.c b/plugins/ruby/ruby.c index 6d1434c..2da14f7 100644 --- a/plugins/ruby/ruby.c +++ b/plugins/ruby/ruby.c @@ -417,6 +417,16 @@ plugin_rb_can_trim (void *handle) return RTEST (rv); } +/* We can't guarantee that errno is stable across language binding + * glue code, so this callback is implemented in C only, and not + * exposed in Ruby. + */ +static int +plugin_rb_errno_is_reliable (void *handle) +{ + return 0; +} + #define plugin_rb_config_help \ "script=<FILENAME> (required) The Ruby plugin to run.\n" \ "[other arguments may be used by the plugin that you load]" @@ -450,6 +460,8 @@ static struct nbdkit_plugin plugin = { .pwrite = plugin_rb_pwrite, .flush = plugin_rb_flush, .trim = plugin_rb_trim, + + .errno_is_reliable = plugin_rb_errno_is_reliable, }; NBDKIT_REGISTER_PLUGIN(plugin) diff --git a/src/connections.c b/src/connections.c index c0f0567..23e82a9 100644 --- a/src/connections.c +++ b/src/connections.c @@ -498,7 +498,10 @@ negotiate_handshake (struct connection *conn) int r; plugin_lock_request (conn); - if (!newstyle) + conn->errno_is_reliable = plugin_errno_is_reliable (conn); + if (conn->errno_is_reliable < 0) + r = -1; + else if (!newstyle) r = _negotiate_handshake_oldstyle (conn); else r = _negotiate_handshake_newstyle (conn); @@ -602,6 +605,18 @@ validate_request (struct connection *conn, return 1; /* Commands validates. */ } +/* Grab the appropriate error value. + */ +static int +get_error (struct connection *conn) +{ + int ret = 0; + + if (conn->errno_is_reliable) + ret = errno; + return ret ? ret : EIO; +} + /* 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,8 +626,7 @@ 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 - * set) and returns 0. + * On read/write errors, sets *error appropriately and returns 0. */ static int _handle_request (struct connection *conn, @@ -632,7 +646,7 @@ _handle_request (struct connection *conn, case NBD_CMD_READ: r = plugin_pread (conn, buf, count, offset); if (r == -1) { - *error = errno ? errno : EIO; + *error = get_error (conn); return 0; } break; @@ -640,7 +654,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 (conn); return 0; } break; @@ -648,7 +662,7 @@ _handle_request (struct connection *conn, case NBD_CMD_FLUSH: r = plugin_flush (conn); if (r == -1) { - *error = errno ? errno : EIO; + *error = get_error (conn); return 0; } break; @@ -656,7 +670,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 (conn); return 0; } break; @@ -664,7 +678,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 (conn); return 0; } break; @@ -676,7 +690,7 @@ _handle_request (struct connection *conn, if (flush_after_command) { r = plugin_flush (conn); if (r == -1) { - *error = errno ? errno : EIO; + *error = get_error (conn); return 0; } } diff --git a/src/internal.h b/src/internal.h index 4632f51..2129337 100644 --- a/src/internal.h +++ b/src/internal.h @@ -120,6 +120,7 @@ struct connection { int can_flush; int is_rotational; int can_trim; + int errno_is_reliable; }; extern int handle_single_connection (int sockin, int sockout); @@ -143,6 +144,7 @@ extern void plugin_unlock_request (struct connection *conn); extern int plugin_open (struct connection *conn, int readonly); extern void plugin_close (struct connection *conn); extern int64_t plugin_get_size (struct connection *conn); +extern int plugin_errno_is_reliable (struct connection *conn); extern int plugin_can_write (struct connection *conn); extern int plugin_can_flush (struct connection *conn); extern int plugin_is_rotational (struct connection *conn); diff --git a/src/plugins.c b/src/plugins.c index 92f8505..d486f2d 100644 --- a/src/plugins.c +++ b/src/plugins.c @@ -395,6 +395,21 @@ plugin_get_size (struct connection *conn) } int +plugin_errno_is_reliable (struct connection *conn) +{ + assert (dl); + assert (conn->handle); + + debug ("errno_is_reliable"); + + if (plugin.errno_is_reliable) + return plugin.errno_is_reliable (conn->handle); + + /* Default to 1, for backwards compatibility (correct for C plugins) */ + return 1; +} + +int plugin_can_write (struct connection *conn) { assert (dl); -- 2.9.3

The last patch fixed the plugin interface to not use a bogus value of errno from other language bindings, but hard-coding things to EIO is not nice either. 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, and is intentionally wiped before each callback. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- v2: rebase on top of .errno_is_implicit --- docs/nbdkit-plugin.pod | 59 ++++++++++++++++++++++++++++++++----------------- include/nbdkit-plugin.h | 1 + src/connections.c | 8 +++++-- src/errors.c | 6 +++++ src/internal.h | 2 ++ src/plugins.c | 10 +++++++-- src/tls.c | 30 +++++++++++++++++++++++-- 7 files changed, 90 insertions(+), 26 deletions(-) diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index 5790f9e..4b3a492 100644 --- a/docs/nbdkit-plugin.pod +++ b/docs/nbdkit-plugin.pod @@ -143,12 +143,14 @@ 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. When a callback fails, -the NBD protocol sends an error code to the client; the -C<.errno_is_reliable> callback controls whether this error code -will default to something based on C<errno>, or if it will just -be hard-coded to C<EIO>. +C<nbdkit_error> to report an error message; additionally, if the +callback is involved in serving data, the plugin should call +C<nbdkit_set_error> to influence the error code that will be sent to +the client. These two functions can be called in either order. Then, +the callback should return the appropriate error indication, eg. NULL +or -1. If the call to C<nbdkit_set_error> is omitted while serving +data, then the C<.errno_is_reliable> callback controls whether to fall +back to the value of C<errno> or a hard-coded C<EIO>. C<nbdkit_error> has the following prototype and works like L<printf(3)>: @@ -158,6 +160,12 @@ L<printf(3)>: For convenience, C<nbdkit_error> preserves the value of C<errno>. +C<nbdkit_set_error> can be called at any time, but only has an impact +during callbacks for serving data, and only when the callback returns +an indication of failure. It has the following prototype: + + void nbdkit_set_error (int err); + =head1 FILENAMES AND PATHS The server usually (not always) changes directory to C</> before it @@ -382,13 +390,18 @@ C<.trim> callback has been defined. int errno_is_reliable (void *handle); -This is called during the option negotiation phase to find out if the -handle wants to reflect C<errno> to the client on failures, or to just -hard-code C<EIO>. Typically, a plugin written in C will want to use -C<errno>, while a plugin written in any other language generally -cannot control the value of C<errno> after the glue code of the -language binding wrappers; therefore, this callback is not needed in -the bindings of other languages. +This is called during the option negotiation phase to find out what +errors to reflect to the client. An error explicitly reported to +C<nbdkit_set_error> takes precedence, but plugins written in C can +often get by with implicitly using the value stored in C<errno>, when +this callback returns true. On the other hand, a plugin written in any +other language generally cannot control the value of C<errno> after +the glue code of the language binding wrappers; returning false forces +the use of C<EIO> as the fallback error rather than using an +unreliable value in C<errno>. This callback is not needed in the +bindings of other languages. The results of this callback do not +matter if all data serving callbacks correctly call +C<nbdkit_set_error>. If there is an error, C<.errno_is_reliable> should call C<nbdkit_error> with an error message and return C<-1>. @@ -413,7 +426,8 @@ to indicate there was I<no> error. If there is an error (including a short read which couldn't be recovered from), C<.pread> should call C<nbdkit_error> with an error -message and return C<-1>. +message, and C<nbdkit_set_error> to record an appropriate error +(unless C<errno> is sufficient), then return C<-1>. =head2 C<.pwrite> @@ -432,7 +446,8 @@ callback should return C<0> to indicate there was I<no> error. If there is an error (including a short write which couldn't be recovered from), C<.pwrite> should call C<nbdkit_error> with an error -message and return C<-1>. +message, and C<nbdkit_set_error> to record an appropriate error +(unless C<errno> is sufficient), then return C<-1>. =head2 C<.flush> @@ -444,7 +459,8 @@ completely written to a permanent medium. If that is not possible then you can omit this callback. If there is an error, C<.flush> should call C<nbdkit_error> with an -error message and return C<-1>. +error message, and C<nbdkit_set_error> to record an appropriate error +(unless C<errno> is sufficient), then return C<-1>. =head2 C<.trim> @@ -455,7 +471,8 @@ in the backing store. If that is not possible then you can omit this callback. If there is an error, C<.trim> should call C<nbdkit_error> with an -error message and return C<-1>. +error message, and C<nbdkit_set_error> to record an appropriate error +(unless C<errno> is sufficient), then return C<-1>. =head2 C<.zero> @@ -465,8 +482,9 @@ During the data serving phase, this callback is used to write C<count> bytes of zeroes at C<offset> in the backing store. If C<may_trim> is non-zero, the operation can punch a hole instead of writing actual zero bytes, but only if subsequent reads from the hole read as zeroes. -If this callback is omitted, or if it fails with C<errno> set to -EOPNOTSUPP, then C<.pwrite> will be used instead. +If this callback is omitted, or if it fails with C<EOPNOTSUPP> +(whether by C<nbdkit_set_error or C<errno>), then C<.pwrite> will be +used instead. The callback must write the whole C<count> bytes if it can. The NBD protocol doesn't allow partial writes (instead, these would be @@ -474,7 +492,8 @@ errors). If the whole C<count> bytes was written successfully, the callback should return C<0> to indicate there was I<no> error. If there is an error, C<.zero> should call C<nbdkit_error> with an -error message and return C<-1>. +error message, and C<nbdkit_set_error> to record an appropriate error +(unless C<errno> is sufficient), then return C<-1>. =head1 THREADS diff --git a/include/nbdkit-plugin.h b/include/nbdkit-plugin.h index 8cb33a0..28872e0 100644 --- a/include/nbdkit-plugin.h +++ b/include/nbdkit-plugin.h @@ -93,6 +93,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 23e82a9..cf06ac5 100644 --- a/src/connections.c +++ b/src/connections.c @@ -610,9 +610,9 @@ validate_request (struct connection *conn, static int get_error (struct connection *conn) { - int ret = 0; + int ret = tls_get_error (); - if (conn->errno_is_reliable) + if (!ret && conn->errno_is_reliable) ret = errno; return ret ? ret : EIO; } @@ -642,6 +642,10 @@ _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); 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 2129337..aa7dbd3 100644 --- a/src/internal.h +++ b/src/internal.h @@ -169,6 +169,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 d486f2d..837a54f 100644 --- a/src/plugins.c +++ b/src/plugins.c @@ -536,7 +536,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); @@ -546,11 +546,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 && conn->errno_is_reliable) + 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..5d380e3 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,26 @@ 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; +} + +/* This preserves errno, for convenience. + */ +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

Add a python language binding for the .zero callback, used for implementing NBD_CMD_WRITE_ZEROES. The caller doesn't have to return anything, but should use nbdkit.set_error(errno.EOPNOTSUPP) to get an automatic fallback to pwrite. Enhance the example to show the use of the fallback mechanism, and to serve as a test of nbdkit.set_error(). Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- v2: rebase to .errno_is_reliable --- plugins/python/example.py | 11 ++++++++ plugins/python/nbdkit-python-plugin.pod | 20 ++++++++++++++ plugins/python/python.c | 46 +++++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+) diff --git a/plugins/python/example.py b/plugins/python/example.py index 184896e..8826eba 100644 --- a/plugins/python/example.py +++ b/plugins/python/example.py @@ -24,6 +24,9 @@ # ><fs> mount /dev/sda1 / # ><fs> [etc] +import nbdkit +import errno + # This is the string used to store the emulated disk (initially all # zero bytes). There is one disk per nbdkit instance, so if you # reconnect to the same server you should see the same disk. You @@ -56,3 +59,11 @@ def pwrite(h, buf, offset): global disk end = offset + len (buf) disk[offset:end] = buf + +def zero(h, count, offset, may_trim): + global disk + if may_trim: + disk[offset:offset+count] = bytearray(count) + else: + nbdkit.set_error(errno.EOPNOTSUPP) + raise Exception diff --git a/plugins/python/nbdkit-python-plugin.pod b/plugins/python/nbdkit-python-plugin.pod index a1e3d2b..1c57e15 100644 --- a/plugins/python/nbdkit-python-plugin.pod +++ b/plugins/python/nbdkit-python-plugin.pod @@ -215,6 +215,26 @@ The body of your C<trim> function should "punch a hole" in the backing store. If the trim fails, your function should throw an exception, optionally using C<nbdkit.set_error> first. +=item C<zero> + +(Optional) + + def zero(h, count, offset, may_trim): + # no return value + +The body of your C<zero> function should ensure that C<count> bytes +of the disk, starting at C<offset>, will read back as zero. If +C<may_trim> is true, the operation may be optimized as a trim as long +as subsequent reads see zeroes. + +NBD only supports whole writes, so your function should try to +write the whole region (perhaps requiring a loop). If the write +fails or is partial, your function should throw an exception, +optionally using C<nbdkit.set_error> first. In particular, if +you would like to automatically fall back to C<pwrite> (perhaps +because there is nothing to optimize if C<may_trim> is false), +use C<nbdkit.set_error(errno.EOPNOTSUPP)>. + =back =head2 MISSING CALLBACKS diff --git a/plugins/python/python.c b/plugins/python/python.c index 631411e..895a361 100644 --- a/plugins/python/python.c +++ b/plugins/python/python.c @@ -54,6 +54,8 @@ static const char *script; static PyObject *module; +static int last_error; + static PyObject * set_error (PyObject *self, PyObject *args) { @@ -62,6 +64,7 @@ set_error (PyObject *self, PyObject *args) if (!PyArg_ParseTuple(args, "i", &err)) return NULL; nbdkit_set_error (err); + last_error = err; Py_RETURN_NONE; } @@ -441,6 +444,48 @@ py_trim (void *handle, uint32_t count, uint64_t offset) } static int +py_zero (void *handle, uint32_t count, uint64_t offset, int may_trim) +{ + PyObject *obj = handle; + PyObject *fn; + PyObject *args; + PyObject *r; + + if (callback_defined ("zero", &fn)) { + PyErr_Clear (); + + last_error = 0; + args = PyTuple_New (4); + Py_INCREF (obj); /* decremented by Py_DECREF (args) */ + PyTuple_SetItem (args, 0, obj); + PyTuple_SetItem (args, 1, PyLong_FromUnsignedLongLong (count)); + PyTuple_SetItem (args, 2, PyLong_FromUnsignedLongLong (offset)); + PyTuple_SetItem (args, 3, PyBool_FromLong (may_trim)); + r = PyObject_CallObject (fn, args); + Py_DECREF (fn); + Py_DECREF (args); + if (last_error == EOPNOTSUPP) { + /* When user requests this particular error, we want to + gracefully fall back, and to accomodate both a normal return + and an exception. */ + nbdkit_debug ("zero requested falling back to pwrite"); + if (r) + Py_DECREF (r); + PyErr_Clear (); + return -1; + } + if (check_python_failure ("zero") == -1) + return -1; + Py_DECREF (r); + return 0; + } + + nbdkit_debug ("zero missing, falling back to pwrite"); + nbdkit_set_error (EOPNOTSUPP); + return -1; +} + +static int py_can_write (void *handle) { PyObject *obj = handle; @@ -607,6 +652,7 @@ static struct nbdkit_plugin plugin = { .pwrite = py_pwrite, .flush = py_flush, .trim = py_trim, + .zero = py_zero, .errno_is_reliable = py_errno_is_reliable, }; -- 2.9.3