Time to take advantage of C typing rules. First, make 'struct
context' a derived class with 'struct nbdkit_next_ops' as its first
member, so that a pointer to one is now the same pointer to the other
at the ABI level. Next, note that all entries in filters.h's next_ops
are backend_* functions, so we can move that static list into
backend.h instead, and populate next_ops as part of backend_open.
Since filters still see 'nbdkit_context*' typedef'd to 'void*', they
are none the wiser, even though filters.c is now passing the same
pointer to two different parameters (since &c_next->next == c_next).
Yes, that means that next_ops->FOO(nxdata) can now be equally spelled
as next_ops->FOO(next_ops).
The oddball (once again) is the retry filter; since
nbdkit_backend_reopen only affects one of the two pointers, the filter
is now updated to ignore one parameter in favor of the other.
A later patch will then do the mechanical change to all the remaining
filters to be fully type-safe at the same time we reduce the parameter
duplication to just pass a single copy of context. The upshot of this
refactoring is that it also becomes easier to introduce a new function
for opening a context from a filter, which can return a single pointer
containing everything the filter needs to call into the plugin.
---
include/nbdkit-filter.h | 2 +-
server/internal.h | 2 +
server/backend.c | 23 +++++++++++
server/filters.c | 64 ++++++++++-------------------
filters/retry/retry.c | 84 +++++++++++++++++++-------------------
tests/test-layers-filter.c | 1 +
6 files changed, 90 insertions(+), 86 deletions(-)
diff --git a/include/nbdkit-filter.h b/include/nbdkit-filter.h
index d16ea330..b47e78c3 100644
--- a/include/nbdkit-filter.h
+++ b/include/nbdkit-filter.h
@@ -57,7 +57,7 @@ typedef struct backend nbdkit_backend;
typedef struct context nbdkit_next;
#elif defined NBDKIT_RETRY_FILTER /* Hack to expose reopen to retry filter */
typedef struct nbdkit_backend nbdkit_backend;
-typedef struct nbdkit_next nbdkit_next;
+typedef struct nbdkit_next_ops nbdkit_next;
#else
typedef void nbdkit_backend;
typedef void nbdkit_next;
diff --git a/server/internal.h b/server/internal.h
index e5e8f797..23639bcd 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -203,6 +203,8 @@ enum {
};
struct context {
+ struct nbdkit_next_ops next; /* Must be first member, for ABI reasons */
+
void *handle; /* Plugin or filter handle. */
struct backend *b; /* Backend that provided handle. */
diff --git a/server/backend.c b/server/backend.c
index 94ff0caa..58d10a3f 100644
--- a/server/backend.c
+++ b/server/backend.c
@@ -211,6 +211,28 @@ backend_default_export (struct backend *b, int readonly)
return conn->default_exportname[b->i];
}
+static struct nbdkit_next_ops next_ops = {
+ .export_description = backend_export_description,
+ .get_size = backend_get_size,
+ .can_write = backend_can_write,
+ .can_flush = backend_can_flush,
+ .is_rotational = backend_is_rotational,
+ .can_trim = backend_can_trim,
+ .can_zero = backend_can_zero,
+ .can_fast_zero = backend_can_fast_zero,
+ .can_extents = backend_can_extents,
+ .can_fua = backend_can_fua,
+ .can_multi_conn = backend_can_multi_conn,
+ .can_cache = backend_can_cache,
+ .pread = backend_pread,
+ .pwrite = backend_pwrite,
+ .flush = backend_flush,
+ .trim = backend_trim,
+ .zero = backend_zero,
+ .extents = backend_extents,
+ .cache = backend_cache,
+};
+
struct context *
backend_open (struct backend *b, int readonly, const char *exportname)
{
@@ -226,6 +248,7 @@ backend_open (struct backend *b, int readonly, const char
*exportname)
b->name, readonly, exportname, conn->using_tls);
assert (conn->contexts[b->i] == NULL);
+ c->next = next_ops;
c->handle = NULL;
c->b = b;
c->state = 0;
diff --git a/server/filters.c b/server/filters.c
index 5f2087d1..a200d61b 100644
--- a/server/filters.c
+++ b/server/filters.c
@@ -303,28 +303,6 @@ filter_close (struct context *c)
f->filter.close (c->handle);
}
-static struct nbdkit_next_ops next_ops = {
- .export_description = backend_export_description,
- .get_size = backend_get_size,
- .can_write = backend_can_write,
- .can_flush = backend_can_flush,
- .is_rotational = backend_is_rotational,
- .can_trim = backend_can_trim,
- .can_zero = backend_can_zero,
- .can_fast_zero = backend_can_fast_zero,
- .can_extents = backend_can_extents,
- .can_fua = backend_can_fua,
- .can_multi_conn = backend_can_multi_conn,
- .can_cache = backend_can_cache,
- .pread = backend_pread,
- .pwrite = backend_pwrite,
- .flush = backend_flush,
- .trim = backend_trim,
- .zero = backend_zero,
- .extents = backend_extents,
- .cache = backend_cache,
-};
-
static int
filter_prepare (struct context *c, int readonly)
{
@@ -334,7 +312,7 @@ filter_prepare (struct context *c, int readonly)
struct context *c_next = get_context (conn, b->next);
if (f->filter.prepare &&
- f->filter.prepare (&next_ops, c_next, c->handle, readonly) == -1)
+ f->filter.prepare (&c_next->next, c_next, c->handle, readonly) == -1)
return -1;
return 0;
@@ -349,7 +327,7 @@ filter_finalize (struct context *c)
struct context *c_next = get_context (conn, b->next);
if (f->filter.finalize &&
- f->filter.finalize (&next_ops, c_next, c->handle) == -1)
+ f->filter.finalize (&c_next->next, c_next, c->handle) == -1)
return -1;
return 0;
}
@@ -363,7 +341,7 @@ filter_export_description (struct context *c)
struct context *c_next = get_context (conn, b->next);
if (f->filter.export_description)
- return f->filter.export_description (&next_ops, c_next, c->handle);
+ return f->filter.export_description (&c_next->next, c_next, c->handle);
else
return backend_export_description (c_next);
}
@@ -377,7 +355,7 @@ filter_get_size (struct context *c)
struct context *c_next = get_context (conn, b->next);
if (f->filter.get_size)
- return f->filter.get_size (&next_ops, c_next, c->handle);
+ return f->filter.get_size (&c_next->next, c_next, c->handle);
else
return backend_get_size (c_next);
}
@@ -391,7 +369,7 @@ filter_can_write (struct context *c)
struct context *c_next = get_context (conn, b->next);
if (f->filter.can_write)
- return f->filter.can_write (&next_ops, c_next, c->handle);
+ return f->filter.can_write (&c_next->next, c_next, c->handle);
else
return backend_can_write (c_next);
}
@@ -405,7 +383,7 @@ filter_can_flush (struct context *c)
struct context *c_next = get_context (conn, b->next);
if (f->filter.can_flush)
- return f->filter.can_flush (&next_ops, c_next, c->handle);
+ return f->filter.can_flush (&c_next->next, c_next, c->handle);
else
return backend_can_flush (c_next);
}
@@ -419,7 +397,7 @@ filter_is_rotational (struct context *c)
struct context *c_next = get_context (conn, b->next);
if (f->filter.is_rotational)
- return f->filter.is_rotational (&next_ops, c_next, c->handle);
+ return f->filter.is_rotational (&c_next->next, c_next, c->handle);
else
return backend_is_rotational (c_next);
}
@@ -433,7 +411,7 @@ filter_can_trim (struct context *c)
struct context *c_next = get_context (conn, b->next);
if (f->filter.can_trim)
- return f->filter.can_trim (&next_ops, c_next, c->handle);
+ return f->filter.can_trim (&c_next->next, c_next, c->handle);
else
return backend_can_trim (c_next);
}
@@ -447,7 +425,7 @@ filter_can_zero (struct context *c)
struct context *c_next = get_context (conn, b->next);
if (f->filter.can_zero)
- return f->filter.can_zero (&next_ops, c_next, c->handle);
+ return f->filter.can_zero (&c_next->next, c_next, c->handle);
else
return backend_can_zero (c_next);
}
@@ -461,7 +439,7 @@ filter_can_fast_zero (struct context *c)
struct context *c_next = get_context (conn, b->next);
if (f->filter.can_fast_zero)
- return f->filter.can_fast_zero (&next_ops, c_next, c->handle);
+ return f->filter.can_fast_zero (&c_next->next, c_next, c->handle);
else
return backend_can_fast_zero (c_next);
}
@@ -475,7 +453,7 @@ filter_can_extents (struct context *c)
struct context *c_next = get_context (conn, b->next);
if (f->filter.can_extents)
- return f->filter.can_extents (&next_ops, c_next, c->handle);
+ return f->filter.can_extents (&c_next->next, c_next, c->handle);
else
return backend_can_extents (c_next);
}
@@ -489,7 +467,7 @@ filter_can_fua (struct context *c)
struct context *c_next = get_context (conn, b->next);
if (f->filter.can_fua)
- return f->filter.can_fua (&next_ops, c_next, c->handle);
+ return f->filter.can_fua (&c_next->next, c_next, c->handle);
else
return backend_can_fua (c_next);
}
@@ -503,7 +481,7 @@ filter_can_multi_conn (struct context *c)
struct context *c_next = get_context (conn, b->next);
if (f->filter.can_multi_conn)
- return f->filter.can_multi_conn (&next_ops, c_next, c->handle);
+ return f->filter.can_multi_conn (&c_next->next, c_next, c->handle);
else
return backend_can_multi_conn (c_next);
}
@@ -517,7 +495,7 @@ filter_can_cache (struct context *c)
struct context *c_next = get_context (conn, b->next);
if (f->filter.can_cache)
- return f->filter.can_cache (&next_ops, c_next, c->handle);
+ return f->filter.can_cache (&c_next->next, c_next, c->handle);
else
return backend_can_cache (c_next);
}
@@ -533,7 +511,7 @@ filter_pread (struct context *c,
struct context *c_next = get_context (conn, b->next);
if (f->filter.pread)
- return f->filter.pread (&next_ops, c_next, c->handle,
+ return f->filter.pread (&c_next->next, c_next, c->handle,
buf, count, offset, flags, err);
else
return backend_pread (c_next, buf, count, offset, flags, err);
@@ -550,7 +528,7 @@ filter_pwrite (struct context *c,
struct context *c_next = get_context (conn, b->next);
if (f->filter.pwrite)
- return f->filter.pwrite (&next_ops, c_next, c->handle,
+ return f->filter.pwrite (&c_next->next, c_next, c->handle,
buf, count, offset, flags, err);
else
return backend_pwrite (c_next, buf, count, offset, flags, err);
@@ -566,7 +544,7 @@ filter_flush (struct context *c,
struct context *c_next = get_context (conn, b->next);
if (f->filter.flush)
- return f->filter.flush (&next_ops, c_next, c->handle, flags, err);
+ return f->filter.flush (&c_next->next, c_next, c->handle, flags, err);
else
return backend_flush (c_next, flags, err);
}
@@ -582,7 +560,7 @@ filter_trim (struct context *c,
struct context *c_next = get_context (conn, b->next);
if (f->filter.trim)
- return f->filter.trim (&next_ops, c_next, c->handle, count, offset,
+ return f->filter.trim (&c_next->next, c_next, c->handle, count, offset,
flags, err);
else
return backend_trim (c_next, count, offset, flags, err);
@@ -598,7 +576,7 @@ filter_zero (struct context *c,
struct context *c_next = get_context (conn, b->next);
if (f->filter.zero)
- return f->filter.zero (&next_ops, c_next, c->handle,
+ return f->filter.zero (&c_next->next, c_next, c->handle,
count, offset, flags, err);
else
return backend_zero (c_next, count, offset, flags, err);
@@ -615,7 +593,7 @@ filter_extents (struct context *c,
struct context *c_next = get_context (conn, b->next);
if (f->filter.extents)
- return f->filter.extents (&next_ops, c_next, c->handle,
+ return f->filter.extents (&c_next->next, c_next, c->handle,
count, offset, flags,
extents, err);
else
@@ -634,7 +612,7 @@ filter_cache (struct context *c,
struct context *c_next = get_context (conn, b->next);
if (f->filter.cache)
- return f->filter.cache (&next_ops, c_next, c->handle,
+ return f->filter.cache (&c_next->next, c_next, c->handle,
count, offset, flags, err);
else
return backend_cache (c_next, count, offset, flags, err);
diff --git a/filters/retry/retry.c b/filters/retry/retry.c
index 8decee6a..a2b5dc66 100644
--- a/filters/retry/retry.c
+++ b/filters/retry/retry.c
@@ -164,10 +164,10 @@ struct retry_data {
};
static bool
-valid_range (struct nbdkit_next_ops *next_ops, void *nxdata,
+valid_range (struct nbdkit_next_ops *next,
uint32_t count, uint64_t offset, bool is_write, int *err)
{
- if ((int64_t) offset + count > next_ops->get_size (nxdata)) {
+ if ((int64_t) offset + count > next->get_size (next)) {
*err = is_write ? ENOSPC : EIO;
return false;
}
@@ -176,7 +176,7 @@ valid_range (struct nbdkit_next_ops *next_ops, void *nxdata,
static bool
do_retry (struct retry_handle *h, struct retry_data *data,
- nbdkit_next **nxdata, const char *method, int *err)
+ nbdkit_next **next, const char *method, int *err)
{
/* If it's the first retry, initialize the other fields in *data. */
if (data->retry == 0)
@@ -210,7 +210,7 @@ do_retry (struct retry_handle *h, struct retry_data *data,
/* Reopen the connection. */
h->reopens++;
if (nbdkit_backend_reopen (h->backend, h->readonly || force_readonly,
- h->exportname, nxdata) == -1) {
+ h->exportname, next) == -1) {
/* If the reopen fails we treat it the same way as a command
* failing.
*/
@@ -225,7 +225,7 @@ do_retry (struct retry_handle *h, struct retry_data *data,
}
static int
-retry_pread (struct nbdkit_next_ops *next_ops, nbdkit_next *nxdata,
+retry_pread (struct nbdkit_next_ops *next_ops, nbdkit_next *next,
void *handle, void *buf, uint32_t count, uint64_t offset,
uint32_t flags, int *err)
{
@@ -234,11 +234,11 @@ retry_pread (struct nbdkit_next_ops *next_ops, nbdkit_next *nxdata,
int r;
again:
- if (! (h->open && valid_range (next_ops, nxdata, count, offset, false,
err)))
+ if (! (h->open && valid_range (next, count, offset, false, err)))
r = -1;
else
- r = next_ops->pread (nxdata, buf, count, offset, flags, err);
- if (r == -1 && do_retry (h, &data, &nxdata, "pread", err))
+ r = next->pread (next, buf, count, offset, flags, err);
+ if (r == -1 && do_retry (h, &data, &next, "pread", err))
goto again;
return r;
@@ -246,7 +246,7 @@ retry_pread (struct nbdkit_next_ops *next_ops, nbdkit_next *nxdata,
/* Write. */
static int
-retry_pwrite (struct nbdkit_next_ops *next_ops, nbdkit_next *nxdata,
+retry_pwrite (struct nbdkit_next_ops *next_ops, nbdkit_next *next,
void *handle,
const void *buf, uint32_t count, uint64_t offset,
uint32_t flags, int *err)
@@ -260,20 +260,20 @@ retry_pwrite (struct nbdkit_next_ops *next_ops, nbdkit_next
*nxdata,
*err = EROFS;
return -1;
}
- if (! (h->open && valid_range (next_ops, nxdata, count, offset, true,
err)))
+ if (! (h->open && valid_range (next, count, offset, true, err)))
r = -1;
- else if (next_ops->can_write (nxdata) != 1) {
+ else if (next->can_write (next) != 1) {
*err = EROFS;
r = -1;
}
else if (flags & NBDKIT_FLAG_FUA &&
- next_ops->can_fua (nxdata) <= NBDKIT_FUA_NONE) {
+ next->can_fua (next) <= NBDKIT_FUA_NONE) {
*err = EIO;
r = -1;
}
else
- r = next_ops->pwrite (nxdata, buf, count, offset, flags, err);
- if (r == -1 && do_retry (h, &data, &nxdata, "pwrite", err))
+ r = next->pwrite (next, buf, count, offset, flags, err);
+ if (r == -1 && do_retry (h, &data, &next, "pwrite", err))
goto again;
return r;
@@ -281,7 +281,7 @@ retry_pwrite (struct nbdkit_next_ops *next_ops, nbdkit_next *nxdata,
/* Trim. */
static int
-retry_trim (struct nbdkit_next_ops *next_ops, nbdkit_next *nxdata,
+retry_trim (struct nbdkit_next_ops *next_ops, nbdkit_next *next,
void *handle,
uint32_t count, uint64_t offset, uint32_t flags,
int *err)
@@ -295,20 +295,20 @@ retry_trim (struct nbdkit_next_ops *next_ops, nbdkit_next *nxdata,
*err = EROFS;
return -1;
}
- if (! (h->open && valid_range (next_ops, nxdata, count, offset, true,
err)))
+ if (! (h->open && valid_range (next, count, offset, true, err)))
r = -1;
- else if (next_ops->can_trim (nxdata) != 1) {
+ else if (next->can_trim (next) != 1) {
*err = EROFS;
r = -1;
}
else if (flags & NBDKIT_FLAG_FUA &&
- next_ops->can_fua (nxdata) <= NBDKIT_FUA_NONE) {
+ next->can_fua (next) <= NBDKIT_FUA_NONE) {
*err = EIO;
r = -1;
}
else
- r = next_ops->trim (nxdata, count, offset, flags, err);
- if (r == -1 && do_retry (h, &data, &nxdata, "trim", err))
+ r = next->trim (next, count, offset, flags, err);
+ if (r == -1 && do_retry (h, &data, &next, "trim", err))
goto again;
return r;
@@ -316,7 +316,7 @@ retry_trim (struct nbdkit_next_ops *next_ops, nbdkit_next *nxdata,
/* Flush. */
static int
-retry_flush (struct nbdkit_next_ops *next_ops, nbdkit_next *nxdata,
+retry_flush (struct nbdkit_next_ops *next_ops, nbdkit_next *next,
void *handle, uint32_t flags,
int *err)
{
@@ -327,13 +327,13 @@ retry_flush (struct nbdkit_next_ops *next_ops, nbdkit_next *nxdata,
again:
if (! h->open)
r = -1;
- else if (next_ops->can_flush (nxdata) != 1) {
+ else if (next->can_flush (next) != 1) {
*err = EIO;
r = -1;
}
else
- r = next_ops->flush (nxdata, flags, err);
- if (r == -1 && do_retry (h, &data, &nxdata, "flush", err))
+ r = next->flush (next, flags, err);
+ if (r == -1 && do_retry (h, &data, &next, "flush", err))
goto again;
return r;
@@ -341,7 +341,7 @@ retry_flush (struct nbdkit_next_ops *next_ops, nbdkit_next *nxdata,
/* Zero. */
static int
-retry_zero (struct nbdkit_next_ops *next_ops, nbdkit_next *nxdata,
+retry_zero (struct nbdkit_next_ops *next_ops, nbdkit_next *next,
void *handle,
uint32_t count, uint64_t offset, uint32_t flags,
int *err)
@@ -356,24 +356,24 @@ retry_zero (struct nbdkit_next_ops *next_ops, nbdkit_next *nxdata,
return -1;
}
if (flags & NBDKIT_FLAG_FAST_ZERO &&
- (! h->open || next_ops->can_fast_zero (nxdata) != 1)) {
+ (! h->open || next->can_fast_zero (next) != 1)) {
*err = EOPNOTSUPP;
return -1;
}
- if (! (h->open && valid_range (next_ops, nxdata, count, offset, true,
err)))
+ if (! (h->open && valid_range (next, count, offset, true, err)))
r = -1;
- else if (next_ops->can_zero (nxdata) <= NBDKIT_ZERO_NONE) {
+ else if (next->can_zero (next) <= NBDKIT_ZERO_NONE) {
*err = EROFS;
r = -1;
}
else if (flags & NBDKIT_FLAG_FUA &&
- next_ops->can_fua (nxdata) <= NBDKIT_FUA_NONE) {
+ next->can_fua (next) <= NBDKIT_FUA_NONE) {
*err = EIO;
r = -1;
}
else
- r = next_ops->zero (nxdata, count, offset, flags, err);
- if (r == -1 && do_retry (h, &data, &nxdata, "zero", err))
+ r = next->zero (next, count, offset, flags, err);
+ if (r == -1 && do_retry (h, &data, &next, "zero", err))
goto again;
return r;
@@ -381,7 +381,7 @@ retry_zero (struct nbdkit_next_ops *next_ops, nbdkit_next *nxdata,
/* Extents. */
static int
-retry_extents (struct nbdkit_next_ops *next_ops, nbdkit_next *nxdata,
+retry_extents (struct nbdkit_next_ops *next_ops, nbdkit_next *next,
void *handle,
uint32_t count, uint64_t offset, uint32_t flags,
struct nbdkit_extents *extents, int *err)
@@ -393,23 +393,23 @@ retry_extents (struct nbdkit_next_ops *next_ops, nbdkit_next
*nxdata,
size_t i;
again:
- if (! (h->open && valid_range (next_ops, nxdata, count, offset, false,
err)))
+ if (! (h->open && valid_range (next, count, offset, false, err)))
r = -1;
- else if (next_ops->can_extents (nxdata) != 1) {
+ else if (next->can_extents (next) != 1) {
*err = EIO;
r = -1;
}
else {
/* Each retry must begin with extents reset to the right beginning. */
nbdkit_extents_free (extents2);
- extents2 = nbdkit_extents_new (offset, next_ops->get_size (nxdata));
+ extents2 = nbdkit_extents_new (offset, next->get_size (next));
if (extents2 == NULL) {
*err = errno;
return -1; /* Not worth a retry after ENOMEM. */
}
- r = next_ops->extents (nxdata, count, offset, flags, extents2, err);
+ r = next->extents (next, count, offset, flags, extents2, err);
}
- if (r == -1 && do_retry (h, &data, &nxdata, "extents", err))
+ if (r == -1 && do_retry (h, &data, &next, "extents", err))
goto again;
if (r == 0) {
@@ -429,7 +429,7 @@ retry_extents (struct nbdkit_next_ops *next_ops, nbdkit_next *nxdata,
/* Cache. */
static int
-retry_cache (struct nbdkit_next_ops *next_ops, nbdkit_next *nxdata,
+retry_cache (struct nbdkit_next_ops *next_ops, nbdkit_next *next,
void *handle,
uint32_t count, uint64_t offset, uint32_t flags,
int *err)
@@ -439,15 +439,15 @@ retry_cache (struct nbdkit_next_ops *next_ops, nbdkit_next *nxdata,
int r;
again:
- if (! (h->open && valid_range (next_ops, nxdata, count, offset, false,
err)))
+ if (! (h->open && valid_range (next, count, offset, false, err)))
r = -1;
- else if (next_ops->can_cache (nxdata) <= NBDKIT_CACHE_NONE) {
+ else if (next->can_cache (next) <= NBDKIT_CACHE_NONE) {
*err = EIO;
r = -1;
}
else
- r = next_ops->cache (nxdata, count, offset, flags, err);
- if (r == -1 && do_retry (h, &data, &nxdata, "cache", err))
+ r = next->cache (next, count, offset, flags, err);
+ if (r == -1 && do_retry (h, &data, &next, "cache", err))
goto again;
return r;
diff --git a/tests/test-layers-filter.c b/tests/test-layers-filter.c
index fc2fcbfd..ed06a008 100644
--- a/tests/test-layers-filter.c
+++ b/tests/test-layers-filter.c
@@ -164,6 +164,7 @@ test_layers_filter_prepare (struct nbdkit_next_ops *next_ops, void
*nxdata,
assert (h->next_ops == NULL);
h->next_ops = next_ops;
h->nxdata = nxdata;
+ assert (next_ops == nxdata);
DEBUG_FUNCTION;
return 0;
}
--
2.30.1