search for: errno_is_preserved

On 02/06/2017 08:57 AM, Richard W.M. Jones wrote: > The callback doesn't make much sense: Could the value change > per-connection? Unlikely. This is a property of the plugin as a > whole. > > I changed the name to "errno_is_preserved", because it's not about the > reliability of errno, but about whether errno is preserved across > calls. Makes it possible for a regression in C plugins (previously, such plugins implicitly behaved as if .errno_is_preserved was 1, now such plugins default to .errno_is_preserved ==...

Ever since commit 69ae137f, the docs claimed that we check .errno_is_preserved == 1, but the code has checked for != 0. Furthermore, the mention of .errno_is_preserved in the docs was rather hidden; a new section will make it easier to add future knobs that likewise affect the plugin as a whole and not an individual connection. Fixes: 69ae137f Signed-off-by: Eric Blake <e...

...o use fork() */ }; NBDKIT_REGISTER_PLUGIN(plugin) diff --git a/plugins/data/data.c b/plugins/data/data.c index 14fb8997..6e7d4296 100644 --- a/plugins/data/data.c +++ b/plugins/data/data.c @@ -433,6 +433,7 @@ static struct nbdkit_plugin plugin = { * paths from failed system calls. */ .errno_is_preserved = 1, + .fork_safe = 1, /* no use of fork() */ }; NBDKIT_REGISTER_PLUGIN(plugin) diff --git a/plugins/example2/example2.c b/plugins/example2/example2.c index 6adc100a..0ce081c8 100644 --- a/plugins/example2/example2.c +++ b/plugins/example2/example2.c @@ -224,6 +224,7 @@ static struct nb...

See patch 1 for rationale.

The callback doesn't make much sense: Could the value change per-connection? Unlikely. This is a property of the plugin as a whole. I changed the name to "errno_is_preserved", because it's not about the reliability of errno, but about whether errno is preserved across calls. --- docs/nbdkit-plugin.pod | 37 +++++++++------------------------ include/nbdkit-plugin.h | 2 +- plugins/ocaml/nbdkit-ocaml-plugin.pod | 11 ---------...

...econd one is also simple enough, but not enough of a bug for me to push tonight. The third is something I noticed while working on sh, but is really more about docs vs. plugins in general. There, we could either change the code to match the docs (breaking backwards behavior for a plugin that set .errno_is_preserved=2) [what my patch did], or change the docs to match the code (mention that any non-zero value will do). Preference? Eric Blake (3): sh: Fix flags when none are present sh: Avoid setenv after fork plugins: Match docs for .errno_is_preserved docs/nbdkit-plugin.pod | 16 ++++++++++++++++ plug...

...in that relied on errno but not the error stored in thread-local storage. Better is to change the backend interface to just pass the direct error value, by moving the decoding of thread-local vs. errno into plugins.c. With the change in decoding location, the backend interface no longer needs an .errno_is_preserved() callback. For maximum convenience, this change lets a filter return an error either by passing through the underlying plugin return (a positive error) or by setting -1 and storing something in errno. However, I did have to tweak some of the existing filters to actually handle and/or return the...

...rrno == EINTR. (2) It writes to them to set particular errno values, eg. errno = EIO. (3) It saves them around functions such as nbdkit_error(), which is (1) + (2) but might be a distinct operation eg. using cleanups. (4) It prints them with perror and %m. (5) It handles plugins which claim errno_is_preserved. My first thought was we could define a macro which does: #ifdef WIN32 #define errno (translate_to_errno (WSAGetLastError ())) #endif which reads the last error and translates WSA* to E* codes. This would solve (1) and is not very invasive for existing code. We'd have to then need to wr...

