Previously, we let a plugin set an error in either thread-local storage (nbdkit_set_error()) or errno, then connections.c would decode which error to use. But with filters in the mix, it is very difficult for a filter to know what error was set by the plugin (particularly since nbdkit_set_error() has no public counterpart for reading the thread-local storage). What's more, if a filter does any non-trivial processing after calling into next_ops, it is very probable that errno might be corrupted, which would affect the error returned by a plugin 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 right error; which means this IS a subtle change to the filter interface (and worse, one that cannot be detected by the compiler because the API signatures did not change). However, more ABI changes are planned with adding FUA support, so as long as it is all done as part of the same release, we are okay. Also note that only nbdkit-plugin.h declares nbdkit_set_error(); we can actually keep it this way (filters do not need to call that function). Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- docs/nbdkit-filter.pod | 83 +++++++++++++++++++++++++++++++++++++------ src/internal.h | 1 - src/connections.c | 45 ++++++----------------- src/filters.c | 81 +++++++++++++++++++++++++---------------- src/plugins.c | 66 +++++++++++++++++++--------------- filters/cache/cache.c | 49 +++++++++++++++---------- filters/cow/cow.c | 28 +++++++++------ filters/partition/partition.c | 2 +- 8 files changed, 221 insertions(+), 134 deletions(-) diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod index eb72dae..4ddf25d 100644 --- a/docs/nbdkit-filter.pod +++ b/docs/nbdkit-filter.pod @@ -163,10 +163,14 @@ short-circuited. The filter’s other methods like C<.prepare>, C<.get_size>, C<.pread> etc ― always called in the context of a connection ― are passed a -pointer to C<struct nbdkit_next_ops> which contains a subset of the -plugin methods that can be called during a connection. It is possible -for a filter to issue (for example) extra read calls in response to a -single C<.pwrite> call. +pointer to C<struct nbdkit_next_ops> which contains a comparable set +of accessors to plugin methods that can be called during a connection. +It is possible for a filter to issue (for example) extra read calls in +response to a single C<.pwrite> call. Note that the semantics of the +functions in C<struct nbdkit_next_ops> are slightly different from +what a plugin implements: for example, while a plugin's C<.pread> +returns -1 on error, C<next_ops->pread> returns a positive errno +value. You can modify parameters when you call the C<next> function. However be careful when modifying strings because for some methods @@ -324,6 +328,11 @@ will see. The returned size must be E<ge> 0. If there is an error, C<.get_size> should call C<nbdkit_error> with an error message and return C<-1>. +If this function is called more than once for the same connection, it +should return the same value; similarly, the filter may cache +C<next_ops->get_size> for a given connection rather than repeating +calls. Note that if C<next_ops->get_size> fails, the value of +C<errno> is indeterminate. =head2 C<.can_write> @@ -346,7 +355,11 @@ should call C<nbdkit_error> with an error message and return C<-1>. These intercept the corresponding plugin methods. If there is an error, the callback should call C<nbdkit_error> with an -error message and return C<-1>. +error message and return C<-1>. If these functions are called more +than once for the same connection, they should return the same value; +similarly, the filter may cache the results of each counterpart in +C<next_ops> for a given connection rather than repeating calls. Note +that if C<next_ops> fails, the value of C<errno> is indeterminate. =head2 C<.pread> @@ -356,9 +369,15 @@ error message and return C<-1>. This intercepts the plugin C<.pread> method and can be used to read or modify data read by the plugin. +Note that unlike the plugin C<.pread> which returns -1 on error, +C<next_ops->pread> returns a positive errno value on error; the filter +should use this return value to learn why the plugin failed, and not +rely on C<errno>. + 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 B<and> set C<errno>, then return C<-1>. +message B<and> either return -1 with C<errno> set or return a positive +errno value corresponding to the problem. =head2 C<.pwrite> @@ -369,9 +388,15 @@ message B<and> set C<errno>, then return C<-1>. This intercepts the plugin C<.pwrite> method and can be used to modify data written by the plugin. +Note that unlike the plugin C<.pwrite> which returns -1 on error, +C<next_ops->pwrite> returns a positive errno value on error; the +filter should use this return value to learn why the plugin failed, +and not rely on C<errno>. + 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 B<and> set C<errno>, then return C<-1>. +message B<and> either return -1 with C<errno> set or return a positive +errno value corresponding to the problem. =head2 C<.flush> @@ -381,8 +406,14 @@ message B<and> set C<errno>, then return C<-1>. This intercepts the plugin C<.flush> method and can be used to modify flush requests. +Note that unlike the plugin C<.flush> which returns -1 on error, +C<next_ops->flush> returns a positive errno value on error; the filter +should use this return value to learn why the plugin failed, and not +rely on C<errno>. + If there is an error, C<.flush> should call C<nbdkit_error> with an -error message B<and> set C<errno>, then return C<-1>. +error message B<and> either return -1 with C<errno> set or return a +positive errno value corresponding to the problem. =head2 C<.trim> @@ -392,8 +423,14 @@ error message B<and> set C<errno>, then return C<-1>. This intercepts the plugin C<.trim> method and can be used to modify trim requests. +Note that unlike the plugin C<.trim> which returns -1 on error, +C<next_ops->trim> returns a positive errno value on error; the filter +should use this return value to learn why the plugin failed, and not +rely on C<errno>. + If there is an error, C<.trim> should call C<nbdkit_error> with an -error message B<and> set C<errno>, then return C<-1>. +error message B<and> either return -1 with C<errno> set or return a +positive errno value corresponding to the problem. =head2 C<.zero> @@ -403,8 +440,34 @@ error message B<and> set C<errno>, then return C<-1>. This intercepts the plugin C<.zero> method and can be used to modify zero requests. +Note that unlike the plugin C<.zero> which returns -1 on error, +C<next_ops->zero> returns a positive errno value on error; the filter +should use this return value to learn why the plugin failed, and not +rely on C<errno>. Furthermore, C<next_ops->zero> will never return +C<ENOTSUP>; the plugin will have already fallen back to using +C<.pwrite> instead. + If there is an error, C<.zero> should call C<nbdkit_error> with an -error message B<and> set C<errno>, then return C<-1>. +error message B<and> either return -1 with C<errno> set or return a +positive errno value corresponding to the problem. The filter should +never fail with C<ENOTSUP> (while plugins have automatic fallback to +C<.pwrite>, filters do not). + +=head1 ERROR HANDLING + +If there is an error in the filter itself, the filter should call +C<nbdkit_error> to report an error message. If the callback is +involved in serving data, the return value determines the error code +that will be sent to the client; other callbacks should return the +appropriate error indication, eg. C<NULL> or C<-1>. + +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 DEBUGGING diff --git a/src/internal.h b/src/internal.h index 3cbfde5..2cda390 100644 --- a/src/internal.h +++ b/src/internal.h @@ -176,7 +176,6 @@ struct backend { void (*dump_fields) (struct backend *); void (*config) (struct backend *, const char *key, const char *value); void (*config_complete) (struct backend *); - int (*errno_is_preserved) (struct backend *); int (*open) (struct backend *, struct connection *conn, int readonly); int (*prepare) (struct backend *, struct connection *conn); int (*finalize) (struct backend *, struct connection *conn); diff --git a/src/connections.c b/src/connections.c index 75c2c2d..2959493 100644 --- a/src/connections.c +++ b/src/connections.c @@ -43,6 +43,7 @@ #include <endian.h> #include <sys/types.h> #include <stddef.h> +#include <assert.h> #include <pthread.h> @@ -912,18 +913,6 @@ validate_request (struct connection *conn, return true; /* Command validates. */ } -/* Grab the appropriate error value. - */ -static int -get_error (struct connection *conn) -{ - int ret = threadlocal_get_error (); - - if (!ret && backend->errno_is_preserved (backend)) - 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 @@ -942,49 +931,36 @@ handle_request (struct connection *conn, uint32_t f = 0; bool fua = conn->can_flush && (flags & NBD_CMD_FLAG_FUA); - /* The plugin should call nbdkit_set_error() to request a particular - error, otherwise we fallback to errno or EIO. */ + /* Clear the error, so that we know if the plugin calls + nbdkit_set_error() or relied on errno. */ threadlocal_set_error (0); switch (cmd) { case NBD_CMD_READ: - if (backend->pread (backend, conn, buf, count, offset, 0) == -1) - return get_error (conn); - break; + return backend->pread (backend, conn, buf, count, offset, 0); case NBD_CMD_WRITE: if (fua) f |= NBDKIT_FLAG_FUA; - if (backend->pwrite (backend, conn, buf, count, offset, f) == -1) - return get_error (conn); - break; + return backend->pwrite (backend, conn, buf, count, offset, f); case NBD_CMD_FLUSH: - if (backend->flush (backend, conn, 0) == -1) - return get_error (conn); - break; + return backend->flush (backend, conn, 0); case NBD_CMD_TRIM: if (fua) f |= NBDKIT_FLAG_FUA; - if (backend->trim (backend, conn, count, offset, f) == -1) - return get_error (conn); - break; + return backend->trim (backend, conn, count, offset, f); case NBD_CMD_WRITE_ZEROES: if (!(flags & NBD_CMD_FLAG_NO_HOLE)) f |= NBDKIT_FLAG_MAY_TRIM; if (fua) f |= NBDKIT_FLAG_FUA; - if (backend->zero (backend, conn, count, offset, f) == -1) - return get_error (conn); - break; - - default: - abort (); + return backend->zero (backend, conn, count, offset, f); } - - return 0; + /* Unreachable */ + abort (); } static int @@ -1130,6 +1106,7 @@ recv_request_send_reply (struct connection *conn) else { lock_request (conn); error = handle_request (conn, cmd, flags, offset, count, buf); + assert ((int) error >= 0); unlock_request (conn); } diff --git a/src/filters.c b/src/filters.c index 40c4913..1003dc7 100644 --- a/src/filters.c +++ b/src/filters.c @@ -107,14 +107,6 @@ filter_thread_model (struct backend *b) /* These are actually passing through to the final plugin, hence * the function names. */ -static int -plugin_errno_is_preserved (struct backend *b) -{ - struct backend_filter *f = container_of (b, struct backend_filter, backend); - - return f->backend.next->errno_is_preserved (f->backend.next); -} - static const char * plugin_name (struct backend *b) { @@ -463,17 +455,23 @@ filter_pread (struct backend *b, struct connection *conn, struct backend_filter *f = container_of (b, struct backend_filter, backend); void *handle = connection_get_handle (conn, f->backend.i); struct b_conn nxdata = { .b = f->backend.next, .conn = conn }; + int r; assert (flags == 0); debug ("pread count=%" PRIu32 " offset=%" PRIu64, count, offset); - if (f->filter.pread) - return f->filter.pread (&next_ops, &nxdata, handle, - buf, count, offset); + if (f->filter.pread) { + r = f->filter.pread (&next_ops, &nxdata, handle, + buf, count, offset); + if (r < 0) + r = errno; + } else - return f->backend.next->pread (f->backend.next, conn, - buf, count, offset, flags); + r = f->backend.next->pread (f->backend.next, conn, + buf, count, offset, flags); + assert (r >= 0); + return r; } static int @@ -485,18 +483,24 @@ filter_pwrite (struct backend *b, struct connection *conn, void *handle = connection_get_handle (conn, f->backend.i); struct b_conn nxdata = { .b = f->backend.next, .conn = conn }; bool fua = flags & NBDKIT_FLAG_FUA; + int r; assert (!(flags & ~NBDKIT_FLAG_FUA)); debug ("pwrite count=%" PRIu32 " offset=%" PRIu64 " fua=%d", count, offset, fua); - if (f->filter.pwrite) - return f->filter.pwrite (&next_ops, &nxdata, handle, - buf, count, offset); + if (f->filter.pwrite) { + r = f->filter.pwrite (&next_ops, &nxdata, handle, + buf, count, offset); + if (r < 0) + r = errno; + } else - return f->backend.next->pwrite (f->backend.next, conn, - buf, count, offset, flags); + r = f->backend.next->pwrite (f->backend.next, conn, + buf, count, offset, flags); + assert (r >= 0); + return r; } static int @@ -505,15 +509,21 @@ filter_flush (struct backend *b, struct connection *conn, uint32_t flags) struct backend_filter *f = container_of (b, struct backend_filter, backend); void *handle = connection_get_handle (conn, f->backend.i); struct b_conn nxdata = { .b = f->backend.next, .conn = conn }; + int r; assert (flags == 0); debug ("flush"); - if (f->filter.flush) - return f->filter.flush (&next_ops, &nxdata, handle); + if (f->filter.flush) { + r= f->filter.flush (&next_ops, &nxdata, handle); + if (r < 0) + r = errno; + } else - return f->backend.next->flush (f->backend.next, conn, flags); + r = f->backend.next->flush (f->backend.next, conn, flags); + assert (r >= 0); + return r; } static int @@ -524,15 +534,21 @@ filter_trim (struct backend *b, struct connection *conn, struct backend_filter *f = container_of (b, struct backend_filter, backend); void *handle = connection_get_handle (conn, f->backend.i); struct b_conn nxdata = { .b = f->backend.next, .conn = conn }; + int r; assert (flags == 0); debug ("trim count=%" PRIu32 " offset=%" PRIu64, count, offset); - if (f->filter.trim) - return f->filter.trim (&next_ops, &nxdata, handle, count, offset); + if (f->filter.trim) { + r = f->filter.trim (&next_ops, &nxdata, handle, count, offset); + if (r < 0) + r = errno; + } else - return f->backend.next->trim (f->backend.next, conn, count, offset, flags); + r = f->backend.next->trim (f->backend.next, conn, count, offset, flags); + assert (r >= 0); + return r; } static int @@ -543,18 +559,24 @@ filter_zero (struct backend *b, struct connection *conn, void *handle = connection_get_handle (conn, f->backend.i); struct b_conn nxdata = { .b = f->backend.next, .conn = conn }; int may_trim = (flags & NBDKIT_FLAG_MAY_TRIM) != 0; + int r; assert (!(flags & ~(NBDKIT_FLAG_MAY_TRIM | NBDKIT_FLAG_FUA))); debug ("zero count=%" PRIu32 " offset=%" PRIu64 " may_trim=%d", count, offset, may_trim); - if (f->filter.zero) - return f->filter.zero (&next_ops, &nxdata, handle, - count, offset, may_trim); + if (f->filter.zero) { + r = f->filter.zero (&next_ops, &nxdata, handle, + count, offset, may_trim); + if (r < 0) + r = errno; + } else - return f->backend.next->zero (f->backend.next, conn, - count, offset, flags); + r = f->backend.next->zero (f->backend.next, conn, + count, offset, flags); + assert (r >= 0 && r != ENOTSUP); + return r; } static struct backend filter_functions = { @@ -567,7 +589,6 @@ static struct backend filter_functions = { .dump_fields = filter_dump_fields, .config = filter_config, .config_complete = filter_config_complete, - .errno_is_preserved = plugin_errno_is_preserved, .open = filter_open, .prepare = filter_prepare, .finalize = filter_finalize, diff --git a/src/plugins.c b/src/plugins.c index dba3e24..c49c0f0 100644 --- a/src/plugins.c +++ b/src/plugins.c @@ -227,14 +227,6 @@ plugin_config_complete (struct backend *b) exit (EXIT_FAILURE); } -static int -plugin_errno_is_preserved (struct backend *b) -{ - struct backend_plugin *p = container_of (b, struct backend_plugin, backend); - - return p->plugin.errno_is_preserved; -} - static int plugin_open (struct backend *b, struct connection *conn, int readonly) { @@ -358,11 +350,24 @@ plugin_can_trim (struct backend *b, struct connection *conn) return p->plugin.trim != NULL; } +/* Grab the appropriate error value. + */ +static int +get_error (struct backend_plugin *p) +{ + int ret = threadlocal_get_error (); + + if (!ret && p->plugin.errno_is_preserved) + ret = errno; + return ret ? ret : EIO; +} + static int plugin_pread (struct backend *b, struct connection *conn, void *buf, uint32_t count, uint64_t offset, uint32_t flags) { struct backend_plugin *p = container_of (b, struct backend_plugin, backend); + int r; assert (connection_get_handle (conn, 0)); assert (p->plugin.pread != NULL); @@ -370,25 +375,29 @@ plugin_pread (struct backend *b, struct connection *conn, debug ("pread count=%" PRIu32 " offset=%" PRIu64, count, offset); - return p->plugin.pread (connection_get_handle (conn, 0), buf, count, offset); + r = p->plugin.pread (connection_get_handle (conn, 0), buf, count, offset); + if (r < 0) + r = get_error (p); + return r; } static int plugin_flush (struct backend *b, struct connection *conn, uint32_t flags) { struct backend_plugin *p = container_of (b, struct backend_plugin, backend); + int r; assert (connection_get_handle (conn, 0)); assert (!flags); debug ("flush"); - if (p->plugin.flush != NULL) - return p->plugin.flush (connection_get_handle (conn, 0)); - else { - errno = EINVAL; - return -1; - } + if (!p->plugin.flush) + return EINVAL; + r = p->plugin.flush (connection_get_handle (conn, 0)); + if (r < 0) + r = get_error (p); + return r; } static int @@ -408,14 +417,14 @@ plugin_pwrite (struct backend *b, struct connection *conn, if (p->plugin.pwrite != NULL) r = p->plugin.pwrite (connection_get_handle (conn, 0), buf, count, offset); - else { - errno = EROFS; - return -1; - } + else + return EROFS; if (r == 0 && fua) { assert (p->plugin.flush); r = plugin_flush (b, conn, 0); } + if (r < 0) + r = get_error (p); return r; } @@ -435,14 +444,14 @@ plugin_trim (struct backend *b, struct connection *conn, if (p->plugin.trim != NULL) r = p->plugin.trim (connection_get_handle (conn, 0), count, offset); - else { - errno = EINVAL; - return -1; - } + else + return EINVAL; if (r == 0 && fua) { assert (p->plugin.flush); r = plugin_flush (b, conn, 0); } + if (r < 0) + r = get_error (p); return r; } @@ -472,7 +481,7 @@ plugin_zero (struct backend *b, struct connection *conn, count, offset, may_trim); if (result == -1) { err = threadlocal_get_error (); - if (!err && plugin_errno_is_preserved (b)) + if (!err && p->plugin.errno_is_preserved) err = errno; } if (result == 0 || err != EOPNOTSUPP) @@ -483,10 +492,8 @@ plugin_zero (struct backend *b, struct connection *conn, threadlocal_set_error (0); limit = count < MAX_REQUEST_SIZE ? count : MAX_REQUEST_SIZE; buf = calloc (limit, 1); - if (!buf) { - errno = ENOMEM; - return -1; - } + if (!buf) + return ENOMEM; while (count) { result = p->plugin.pwrite (connection_get_handle (conn, 0), @@ -507,6 +514,8 @@ plugin_zero (struct backend *b, struct connection *conn, assert (p->plugin.flush); result = plugin_flush (b, conn, 0); } + if (result < 0) + result = get_error (p); return result; } @@ -520,7 +529,6 @@ static struct backend plugin_functions = { .dump_fields = plugin_dump_fields, .config = plugin_config, .config_complete = plugin_config_complete, - .errno_is_preserved = plugin_errno_is_preserved, .open = plugin_open, .prepare = plugin_prepare, .finalize = plugin_finalize, diff --git a/filters/cache/cache.c b/filters/cache/cache.c index 9473f2c..2ae6f01 100644 --- a/filters/cache/cache.c +++ b/filters/cache/cache.c @@ -292,6 +292,7 @@ blk_writethrough (struct nbdkit_next_ops *next_ops, void *nxdata, uint64_t blknum, const uint8_t *block) { off_t offset = blknum * BLKSIZE; + int r; nbdkit_debug ("cache: blk_writethrough block %" PRIu64 " (offset %" PRIu64 ")", @@ -302,8 +303,9 @@ blk_writethrough (struct nbdkit_next_ops *next_ops, void *nxdata, return -1; } - if (next_ops->pwrite (nxdata, block, BLKSIZE, offset) == -1) - return -1; + r = next_ops->pwrite (nxdata, block, BLKSIZE, offset); + if (r) + return r; blk_set_bitmap_entry (blknum, BLOCK_CLEAN); @@ -341,6 +343,7 @@ cache_pread (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, void *buf, uint32_t count, uint64_t offset) { uint8_t *block; + int r; block = malloc (BLKSIZE); if (block == NULL) { @@ -357,9 +360,10 @@ cache_pread (struct nbdkit_next_ops *next_ops, void *nxdata, if (n > count) n = count; - if (blk_read (next_ops, nxdata, blknum, block) == -1) { + r = blk_read (next_ops, nxdata, blknum, block); + if (r) { free (block); - return -1; + return r; } memcpy (buf, &block[blkoffs], n); @@ -379,6 +383,7 @@ cache_pwrite (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, const void *buf, uint32_t count, uint64_t offset) { uint8_t *block; + int r; block = malloc (BLKSIZE); if (block == NULL) { @@ -396,14 +401,16 @@ cache_pwrite (struct nbdkit_next_ops *next_ops, void *nxdata, n = count; /* Do a read-modify-write operation on the current block. */ - if (blk_read (next_ops, nxdata, blknum, block) == -1) { + r = blk_read (next_ops, nxdata, blknum, block); + if (r) { free (block); - return -1; + return r; } memcpy (&block[blkoffs], buf, n); - if (blk_writeback (next_ops, nxdata, blknum, block) == -1) { + r = blk_writeback (next_ops, nxdata, blknum, block); + if (r) { free (block); - return -1; + return r; } buf += n; @@ -421,6 +428,7 @@ cache_zero (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, uint32_t count, uint64_t offset, int may_trim) { uint8_t *block; + int r; block = malloc (BLKSIZE); if (block == NULL) { @@ -437,14 +445,16 @@ cache_zero (struct nbdkit_next_ops *next_ops, void *nxdata, if (n > count) n = count; - if (blk_read (next_ops, nxdata, blknum, block) == -1) { + r = blk_read (next_ops, nxdata, blknum, block); + if (r) { free (block); - return -1; + return r; } memset (&block[blkoffs], 0, n); - if (blk_writeback (next_ops, nxdata, blknum, block) == -1) { + r = blk_writeback (next_ops, nxdata, blknum, block); + if (r) { free (block); - return -1; + return r; } count -= n; @@ -463,7 +473,7 @@ cache_flush (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle) uint64_t i, j; uint64_t blknum; enum bm_entry state; - unsigned errors = 0; + int error = 0, r; if (cache_mode == CACHE_MODE_UNSAFE) return 0; @@ -494,10 +504,10 @@ cache_flush (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle) /* Perform a read + writethrough which will read from the * cache and write it through to the underlying storage. */ - if (blk_read (next_ops, nxdata, blknum, block) == -1 || - blk_writethrough (next_ops, nxdata, blknum, block)) { + if ((r = blk_read (next_ops, nxdata, blknum, block)) || + (r = blk_writethrough (next_ops, nxdata, blknum, block))) { nbdkit_error ("cache: flush of block %" PRIu64 " failed", blknum); - errors++; + error = error ? error : r; } } } @@ -507,10 +517,11 @@ cache_flush (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle) free (block); /* Now issue a flush request to the underlying storage. */ - if (next_ops->flush (nxdata) == -1) - errors++; + r = next_ops->flush (nxdata); + if (r) + error = error ? error : r; - return errors == 0 ? 0 : -1; + return error; } static struct nbdkit_filter filter = { diff --git a/filters/cow/cow.c b/filters/cow/cow.c index 5c2fcd0..18debf7 100644 --- a/filters/cow/cow.c +++ b/filters/cow/cow.c @@ -313,6 +313,7 @@ cow_pread (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, void *buf, uint32_t count, uint64_t offset) { uint8_t *block; + int r; block = malloc (BLKSIZE); if (block == NULL) { @@ -329,9 +330,10 @@ cow_pread (struct nbdkit_next_ops *next_ops, void *nxdata, if (n > count) n = count; - if (blk_read (next_ops, nxdata, blknum, block) == -1) { + r = blk_read (next_ops, nxdata, blknum, block); + if (r) { free (block); - return -1; + return r; } memcpy (buf, &block[blkoffs], n); @@ -351,6 +353,7 @@ cow_pwrite (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, const void *buf, uint32_t count, uint64_t offset) { uint8_t *block; + int r; block = malloc (BLKSIZE); if (block == NULL) { @@ -368,14 +371,16 @@ cow_pwrite (struct nbdkit_next_ops *next_ops, void *nxdata, n = count; /* Do a read-modify-write operation on the current block. */ - if (blk_read (next_ops, nxdata, blknum, block) == -1) { + r = blk_read (next_ops, nxdata, blknum, block); + if (r) { free (block); - return -1; + return r; } memcpy (&block[blkoffs], buf, n); - if (blk_write (blknum, block) == -1) { + r = blk_write (blknum, block); + if (r) { free (block); - return -1; + return r; } buf += n; @@ -393,6 +398,7 @@ cow_zero (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, uint32_t count, uint64_t offset, int may_trim) { uint8_t *block; + int r; block = malloc (BLKSIZE); if (block == NULL) { @@ -412,14 +418,16 @@ cow_zero (struct nbdkit_next_ops *next_ops, void *nxdata, /* XXX There is the possibility of optimizing this: ONLY if we are * writing a whole, aligned block, then use FALLOC_FL_ZERO_RANGE. */ - if (blk_read (next_ops, nxdata, blknum, block) == -1) { + r = blk_read (next_ops, nxdata, blknum, block); + if (r) { free (block); - return -1; + return r; } memset (&block[blkoffs], 0, n); - if (blk_write (blknum, block) == -1) { + r = blk_write (blknum, block); + if (r) { free (block); - return -1; + return r; } count -= n; diff --git a/filters/partition/partition.c b/filters/partition/partition.c index f74b3af..bf5238d 100644 --- a/filters/partition/partition.c +++ b/filters/partition/partition.c @@ -216,7 +216,7 @@ find_gpt_partition (struct nbdkit_next_ops *next_ops, void *nxdata, * partition_prepare call above. */ if (next_ops->pread (nxdata, partition_bytes, sizeof partition_bytes, - 2*512 + i*128) == -1) + 2*512 + i*128)) return -1; get_gpt_partition (partition_bytes, &partition); if (memcmp (partition.partition_type_guid, -- 2.14.3