.../* Finish the newstyle handshake. */ - r = plugin_get_size (conn); + r = backend->get_size (backend, conn); if (r == -1) return -1; if (r < 0) { @@ -848,7 +850,7 @@ get_error (struct connection *conn) { int ret = threadlocal_get_error (); - if (!ret && plugin_errno_is_preserved ()) + if (!ret && backend->errno_is_preserved (backend)) ret = errno; return ret ? ret : EIO; } @@ -881,28 +883,28 @@ handle_request (struct connection *conn, switch (cmd) { case NBD_CMD_READ: - if (plugin_pread (conn, buf, count, offset) == -1) + if (backend->...

.../* Finish the newstyle handshake. */ - r = plugin_get_size (conn); + r = backend->get_size (backend, conn); if (r == -1) return -1; if (r < 0) { @@ -848,7 +850,7 @@ get_error (struct connection *conn) { int ret = threadlocal_get_error (); - if (!ret && plugin_errno_is_preserved ()) + if (!ret && backend->errno_is_preserved (backend)) ret = errno; return ret ? ret : EIO; } @@ -881,28 +883,28 @@ handle_request (struct connection *conn, switch (cmd) { case NBD_CMD_READ: - if (plugin_pread (conn, buf, count, offset) == -1) + if (backend->...

.../* Finish the newstyle handshake. */ - r = plugin_get_size (conn); + r = backend->get_size (backend, conn); if (r == -1) return -1; if (r < 0) { @@ -848,7 +850,7 @@ get_error (struct connection *conn) { int ret = threadlocal_get_error (); - if (!ret && plugin_errno_is_preserved ()) + if (!ret && backend->errno_is_preserved (backend)) ret = errno; return ret ? ret : EIO; } @@ -881,28 +883,28 @@ handle_request (struct connection *conn, switch (cmd) { case NBD_CMD_READ: - if (plugin_pread (conn, buf, count, offset) == -1) + if (backend->...

This is a major rewrite compared to my v1 series, where I've tried a lot harder to ensure that we still accommodate building on Haiku (although I have not actually yet fired up a Haiku VM to try it for myself). I also managed to make the sh plugin fully parallel, on capable platforms. See also my question on patch 10 on whether I've picked the best naming convention. Eric Blake (17):

v1 -> v2: - Fixed everything mentioned in the review. Rich.

...uint32_t count, uint64_t offset) @@ -192,6 +199,7 @@ static struct nbdkit_plugin plugin = { .magic_config_key = "dir", .open = floppy_open, .get_size = floppy_get_size, + .can_multi_conn = floppy_can_multi_conn, .pread = floppy_pread, .errno_is_preserved = 1, }; diff --git a/plugins/iso/iso.c b/plugins/iso/iso.c index 7ed5e7a..7431b48 100644 --- a/plugins/iso/iso.c +++ b/plugins/iso/iso.c @@ -258,6 +258,13 @@ iso_get_size (void *handle) return statbuf.st_size; } +/* Serves the same data over multiple connections. */ +static int +iso_can_mult...

...2_t count, uint64_t offset); + int (*pwrite_old1) (void *handle, const void *buf, uint32_t count, uint64_t offset); int (*flush) (void *handle); 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_preserved; void (*dump_plugin) (void); + int (*pwrite) (void *handle, const void *buf, uint32_t count, uint64_t offset, unsigned flags); This is binary-compatible with old plugins, but not source compatible, so: (2) Plugsin may optionally define NBDKIT_PLUGIN_LEVEL before including <nbdkit-plu...

...assuming that Rust even collects this from low-level syscalls?). For historical reasons nbdkit plugins written in C set errno on error and nbdkit server reads the errno and may translate it into NBD errors on the wire (so it's generally important to get it right). However your plugin may set .errno_is_preserved = 0, then you can call nbdkit_set_error() whenever convenient before returning an error to set the errno that nbdkit will use. In OCaml plugins we have to go through some hoops to convert OCaml errno (which use a different numbering) to system errno: https://github.com/libguestfs/nbdkit/blob/904c...

Somewhat invasive but mostly mechanical change to how plugins are called. This patch is in preparation for adding a second backend subtype for filters. Rich.

.... I need to look at exactly >> what's going on here. >> >> Number (5) is actually fairly easy to deal with because there's only >> one place where we handle the errno returned by plugins >> (server/plugins.c:get_error). I think we'd probably want >> errno_is_preserved to mean "WSAGetLastError" or "GetLastError" >> contains something of interest. >> >> Thoughts? >> >> Also I really need to look at how some other portable libraries like >> curl and gnutls are handling this. Maybe they've already come up...

...em to set particular errno values, eg. errno = EIO. > > (3) It saves them around functions such as nbdkit_error(), which > is (1) + (2) but might be a distinct operation eg. using cleanups. > > (4) It prints them with perror and %m. > > (5) It handles plugins which claim errno_is_preserved. > > My first thought was we could define a macro which does: > > #ifdef WIN32 > #define errno (translate_to_errno (WSAGetLastError ())) > #endif > > which reads the last error and translates WSA* to E* codes. This > would solve (1) and is not very invasive for exi...

Here's what I'm currently playing with; I'm not ready to commit anything until I rebase my FUA work on top of this, as I only want to break filter ABI once between releases. Eric Blake (2): backend: Rework internal/filter error return semantics filters: Add log filter TODO | 2 - docs/nbdkit-filter.pod | 84 +++++++-- docs/nbdkit.pod