Eric Blake
2019-Aug-30 03:08 UTC
[Libguestfs] [nbdkit PATCH 0/9] can_FOO caching, more filter validation
It's easy to use the sh script to demonstrate that nbdkit is inefficiently calling into .get_size, .can_fua, and friends more than necessary. We've also commented on the list in the past that it would be nice to ensure that when filters call into next_ops, they are not violating constraints (as we've have to fix several bugs in the past where we did not have such checking to protect ourselves). This series fixes both (well, we may still have buggy filters, but if so, now we'll get earlier assertion failures instead of random plugin behavior). Another benefit - this gives us a single point for adding any code for performing NBD_INFO_BLOCKSIZE support, where we'll have to hoist what the blocksize filter is currently doing into something that nbdkit does from any one backend calling into another with different sizing constraints. However, that is not done in this series. My pending work on fast zeroes was easy enough to rebase on top of this (in fact, I wrote this because I noticed that .can_zero was being called more than necessary in implementing .can_fast_zero). Eric Blake (9): server: Fewer dereferences in filter server: Consolidate common backend tasks into new backend.c server: Create new backend_* functions for central handling server: Rework storage of per-backend handle server: Cache per-connection size server: Cache per-connection can_FOO flags filters: Change semantics of .can_zero server: Move fallbacks from protocol.c to backend.c server: Move command validation from protocol.c to backend.c docs/nbdkit-filter.pod | 62 +++- docs/nbdkit-plugin.pod | 6 +- server/internal.h | 108 +++++- server/Makefile.am | 3 +- server/backend.c | 515 +++++++++++++++++++++++++++ server/connections.c | 51 +-- server/filters.c | 379 ++++++-------------- server/main.c | 2 +- server/plugins.c | 192 +++------- server/protocol-handshake-newstyle.c | 20 +- server/protocol-handshake-oldstyle.c | 3 +- server/protocol-handshake.c | 97 +++-- server/protocol.c | 201 +++-------- include/nbdkit-filter.h | 4 + filters/blocksize/blocksize.c | 2 - filters/cache/cache.c | 1 - filters/cow/cow.c | 16 +- filters/nozero/nozero.c | 9 +- tests/test-layers.c | 50 +-- 19 files changed, 963 insertions(+), 758 deletions(-) create mode 100644 server/backend.c -- 2.21.0
Eric Blake
2019-Aug-30 03:08 UTC
[Libguestfs] [nbdkit PATCH 1/9] server: Fewer dereferences in filter
Anywhere that we compute 'f = container_of(b)' then use
'f->backend.',
we can instead just use 'b->'. Similarly, during registration, we
can
use the 'filename' input rather than our just-copied
'f->filename'.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
server/filters.c | 170 ++++++++++++++++++++++-------------------------
1 file changed, 79 insertions(+), 91 deletions(-)
diff --git a/server/filters.c b/server/filters.c
index bb6394fb..287c8747 100644
--- a/server/filters.c
+++ b/server/filters.c
@@ -68,7 +68,7 @@ filter_free (struct backend *b)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- f->backend.next->free (f->backend.next);
+ b->next->free (b->next);
/* Acquiring this lock prevents any filter callbacks from running
* simultaneously.
@@ -94,7 +94,7 @@ filter_thread_model (struct backend *b)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
int filter_thread_model = NBDKIT_THREAD_MODEL_PARALLEL;
- int thread_model = f->backend.next->thread_model (f->backend.next);
+ int thread_model = b->next->thread_model (b->next);
if (f->filter.thread_model) {
filter_thread_model = f->filter.thread_model ();
@@ -114,9 +114,7 @@ filter_thread_model (struct backend *b)
static const char *
plugin_name (struct backend *b)
{
- struct backend_filter *f = container_of (b, struct backend_filter, backend);
-
- return f->backend.next->plugin_name (f->backend.next);
+ return b->next->plugin_name (b->next);
}
static const char *
@@ -161,9 +159,7 @@ filter_usage (struct backend *b)
static void
filter_dump_fields (struct backend *b)
{
- struct backend_filter *f = container_of (b, struct backend_filter, backend);
-
- f->backend.next->dump_fields (f->backend.next);
+ b->next->dump_fields (b->next);
}
static int
@@ -183,11 +179,11 @@ filter_config (struct backend *b, const char *key, const
char *value)
f->name, key, value);
if (f->filter.config) {
- if (f->filter.config (next_config, f->backend.next, key, value) ==
-1)
+ if (f->filter.config (next_config, b->next, key, value) == -1)
exit (EXIT_FAILURE);
}
else
- f->backend.next->config (f->backend.next, key, value);
+ b->next->config (b->next, key, value);
}
static int
@@ -206,11 +202,11 @@ filter_config_complete (struct backend *b)
debug ("%s: config_complete", f->name);
if (f->filter.config_complete) {
- if (f->filter.config_complete (next_config_complete, f->backend.next)
== -1)
+ if (f->filter.config_complete (next_config_complete, b->next) == -1)
exit (EXIT_FAILURE);
}
else
- f->backend.next->config_complete (f->backend.next);
+ b->next->config_complete (b->next);
}
/* magic_config_key only applies to plugins, so this passes the
@@ -219,9 +215,7 @@ filter_config_complete (struct backend *b)
static const char *
plugin_magic_config_key (struct backend *b)
{
- struct backend_filter *f = container_of (b, struct backend_filter, backend);
-
- return f->backend.next->magic_config_key (f->backend.next);
+ return b->next->magic_config_key (b->next);
}
static int
@@ -236,7 +230,7 @@ static int
filter_open (struct backend *b, struct connection *conn, int readonly)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- struct b_conn nxdata = { .b = f->backend.next, .conn = conn };
+ struct b_conn nxdata = { .b = b->next, .conn = conn };
void *handle;
debug ("%s: open readonly=%d", f->name, readonly);
@@ -245,24 +239,24 @@ filter_open (struct backend *b, struct connection *conn,
int readonly)
handle = f->filter.open (next_open, &nxdata, readonly);
if (handle == NULL)
return -1;
- connection_set_handle (conn, f->backend.i, handle);
+ connection_set_handle (conn, b->i, handle);
return 0;
}
else
- return f->backend.next->open (f->backend.next, conn, readonly);
+ return b->next->open (b->next, conn, readonly);
}
static void
filter_close (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);
+ void *handle = connection_get_handle (conn, b->i);
debug ("%s: close", f->name);
if (f->filter.close)
f->filter.close (handle);
- f->backend.next->close (f->backend.next, conn);
+ b->next->close (b->next, conn);
}
/* The next_functions structure contains pointers to backend
@@ -459,15 +453,15 @@ static int
filter_prepare (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 };
+ void *handle = connection_get_handle (conn, b->i);
+ struct b_conn nxdata = { .b = b->next, .conn = conn };
debug ("%s: prepare", f->name);
/* Call these in order starting from the filter closest to the
* plugin.
*/
- if (f->backend.next->prepare (f->backend.next, conn) == -1)
+ if (b->next->prepare (b->next, conn) == -1)
return -1;
if (f->filter.prepare &&
@@ -481,8 +475,8 @@ static int
filter_finalize (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 };
+ void *handle = connection_get_handle (conn, b->i);
+ struct b_conn nxdata = { .b = b->next, .conn = conn };
debug ("%s: finalize", f->name);
@@ -493,157 +487,157 @@ filter_finalize (struct backend *b, struct connection
*conn)
f->filter.finalize (&next_ops, &nxdata, handle) == -1)
return -1;
- return f->backend.next->finalize (f->backend.next, conn);
+ return b->next->finalize (b->next, conn);
}
static int64_t
filter_get_size (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 };
+ void *handle = connection_get_handle (conn, b->i);
+ struct b_conn nxdata = { .b = b->next, .conn = conn };
debug ("%s: get_size", f->name);
if (f->filter.get_size)
return f->filter.get_size (&next_ops, &nxdata, handle);
else
- return f->backend.next->get_size (f->backend.next, conn);
+ return b->next->get_size (b->next, conn);
}
static int
filter_can_write (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 };
+ void *handle = connection_get_handle (conn, b->i);
+ struct b_conn nxdata = { .b = b->next, .conn = conn };
debug ("%s: can_write", f->name);
if (f->filter.can_write)
return f->filter.can_write (&next_ops, &nxdata, handle);
else
- return f->backend.next->can_write (f->backend.next, conn);
+ return b->next->can_write (b->next, conn);
}
static int
filter_can_flush (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 };
+ void *handle = connection_get_handle (conn, b->i);
+ struct b_conn nxdata = { .b = b->next, .conn = conn };
debug ("%s: can_flush", f->name);
if (f->filter.can_flush)
return f->filter.can_flush (&next_ops, &nxdata, handle);
else
- return f->backend.next->can_flush (f->backend.next, conn);
+ return b->next->can_flush (b->next, conn);
}
static int
filter_is_rotational (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 };
+ void *handle = connection_get_handle (conn, b->i);
+ struct b_conn nxdata = { .b = b->next, .conn = conn };
debug ("%s: is_rotational", f->name);
if (f->filter.is_rotational)
return f->filter.is_rotational (&next_ops, &nxdata, handle);
else
- return f->backend.next->is_rotational (f->backend.next, conn);
+ return b->next->is_rotational (b->next, conn);
}
static int
filter_can_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 };
+ void *handle = connection_get_handle (conn, b->i);
+ struct b_conn nxdata = { .b = b->next, .conn = conn };
debug ("%s: can_trim", f->name);
if (f->filter.can_trim)
return f->filter.can_trim (&next_ops, &nxdata, handle);
else
- return f->backend.next->can_trim (f->backend.next, conn);
+ return b->next->can_trim (b->next, conn);
}
static int
filter_can_zero (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 };
+ void *handle = connection_get_handle (conn, b->i);
+ struct b_conn nxdata = { .b = b->next, .conn = conn };
debug ("%s: can_zero", f->name);
if (f->filter.can_zero)
return f->filter.can_zero (&next_ops, &nxdata, handle);
else
- return f->backend.next->can_zero (f->backend.next, conn);
+ return b->next->can_zero (b->next, conn);
}
static int
filter_can_extents (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 };
+ void *handle = connection_get_handle (conn, b->i);
+ struct b_conn nxdata = { .b = b->next, .conn = conn };
debug ("%s: can_extents", f->name);
if (f->filter.can_extents)
return f->filter.can_extents (&next_ops, &nxdata, handle);
else
- return f->backend.next->can_extents (f->backend.next, conn);
+ return b->next->can_extents (b->next, conn);
}
static int
filter_can_fua (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 };
+ void *handle = connection_get_handle (conn, b->i);
+ struct b_conn nxdata = { .b = b->next, .conn = conn };
debug ("%s: can_fua", f->name);
if (f->filter.can_fua)
return f->filter.can_fua (&next_ops, &nxdata, handle);
else
- return f->backend.next->can_fua (f->backend.next, conn);
+ return b->next->can_fua (b->next, conn);
}
static int
filter_can_multi_conn (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 };
+ void *handle = connection_get_handle (conn, b->i);
+ struct b_conn nxdata = { .b = b->next, .conn = conn };
debug ("%s: can_multi_conn", f->name);
if (f->filter.can_multi_conn)
return f->filter.can_multi_conn (&next_ops, &nxdata, handle);
else
- return f->backend.next->can_multi_conn (f->backend.next, conn);
+ return b->next->can_multi_conn (b->next, conn);
}
static int
filter_can_cache (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 };
+ void *handle = connection_get_handle (conn, b->i);
+ struct b_conn nxdata = { .b = b->next, .conn = conn };
debug ("%s: can_cache", f->name);
if (f->filter.can_cache)
return f->filter.can_cache (&next_ops, &nxdata, handle);
else
- return f->backend.next->can_cache (f->backend.next, conn);
+ return b->next->can_cache (b->next, conn);
}
static int
@@ -652,8 +646,8 @@ filter_pread (struct backend *b, struct connection *conn,
uint32_t flags, int *err)
{
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 };
+ void *handle = connection_get_handle (conn, b->i);
+ struct b_conn nxdata = { .b = b->next, .conn = conn };
assert (flags == 0);
@@ -664,8 +658,7 @@ filter_pread (struct backend *b, struct connection *conn,
return f->filter.pread (&next_ops, &nxdata, handle,
buf, count, offset, flags, err);
else
- return f->backend.next->pread (f->backend.next, conn,
- buf, count, offset, flags, err);
+ return b->next->pread (b->next, conn, buf, count, offset, flags,
err);
}
static int
@@ -674,8 +667,8 @@ filter_pwrite (struct backend *b, struct connection *conn,
uint32_t flags, int *err)
{
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 };
+ void *handle = connection_get_handle (conn, b->i);
+ struct b_conn nxdata = { .b = b->next, .conn = conn };
assert (!(flags & ~NBDKIT_FLAG_FUA));
@@ -686,8 +679,7 @@ filter_pwrite (struct backend *b, struct connection *conn,
return f->filter.pwrite (&next_ops, &nxdata, handle,
buf, count, offset, flags, err);
else
- return f->backend.next->pwrite (f->backend.next, conn,
- buf, count, offset, flags, err);
+ return b->next->pwrite (b->next, conn, buf, count, offset, flags,
err);
}
static int
@@ -695,8 +687,8 @@ filter_flush (struct backend *b, struct connection *conn,
uint32_t flags,
int *err)
{
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 };
+ void *handle = connection_get_handle (conn, b->i);
+ struct b_conn nxdata = { .b = b->next, .conn = conn };
assert (flags == 0);
@@ -705,7 +697,7 @@ filter_flush (struct backend *b, struct connection *conn,
uint32_t flags,
if (f->filter.flush)
return f->filter.flush (&next_ops, &nxdata, handle, flags, err);
else
- return f->backend.next->flush (f->backend.next, conn, flags, err);
+ return b->next->flush (b->next, conn, flags, err);
}
static int
@@ -714,8 +706,8 @@ filter_trim (struct backend *b, struct connection *conn,
uint32_t flags, int *err)
{
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 };
+ void *handle = connection_get_handle (conn, b->i);
+ struct b_conn nxdata = { .b = b->next, .conn = conn };
assert (flags == 0);
@@ -726,8 +718,7 @@ filter_trim (struct backend *b, struct connection *conn,
return f->filter.trim (&next_ops, &nxdata, handle, count,
offset, flags,
err);
else
- return f->backend.next->trim (f->backend.next, conn, count,
offset, flags,
- err);
+ return b->next->trim (b->next, conn, count, offset, flags, err);
}
static int
@@ -735,8 +726,8 @@ filter_zero (struct backend *b, struct connection *conn,
uint32_t count, uint64_t offset, uint32_t flags, int *err)
{
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 };
+ void *handle = connection_get_handle (conn, b->i);
+ struct b_conn nxdata = { .b = b->next, .conn = conn };
assert (!(flags & ~(NBDKIT_FLAG_MAY_TRIM | NBDKIT_FLAG_FUA)));
@@ -747,8 +738,7 @@ filter_zero (struct backend *b, struct connection *conn,
return f->filter.zero (&next_ops, &nxdata, handle,
count, offset, flags, err);
else
- return f->backend.next->zero (f->backend.next, conn,
- count, offset, flags, err);
+ return b->next->zero (b->next, conn, count, offset, flags, err);
}
static int
@@ -757,8 +747,8 @@ filter_extents (struct backend *b, struct connection *conn,
struct nbdkit_extents *extents, int *err)
{
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 };
+ void *handle = connection_get_handle (conn, b->i);
+ struct b_conn nxdata = { .b = b->next, .conn = conn };
assert (!(flags & ~NBDKIT_FLAG_REQ_ONE));
@@ -770,9 +760,8 @@ filter_extents (struct backend *b, struct connection *conn,
count, offset, flags,
extents, err);
else
- return f->backend.next->extents (f->backend.next, conn,
- count, offset, flags,
- extents, err);
+ return b->next->extents (b->next, conn, count, offset, flags,
+ extents, err);
}
static int
@@ -781,8 +770,8 @@ filter_cache (struct backend *b, struct connection *conn,
uint32_t flags, int *err)
{
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 };
+ void *handle = connection_get_handle (conn, b->i);
+ struct b_conn nxdata = { .b = b->next, .conn = conn };
assert (flags == 0);
@@ -793,8 +782,7 @@ filter_cache (struct backend *b, struct connection *conn,
return f->filter.cache (&next_ops, &nxdata, handle,
count, offset, flags, err);
else
- return f->backend.next->cache (f->backend.next, conn,
- count, offset, flags, err);
+ return b->next->cache (b->next, conn, count, offset, flags, err);
}
static struct backend filter_functions = {
@@ -858,7 +846,7 @@ filter_register (struct backend *next, size_t index, const
char *filename,
if (f->filename == NULL) goto out_of_memory;
f->dl = dl;
- debug ("registering filter %s", f->filename);
+ debug ("registering filter %s", filename);
/* Call the initialization function which returns the address of the
* filter's own 'struct nbdkit_filter'.
@@ -866,7 +854,7 @@ filter_register (struct backend *next, size_t index, const
char *filename,
filter = filter_init ();
if (!filter) {
fprintf (stderr, "%s: %s: filter registration function failed\n",
- program_name, f->filename);
+ program_name, filename);
exit (EXIT_FAILURE);
}
@@ -887,14 +875,14 @@ filter_register (struct backend *next, size_t index, const
char *filename,
fprintf (stderr,
"%s: %s: filter is incompatible with this version of nbdkit,
"
"and appears to stem from nbdkit 1.14 or earlier\n",
- program_name, f->filename);
+ program_name, filename);
exit (EXIT_FAILURE);
}
if (strcmp (filter->_nbdkit_version, NBDKIT_VERSION_STRING) != 0) {
fprintf (stderr,
"%s: %s: filter is incompatible with this version of nbdkit
"
"(_nbdkit_version = %s)\n",
- program_name, f->filename, filter->_nbdkit_version);
+ program_name, filename, filter->_nbdkit_version);
exit (EXIT_FAILURE);
}
@@ -903,14 +891,14 @@ filter_register (struct backend *next, size_t index, const
char *filename,
/* Only filter.name is required. */
if (f->filter.name == NULL) {
fprintf (stderr, "%s: %s: filter must have a .name field\n",
- program_name, f->filename);
+ program_name, filename);
exit (EXIT_FAILURE);
}
len = strlen (f->filter.name);
if (len == 0) {
fprintf (stderr, "%s: %s: filter.name field must not be empty\n",
- program_name, f->filename);
+ program_name, filename);
exit (EXIT_FAILURE);
}
for (i = 0; i < len; ++i) {
@@ -920,7 +908,7 @@ filter_register (struct backend *next, size_t index, const
char *filename,
fprintf (stderr,
"%s: %s: filter.name ('%s') field "
"must contain only ASCII alphanumeric characters\n",
- program_name, f->filename, f->filter.name);
+ program_name, filename, f->filter.name);
exit (EXIT_FAILURE);
}
}
--
2.21.0
Eric Blake
2019-Aug-30 03:08 UTC
[Libguestfs] [nbdkit PATCH 2/9] server: Consolidate common backend tasks into new backend.c
Both plugin.c and filter.c add the same three fields on top of struct
backend, and use very similar code in initializing them. Let's stop
the duplication, by moving those three fields into struct backend, and
creating a new backend.c for manipulating them. In turn, we can drop
the backend->name() accessor in favor of just directly accessing the
name field. This is a net reduction in lines of code (remember, the
diffstat is also counting comment additions, including license
boilerplate).
Signed-off-by: Eric Blake <eblake@redhat.com>
---
server/internal.h | 21 +++++++-
server/Makefile.am | 3 +-
server/backend.c | 121 ++++++++++++++++++++++++++++++++++++++++++
server/filters.c | 129 +++++++++++----------------------------------
server/main.c | 2 +-
server/plugins.c | 102 ++++++++---------------------------
6 files changed, 195 insertions(+), 183 deletions(-)
create mode 100644 server/backend.c
diff --git a/server/internal.h b/server/internal.h
index a9692bbc..3af6ca16 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -242,6 +242,7 @@ extern void log_stderr_verror (const char *fs, va_list args)
extern void log_syslog_verror (const char *fs, va_list args)
__attribute__((__format__ (printf, 1, 0)));
+/* backend.c */
struct backend {
/* Next filter or plugin in the chain. This is always NULL for
* plugins and never NULL for filters.
@@ -255,9 +256,18 @@ struct backend {
*/
size_t i;
+ /* A copy of the backend name that survives a dlclose. */
+ char *name;
+
+ /* The file the backend was loaded from. */
+ char *filename;
+
+ /* The dlopen handle for the backend. */
+ void *dl;
+
+ /* Backend callbacks. All are required. */
void (*free) (struct backend *);
int (*thread_model) (struct backend *);
- const char *(*name) (struct backend *);
const char *(*plugin_name) (struct backend *);
void (*usage) (struct backend *);
const char *(*version) (struct backend *);
@@ -298,6 +308,15 @@ struct backend {
uint64_t offset, uint32_t flags, int *err);
};
+extern void backend_init (struct backend *b, struct backend *next, size_t
index,
+ const char *filename, void *dl, const char *type)
+ __attribute__((__nonnull__ (1, 4, 5, 6)));
+extern void backend_set_name (struct backend *b, const char *name,
+ const char *type)
+ __attribute__((__nonnull__ (1, 3)));
+extern void backend_unload (struct backend *b, void (*unload) (void))
+ __attribute__((__nonnull__ (1)));
+
/* plugins.c */
extern struct backend *plugin_register (size_t index, const char *filename,
void *dl, struct nbdkit_plugin
*(*plugin_init) (void))
diff --git a/server/Makefile.am b/server/Makefile.am
index 29674866..a0417639 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -1,5 +1,5 @@
# nbdkit
-# Copyright (C) 2013-2018 Red Hat Inc.
+# Copyright (C) 2013-2019 Red Hat Inc.
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are
@@ -36,6 +36,7 @@ EXTRA_DIST = nbdkit.syms
sbin_PROGRAMS = nbdkit
nbdkit_SOURCES = \
+ backend.c \
background.c \
captive.c \
connections.c \
diff --git a/server/backend.c b/server/backend.c
new file mode 100644
index 00000000..c7ee2d05
--- /dev/null
+++ b/server/backend.c
@@ -0,0 +1,121 @@
+/* nbdkit
+ * Copyright (C) 2013-2019 Red Hat Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * * Neither the name of Red Hat nor the names of its contributors may be
+ * used to endorse or promote products derived from this software without
+ * specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+ * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <config.h>
+
+#include <ctype.h>
+#include <dlfcn.h>
+#include <stdio.h>
+#include <string.h>
+
+#include "internal.h"
+
+/* Helpers for registering a new backend. */
+
+void
+backend_init (struct backend *b, struct backend *next, size_t index,
+ const char *filename, void *dl, const char *type)
+{
+ b->next = next;
+ b->i = index;
+ b->filename = strdup (filename);
+ if (b->filename == NULL) {
+ perror ("strdup");
+ exit (EXIT_FAILURE);
+ }
+ b->dl = dl;
+
+ debug ("registering %s %s", type, filename);
+}
+
+void
+backend_set_name (struct backend *b, const char *name, const char *type)
+{
+ size_t i, len;
+
+ /* name is required. */
+ if (name == NULL) {
+ fprintf (stderr, "%s: %s: %s must have a .name field\n",
+ program_name, b->filename, type);
+ exit (EXIT_FAILURE);
+ }
+
+ len = strlen (name);
+ if (len == 0) {
+ fprintf (stderr, "%s: %s: %s.name field must not be empty\n",
+ program_name, b->filename, type);
+ exit (EXIT_FAILURE);
+ }
+ for (i = 0; i < len; ++i) {
+ unsigned char c = name[i];
+
+ if (!(isascii (c) && isalnum (c))) {
+ fprintf (stderr,
+ "%s: %s: %s.name ('%s') field "
+ "must contain only ASCII alphanumeric characters\n",
+ program_name, b->filename, type, name);
+ exit (EXIT_FAILURE);
+ }
+ }
+
+ /* Copy the module's name into local storage, so that name
+ * survives past unload.
+ */
+ b->name = strdup (name);
+ if (b->name == NULL) {
+ perror ("strdup");
+ exit (EXIT_FAILURE);
+ }
+
+ debug ("registered %s %s (name %s)", type, b->filename,
b->name);
+}
+
+void
+backend_unload (struct backend *b, void (*unload) (void))
+{
+ /* Acquiring this lock prevents any other backend callbacks from running
+ * simultaneously.
+ */
+ lock_unload ();
+
+ debug ("%s: unload", b->name);
+ if (unload)
+ unload ();
+
+ if (DO_DLCLOSE)
+ dlclose (b->dl);
+ free (b->filename);
+
+ unlock_unload ();
+
+ free (b->name);
+}
diff --git a/server/filters.c b/server/filters.c
index 287c8747..1f76bf61 100644
--- a/server/filters.c
+++ b/server/filters.c
@@ -48,9 +48,6 @@
*/
struct backend_filter {
struct backend backend;
- char *name; /* copy of filter.name */
- char *filename;
- void *dl;
struct nbdkit_filter filter;
};
@@ -70,22 +67,7 @@ filter_free (struct backend *b)
b->next->free (b->next);
- /* Acquiring this lock prevents any filter callbacks from running
- * simultaneously.
- */
- lock_unload ();
-
- debug ("%s: unload", f->name);
- if (f->filter.unload)
- f->filter.unload ();
-
- if (DO_DLCLOSE)
- dlclose (f->dl);
- free (f->filename);
-
- unlock_unload ();
-
- free (f->name);
+ backend_unload (b, f->filter.unload);
free (f);
}
@@ -117,14 +99,6 @@ plugin_name (struct backend *b)
return b->next->plugin_name (b->next);
}
-static const char *
-filter_name (struct backend *b)
-{
- struct backend_filter *f = container_of (b, struct backend_filter, backend);
-
- return f->name;
-}
-
static const char *
filter_version (struct backend *b)
{
@@ -139,11 +113,11 @@ filter_usage (struct backend *b)
struct backend_filter *f = container_of (b, struct backend_filter, backend);
const char *p;
- printf ("filter: %s", f->name);
+ printf ("filter: %s", b->name);
if (f->filter.longname)
printf (" (%s)", f->filter.longname);
printf ("\n");
- printf ("(%s)\n", f->filename);
+ printf ("(%s)\n", b->filename);
if (f->filter.description) {
printf ("%s", f->filter.description);
if ((p = strrchr (f->filter.description, '\n')) == NULL || p[1])
@@ -176,7 +150,7 @@ filter_config (struct backend *b, const char *key, const
char *value)
struct backend_filter *f = container_of (b, struct backend_filter, backend);
debug ("%s: config key=%s, value=%s",
- f->name, key, value);
+ b->name, key, value);
if (f->filter.config) {
if (f->filter.config (next_config, b->next, key, value) == -1)
@@ -199,7 +173,7 @@ filter_config_complete (struct backend *b)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- debug ("%s: config_complete", f->name);
+ debug ("%s: config_complete", b->name);
if (f->filter.config_complete) {
if (f->filter.config_complete (next_config_complete, b->next) == -1)
@@ -233,7 +207,7 @@ filter_open (struct backend *b, struct connection *conn, int
readonly)
struct b_conn nxdata = { .b = b->next, .conn = conn };
void *handle;
- debug ("%s: open readonly=%d", f->name, readonly);
+ debug ("%s: open readonly=%d", b->name, readonly);
if (f->filter.open) {
handle = f->filter.open (next_open, &nxdata, readonly);
@@ -252,7 +226,7 @@ filter_close (struct backend *b, struct connection *conn)
struct backend_filter *f = container_of (b, struct backend_filter, backend);
void *handle = connection_get_handle (conn, b->i);
- debug ("%s: close", f->name);
+ debug ("%s: close", b->name);
if (f->filter.close)
f->filter.close (handle);
@@ -456,7 +430,7 @@ filter_prepare (struct backend *b, struct connection *conn)
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- debug ("%s: prepare", f->name);
+ debug ("%s: prepare", b->name);
/* Call these in order starting from the filter closest to the
* plugin.
@@ -478,7 +452,7 @@ filter_finalize (struct backend *b, struct connection *conn)
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- debug ("%s: finalize", f->name);
+ debug ("%s: finalize", b->name);
/* Call these in reverse order to .prepare above, starting from the
* filter furthest away from the plugin.
@@ -497,7 +471,7 @@ filter_get_size (struct backend *b, struct connection *conn)
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- debug ("%s: get_size", f->name);
+ debug ("%s: get_size", b->name);
if (f->filter.get_size)
return f->filter.get_size (&next_ops, &nxdata, handle);
@@ -512,7 +486,7 @@ filter_can_write (struct backend *b, struct connection
*conn)
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- debug ("%s: can_write", f->name);
+ debug ("%s: can_write", b->name);
if (f->filter.can_write)
return f->filter.can_write (&next_ops, &nxdata, handle);
@@ -527,7 +501,7 @@ filter_can_flush (struct backend *b, struct connection
*conn)
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- debug ("%s: can_flush", f->name);
+ debug ("%s: can_flush", b->name);
if (f->filter.can_flush)
return f->filter.can_flush (&next_ops, &nxdata, handle);
@@ -542,7 +516,7 @@ filter_is_rotational (struct backend *b, struct connection
*conn)
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- debug ("%s: is_rotational", f->name);
+ debug ("%s: is_rotational", b->name);
if (f->filter.is_rotational)
return f->filter.is_rotational (&next_ops, &nxdata, handle);
@@ -557,7 +531,7 @@ filter_can_trim (struct backend *b, struct connection *conn)
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- debug ("%s: can_trim", f->name);
+ debug ("%s: can_trim", b->name);
if (f->filter.can_trim)
return f->filter.can_trim (&next_ops, &nxdata, handle);
@@ -572,7 +546,7 @@ filter_can_zero (struct backend *b, struct connection *conn)
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- debug ("%s: can_zero", f->name);
+ debug ("%s: can_zero", b->name);
if (f->filter.can_zero)
return f->filter.can_zero (&next_ops, &nxdata, handle);
@@ -587,7 +561,7 @@ filter_can_extents (struct backend *b, struct connection
*conn)
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- debug ("%s: can_extents", f->name);
+ debug ("%s: can_extents", b->name);
if (f->filter.can_extents)
return f->filter.can_extents (&next_ops, &nxdata, handle);
@@ -602,7 +576,7 @@ filter_can_fua (struct backend *b, struct connection *conn)
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- debug ("%s: can_fua", f->name);
+ debug ("%s: can_fua", b->name);
if (f->filter.can_fua)
return f->filter.can_fua (&next_ops, &nxdata, handle);
@@ -617,7 +591,7 @@ filter_can_multi_conn (struct backend *b, struct connection
*conn)
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- debug ("%s: can_multi_conn", f->name);
+ debug ("%s: can_multi_conn", b->name);
if (f->filter.can_multi_conn)
return f->filter.can_multi_conn (&next_ops, &nxdata, handle);
@@ -632,7 +606,7 @@ filter_can_cache (struct backend *b, struct connection
*conn)
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- debug ("%s: can_cache", f->name);
+ debug ("%s: can_cache", b->name);
if (f->filter.can_cache)
return f->filter.can_cache (&next_ops, &nxdata, handle);
@@ -652,7 +626,7 @@ filter_pread (struct backend *b, struct connection *conn,
assert (flags == 0);
debug ("%s: pread count=%" PRIu32 " offset=%" PRIu64
" flags=0x%" PRIx32,
- f->name, count, offset, flags);
+ b->name, count, offset, flags);
if (f->filter.pread)
return f->filter.pread (&next_ops, &nxdata, handle,
@@ -673,7 +647,7 @@ filter_pwrite (struct backend *b, struct connection *conn,
assert (!(flags & ~NBDKIT_FLAG_FUA));
debug ("%s: pwrite count=%" PRIu32 " offset=%" PRIu64
" flags=0x%" PRIx32,
- f->name, count, offset, flags);
+ b->name, count, offset, flags);
if (f->filter.pwrite)
return f->filter.pwrite (&next_ops, &nxdata, handle,
@@ -692,7 +666,7 @@ filter_flush (struct backend *b, struct connection *conn,
uint32_t flags,
assert (flags == 0);
- debug ("%s: flush flags=0x%" PRIx32, f->name, flags);
+ debug ("%s: flush flags=0x%" PRIx32, b->name, flags);
if (f->filter.flush)
return f->filter.flush (&next_ops, &nxdata, handle, flags, err);
@@ -712,7 +686,7 @@ filter_trim (struct backend *b, struct connection *conn,
assert (flags == 0);
debug ("%s: trim count=%" PRIu32 " offset=%" PRIu64
" flags=0x%" PRIx32,
- f->name, count, offset, flags);
+ b->name, count, offset, flags);
if (f->filter.trim)
return f->filter.trim (&next_ops, &nxdata, handle, count,
offset, flags,
@@ -732,7 +706,7 @@ filter_zero (struct backend *b, struct connection *conn,
assert (!(flags & ~(NBDKIT_FLAG_MAY_TRIM | NBDKIT_FLAG_FUA)));
debug ("%s: zero count=%" PRIu32 " offset=%" PRIu64
" flags=0x%" PRIx32,
- f->name, count, offset, flags);
+ b->name, count, offset, flags);
if (f->filter.zero)
return f->filter.zero (&next_ops, &nxdata, handle,
@@ -753,7 +727,7 @@ filter_extents (struct backend *b, struct connection *conn,
assert (!(flags & ~NBDKIT_FLAG_REQ_ONE));
debug ("%s: extents count=%" PRIu32 " offset=%" PRIu64
" flags=0x%" PRIx32,
- f->name, count, offset, flags);
+ b->name, count, offset, flags);
if (f->filter.extents)
return f->filter.extents (&next_ops, &nxdata, handle,
@@ -776,7 +750,7 @@ filter_cache (struct backend *b, struct connection *conn,
assert (flags == 0);
debug ("%s: cache count=%" PRIu32 " offset=%" PRIu64
" flags=0x%" PRIx32,
- f->name, count, offset, flags);
+ b->name, count, offset, flags);
if (f->filter.cache)
return f->filter.cache (&next_ops, &nxdata, handle,
@@ -788,7 +762,6 @@ filter_cache (struct backend *b, struct connection *conn,
static struct backend filter_functions = {
.free = filter_free,
.thread_model = filter_thread_model,
- .name = filter_name,
.plugin_name = plugin_name,
.usage = filter_usage,
.version = filter_version,
@@ -826,7 +799,6 @@ filter_register (struct backend *next, size_t index, const
char *filename,
{
struct backend_filter *f;
const struct nbdkit_filter *filter;
- size_t i, len;
union overlay {
const char *str;
unsigned int api;
@@ -834,19 +806,12 @@ filter_register (struct backend *next, size_t index, const
char *filename,
f = calloc (1, sizeof *f);
if (f == NULL) {
- out_of_memory:
perror ("strdup");
exit (EXIT_FAILURE);
}
f->backend = filter_functions;
- f->backend.next = next;
- f->backend.i = index;
- f->filename = strdup (filename);
- if (f->filename == NULL) goto out_of_memory;
- f->dl = dl;
-
- debug ("registering filter %s", filename);
+ backend_init (&f->backend, next, index, filename, dl,
"filter");
/* Call the initialization function which returns the address of the
* filter's own 'struct nbdkit_filter'.
@@ -888,47 +853,13 @@ filter_register (struct backend *next, size_t index, const
char *filename,
f->filter = *filter;
- /* Only filter.name is required. */
- if (f->filter.name == NULL) {
- fprintf (stderr, "%s: %s: filter must have a .name field\n",
- program_name, filename);
- exit (EXIT_FAILURE);
- }
-
- len = strlen (f->filter.name);
- if (len == 0) {
- fprintf (stderr, "%s: %s: filter.name field must not be empty\n",
- program_name, filename);
- exit (EXIT_FAILURE);
- }
- for (i = 0; i < len; ++i) {
- if (!((f->filter.name[i] >= '0' &&
f->filter.name[i] <= '9') ||
- (f->filter.name[i] >= 'a' &&
f->filter.name[i] <= 'z') ||
- (f->filter.name[i] >= 'A' &&
f->filter.name[i] <= 'Z'))) {
- fprintf (stderr,
- "%s: %s: filter.name ('%s') field "
- "must contain only ASCII alphanumeric characters\n",
- program_name, filename, f->filter.name);
- exit (EXIT_FAILURE);
- }
- }
-
- /* Copy the module's name into local storage, so that filter.name
- * survives past unload.
- */
- f->name = strdup (f->filter.name);
- if (f->name == NULL) {
- perror ("strdup");
- exit (EXIT_FAILURE);
- }
-
- debug ("registered filter %s (name %s)", f->filename,
f->name);
+ backend_set_name (&f->backend, f->filter.name, "filter");
/* Set debug flags before calling load. */
- set_debug_flags (dl, f->name);
+ set_debug_flags (dl, f->backend.name);
/* Call the on-load callback if it exists. */
- debug ("%s: load", f->name);
+ debug ("%s: load", f->backend.name);
if (f->filter.load)
f->filter.load ();
diff --git a/server/main.c b/server/main.c
index 124e19b7..6999818c 100644
--- a/server/main.c
+++ b/server/main.c
@@ -602,7 +602,7 @@ main (int argc, char *argv[])
display_version ();
for_each_backend (b) {
- printf ("%s", b->name (b));
+ printf ("%s", b->name);
if ((v = b->version (b)) != NULL)
printf (" %s", v);
printf ("\n");
diff --git a/server/plugins.c b/server/plugins.c
index 34cc3cb5..0f70ede0 100644
--- a/server/plugins.c
+++ b/server/plugins.c
@@ -54,9 +54,6 @@
*/
struct backend_plugin {
struct backend backend;
- char *name; /* copy of plugin.name */
- char *filename;
- void *dl;
struct nbdkit_plugin plugin;
};
@@ -65,22 +62,7 @@ plugin_free (struct backend *b)
{
struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
- /* Acquiring this lock prevents any plugin callbacks from running
- * simultaneously.
- */
- lock_unload ();
-
- debug ("%s: unload", p->name);
- if (p->plugin.unload)
- p->plugin.unload ();
-
- if (DO_DLCLOSE)
- dlclose (p->dl);
- free (p->filename);
-
- unlock_unload ();
-
- free (p->name);
+ backend_unload (b, p->plugin.unload);
free (p);
}
@@ -113,9 +95,7 @@ plugin_thread_model (struct backend *b)
static const char *
plugin_name (struct backend *b)
{
- struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
-
- return p->name;
+ return b->name;
}
static void
@@ -124,11 +104,11 @@ plugin_usage (struct backend *b)
struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
const char *t;
- printf ("plugin: %s", p->name);
+ printf ("plugin: %s", b->name);
if (p->plugin.longname)
printf (" (%s)", p->plugin.longname);
printf ("\n");
- printf ("(%s)\n", p->filename);
+ printf ("(%s)\n", b->filename);
if (p->plugin.description) {
printf ("%s", p->plugin.description);
if ((t = strrchr (p->plugin.description, '\n')) == NULL || t[1])
@@ -156,11 +136,11 @@ plugin_dump_fields (struct backend *b)
struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
char *path;
- path = nbdkit_realpath (p->filename);
+ path = nbdkit_realpath (b->filename);
printf ("path=%s\n", path);
free (path);
- printf ("name=%s\n", p->name);
+ printf ("name=%s\n", b->name);
if (p->plugin.version)
printf ("version=%s\n", p->plugin.version);
@@ -220,14 +200,14 @@ plugin_config (struct backend *b, const char *key, const
char *value)
{
struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
- debug ("%s: config key=%s, value=%s", p->name, key, value);
+ debug ("%s: config key=%s, value=%s", b->name, key, value);
if (p->plugin.config == NULL) {
fprintf (stderr,
"%s: %s: this plugin does not need command line
configuration\n"
"Try using: %s --help %s\n",
- program_name, p->filename,
- program_name, p->filename);
+ program_name, b->filename,
+ program_name, b->filename);
exit (EXIT_FAILURE);
}
@@ -240,7 +220,7 @@ plugin_config_complete (struct backend *b)
{
struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
- debug ("%s: config_complete", p->name);
+ debug ("%s: config_complete", b->name);
if (!p->plugin.config_complete)
return;
@@ -266,7 +246,7 @@ plugin_open (struct backend *b, struct connection *conn, int
readonly)
assert (connection_get_handle (conn, 0) == NULL);
assert (p->plugin.open != NULL);
- debug ("%s: open readonly=%d", p->name, readonly);
+ debug ("%s: open readonly=%d", b->name, readonly);
handle = p->plugin.open (readonly);
if (!handle)
@@ -744,7 +724,6 @@ plugin_cache (struct backend *b, struct connection *conn,
static struct backend plugin_functions = {
.free = plugin_free,
.thread_model = plugin_thread_model,
- .name = plugin_name,
.plugin_name = plugin_name,
.usage = plugin_usage,
.version = plugin_version,
@@ -782,23 +761,16 @@ plugin_register (size_t index, const char *filename,
{
struct backend_plugin *p;
const struct nbdkit_plugin *plugin;
- size_t i, len, size;
+ size_t size;
p = malloc (sizeof *p);
if (p == NULL) {
- out_of_memory:
perror ("strdup");
exit (EXIT_FAILURE);
}
p->backend = plugin_functions;
- p->backend.next = NULL;
- p->backend.i = index;
- p->filename = strdup (filename);
- if (p->filename == NULL) goto out_of_memory;
- p->dl = dl;
-
- debug ("registering plugin %s", p->filename);
+ backend_init (&p->backend, NULL, index, filename, dl,
"plugin");
/* Call the initialization function which returns the address of the
* plugin's own 'struct nbdkit_plugin'.
@@ -806,7 +778,7 @@ plugin_register (size_t index, const char *filename,
plugin = plugin_init ();
if (!plugin) {
fprintf (stderr, "%s: %s: plugin registration function failed\n",
- program_name, p->filename);
+ program_name, filename);
exit (EXIT_FAILURE);
}
@@ -815,7 +787,7 @@ plugin_register (size_t index, const char *filename,
fprintf (stderr,
"%s: %s: plugin is incompatible with this version of nbdkit
"
"(_api_version = %d)\n",
- program_name, p->filename, plugin->_api_version);
+ program_name, filename, plugin->_api_version);
exit (EXIT_FAILURE);
}
@@ -833,61 +805,29 @@ plugin_register (size_t index, const char *filename,
/* Check for the minimum fields which must exist in the
* plugin struct.
*/
- if (p->plugin.name == NULL) {
- fprintf (stderr, "%s: %s: plugin must have a .name field\n",
- program_name, p->filename);
- exit (EXIT_FAILURE);
- }
if (p->plugin.open == NULL) {
fprintf (stderr, "%s: %s: plugin must have a .open callback\n",
- program_name, p->filename);
+ program_name, filename);
exit (EXIT_FAILURE);
}
if (p->plugin.get_size == NULL) {
fprintf (stderr, "%s: %s: plugin must have a .get_size
callback\n",
- program_name, p->filename);
+ program_name, filename);
exit (EXIT_FAILURE);
}
if (p->plugin.pread == NULL && p->plugin._pread_old == NULL) {
fprintf (stderr, "%s: %s: plugin must have a .pread callback\n",
- program_name, p->filename);
+ program_name, filename);
exit (EXIT_FAILURE);
}
- len = strlen (p->plugin.name);
- if (len == 0) {
- fprintf (stderr, "%s: %s: plugin.name field must not be empty\n",
- program_name, p->filename);
- exit (EXIT_FAILURE);
- }
- for (i = 0; i < len; ++i) {
- if (!((p->plugin.name[i] >= '0' &&
p->plugin.name[i] <= '9') ||
- (p->plugin.name[i] >= 'a' &&
p->plugin.name[i] <= 'z') ||
- (p->plugin.name[i] >= 'A' &&
p->plugin.name[i] <= 'Z'))) {
- fprintf (stderr,
- "%s: %s: plugin.name ('%s') field "
- "must contain only ASCII alphanumeric characters\n",
- program_name, p->filename, p->plugin.name);
- exit (EXIT_FAILURE);
- }
- }
-
- /* Copy the module's name into local storage, so that plugin.name
- * survives past unload.
- */
- p->name = strdup (p->plugin.name);
- if (p->name == NULL) {
- perror ("strdup");
- exit (EXIT_FAILURE);
- }
-
- debug ("registered plugin %s (name %s)", p->filename,
p->name);
+ backend_set_name (&p->backend, p->plugin.name, "plugin");
/* Set debug flags before calling load. */
- set_debug_flags (dl, p->name);
+ set_debug_flags (dl, p->backend.name);
/* Call the on-load callback if it exists. */
- debug ("%s: load", p->name);
+ debug ("%s: load", p->backend.name);
if (p->plugin.load)
p->plugin.load ();
--
2.21.0
Eric Blake
2019-Aug-30 03:08 UTC
[Libguestfs] [nbdkit PATCH 3/9] server: Create new backend_* functions for central handling
I'm planning to move sanity checking and per-connection caching into
common code (right now, we check and cache things at the connection
level for the final filter, but not at other layers; it would be nicer
to do this at each step through the filters). This starts the process
by extending the just-created backend.c to add wrappers for all of the
handshake and request callbacks present in a filter's next_ops. This
lets us consolidate common debugging output (which requires a tweak to
test-layers), and moves assertion checking (such as what was
duplicated in 0b8ad8c8) into a single location.
Of note: .extents no longer returns 0 for a 0-length request made by a
filter (such a request cannot be made by the client, because
protocol.c:valid_range rejects it); commit 5a1db63d showed a case
where a filter ended up doing that accidentally.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
server/internal.h | 49 +++++++
server/backend.c | 201 +++++++++++++++++++++++++++
server/filters.c | 158 +++++----------------
server/plugins.c | 48 -------
server/protocol-handshake-newstyle.c | 2 +-
server/protocol-handshake-oldstyle.c | 2 +-
server/protocol-handshake.c | 18 +--
server/protocol.c | 36 ++---
tests/test-layers.c | 50 +++----
9 files changed, 331 insertions(+), 233 deletions(-)
diff --git a/server/internal.h b/server/internal.h
index 3af6ca16..93ebeb78 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -317,6 +317,55 @@ extern void backend_set_name (struct backend *b, const char
*name,
extern void backend_unload (struct backend *b, void (*unload) (void))
__attribute__((__nonnull__ (1)));
+extern int64_t backend_get_size (struct backend *b, struct connection *conn)
+ __attribute__((__nonnull__ (1, 2)));
+extern int backend_can_write (struct backend *b, struct connection *conn)
+ __attribute__((__nonnull__ (1, 2)));
+extern int backend_can_flush (struct backend *b, struct connection *conn)
+ __attribute__((__nonnull__ (1, 2)));
+extern int backend_is_rotational (struct backend *b, struct connection *conn)
+ __attribute__((__nonnull__ (1, 2)));
+extern int backend_can_trim (struct backend *b, struct connection *conn)
+ __attribute__((__nonnull__ (1, 2)));
+extern int backend_can_zero (struct backend *b, struct connection *conn)
+ __attribute__((__nonnull__ (1, 2)));
+extern int backend_can_extents (struct backend *b, struct connection *conn)
+ __attribute__((__nonnull__ (1, 2)));
+extern int backend_can_fua (struct backend *b, struct connection *conn)
+ __attribute__((__nonnull__ (1, 2)));
+extern int backend_can_multi_conn (struct backend *b, struct connection *conn)
+ __attribute__((__nonnull__ (1, 2)));
+extern int backend_can_cache (struct backend *b, struct connection *conn)
+ __attribute__((__nonnull__ (1, 2)));
+
+extern int backend_pread (struct backend *b, struct connection *conn,
+ void *buf, uint32_t count, uint64_t offset,
+ uint32_t flags, int *err)
+ __attribute__((__nonnull__ (1, 2, 3, 7)));
+extern int backend_pwrite (struct backend *b, struct connection *conn,
+ const void *buf, uint32_t count, uint64_t offset,
+ uint32_t flags, int *err)
+ __attribute__((__nonnull__ (1, 2, 3, 7)));
+extern int backend_flush (struct backend *b, struct connection *conn,
+ uint32_t flags, int *err)
+ __attribute__((__nonnull__ (1, 2, 4)));
+extern int backend_trim (struct backend *b, struct connection *conn,
+ uint32_t count, uint64_t offset, uint32_t flags,
+ int *err)
+ __attribute__((__nonnull__ (1, 2, 6)));
+extern int backend_zero (struct backend *b, struct connection *conn,
+ uint32_t count, uint64_t offset, uint32_t flags,
+ int *err)
+ __attribute__((__nonnull__ (1, 2, 6)));
+extern int backend_extents (struct backend *b, struct connection *conn,
+ uint32_t count, uint64_t offset, uint32_t flags,
+ struct nbdkit_extents *extents, int *err)
+ __attribute__((__nonnull__ (1, 2, 6, 7)));
+extern int backend_cache (struct backend *b, struct connection *conn,
+ uint32_t count, uint64_t offset,
+ uint32_t flags, int *err)
+ __attribute__((__nonnull__ (1, 2, 6)));
+
/* plugins.c */
extern struct backend *plugin_register (size_t index, const char *filename,
void *dl, struct nbdkit_plugin
*(*plugin_init) (void))
diff --git a/server/backend.c b/server/backend.c
index c7ee2d05..ce3c5e2b 100644
--- a/server/backend.c
+++ b/server/backend.c
@@ -32,8 +32,10 @@
#include <config.h>
+#include <assert.h>
#include <ctype.h>
#include <dlfcn.h>
+#include <inttypes.h>
#include <stdio.h>
#include <string.h>
@@ -119,3 +121,202 @@ backend_unload (struct backend *b, void (*unload) (void))
free (b->name);
}
+
+int64_t
+backend_get_size (struct backend *b, struct connection *conn)
+{
+ debug ("%s: get_size", b->name);
+
+ /* TODO caching */
+ return b->get_size (b, conn);
+}
+
+int
+backend_can_write (struct backend *b, struct connection *conn)
+{
+ debug ("%s: can_write", b->name);
+
+ return b->can_write (b, conn);
+}
+
+int
+backend_can_flush (struct backend *b, struct connection *conn)
+{
+ debug ("%s: can_flush", b->name);
+
+ return b->can_flush (b, conn);
+}
+
+int
+backend_is_rotational (struct backend *b, struct connection *conn)
+{
+ debug ("%s: is_rotational", b->name);
+
+ return b->is_rotational (b, conn);
+}
+
+int
+backend_can_trim (struct backend *b, struct connection *conn)
+{
+ debug ("%s: can_trim", b->name);
+
+ return b->can_trim (b, conn);
+}
+
+int
+backend_can_zero (struct backend *b, struct connection *conn)
+{
+ debug ("%s: can_zero", b->name);
+
+ return b->can_zero (b, conn);
+}
+
+int
+backend_can_extents (struct backend *b, struct connection *conn)
+{
+ debug ("%s: can_extents", b->name);
+
+ return b->can_extents (b, conn);
+}
+
+int
+backend_can_fua (struct backend *b, struct connection *conn)
+{
+ debug ("%s: can_fua", b->name);
+
+ return b->can_fua (b, conn);
+}
+
+int
+backend_can_multi_conn (struct backend *b, struct connection *conn)
+{
+ debug ("%s: can_multi_conn", b->name);
+
+ return b->can_multi_conn (b, conn);
+}
+
+int
+backend_can_cache (struct backend *b, struct connection *conn)
+{
+ debug ("%s: can_cache", b->name);
+
+ return b->can_cache (b, conn);
+}
+
+int
+backend_pread (struct backend *b, struct connection *conn,
+ void *buf, uint32_t count, uint64_t offset,
+ uint32_t flags, int *err)
+{
+ int r;
+
+ assert (flags == 0);
+ debug ("%s: pread count=%" PRIu32 " offset=%" PRIu64,
+ b->name, count, offset);
+
+ r = b->pread (b, conn, buf, count, offset, flags, err);
+ if (r == -1)
+ assert (*err);
+ return r;
+}
+
+int
+backend_pwrite (struct backend *b, struct connection *conn,
+ const void *buf, uint32_t count, uint64_t offset,
+ uint32_t flags, int *err)
+{
+ int r;
+
+ assert (!(flags & ~NBDKIT_FLAG_FUA));
+ debug ("%s: pwrite count=%" PRIu32 " offset=%" PRIu64
" fua=%d",
+ b->name, count, offset, !!(flags & NBDKIT_FLAG_FUA));
+
+ r = b->pwrite (b, conn, buf, count, offset, flags, err);
+ if (r == -1)
+ assert (*err);
+ return r;
+}
+
+int
+backend_flush (struct backend *b, struct connection *conn,
+ uint32_t flags, int *err)
+{
+ int r;
+
+ assert (flags == 0);
+ debug ("%s: flush", b->name);
+
+ r = b->flush (b, conn, flags, err);
+ if (r == -1)
+ assert (*err);
+ return r;
+}
+
+int
+backend_trim (struct backend *b, struct connection *conn,
+ uint32_t count, uint64_t offset, uint32_t flags,
+ int *err)
+{
+ int r;
+
+ assert (flags == 0);
+ debug ("%s: trim count=%" PRIu32 " offset=%" PRIu64
" fua=%d",
+ b->name, count, offset, !!(flags & NBDKIT_FLAG_FUA));
+
+ r = b->trim (b, conn, count, offset, flags, err);
+ if (r == -1)
+ assert (*err);
+ return r;
+}
+
+int
+backend_zero (struct backend *b, struct connection *conn,
+ uint32_t count, uint64_t offset, uint32_t flags,
+ int *err)
+{
+ int r;
+
+ assert (!(flags & ~(NBDKIT_FLAG_MAY_TRIM | NBDKIT_FLAG_FUA)));
+ debug ("%s: zero count=%" PRIu32 " offset=%" PRIu64
" may_trim=%d fua=%d",
+ b->name, count, offset, !!(flags & NBDKIT_FLAG_MAY_TRIM),
+ !!(flags & NBDKIT_FLAG_FUA));
+
+ r = b->zero (b, conn, count, offset, flags, err);
+ if (r == -1)
+ assert (*err && *err != ENOTSUP && *err != EOPNOTSUPP);
+ return r;
+}
+
+int
+backend_extents (struct backend *b, struct connection *conn,
+ uint32_t count, uint64_t offset, uint32_t flags,
+ struct nbdkit_extents *extents, int *err)
+{
+ int r;
+
+ assert (!(flags & ~NBDKIT_FLAG_REQ_ONE));
+ debug ("%s: extents count=%" PRIu32 " offset=%" PRIu64
" req_one=%d",
+ b->name, count, offset, !!(flags & NBDKIT_FLAG_REQ_ONE));
+
+ r = b->extents (b, conn, count, offset, flags, extents, err);
+ if (r == -1)
+ assert (*err);
+ return r;
+}
+
+int
+backend_cache (struct backend *b, struct connection *conn,
+ uint32_t count, uint64_t offset,
+ uint32_t flags, int *err)
+{
+ int r;
+
+ assert (flags == 0);
+ debug ("%s: cache count=%" PRIu32 " offset=%" PRIu64,
+ b->name, count, offset);
+
+ r = b->cache (b, conn, count, offset, flags, err);
+ if (r == -1)
+ assert (*err);
+ return r;
+}
diff --git a/server/filters.c b/server/filters.c
index 1f76bf61..acb44bda 100644
--- a/server/filters.c
+++ b/server/filters.c
@@ -244,70 +244,70 @@ static int64_t
next_get_size (void *nxdata)
{
struct b_conn *b_conn = nxdata;
- return b_conn->b->get_size (b_conn->b, b_conn->conn);
+ return backend_get_size (b_conn->b, b_conn->conn);
}
static int
next_can_write (void *nxdata)
{
struct b_conn *b_conn = nxdata;
- return b_conn->b->can_write (b_conn->b, b_conn->conn);
+ return backend_can_write (b_conn->b, b_conn->conn);
}
static int
next_can_flush (void *nxdata)
{
struct b_conn *b_conn = nxdata;
- return b_conn->b->can_flush (b_conn->b, b_conn->conn);
+ return backend_can_flush (b_conn->b, b_conn->conn);
}
static int
next_is_rotational (void *nxdata)
{
struct b_conn *b_conn = nxdata;
- return b_conn->b->is_rotational (b_conn->b, b_conn->conn);
+ return backend_is_rotational (b_conn->b, b_conn->conn);
}
static int
next_can_trim (void *nxdata)
{
struct b_conn *b_conn = nxdata;
- return b_conn->b->can_trim (b_conn->b, b_conn->conn);
+ return backend_can_trim (b_conn->b, b_conn->conn);
}
static int
next_can_zero (void *nxdata)
{
struct b_conn *b_conn = nxdata;
- return b_conn->b->can_zero (b_conn->b, b_conn->conn);
+ return backend_can_zero (b_conn->b, b_conn->conn);
}
static int
next_can_extents (void *nxdata)
{
struct b_conn *b_conn = nxdata;
- return b_conn->b->can_extents (b_conn->b, b_conn->conn);
+ return backend_can_extents (b_conn->b, b_conn->conn);
}
static int
next_can_fua (void *nxdata)
{
struct b_conn *b_conn = nxdata;
- return b_conn->b->can_fua (b_conn->b, b_conn->conn);
+ return backend_can_fua (b_conn->b, b_conn->conn);
}
static int
next_can_multi_conn (void *nxdata)
{
struct b_conn *b_conn = nxdata;
- return b_conn->b->can_multi_conn (b_conn->b, b_conn->conn);
+ return backend_can_multi_conn (b_conn->b, b_conn->conn);
}
static int
next_can_cache (void *nxdata)
{
struct b_conn *b_conn = nxdata;
- return b_conn->b->can_cache (b_conn->b, b_conn->conn);
+ return backend_can_cache (b_conn->b, b_conn->conn);
}
static int
@@ -315,13 +315,8 @@ next_pread (void *nxdata, void *buf, uint32_t count,
uint64_t offset,
uint32_t flags, int *err)
{
struct b_conn *b_conn = nxdata;
- int r;
-
- r = b_conn->b->pread (b_conn->b, b_conn->conn, buf, count,
offset, flags,
+ return backend_pread (b_conn->b, b_conn->conn, buf, count, offset,
flags,
err);
- if (r == -1)
- assert (*err);
- return r;
}
static int
@@ -329,25 +324,15 @@ next_pwrite (void *nxdata, const void *buf, uint32_t
count, uint64_t offset,
uint32_t flags, int *err)
{
struct b_conn *b_conn = nxdata;
- int r;
-
- r = b_conn->b->pwrite (b_conn->b, b_conn->conn, buf, count,
offset, flags,
+ return backend_pwrite (b_conn->b, b_conn->conn, buf, count, offset,
flags,
err);
- if (r == -1)
- assert (*err);
- return r;
}
static int
next_flush (void *nxdata, uint32_t flags, int *err)
{
struct b_conn *b_conn = nxdata;
- int r;
-
- r = b_conn->b->flush (b_conn->b, b_conn->conn, flags, err);
- if (r == -1)
- assert (*err);
- return r;
+ return backend_flush (b_conn->b, b_conn->conn, flags, err);
}
static int
@@ -355,12 +340,7 @@ next_trim (void *nxdata, uint32_t count, uint64_t offset,
uint32_t flags,
int *err)
{
struct b_conn *b_conn = nxdata;
- int r;
-
- r = b_conn->b->trim (b_conn->b, b_conn->conn, count, offset,
flags, err);
- if (r == -1)
- assert (*err);
- return r;
+ return backend_trim (b_conn->b, b_conn->conn, count, offset, flags,
err);
}
static int
@@ -368,12 +348,7 @@ next_zero (void *nxdata, uint32_t count, uint64_t offset,
uint32_t flags,
int *err)
{
struct b_conn *b_conn = nxdata;
- int r;
-
- r = b_conn->b->zero (b_conn->b, b_conn->conn, count, offset,
flags, err);
- if (r == -1)
- assert (*err && *err != ENOTSUP && *err != EOPNOTSUPP);
- return r;
+ return backend_zero (b_conn->b, b_conn->conn, count, offset, flags,
err);
}
static int
@@ -381,13 +356,8 @@ next_extents (void *nxdata, uint32_t count, uint64_t
offset, uint32_t flags,
struct nbdkit_extents *extents, int *err)
{
struct b_conn *b_conn = nxdata;
- int r;
-
- r = b_conn->b->extents (b_conn->b, b_conn->conn, count, offset,
flags,
+ return backend_extents (b_conn->b, b_conn->conn, count, offset, flags,
extents, err);
- if (r == -1)
- assert (*err);
- return r;
}
static int
@@ -395,12 +365,7 @@ next_cache (void *nxdata, uint32_t count, uint64_t offset,
uint32_t flags, int *err)
{
struct b_conn *b_conn = nxdata;
- int r;
-
- r = b_conn->b->cache (b_conn->b, b_conn->conn, count, offset,
flags, err);
- if (r == -1)
- assert (*err);
- return r;
+ return backend_cache (b_conn->b, b_conn->conn, count, offset, flags,
err);
}
static struct nbdkit_next_ops next_ops = {
@@ -471,12 +436,10 @@ filter_get_size (struct backend *b, struct connection
*conn)
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- debug ("%s: get_size", b->name);
-
if (f->filter.get_size)
return f->filter.get_size (&next_ops, &nxdata, handle);
else
- return b->next->get_size (b->next, conn);
+ return backend_get_size (b->next, conn);
}
static int
@@ -486,12 +449,10 @@ filter_can_write (struct backend *b, struct connection
*conn)
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- debug ("%s: can_write", b->name);
-
if (f->filter.can_write)
return f->filter.can_write (&next_ops, &nxdata, handle);
else
- return b->next->can_write (b->next, conn);
+ return backend_can_write (b->next, conn);
}
static int
@@ -501,12 +462,10 @@ filter_can_flush (struct backend *b, struct connection
*conn)
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- debug ("%s: can_flush", b->name);
-
if (f->filter.can_flush)
return f->filter.can_flush (&next_ops, &nxdata, handle);
else
- return b->next->can_flush (b->next, conn);
+ return backend_can_flush (b->next, conn);
}
static int
@@ -516,12 +475,10 @@ filter_is_rotational (struct backend *b, struct connection
*conn)
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- debug ("%s: is_rotational", b->name);
-
if (f->filter.is_rotational)
return f->filter.is_rotational (&next_ops, &nxdata, handle);
else
- return b->next->is_rotational (b->next, conn);
+ return backend_is_rotational (b->next, conn);
}
static int
@@ -531,12 +488,10 @@ filter_can_trim (struct backend *b, struct connection
*conn)
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- debug ("%s: can_trim", b->name);
-
if (f->filter.can_trim)
return f->filter.can_trim (&next_ops, &nxdata, handle);
else
- return b->next->can_trim (b->next, conn);
+ return backend_can_trim (b->next, conn);
}
static int
@@ -546,12 +501,10 @@ filter_can_zero (struct backend *b, struct connection
*conn)
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- debug ("%s: can_zero", b->name);
-
if (f->filter.can_zero)
return f->filter.can_zero (&next_ops, &nxdata, handle);
else
- return b->next->can_zero (b->next, conn);
+ return backend_can_zero (b->next, conn);
}
static int
@@ -561,12 +514,10 @@ filter_can_extents (struct backend *b, struct connection
*conn)
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- debug ("%s: can_extents", b->name);
-
if (f->filter.can_extents)
return f->filter.can_extents (&next_ops, &nxdata, handle);
else
- return b->next->can_extents (b->next, conn);
+ return backend_can_extents (b->next, conn);
}
static int
@@ -576,12 +527,10 @@ filter_can_fua (struct backend *b, struct connection
*conn)
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- debug ("%s: can_fua", b->name);
-
if (f->filter.can_fua)
return f->filter.can_fua (&next_ops, &nxdata, handle);
else
- return b->next->can_fua (b->next, conn);
+ return backend_can_fua (b->next, conn);
}
static int
@@ -591,12 +540,10 @@ filter_can_multi_conn (struct backend *b, struct
connection *conn)
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- debug ("%s: can_multi_conn", b->name);
-
if (f->filter.can_multi_conn)
return f->filter.can_multi_conn (&next_ops, &nxdata, handle);
else
- return b->next->can_multi_conn (b->next, conn);
+ return backend_can_multi_conn (b->next, conn);
}
static int
@@ -606,12 +553,10 @@ filter_can_cache (struct backend *b, struct connection
*conn)
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- debug ("%s: can_cache", b->name);
-
if (f->filter.can_cache)
return f->filter.can_cache (&next_ops, &nxdata, handle);
else
- return b->next->can_cache (b->next, conn);
+ return backend_can_cache (b->next, conn);
}
static int
@@ -623,16 +568,11 @@ filter_pread (struct backend *b, struct connection *conn,
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- assert (flags == 0);
-
- debug ("%s: pread count=%" PRIu32 " offset=%" PRIu64
" flags=0x%" PRIx32,
- b->name, count, offset, flags);
-
if (f->filter.pread)
return f->filter.pread (&next_ops, &nxdata, handle,
buf, count, offset, flags, err);
else
- return b->next->pread (b->next, conn, buf, count, offset, flags,
err);
+ return backend_pread (b->next, conn, buf, count, offset, flags, err);
}
static int
@@ -644,16 +584,11 @@ filter_pwrite (struct backend *b, struct connection *conn,
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- assert (!(flags & ~NBDKIT_FLAG_FUA));
-
- debug ("%s: pwrite count=%" PRIu32 " offset=%" PRIu64
" flags=0x%" PRIx32,
- b->name, count, offset, flags);
-
if (f->filter.pwrite)
return f->filter.pwrite (&next_ops, &nxdata, handle,
buf, count, offset, flags, err);
else
- return b->next->pwrite (b->next, conn, buf, count, offset, flags,
err);
+ return backend_pwrite (b->next, conn, buf, count, offset, flags, err);
}
static int
@@ -664,14 +599,10 @@ filter_flush (struct backend *b, struct connection *conn,
uint32_t flags,
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- assert (flags == 0);
-
- debug ("%s: flush flags=0x%" PRIx32, b->name, flags);
-
if (f->filter.flush)
return f->filter.flush (&next_ops, &nxdata, handle, flags, err);
else
- return b->next->flush (b->next, conn, flags, err);
+ return backend_flush (b->next, conn, flags, err);
}
static int
@@ -683,16 +614,11 @@ filter_trim (struct backend *b, struct connection *conn,
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- assert (flags == 0);
-
- debug ("%s: trim count=%" PRIu32 " offset=%" PRIu64
" flags=0x%" PRIx32,
- b->name, count, offset, flags);
-
if (f->filter.trim)
return f->filter.trim (&next_ops, &nxdata, handle, count,
offset, flags,
err);
else
- return b->next->trim (b->next, conn, count, offset, flags, err);
+ return backend_trim (b->next, conn, count, offset, flags, err);
}
static int
@@ -703,16 +629,11 @@ filter_zero (struct backend *b, struct connection *conn,
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- assert (!(flags & ~(NBDKIT_FLAG_MAY_TRIM | NBDKIT_FLAG_FUA)));
-
- debug ("%s: zero count=%" PRIu32 " offset=%" PRIu64
" flags=0x%" PRIx32,
- b->name, count, offset, flags);
-
if (f->filter.zero)
return f->filter.zero (&next_ops, &nxdata, handle,
count, offset, flags, err);
else
- return b->next->zero (b->next, conn, count, offset, flags, err);
+ return backend_zero (b->next, conn, count, offset, flags, err);
}
static int
@@ -724,18 +645,13 @@ filter_extents (struct backend *b, struct connection
*conn,
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- assert (!(flags & ~NBDKIT_FLAG_REQ_ONE));
-
- debug ("%s: extents count=%" PRIu32 " offset=%" PRIu64
" flags=0x%" PRIx32,
- b->name, count, offset, flags);
-
if (f->filter.extents)
return f->filter.extents (&next_ops, &nxdata, handle,
count, offset, flags,
extents, err);
else
- return b->next->extents (b->next, conn, count, offset, flags,
- extents, err);
+ return backend_extents (b->next, conn, count, offset, flags,
+ extents, err);
}
static int
@@ -747,16 +663,12 @@ filter_cache (struct backend *b, struct connection *conn,
void *handle = connection_get_handle (conn, b->i);
struct b_conn nxdata = { .b = b->next, .conn = conn };
- assert (flags == 0);
-
- debug ("%s: cache count=%" PRIu32 " offset=%" PRIu64
" flags=0x%" PRIx32,
- b->name, count, offset, flags);
if (f->filter.cache)
return f->filter.cache (&next_ops, &nxdata, handle,
count, offset, flags, err);
else
- return b->next->cache (b->next, conn, count, offset, flags, err);
+ return backend_cache (b->next, conn, count, offset, flags, err);
}
static struct backend filter_functions = {
diff --git a/server/plugins.c b/server/plugins.c
index 0f70ede0..da8931a3 100644
--- a/server/plugins.c
+++ b/server/plugins.c
@@ -295,8 +295,6 @@ plugin_get_size (struct backend *b, struct connection *conn)
assert (connection_get_handle (conn, 0));
assert (p->plugin.get_size != NULL);
- debug ("get_size");
-
return p->plugin.get_size (connection_get_handle (conn, 0));
}
@@ -307,8 +305,6 @@ plugin_can_write (struct backend *b, struct connection
*conn)
assert (connection_get_handle (conn, 0));
- debug ("can_write");
-
if (p->plugin.can_write)
return p->plugin.can_write (connection_get_handle (conn, 0));
else
@@ -322,8 +318,6 @@ plugin_can_flush (struct backend *b, struct connection
*conn)
assert (connection_get_handle (conn, 0));
- debug ("can_flush");
-
if (p->plugin.can_flush)
return p->plugin.can_flush (connection_get_handle (conn, 0));
else
@@ -337,8 +331,6 @@ plugin_is_rotational (struct backend *b, struct connection
*conn)
assert (connection_get_handle (conn, 0));
- debug ("is_rotational");
-
if (p->plugin.is_rotational)
return p->plugin.is_rotational (connection_get_handle (conn, 0));
else
@@ -352,8 +344,6 @@ plugin_can_trim (struct backend *b, struct connection *conn)
assert (connection_get_handle (conn, 0));
- debug ("can_trim");
-
if (p->plugin.can_trim)
return p->plugin.can_trim (connection_get_handle (conn, 0));
else
@@ -367,8 +357,6 @@ plugin_can_zero (struct backend *b, struct connection *conn)
assert (connection_get_handle (conn, 0));
- debug ("can_zero");
-
/* Note the special case here: the plugin's .can_zero controls only
* whether we call .zero; while the backend expects .can_zero to
* return whether to advertise zero support. Since we ALWAYS know
@@ -388,8 +376,6 @@ plugin_can_extents (struct backend *b, struct connection
*conn)
assert (connection_get_handle (conn, 0));
- debug ("can_extents");
-
if (p->plugin.can_extents)
return p->plugin.can_extents (connection_get_handle (conn, 0));
else
@@ -404,8 +390,6 @@ plugin_can_fua (struct backend *b, struct connection *conn)
assert (connection_get_handle (conn, 0));
- debug ("can_fua");
-
/* The plugin must use API version 2 and have .can_fua return
NBDKIT_FUA_NATIVE before we will pass the FUA flag on. */
if (p->plugin.can_fua) {
@@ -427,8 +411,6 @@ plugin_can_multi_conn (struct backend *b, struct connection
*conn)
assert (connection_get_handle (conn, 0));
- debug ("can_multi_conn");
-
if (p->plugin.can_multi_conn)
return p->plugin.can_multi_conn (connection_get_handle (conn, 0));
else
@@ -442,8 +424,6 @@ plugin_can_cache (struct backend *b, struct connection
*conn)
assert (connection_get_handle (conn, 0));
- debug ("can_cache");
-
if (p->plugin.can_cache)
return p->plugin.can_cache (connection_get_handle (conn, 0));
if (p->plugin.cache)
@@ -482,9 +462,6 @@ plugin_pread (struct backend *b, struct connection *conn,
assert (connection_get_handle (conn, 0));
assert (p->plugin.pread || p->plugin._pread_old);
- assert (!flags);
-
- debug ("pread count=%" PRIu32 " offset=%" PRIu64, count,
offset);
if (p->plugin.pread)
r = p->plugin.pread (connection_get_handle (conn, 0), buf, count,
offset,
@@ -507,8 +484,6 @@ plugin_flush (struct backend *b, struct connection *conn,
uint32_t flags,
assert (connection_get_handle (conn, 0));
assert (!flags);
- debug ("flush");
-
if (p->plugin.flush)
r = p->plugin.flush (connection_get_handle (conn, 0), 0);
else if (p->plugin._flush_old)
@@ -533,10 +508,6 @@ plugin_pwrite (struct backend *b, struct connection *conn,
bool need_flush = false;
assert (connection_get_handle (conn, 0));
- assert (!(flags & ~NBDKIT_FLAG_FUA));
-
- debug ("pwrite count=%" PRIu32 " offset=%" PRIu64 "
fua=%d", count, offset,
- fua);
if (fua && plugin_can_fua (b, conn) != NBDKIT_FUA_NATIVE) {
flags &= ~NBDKIT_FLAG_FUA;
@@ -569,10 +540,6 @@ plugin_trim (struct backend *b, struct connection *conn,
bool need_flush = false;
assert (connection_get_handle (conn, 0));
- assert (!(flags & ~NBDKIT_FLAG_FUA));
-
- debug ("trim count=%" PRIu32 " offset=%" PRIu64 "
fua=%d", count, offset,
- fua);
if (fua && plugin_can_fua (b, conn) != NBDKIT_FUA_NATIVE) {
flags &= ~NBDKIT_FLAG_FUA;
@@ -606,10 +573,6 @@ plugin_zero (struct backend *b, struct connection *conn,
int can_zero = 1; /* TODO cache this per-connection? */
assert (connection_get_handle (conn, 0));
- assert (!(flags & ~(NBDKIT_FLAG_MAY_TRIM | NBDKIT_FLAG_FUA)));
-
- debug ("zero count=%" PRIu32 " offset=%" PRIu64 "
may_trim=%d fua=%d",
- count, offset, may_trim, fua);
if (fua && plugin_can_fua (b, conn) != NBDKIT_FUA_NATIVE) {
flags &= ~NBDKIT_FLAG_FUA;
@@ -670,21 +633,13 @@ plugin_extents (struct backend *b, struct connection
*conn,
struct nbdkit_extents *extents, int *err)
{
struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
- bool req_one = flags & NBDKIT_FLAG_REQ_ONE;
int r;
assert (connection_get_handle (conn, 0));
- assert (!(flags & ~NBDKIT_FLAG_REQ_ONE));
/* This should be true because plugin_can_extents checks it. */
assert (p->plugin.extents);
- debug ("extents count=%" PRIu32 " offset=%" PRIu64 "
req_one=%d",
- count, offset, req_one);
-
- if (!count)
- return 0;
-
r = p->plugin.extents (connection_get_handle (conn, 0), count, offset,
flags, extents);
if (r >= 0 && nbdkit_extents_count (extents) < 1) {
@@ -706,9 +661,6 @@ plugin_cache (struct backend *b, struct connection *conn,
int r;
assert (connection_get_handle (conn, 0));
- assert (!flags);
-
- debug ("cache count=%" PRIu32 " offset=%" PRIu64, count,
offset);
/* A plugin may advertise caching but not provide .cache; in that
* case, caching is explicitly a no-op. */
diff --git a/server/protocol-handshake-newstyle.c
b/server/protocol-handshake-newstyle.c
index 6ccb76f3..28817317 100644
--- a/server/protocol-handshake-newstyle.c
+++ b/server/protocol-handshake-newstyle.c
@@ -205,7 +205,7 @@ finish_newstyle_options (struct connection *conn)
{
int64_t r;
- r = backend->get_size (backend, conn);
+ r = backend_get_size (backend, conn);
if (r == -1)
return -1;
if (r < 0) {
diff --git a/server/protocol-handshake-oldstyle.c
b/server/protocol-handshake-oldstyle.c
index 9fde1ca0..af3ac488 100644
--- a/server/protocol-handshake-oldstyle.c
+++ b/server/protocol-handshake-oldstyle.c
@@ -59,7 +59,7 @@ protocol_handshake_oldstyle (struct connection *conn)
return -1;
}
- r = backend->get_size (backend, conn);
+ r = backend_get_size (backend, conn);
if (r == -1)
return -1;
if (r < 0) {
diff --git a/server/protocol-handshake.c b/server/protocol-handshake.c
index 0f3bd280..4d12b3dc 100644
--- a/server/protocol-handshake.c
+++ b/server/protocol-handshake.c
@@ -52,7 +52,7 @@ protocol_compute_eflags (struct connection *conn, uint16_t
*flags)
uint16_t eflags = NBD_FLAG_HAS_FLAGS;
int fl;
- fl = backend->can_write (backend, conn);
+ fl = backend_can_write (backend, conn);
if (fl == -1)
return -1;
if (readonly || !fl) {
@@ -60,7 +60,7 @@ protocol_compute_eflags (struct connection *conn, uint16_t
*flags)
conn->readonly = true;
}
if (!conn->readonly) {
- fl = backend->can_zero (backend, conn);
+ fl = backend_can_zero (backend, conn);
if (fl == -1)
return -1;
if (fl) {
@@ -68,7 +68,7 @@ protocol_compute_eflags (struct connection *conn, uint16_t
*flags)
conn->can_zero = true;
}
- fl = backend->can_trim (backend, conn);
+ fl = backend_can_trim (backend, conn);
if (fl == -1)
return -1;
if (fl) {
@@ -76,7 +76,7 @@ protocol_compute_eflags (struct connection *conn, uint16_t
*flags)
conn->can_trim = true;
}
- fl = backend->can_fua (backend, conn);
+ fl = backend_can_fua (backend, conn);
if (fl == -1)
return -1;
if (fl) {
@@ -85,7 +85,7 @@ protocol_compute_eflags (struct connection *conn, uint16_t
*flags)
}
}
- fl = backend->can_flush (backend, conn);
+ fl = backend_can_flush (backend, conn);
if (fl == -1)
return -1;
if (fl) {
@@ -93,7 +93,7 @@ protocol_compute_eflags (struct connection *conn, uint16_t
*flags)
conn->can_flush = true;
}
- fl = backend->is_rotational (backend, conn);
+ fl = backend_is_rotational (backend, conn);
if (fl == -1)
return -1;
if (fl) {
@@ -104,7 +104,7 @@ protocol_compute_eflags (struct connection *conn, uint16_t
*flags)
/* multi-conn is useless if parallel connections are not allowed */
if (backend->thread_model (backend) >
NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) {
- fl = backend->can_multi_conn (backend, conn);
+ fl = backend_can_multi_conn (backend, conn);
if (fl == -1)
return -1;
if (fl) {
@@ -113,7 +113,7 @@ protocol_compute_eflags (struct connection *conn, uint16_t
*flags)
}
}
- fl = backend->can_cache (backend, conn);
+ fl = backend_can_cache (backend, conn);
if (fl == -1)
return -1;
if (fl) {
@@ -127,7 +127,7 @@ protocol_compute_eflags (struct connection *conn, uint16_t
*flags)
* per connection and store the result in the handle anyway. This
* protocol_compute_eflags function is a bit misnamed XXX.
*/
- fl = backend->can_extents (backend, conn);
+ fl = backend_can_extents (backend, conn);
if (fl == -1)
return -1;
if (fl)
diff --git a/server/protocol.c b/server/protocol.c
index 72f6f2a8..72efcaa6 100644
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -240,35 +240,27 @@ handle_request (struct connection *conn,
switch (cmd) {
case NBD_CMD_READ:
- if (backend->pread (backend, conn, buf, count, offset, 0, &err) ==
-1) {
- assert (err);
+ if (backend_pread (backend, conn, buf, count, offset, 0, &err) == -1)
return err;
- }
break;
case NBD_CMD_WRITE:
if (fua)
f |= NBDKIT_FLAG_FUA;
- if (backend->pwrite (backend, conn, buf, count, offset, f, &err) ==
-1) {
- assert (err);
+ if (backend_pwrite (backend, conn, buf, count, offset, f, &err) == -1)
return err;
- }
break;
case NBD_CMD_FLUSH:
- if (backend->flush (backend, conn, 0, &err) == -1) {
- assert (err);
+ if (backend_flush (backend, conn, 0, &err) == -1)
return err;
- }
break;
case NBD_CMD_TRIM:
if (fua)
f |= NBDKIT_FLAG_FUA;
- if (backend->trim (backend, conn, count, offset, f, &err) == -1) {
- assert (err);
+ if (backend_trim (backend, conn, count, offset, f, &err) == -1)
return err;
- }
break;
case NBD_CMD_CACHE:
@@ -278,18 +270,14 @@ handle_request (struct connection *conn,
while (count) {
limit = MIN (count, sizeof buf);
- if (backend->pread (backend, conn, buf, limit, offset, flags,
- &err) == -1) {
- assert (err);
+ if (backend_pread (backend, conn, buf, limit, offset, flags,
+ &err) == -1)
return err;
- }
count -= limit;
}
}
- else if (backend->cache (backend, conn, count, offset, 0, &err) ==
-1) {
- assert (err);
+ else if (backend_cache (backend, conn, count, offset, 0, &err) == -1)
return err;
- }
break;
case NBD_CMD_WRITE_ZEROES:
@@ -297,10 +285,8 @@ handle_request (struct connection *conn,
f |= NBDKIT_FLAG_MAY_TRIM;
if (fua)
f |= NBDKIT_FLAG_FUA;
- if (backend->zero (backend, conn, count, offset, f, &err) == -1) {
- assert (err && err != ENOTSUP && err != EOPNOTSUPP);
+ if (backend_zero (backend, conn, count, offset, f, &err) == -1)
return err;
- }
break;
case NBD_CMD_BLOCK_STATUS:
@@ -312,11 +298,9 @@ handle_request (struct connection *conn,
if (conn->can_extents) {
if (flags & NBD_CMD_FLAG_REQ_ONE)
f |= NBDKIT_FLAG_REQ_ONE;
- if (backend->extents (backend, conn, count, offset, f,
- extents, &err) == -1) {
- assert (err);
+ if (backend_extents (backend, conn, count, offset, f,
+ extents, &err) == -1)
return err;
- }
}
else {
int r;
diff --git a/tests/test-layers.c b/tests/test-layers.c
index 6617cd73..a7184d72 100644
--- a/tests/test-layers.c
+++ b/tests/test-layers.c
@@ -1,5 +1,5 @@
/* nbdkit
- * Copyright (C) 2018 Red Hat Inc.
+ * Copyright (C) 2018-2019 Red Hat Inc.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -400,13 +400,13 @@ main (int argc, char *argv[])
sleep (1);
log_verify_seen_in_order
- ("testlayersfilter3: pread count=512 offset=0 flags=0x0",
+ ("testlayersfilter3: pread count=512 offset=0",
"filter3: test_layers_filter_pread",
- "testlayersfilter2: pread count=512 offset=0 flags=0x0",
+ "testlayersfilter2: pread count=512 offset=0",
"filter2: test_layers_filter_pread",
- "testlayersfilter1: pread count=512 offset=0 flags=0x0",
+ "testlayersfilter1: pread count=512 offset=0",
"filter1: test_layers_filter_pread",
- "testlayersplugin: debug: pread count=512 offset=0",
+ "testlayersplugin: pread count=512 offset=0",
"test_layers_plugin_pread",
NULL);
@@ -434,13 +434,13 @@ main (int argc, char *argv[])
sleep (1);
log_verify_seen_in_order
- ("testlayersfilter3: pwrite count=512 offset=0 flags=0x0",
+ ("testlayersfilter3: pwrite count=512 offset=0 fua=0",
"filter3: test_layers_filter_pwrite",
- "testlayersfilter2: pwrite count=512 offset=0 flags=0x0",
+ "testlayersfilter2: pwrite count=512 offset=0 fua=0",
"filter2: test_layers_filter_pwrite",
- "testlayersfilter1: pwrite count=512 offset=0 flags=0x0",
+ "testlayersfilter1: pwrite count=512 offset=0 fua=0",
"filter1: test_layers_filter_pwrite",
- "testlayersplugin: debug: pwrite count=512 offset=0",
+ "testlayersplugin: pwrite count=512 offset=0 fua=0",
"test_layers_plugin_pwrite",
NULL);
@@ -464,13 +464,13 @@ main (int argc, char *argv[])
sleep (1);
log_verify_seen_in_order
- ("testlayersfilter3: flush flags=0x0",
+ ("testlayersfilter3: flush",
"filter3: test_layers_filter_flush",
- "testlayersfilter2: flush flags=0x0",
+ "testlayersfilter2: flush",
"filter2: test_layers_filter_flush",
- "testlayersfilter1: flush flags=0x0",
+ "testlayersfilter1: flush",
"filter1: test_layers_filter_flush",
- "testlayersplugin: debug: flush",
+ "testlayersplugin: flush",
"test_layers_plugin_flush",
NULL);
@@ -494,13 +494,13 @@ main (int argc, char *argv[])
sleep (1);
log_verify_seen_in_order
- ("testlayersfilter3: trim count=512 offset=0 flags=0x0",
+ ("testlayersfilter3: trim count=512 offset=0 fua=0",
"filter3: test_layers_filter_trim",
- "testlayersfilter2: trim count=512 offset=0 flags=0x0",
+ "testlayersfilter2: trim count=512 offset=0 fua=0",
"filter2: test_layers_filter_trim",
- "testlayersfilter1: trim count=512 offset=0 flags=0x0",
+ "testlayersfilter1: trim count=512 offset=0 fua=0",
"filter1: test_layers_filter_trim",
- "testlayersplugin: debug: trim count=512 offset=0",
+ "testlayersplugin: trim count=512 offset=0 fua=0",
"test_layers_plugin_trim",
NULL);
@@ -524,13 +524,13 @@ main (int argc, char *argv[])
sleep (1);
log_verify_seen_in_order
- ("testlayersfilter3: zero count=512 offset=0 flags=0x1",
+ ("testlayersfilter3: zero count=512 offset=0 may_trim=1 fua=0",
"filter3: test_layers_filter_zero",
- "testlayersfilter2: zero count=512 offset=0 flags=0x1",
+ "testlayersfilter2: zero count=512 offset=0 may_trim=1 fua=0",
"filter2: test_layers_filter_zero",
- "testlayersfilter1: zero count=512 offset=0 flags=0x1",
+ "testlayersfilter1: zero count=512 offset=0 may_trim=1 fua=0",
"filter1: test_layers_filter_zero",
- "testlayersplugin: debug: zero count=512 offset=0 may_trim=1
fua=0",
+ "testlayersplugin: zero count=512 offset=0 may_trim=1 fua=0",
"test_layers_plugin_zero",
NULL);
@@ -554,13 +554,13 @@ main (int argc, char *argv[])
sleep (1);
log_verify_seen_in_order
- ("testlayersfilter3: cache count=512 offset=0 flags=0x0",
+ ("testlayersfilter3: cache count=512 offset=0",
"filter3: test_layers_filter_cache",
- "testlayersfilter2: cache count=512 offset=0 flags=0x0",
+ "testlayersfilter2: cache count=512 offset=0",
"filter2: test_layers_filter_cache",
- "testlayersfilter1: cache count=512 offset=0 flags=0x0",
+ "testlayersfilter1: cache count=512 offset=0",
"filter1: test_layers_filter_cache",
- "testlayersplugin: debug: cache count=512 offset=0",
+ "testlayersplugin: cache count=512 offset=0",
"test_layers_plugin_cache",
NULL);
--
2.21.0
Eric Blake
2019-Aug-30 03:08 UTC
[Libguestfs] [nbdkit PATCH 4/9] server: Rework storage of per-backend handle
Right now, each connection maintains a bare list of void* handles
obtained from the backends, which is reallocated on-the-fly as
additional filters go through .open (and where a filter that skips
.open does not necessarily resize the array). However, we know the
size of the array in advance (because we know how many filters were
loaded) and can avoid realloc churn; and we also want to move the code
towards being able to cache more than just the handle from the
filter/plugin. So it makes sense to have each connection maintain an
array of structs that includes the handle and anything else associated
with a given backend/connection combination.
This patch refactors the allocation and tracking of handles, while
upcoming patches add some caching made possible by the new array of
structs.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
server/internal.h | 15 ++++++++++----
server/backend.c | 7 +++++++
server/connections.c | 47 ++++++++++++--------------------------------
server/filters.c | 2 +-
server/plugins.c | 4 ++--
5 files changed, 34 insertions(+), 41 deletions(-)
diff --git a/server/internal.h b/server/internal.h
index 93ebeb78..9bf84022 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -148,6 +148,12 @@ typedef int (*connection_send_function) (struct connection
*,
typedef void (*connection_close_function) (struct connection *)
__attribute__((__nonnull__ (1)));
+struct b_conn_handle {
+ void *handle;
+
+ // TODO add per-backend caching
+};
+
struct connection {
pthread_mutex_t request_lock;
pthread_mutex_t read_lock;
@@ -158,7 +164,7 @@ struct connection {
void *crypto_session;
int nworkers;
- void **handles;
+ struct b_conn_handle *handles;
size_t nr_handles;
uint32_t cflags;
@@ -185,9 +191,6 @@ struct connection {
};
extern int handle_single_connection (int sockin, int sockout);
-extern int connection_set_handle (struct connection *conn,
- size_t i, void *handle)
- __attribute__((__nonnull__ (1 /* not 3 */)));
extern void *connection_get_handle (struct connection *conn, size_t i)
__attribute__((__nonnull__ (1)));
extern int connection_get_status (struct connection *conn)
@@ -317,6 +320,10 @@ extern void backend_set_name (struct backend *b, const char
*name,
extern void backend_unload (struct backend *b, void (*unload) (void))
__attribute__((__nonnull__ (1)));
+extern void backend_set_handle (struct backend *b, struct connection *conn,
+ void *handle)
+ __attribute__((__nonnull__ (1, 2 /* not 3 */)));
+
extern int64_t backend_get_size (struct backend *b, struct connection *conn)
__attribute__((__nonnull__ (1, 2)));
extern int backend_can_write (struct backend *b, struct connection *conn)
diff --git a/server/backend.c b/server/backend.c
index ce3c5e2b..b2054aa2 100644
--- a/server/backend.c
+++ b/server/backend.c
@@ -122,6 +122,13 @@ backend_unload (struct backend *b, void (*unload) (void))
free (b->name);
}
+void
+backend_set_handle (struct backend *b, struct connection *conn, void *handle)
+{
+ assert (b->i < conn->nr_handles);
+ conn->handles[b->i].handle = handle;
+}
+
int64_t
backend_get_size (struct backend *b, struct connection *conn)
{
diff --git a/server/connections.c b/server/connections.c
index 0184afea..1c501002 100644
--- a/server/connections.c
+++ b/server/connections.c
@@ -60,37 +60,11 @@ static int raw_send_other (struct connection *, const void
*buf, size_t len,
int flags);
static void raw_close (struct connection *);
-int
-connection_set_handle (struct connection *conn, size_t i, void *handle)
-{
- size_t j;
-
- if (i < conn->nr_handles)
- conn->handles[i] = handle;
- else {
- j = conn->nr_handles;
- conn->nr_handles = i+1;
- conn->handles = realloc (conn->handles,
- conn->nr_handles * sizeof (void *));
- if (conn->handles == NULL) {
- perror ("realloc");
- conn->nr_handles = 0;
- return -1;
- }
- for (; j < i; ++j)
- conn->handles[j] = NULL;
- conn->handles[i] = handle;
- }
- return 0;
-}
-
void *
connection_get_handle (struct connection *conn, size_t i)
{
- if (i < conn->nr_handles)
- return conn->handles[i];
- else
- return NULL;
+ assert (i < conn->nr_handles);
+ return conn->handles[i].handle;
}
int
@@ -292,6 +266,13 @@ new_connection (int sockin, int sockout, int nworkers)
perror ("malloc");
return NULL;
}
+ conn->handles = calloc (backend->i + 1, sizeof *conn->handles);
+ if (conn->handles == NULL) {
+ perror ("malloc");
+ free (conn);
+ return NULL;
+ }
+ conn->nr_handles = backend->i + 1;
conn->status = 1;
conn->nworkers = nworkers;
@@ -383,12 +364,10 @@ free_connection (struct connection *conn)
* thread will be in the process of unloading it. The plugin.unload
* callback should always be called.
*/
- if (!quit) {
- if (conn->nr_handles > 0 && conn->handles[0]) {
- lock_request (conn);
- backend->close (backend, conn);
- unlock_request (conn);
- }
+ if (!quit && connection_get_handle (conn, 0)) {
+ lock_request (conn);
+ backend->close (backend, conn);
+ unlock_request (conn);
}
if (conn->status_pipe[0] >= 0) {
diff --git a/server/filters.c b/server/filters.c
index acb44bda..614ce2f6 100644
--- a/server/filters.c
+++ b/server/filters.c
@@ -213,7 +213,7 @@ filter_open (struct backend *b, struct connection *conn, int
readonly)
handle = f->filter.open (next_open, &nxdata, readonly);
if (handle == NULL)
return -1;
- connection_set_handle (conn, b->i, handle);
+ backend_set_handle (b, conn, handle);
return 0;
}
else
diff --git a/server/plugins.c b/server/plugins.c
index da8931a3..d1654f8d 100644
--- a/server/plugins.c
+++ b/server/plugins.c
@@ -252,7 +252,7 @@ plugin_open (struct backend *b, struct connection *conn, int
readonly)
if (!handle)
return -1;
- connection_set_handle (conn, 0, handle);
+ backend_set_handle (b, conn, handle);
return 0;
}
@@ -284,7 +284,7 @@ plugin_close (struct backend *b, struct connection *conn)
if (p->plugin.close)
p->plugin.close (connection_get_handle (conn, 0));
- connection_set_handle (conn, 0, NULL);
+ backend_set_handle (b, conn, NULL);
}
static int64_t
--
2.21.0
Eric Blake
2019-Aug-30 03:08 UTC
[Libguestfs] [nbdkit PATCH 5/9] server: Cache per-connection size
We don't know how long a plugin's .get_size() will take, but we also
documented that it shouldn't change per connection and therefore can
be cached. It's not hard to see that we have to consult the size per
connection (see commit b3a43ccd for a test that purposefully exposes
different sizes to separate clients), nor to search the code to see we
already cache it at the protocol level, given that each .extents call
refers to the current size:
$ cat script
case "$1" in
get_size) sleep 1; echo 1m;;
can_extents) ;;
extents) echo 0 1m;;
*) exit 2 ;;
esac
$ /bin/time -f %e \
./nbdkit -U - sh script --run 'qemu-io -r -f raw -c map -c map $nbd'
1 MiB (0x100000) bytes allocated at offset 0 bytes (0x0)
1 MiB (0x100000) bytes allocated at offset 0 bytes (0x0)
1.08
and we see completion in just over a second, so the sleep in get_size
is called only once. But we are not caching it in all filters:
$ /bin/time -f %e \
./nbdkit -U - --filter=offset sh script offset=512k \
--run 'qemu-io -r -f raw -c map -c map $nbd'
512 KiB (0x80000) bytes allocated at offset 0 bytes (0x0)
512 KiB (0x80000) bytes allocated at offset 0 bytes (0x0)
3.13
where the 3 second delay demonstrates that the offset filter calls
next->get_size() once per top-level .extents call.
The previous patches made it easy to support a framework for
per-backend caching, so now to put it to use by remembering the result
of get_size() for each. With that, the above example with offset
filtering speeds up to a completion in just over one second.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
docs/nbdkit-filter.pod | 9 +++++----
server/internal.h | 3 +--
server/backend.c | 7 +++++--
server/connections.c | 3 +++
server/protocol-handshake-newstyle.c | 18 ++++++++++--------
server/protocol-handshake-oldstyle.c | 1 -
server/protocol.c | 5 +++--
7 files changed, 27 insertions(+), 19 deletions(-)
diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod
index cfd664eb..1e2fe99c 100644
--- a/docs/nbdkit-filter.pod
+++ b/docs/nbdkit-filter.pod
@@ -346,10 +346,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-E<gt>get_size> for a given connection rather than
repeating
-calls.
+This function is only called once per connection and cached by nbdkit.
+Similarly, repeated calls to C<next_ops-E<gt>get_size> will returnf
a
+cached value; by calling into the plugin during C<.prepare>, you can
+ensure that later calls during data commands like <.pread> will not
+fail.
=head2 C<.can_write>
diff --git a/server/internal.h b/server/internal.h
index 9bf84022..ec8a894c 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -151,7 +151,7 @@ typedef void (*connection_close_function) (struct connection
*)
struct b_conn_handle {
void *handle;
- // TODO add per-backend caching
+ uint64_t exportsize;
};
struct connection {
@@ -168,7 +168,6 @@ struct connection {
size_t nr_handles;
uint32_t cflags;
- uint64_t exportsize;
uint16_t eflags;
bool readonly;
bool can_flush;
diff --git a/server/backend.c b/server/backend.c
index b2054aa2..374d8540 100644
--- a/server/backend.c
+++ b/server/backend.c
@@ -132,10 +132,13 @@ backend_set_handle (struct backend *b, struct connection
*conn, void *handle)
int64_t
backend_get_size (struct backend *b, struct connection *conn)
{
+ struct b_conn_handle *h = &conn->handles[b->i];
+
debug ("%s: get_size", b->name);
- /* TODO caching */
- return b->get_size (b, conn);
+ if (h->exportsize == -1)
+ h->exportsize = b->get_size (b, conn);
+ return h->exportsize;
}
int
diff --git a/server/connections.c b/server/connections.c
index 1c501002..2a5150cd 100644
--- a/server/connections.c
+++ b/server/connections.c
@@ -260,6 +260,7 @@ new_connection (int sockin, int sockout, int nworkers)
struct connection *conn;
int opt;
socklen_t optlen = sizeof opt;
+ struct backend *b;
conn = calloc (1, sizeof *conn);
if (conn == NULL) {
@@ -273,6 +274,8 @@ new_connection (int sockin, int sockout, int nworkers)
return NULL;
}
conn->nr_handles = backend->i + 1;
+ for_each_backend (b)
+ conn->handles[b->i].exportsize = -1;
conn->status = 1;
conn->nworkers = nworkers;
diff --git a/server/protocol-handshake-newstyle.c
b/server/protocol-handshake-newstyle.c
index 28817317..23579e5d 100644
--- a/server/protocol-handshake-newstyle.c
+++ b/server/protocol-handshake-newstyle.c
@@ -117,7 +117,7 @@ send_newstyle_option_reply_exportname (struct connection
*conn,
static int
send_newstyle_option_reply_info_export (struct connection *conn,
uint32_t option, uint32_t reply,
- uint16_t info)
+ uint16_t info, uint64_t exportsize)
{
struct fixed_new_option_reply fixed_new_option_reply;
struct fixed_new_option_reply_info_export export;
@@ -127,7 +127,7 @@ send_newstyle_option_reply_info_export (struct connection
*conn,
fixed_new_option_reply.reply = htobe32 (reply);
fixed_new_option_reply.replylen = htobe32 (sizeof export);
export.info = htobe16 (info);
- export.exportsize = htobe64 (conn->exportsize);
+ export.exportsize = htobe64 (exportsize);
export.eflags = htobe16 (conn->eflags);
if (conn->send (conn,
@@ -201,7 +201,7 @@ conn_recv_full (struct connection *conn, void *buf, size_t
len,
* in that function.
*/
static int
-finish_newstyle_options (struct connection *conn)
+finish_newstyle_options (struct connection *conn, uint64_t *exportsize)
{
int64_t r;
@@ -213,7 +213,7 @@ finish_newstyle_options (struct connection *conn)
"(%" PRIi64 ")", r);
return -1;
}
- conn->exportsize = (uint64_t) r;
+ *exportsize = r;
if (protocol_compute_eflags (conn, &conn->eflags) < 0)
return -1;
@@ -233,6 +233,7 @@ negotiate_handshake_newstyle_options (struct connection
*conn)
char data[MAX_OPTION_LENGTH+1];
struct new_handshake_finish handshake_finish;
const char *optname;
+ uint64_t exportsize;
for (nr_options = 0; nr_options < MAX_NR_OPTIONS; ++nr_options) {
if (conn_recv_full (conn, &new_option, sizeof new_option,
@@ -281,11 +282,11 @@ negotiate_handshake_newstyle_options (struct connection
*conn)
name_of_nbd_opt (option), data);
/* We have to finish the handshake by sending handshake_finish. */
- if (finish_newstyle_options (conn) == -1)
+ if (finish_newstyle_options (conn, &exportsize) == -1)
return -1;
memset (&handshake_finish, 0, sizeof handshake_finish);
- handshake_finish.exportsize = htobe64 (conn->exportsize);
+ handshake_finish.exportsize = htobe64 (exportsize);
handshake_finish.eflags = htobe16 (conn->eflags);
if (conn->send (conn,
@@ -432,12 +433,13 @@ negotiate_handshake_newstyle_options (struct connection
*conn)
* qemu client in particular does not request this, but will
* fail if we don't send it.
*/
- if (finish_newstyle_options (conn) == -1)
+ if (finish_newstyle_options (conn, &exportsize) == -1)
return -1;
if (send_newstyle_option_reply_info_export (conn, option,
NBD_REP_INFO,
- NBD_INFO_EXPORT) == -1)
+ NBD_INFO_EXPORT,
+ exportsize) == -1)
return -1;
/* For now we ignore all other info requests (but we must
diff --git a/server/protocol-handshake-oldstyle.c
b/server/protocol-handshake-oldstyle.c
index af3ac488..3d44e9db 100644
--- a/server/protocol-handshake-oldstyle.c
+++ b/server/protocol-handshake-oldstyle.c
@@ -68,7 +68,6 @@ protocol_handshake_oldstyle (struct connection *conn)
return -1;
}
exportsize = (uint64_t) r;
- conn->exportsize = exportsize;
gflags = 0;
if (protocol_compute_eflags (conn, &eflags) < 0)
diff --git a/server/protocol.c b/server/protocol.c
index 72efcaa6..06f1ee15 100644
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -53,8 +53,9 @@
static bool
valid_range (struct connection *conn, uint64_t offset, uint32_t count)
{
- uint64_t exportsize = conn->exportsize;
+ uint64_t exportsize = backend_get_size (backend, conn);
+ assert (exportsize <= INT64_MAX); /* Guaranteed by negotiation phase */
return count > 0 && offset <= exportsize && offset +
count <= exportsize;
}
@@ -705,7 +706,7 @@ protocol_recv_request_send_reply (struct connection *conn)
/* Allocate the extents list for block status only. */
if (cmd == NBD_CMD_BLOCK_STATUS) {
- extents = nbdkit_extents_new (offset, conn->exportsize);
+ extents = nbdkit_extents_new (offset, backend_get_size (backend, conn));
if (extents == NULL) {
error = ENOMEM;
goto send_reply;
--
2.21.0
Eric Blake
2019-Aug-30 03:08 UTC
[Libguestfs] [nbdkit PATCH 6/9] server: Cache per-connection can_FOO flags
Similar to the previous patch in caching size, we want to avoid
calling into the plugin more than once per connection on any of the
flag determination callbacks.
The following script demonstrates the speedup, where we avoid repeated
calls into a slow can_fua. Pre-patch:
$ cat script
case "$1" in
get_size) echo 1m;;
can_fua) sleep 1; echo native;;
can_write | can_zero | pwrite | zero) ;;
*) exit 2 ;;
esac
$ /bin/time -f %e ./nbdkit --filter=blocksize sh script maxlen=4k \
--run 'qemu-io -f raw -c "w -z -f 0 16k" $nbd'
wrote 16384/16384 bytes at offset 0
16 KiB, 1 ops; 0:00:05.07 (3.157 KiB/sec and 0.1973 ops/sec)
6.14
there are six executions (one to determine eflags, one by the
blocksize filter to determine whether to pass the flag down or call
flush, and four by each .zero call over the split range from
blocksize). After:
$ /bin/time -f %e ./nbdkit --filter=blocksize sh script maxlen=4k \
--run 'qemu-io -f raw -c "w -z -f 0 16k" $nbd'
wrote 16384/16384 bytes at offset 0
16 KiB, 1 ops; 00.03 sec (585.570 KiB/sec and 36.5981 ops/sec)
1.13
we've reduced things to a single call, and everything else using the
cache.
Note that the cache of can_zero answers the filter semantics (whether
we advertise zero support in eflags) and not the plugin semantics
(whether we should attempt .zero or just automatically fall back to
.pwrite); that will be improved in the next patch.
In the cow filter, adding a call to next_ops->can_cache in .prepare
means that .cache no longer has to worry about can_cache failing (the
can_FOO methods do not guarantee easy access to a sane errno value).
Signed-off-by: Eric Blake <eblake@redhat.com>
---
docs/nbdkit-filter.pod | 9 +--
docs/nbdkit-plugin.pod | 6 +-
server/internal.h | 19 +++---
server/backend.c | 75 ++++++++++++++++++++---
server/connections.c | 3 +-
server/plugins.c | 7 ++-
server/protocol-handshake.c | 87 +++++++++++----------------
server/protocol.c | 110 +++++++++++++++++++++-------------
filters/blocksize/blocksize.c | 2 -
filters/cache/cache.c | 1 -
filters/cow/cow.c | 16 +++--
11 files changed, 204 insertions(+), 131 deletions(-)
diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod
index 1e2fe99c..3333d6b5 100644
--- a/docs/nbdkit-filter.pod
+++ b/docs/nbdkit-filter.pod
@@ -415,10 +415,11 @@ on to only the final sub-request, or by dropping the flag
and ending
with a direct call to C<next_ops-E<gt>flush>).
If there is an error, the callback should call C<nbdkit_error> with an
-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.
+error message and return C<-1>. These functions are only called once
+per connection and cached by nbdkit. Similarly, repeated calls to any
+of the C<next_ops> counterparts will return a cached value; by calling
+into the plugin during C<.prepare>, you can ensure that later use of
+the cached values during data commands like <.pwrite> will not fail.
=head2 C<.pread>
diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
index bc3d9749..17239a5c 100644
--- a/docs/nbdkit-plugin.pod
+++ b/docs/nbdkit-plugin.pod
@@ -150,10 +150,8 @@ A new client has connected.
These are called during option negotiation with the client, but before
any data is served. These callbacks may return different values
-across different C<.open> calls, but within a single connection, must
-always return the same value; other code in nbdkit may cache the
-per-connection value returned rather than using the callback a second
-time.
+across different C<.open> calls, but within a single connection, they
+are called at most once and cached by nbdkit for that connection.
=item C<.pread>, C<.pwrite> and other data serving callbacks
diff --git a/server/internal.h b/server/internal.h
index ec8a894c..ddb79623 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -152,6 +152,15 @@ struct b_conn_handle {
void *handle;
uint64_t exportsize;
+ int can_write;
+ int can_flush;
+ int is_rotational;
+ int can_trim;
+ int can_zero;
+ int can_fua;
+ int can_multi_conn;
+ int can_cache;
+ int can_extents;
};
struct connection {
@@ -169,16 +178,6 @@ struct connection {
uint32_t cflags;
uint16_t eflags;
- bool readonly;
- bool can_flush;
- bool is_rotational;
- bool can_trim;
- bool can_zero;
- bool can_fua;
- bool can_multi_conn;
- bool can_cache;
- bool emulate_cache;
- bool can_extents;
bool using_tls;
bool structured_replies;
bool meta_context_base_allocation;
diff --git a/server/backend.c b/server/backend.c
index 374d8540..196b48e4 100644
--- a/server/backend.c
+++ b/server/backend.c
@@ -144,73 +144,130 @@ backend_get_size (struct backend *b, struct connection
*conn)
int
backend_can_write (struct backend *b, struct connection *conn)
{
+ struct b_conn_handle *h = &conn->handles[b->i];
+
debug ("%s: can_write", b->name);
- return b->can_write (b, conn);
+ if (h->can_write == -1)
+ h->can_write = b->can_write (b, conn);
+ return h->can_write;
}
int
backend_can_flush (struct backend *b, struct connection *conn)
{
+ struct b_conn_handle *h = &conn->handles[b->i];
+
debug ("%s: can_flush", b->name);
- return b->can_flush (b, conn);
+ if (h->can_flush == -1)
+ h->can_flush = b->can_flush (b, conn);
+ return h->can_flush;
}
int
backend_is_rotational (struct backend *b, struct connection *conn)
{
+ struct b_conn_handle *h = &conn->handles[b->i];
+
debug ("%s: is_rotational", b->name);
- return b->is_rotational (b, conn);
+ if (h->is_rotational == -1)
+ h->is_rotational = b->is_rotational (b, conn);
+ return h->is_rotational;
}
int
backend_can_trim (struct backend *b, struct connection *conn)
{
+ struct b_conn_handle *h = &conn->handles[b->i];
+ int r;
+
debug ("%s: can_trim", b->name);
- return b->can_trim (b, conn);
+ if (h->can_trim == -1) {
+ r = backend_can_write (b, conn);
+ if (r != 1) {
+ h->can_trim = 0;
+ return r;
+ }
+ h->can_trim = b->can_trim (b, conn);
+ }
+ return h->can_trim;
}
int
backend_can_zero (struct backend *b, struct connection *conn)
{
+ struct b_conn_handle *h = &conn->handles[b->i];
+ int r;
+
debug ("%s: can_zero", b->name);
- return b->can_zero (b, conn);
+ if (h->can_zero == -1) {
+ r = backend_can_write (b, conn);
+ if (r != 1) {
+ h->can_zero = 0;
+ return r;
+ }
+ h->can_zero = b->can_zero (b, conn);
+ }
+ return h->can_zero;
}
int
backend_can_extents (struct backend *b, struct connection *conn)
{
+ struct b_conn_handle *h = &conn->handles[b->i];
+
debug ("%s: can_extents", b->name);
- return b->can_extents (b, conn);
+ if (h->can_extents == -1)
+ h->can_extents = b->can_extents (b, conn);
+ return h->can_extents;
}
int
backend_can_fua (struct backend *b, struct connection *conn)
{
+ struct b_conn_handle *h = &conn->handles[b->i];
+ int r;
+
debug ("%s: can_fua", b->name);
- return b->can_fua (b, conn);
+ if (h->can_fua == -1) {
+ r = backend_can_write (b, conn);
+ if (r != 1) {
+ h->can_fua = NBDKIT_FUA_NONE;
+ return r;
+ }
+ h->can_fua = b->can_fua (b, conn);
+ }
+ return h->can_fua;
}
int
backend_can_multi_conn (struct backend *b, struct connection *conn)
{
+ struct b_conn_handle *h = &conn->handles[b->i];
+
debug ("%s: can_multi_conn", b->name);
- return b->can_multi_conn (b, conn);
+ if (h->can_multi_conn == -1)
+ h->can_multi_conn = b->can_multi_conn (b, conn);
+ return h->can_multi_conn;
}
int
backend_can_cache (struct backend *b, struct connection *conn)
{
+ struct b_conn_handle *h = &conn->handles[b->i];
+
debug ("%s: can_cache", b->name);
- return b->can_cache (b, conn);
+ if (h->can_cache == -1)
+ h->can_cache = b->can_cache (b, conn);
+ return h->can_cache;
}
int
diff --git a/server/connections.c b/server/connections.c
index 2a5150cd..7609e9a7 100644
--- a/server/connections.c
+++ b/server/connections.c
@@ -274,8 +274,9 @@ new_connection (int sockin, int sockout, int nworkers)
return NULL;
}
conn->nr_handles = backend->i + 1;
+ memset (conn->handles, -1, conn->nr_handles * sizeof
*conn->handles);
for_each_backend (b)
- conn->handles[b->i].exportsize = -1;
+ conn->handles[b->i].handle = NULL;
conn->status = 1;
conn->nworkers = nworkers;
diff --git a/server/plugins.c b/server/plugins.c
index d1654f8d..c8f4af90 100644
--- a/server/plugins.c
+++ b/server/plugins.c
@@ -509,7 +509,7 @@ plugin_pwrite (struct backend *b, struct connection *conn,
assert (connection_get_handle (conn, 0));
- if (fua && plugin_can_fua (b, conn) != NBDKIT_FUA_NATIVE) {
+ if (fua && backend_can_fua (b, conn) != NBDKIT_FUA_NATIVE) {
flags &= ~NBDKIT_FLAG_FUA;
need_flush = true;
}
@@ -541,7 +541,7 @@ plugin_trim (struct backend *b, struct connection *conn,
assert (connection_get_handle (conn, 0));
- if (fua && plugin_can_fua (b, conn) != NBDKIT_FUA_NATIVE) {
+ if (fua && backend_can_fua (b, conn) != NBDKIT_FUA_NATIVE) {
flags &= ~NBDKIT_FLAG_FUA;
need_flush = true;
}
@@ -574,13 +574,14 @@ plugin_zero (struct backend *b, struct connection *conn,
assert (connection_get_handle (conn, 0));
- if (fua && plugin_can_fua (b, conn) != NBDKIT_FUA_NATIVE) {
+ if (fua && backend_can_fua (b, conn) != NBDKIT_FUA_NATIVE) {
flags &= ~NBDKIT_FLAG_FUA;
need_flush = true;
}
if (!count)
return 0;
if (p->plugin.can_zero) {
+ /* Calling backend_can_zero would answer the wrong question */
can_zero = p->plugin.can_zero (connection_get_handle (conn, 0));
assert (can_zero != -1);
}
diff --git a/server/protocol-handshake.c b/server/protocol-handshake.c
index 4d12b3dc..16261c34 100644
--- a/server/protocol-handshake.c
+++ b/server/protocol-handshake.c
@@ -51,87 +51,72 @@ protocol_compute_eflags (struct connection *conn, uint16_t
*flags)
{
uint16_t eflags = NBD_FLAG_HAS_FLAGS;
int fl;
+ bool can_write = true;
fl = backend_can_write (backend, conn);
if (fl == -1)
return -1;
if (readonly || !fl) {
eflags |= NBD_FLAG_READ_ONLY;
- conn->readonly = true;
+ can_write = false;
}
- if (!conn->readonly) {
- fl = backend_can_zero (backend, conn);
- if (fl == -1)
- return -1;
- if (fl) {
- eflags |= NBD_FLAG_SEND_WRITE_ZEROES;
- conn->can_zero = true;
- }
- fl = backend_can_trim (backend, conn);
- if (fl == -1)
- return -1;
- if (fl) {
- eflags |= NBD_FLAG_SEND_TRIM;
- conn->can_trim = true;
- }
+ /* Check all flags even if they won't be advertised, to prime the
+ * cache and thus simplify later EINVAL handling of a client that
+ * makes a non-compliant request that did not match eflags.
+ */
+ fl = backend_can_zero (backend, conn);
+ if (fl == -1)
+ return -1;
+ if (fl && can_write)
+ eflags |= NBD_FLAG_SEND_WRITE_ZEROES;
- fl = backend_can_fua (backend, conn);
- if (fl == -1)
- return -1;
- if (fl) {
- eflags |= NBD_FLAG_SEND_FUA;
- conn->can_fua = true;
- }
- }
+ fl = backend_can_trim (backend, conn);
+ if (fl == -1)
+ return -1;
+ if (fl && can_write)
+ eflags |= NBD_FLAG_SEND_TRIM;
+
+ fl = backend_can_fua (backend, conn);
+ if (fl == -1)
+ return -1;
+ if (fl && can_write)
+ eflags |= NBD_FLAG_SEND_FUA;
fl = backend_can_flush (backend, conn);
if (fl == -1)
return -1;
- if (fl) {
+ if (fl)
eflags |= NBD_FLAG_SEND_FLUSH;
- conn->can_flush = true;
- }
fl = backend_is_rotational (backend, conn);
if (fl == -1)
return -1;
- if (fl) {
+ if (fl)
eflags |= NBD_FLAG_ROTATIONAL;
- conn->is_rotational = true;
- }
- /* multi-conn is useless if parallel connections are not allowed */
- if (backend->thread_model (backend) >
- NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) {
- fl = backend_can_multi_conn (backend, conn);
- if (fl == -1)
- return -1;
- if (fl) {
- eflags |= NBD_FLAG_CAN_MULTI_CONN;
- conn->can_multi_conn = true;
- }
- }
+ /* multi-conn is useless if parallel connections are not allowed. */
+ fl = backend_can_multi_conn (backend, conn);
+ if (fl == -1)
+ return -1;
+ if (fl && (backend->thread_model (backend) >
+ NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS))
+ eflags |= NBD_FLAG_CAN_MULTI_CONN;
fl = backend_can_cache (backend, conn);
if (fl == -1)
return -1;
- if (fl) {
+ if (fl)
eflags |= NBD_FLAG_SEND_CACHE;
- conn->can_cache = true;
- conn->emulate_cache = fl == NBDKIT_CACHE_EMULATE;
- }
- /* The result of this is not returned to callers here (or at any
- * time during the handshake). However it makes sense to do it once
- * per connection and store the result in the handle anyway. This
- * protocol_compute_eflags function is a bit misnamed XXX.
+ /* The result of this is not directly advertised as part of the
+ * handshake, but priming the cache here makes BLOCK_STATUS handling
+ * not have to worry about errors, and makes test-layers easier to
+ * write.
*/
fl = backend_can_extents (backend, conn);
if (fl == -1)
return -1;
- if (fl)
- conn->can_extents = true;
if (conn->structured_replies)
eflags |= NBD_FLAG_SEND_DF;
diff --git a/server/protocol.c b/server/protocol.c
index 06f1ee15..0ecf0b5c 100644
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -64,14 +64,19 @@ validate_request (struct connection *conn,
uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t
count,
uint32_t *error)
{
+ int r;
+
/* Readonly connection? */
- if (conn->readonly &&
- (cmd == NBD_CMD_WRITE || cmd == NBD_CMD_TRIM ||
- cmd == NBD_CMD_WRITE_ZEROES)) {
- nbdkit_error ("invalid request: %s: write request on readonly
connection",
- name_of_nbd_cmd (cmd));
- *error = EROFS;
- return false;
+ if (cmd == NBD_CMD_WRITE || cmd == NBD_CMD_TRIM ||
+ cmd == NBD_CMD_WRITE_ZEROES) {
+ r = backend_can_write (backend, conn);
+ assert (r >= 0); /* Guaranteed by eflags computation */
+ if (!r) {
+ nbdkit_error ("invalid request: %s: write request on readonly
connection",
+ name_of_nbd_cmd (cmd));
+ *error = EROFS;
+ return false;
+ }
}
/* Validate cmd, offset, count. */
@@ -142,10 +147,14 @@ validate_request (struct connection *conn,
*error = EINVAL;
return false;
}
- if (!conn->can_fua && (flags & NBD_CMD_FLAG_FUA)) {
- nbdkit_error ("invalid request: FUA flag not supported");
- *error = EINVAL;
- return false;
+ if (flags & NBD_CMD_FLAG_FUA) {
+ r = backend_can_fua (backend, conn);
+ assert (r >= 0); /* Guaranteed by eflags computation */
+ if (!r) {
+ nbdkit_error ("invalid request: FUA flag not supported");
+ *error = EINVAL;
+ return false;
+ }
}
/* Refuse over-large read and write requests. */
@@ -159,35 +168,51 @@ validate_request (struct connection *conn,
}
/* Flush allowed? */
- if (!conn->can_flush && cmd == NBD_CMD_FLUSH) {
- nbdkit_error ("invalid request: %s: flush operation not
supported",
- name_of_nbd_cmd (cmd));
- *error = EINVAL;
- return false;
+ if (cmd == NBD_CMD_FLUSH) {
+ r = backend_can_flush (backend, conn);
+ assert (r >= 0); /* Guaranteed by eflags computation */
+ if (!r) {
+ nbdkit_error ("invalid request: %s: flush operation not
supported",
+ name_of_nbd_cmd (cmd));
+ *error = EINVAL;
+ return false;
+ }
}
/* Trim allowed? */
- if (!conn->can_trim && cmd == NBD_CMD_TRIM) {
- nbdkit_error ("invalid request: %s: trim operation not
supported",
- name_of_nbd_cmd (cmd));
- *error = EINVAL;
- return false;
+ if (cmd == NBD_CMD_TRIM) {
+ r = backend_can_trim (backend, conn);
+ assert (r >= 0); /* Guaranteed by eflags computation */
+ if (!r) {
+ nbdkit_error ("invalid request: %s: trim operation not
supported",
+ name_of_nbd_cmd (cmd));
+ *error = EINVAL;
+ return false;
+ }
}
/* Zero allowed? */
- if (!conn->can_zero && cmd == NBD_CMD_WRITE_ZEROES) {
- nbdkit_error ("invalid request: %s: write zeroes operation not
supported",
- name_of_nbd_cmd (cmd));
- *error = EINVAL;
- return false;
+ if (cmd == NBD_CMD_WRITE_ZEROES) {
+ r = backend_can_zero (backend, conn);
+ assert (r >= 0); /* Guaranteed by eflags computation */
+ if (!r) {
+ nbdkit_error ("invalid request: %s: write zeroes operation not
supported",
+ name_of_nbd_cmd (cmd));
+ *error = EINVAL;
+ return false;
+ }
}
/* Cache allowed? */
- if (!conn->can_cache && cmd == NBD_CMD_CACHE) {
- nbdkit_error ("invalid request: %s: cache operation not
supported",
- name_of_nbd_cmd (cmd));
- *error = EINVAL;
- return false;
+ if (cmd == NBD_CMD_CACHE) {
+ r = backend_can_cache (backend, conn);
+ assert (r >= 0); /* Guaranteed by eflags computation */
+ if (!r) {
+ nbdkit_error ("invalid request: %s: cache operation not
supported",
+ name_of_nbd_cmd (cmd));
+ *error = EINVAL;
+ return false;
+ }
}
/* Block status allowed? */
@@ -232,8 +257,8 @@ handle_request (struct connection *conn,
void *buf, struct nbdkit_extents *extents)
{
uint32_t f = 0;
- bool fua = conn->can_fua && (flags & NBD_CMD_FLAG_FUA);
int err = 0;
+ int r;
/* Clear the error, so that we know if the plugin calls
* nbdkit_set_error() or relied on errno. */
@@ -246,7 +271,7 @@ handle_request (struct connection *conn,
break;
case NBD_CMD_WRITE:
- if (fua)
+ if (flags & NBD_CMD_FLAG_FUA)
f |= NBDKIT_FLAG_FUA;
if (backend_pwrite (backend, conn, buf, count, offset, f, &err) == -1)
return err;
@@ -258,14 +283,16 @@ handle_request (struct connection *conn,
break;
case NBD_CMD_TRIM:
- if (fua)
+ if (flags & NBD_CMD_FLAG_FUA)
f |= NBDKIT_FLAG_FUA;
if (backend_trim (backend, conn, count, offset, f, &err) == -1)
return err;
break;
case NBD_CMD_CACHE:
- if (conn->emulate_cache) {
+ r = backend_can_cache (backend, conn);
+ assert (r > 0); /* Guaranteed by validate_request */
+ if (r == NBDKIT_CACHE_EMULATE) {
static char buf[MAX_REQUEST_SIZE]; /* data sink, never read */
uint32_t limit;
@@ -284,7 +311,7 @@ handle_request (struct connection *conn,
case NBD_CMD_WRITE_ZEROES:
if (!(flags & NBD_CMD_FLAG_NO_HOLE))
f |= NBDKIT_FLAG_MAY_TRIM;
- if (fua)
+ if (flags & NBD_CMD_FLAG_FUA)
f |= NBDKIT_FLAG_FUA;
if (backend_zero (backend, conn, count, offset, f, &err) == -1)
return err;
@@ -293,10 +320,13 @@ handle_request (struct connection *conn,
case NBD_CMD_BLOCK_STATUS:
/* The other backend methods don't check can_*. That is because
* those methods are implicitly suppressed by returning eflags to
- * the client. However there is no eflag for extents so we must
- * check it here.
+ * the client (see validate_request), but there is no eflag for
+ * extents. We did prime the cache earlier, but must check here
+ * in order to perform a fallback when needed.
*/
- if (conn->can_extents) {
+ r = backend_can_extents (backend, conn);
+ assert (r >= 0); /* Guaranteed during eflags computation */
+ if (r) {
if (flags & NBD_CMD_FLAG_REQ_ONE)
f |= NBDKIT_FLAG_REQ_ONE;
if (backend_extents (backend, conn, count, offset, f,
@@ -304,8 +334,6 @@ handle_request (struct connection *conn,
return err;
}
else {
- int r;
-
/* By default it is safe assume that everything in the range is
* allocated.
*/
diff --git a/filters/blocksize/blocksize.c b/filters/blocksize/blocksize.c
index 0978887f..0fa05301 100644
--- a/filters/blocksize/blocksize.c
+++ b/filters/blocksize/blocksize.c
@@ -138,8 +138,6 @@ blocksize_config_complete (nbdkit_next_config_complete
*next, void *nxdata)
"maxdata=<SIZE> Maximum size for read/write (default
64M).\n" \
"maxlen=<SIZE> Maximum size for trim/zero (default
4G-minblock)."
-/* TODO: Should we have a .prepare to cache per-connection FUA mode? */
-
/* Round size down to avoid issues at end of file. */
static int64_t
blocksize_get_size (struct nbdkit_next_ops *next_ops, void *nxdata,
diff --git a/filters/cache/cache.c b/filters/cache/cache.c
index b5dbccd2..e5f18d9b 100644
--- a/filters/cache/cache.c
+++ b/filters/cache/cache.c
@@ -239,7 +239,6 @@ cache_prepare (struct nbdkit_next_ops *next_ops, void
*nxdata,
r = cache_get_size (next_ops, nxdata, handle);
if (r < 0)
return -1;
- /* TODO: cache per-connection FUA mode? */
return 0;
}
diff --git a/filters/cow/cow.c b/filters/cow/cow.c
index 9d91d432..e4330bf3 100644
--- a/filters/cow/cow.c
+++ b/filters/cow/cow.c
@@ -127,7 +127,7 @@ cow_get_size (struct nbdkit_next_ops *next_ops, void
*nxdata,
return size;
}
-/* Force an early call to cow_get_size, consequently truncating the
+/* Force early calls to populate nbdkit's cache, and truncate the
* overlay to the correct size.
*/
static int
@@ -137,7 +137,14 @@ cow_prepare (struct nbdkit_next_ops *next_ops, void
*nxdata,
int64_t r;
r = cow_get_size (next_ops, nxdata, handle);
- return r >= 0 ? 0 : -1;
+ if (r == -1)
+ return -1;
+
+ r = next_ops->can_cache (nxdata);
+ if (r == -1)
+ return -1;
+
+ return 0;
}
/* Whatever the underlying plugin can or can't do, we can write, we
@@ -427,7 +434,7 @@ cow_cache (struct nbdkit_next_ops *next_ops, void *nxdata,
uint64_t blknum, blkoffs;
int r;
uint64_t remaining = count; /* Rounding out could exceed 32 bits */
- enum cache_mode mode; /* XXX Cache this per connection? */
+ enum cache_mode mode;
switch (next_ops->can_cache (nxdata)) {
case NBDKIT_CACHE_NONE:
@@ -440,8 +447,7 @@ cow_cache (struct nbdkit_next_ops *next_ops, void *nxdata,
mode = BLK_CACHE_PASSTHROUGH;
break;
default:
- *err = EINVAL;
- return -1;
+ abort (); /* .prepare populated the cache */
}
if (cow_on_cache)
mode = BLK_CACHE_COW;
--
2.21.0
Eric Blake
2019-Aug-30 03:08 UTC
[Libguestfs] [nbdkit PATCH 7/9] filters: Change semantics of .can_zero
The tri-state return pattern has proved valuable in other cases where
a backend wants to opt in or out of nbdkit fallbacks. Using a
tri-state return pattern at the backend level unifies the different
semantics between plugin and filter .can_zero return values, and makes
it possible for plugins to use cached results of .can_zero rather than
calling into the plugin on each .zero request. As in other recent
patches, it is easy to write an sh script that demonstrates a
resulting speedup from the caching. All filters are in-tree and do
not have a promise of API compatability, so it's easy to update all of
them (the log filter is okay as-is, the nozero filter is easy to
update, and no other filter overrides .can_zero). But for backwards
compatibility reasons, we cannot change the semantics of the plugin
.can_zero return value while remaining at API version 2; so that
remains a bool, but now selects between EMULATE or NATIVE at the
backend level.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
docs/nbdkit-filter.pod | 44 ++++++++++++++++++++++++++++++-----------
server/backend.c | 2 +-
server/plugins.c | 33 ++++++++++++++++---------------
include/nbdkit-filter.h | 4 ++++
filters/nozero/nozero.c | 9 ++++++++-
5 files changed, 63 insertions(+), 29 deletions(-)
diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod
index 3333d6b5..c83fcbfe 100644
--- a/docs/nbdkit-filter.pod
+++ b/docs/nbdkit-filter.pod
@@ -393,16 +393,37 @@ fail.
These intercept the corresponding plugin methods, and control feature
bits advertised to the client.
-Of note, the C<.can_zero> callback in the filter controls whether the
-server advertises zero support to the client, which is slightly
-different semantics than the plugin; that is,
-C<next_ops-E<gt>can_zero> always returns true for a plugin, even
when
-the plugin's own C<.can_zero> callback returned false, because nbdkit
-implements a fallback to C<.pwrite> at the plugin layer.
+Of note, the semantics of C<.can_zero> callback in the filter are
+slightly different from the plugin, and must be one of three success
+values visible only to filters:
+
+=over 4
+
+=item C<NBDKIT_ZERO_NONE>
+
+Completely suppress advertisement of write zero support (this can only
+be done from filters, not plugins).
+
+=item C<NBDKIT_ZERO_EMULATE>
+
+Inform nbdkit that write zeroes should immediately fall back to
+C<.pwrite> emulation without trying C<.zero> (this value is
returned
+by C<next_ops-E<gt>can_zero> if the plugin returned false in its
+C<.can_zero>).
+
+=item C<NBDKIT_ZERO_NATIVE>
+
+Inform nbdkit that write zeroes should attempt to use C<.zero>,
+although it may still fall back to C<.pwrite> emulation for
C<ENOTSUP>
+or C<EOPNOTSUPP> failures (this value is returned by
+C<next_ops-E<gt>can_zero> if the plugin returned true in its
+C<.can_zero>).
+
+=back
Remember that most of the feature check functions return merely a
-boolean success value, while C<.can_fua> and C<.can_cache> have
three
-success values.
+boolean success value, while C<.can_zero>, C<.can_fua> and
+C<.can_cache> have three success values.
The difference between C<.can_fua> values may affect choices made in
the filter: when splitting a write request that requested FUA from the
@@ -514,9 +535,10 @@ value to return to the client.
This intercepts the plugin C<.zero> method and can be used to modify
zero requests.
-This function will not be called if C<.can_zero> returned false; in
-turn, the filter should not call C<next_ops-E<gt>zero> if
-C<next_ops-E<gt>can_zero> did not return true.
+This function will not be called if C<.can_zero> returned
+C<NBDKIT_ZERO_NONE>; in turn, the filter should not call
+C<next_ops-E<gt>zero> if C<next_ops-E<gt>can_zero>
returned
+C<NBDKIT_ZERO_NONE>.
On input, the parameter C<flags> may include
C<NBDKIT_FLAG_MAY_TRIM>
unconditionally, and C<NBDKIT_FLAG_FUA> based on the result of
diff --git a/server/backend.c b/server/backend.c
index 196b48e4..1e3a0109 100644
--- a/server/backend.c
+++ b/server/backend.c
@@ -207,7 +207,7 @@ backend_can_zero (struct backend *b, struct connection
*conn)
if (h->can_zero == -1) {
r = backend_can_write (b, conn);
if (r != 1) {
- h->can_zero = 0;
+ h->can_zero = NBDKIT_ZERO_NONE;
return r;
}
h->can_zero = b->can_zero (b, conn);
diff --git a/server/plugins.c b/server/plugins.c
index c8f4af90..bff4cd47 100644
--- a/server/plugins.c
+++ b/server/plugins.c
@@ -354,19 +354,26 @@ static int
plugin_can_zero (struct backend *b, struct connection *conn)
{
struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
+ int r;
assert (connection_get_handle (conn, 0));
- /* Note the special case here: the plugin's .can_zero controls only
- * whether we call .zero; while the backend expects .can_zero to
- * return whether to advertise zero support. Since we ALWAYS know
- * how to fall back to .pwrite in plugin_zero(), we ignore the
- * difference between the plugin's true or false return, and only
- * call it to catch a -1 failure during negotiation. */
- if (p->plugin.can_zero &&
- p->plugin.can_zero (connection_get_handle (conn, 0)) == -1)
+ /* Note the special case here: the plugin's .can_zero returns a bool
+ * which controls only whether we call .zero; while the backend
+ * expects .can_zero to return a tri-state on level of support.
+ */
+ if (p->plugin.can_zero) {
+ r = p->plugin.can_zero (connection_get_handle (conn, 0));
+ if (r == -1)
+ return -1;
+ return r ? NBDKIT_ZERO_NATIVE : NBDKIT_ZERO_EMULATE;
+ }
+ if (p->plugin.zero || p->plugin._zero_old)
+ return NBDKIT_ZERO_NATIVE;
+ r = backend_can_write (b, conn);
+ if (r == -1)
return -1;
- return plugin_can_write (b, conn);
+ return r ? NBDKIT_ZERO_EMULATE : NBDKIT_ZERO_NONE;
}
static int
@@ -570,7 +577,6 @@ plugin_zero (struct backend *b, struct connection *conn,
bool fua = flags & NBDKIT_FLAG_FUA;
bool emulate = false;
bool need_flush = false;
- int can_zero = 1; /* TODO cache this per-connection? */
assert (connection_get_handle (conn, 0));
@@ -580,13 +586,8 @@ plugin_zero (struct backend *b, struct connection *conn,
}
if (!count)
return 0;
- if (p->plugin.can_zero) {
- /* Calling backend_can_zero would answer the wrong question */
- can_zero = p->plugin.can_zero (connection_get_handle (conn, 0));
- assert (can_zero != -1);
- }
- if (can_zero) {
+ if (backend_can_zero (b, conn) == NBDKIT_ZERO_NATIVE) {
errno = 0;
if (p->plugin.zero)
r = p->plugin.zero (connection_get_handle (conn, 0), count, offset,
diff --git a/include/nbdkit-filter.h b/include/nbdkit-filter.h
index 29d92755..6232e0e2 100644
--- a/include/nbdkit-filter.h
+++ b/include/nbdkit-filter.h
@@ -43,6 +43,10 @@
extern "C" {
#endif
+#define NBDKIT_ZERO_NONE 0
+#define NBDKIT_ZERO_EMULATE 1
+#define NBDKIT_ZERO_NATIVE 2
+
struct nbdkit_extent {
uint64_t offset;
uint64_t length;
diff --git a/filters/nozero/nozero.c b/filters/nozero/nozero.c
index 964cce9f..d916657e 100644
--- a/filters/nozero/nozero.c
+++ b/filters/nozero/nozero.c
@@ -94,7 +94,14 @@ nozero_prepare (struct nbdkit_next_ops *next_ops, void
*nxdata, void *handle)
static int
nozero_can_zero (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle)
{
- return zeromode != NONE;
+ switch (zeromode) {
+ case NONE:
+ return NBDKIT_ZERO_NONE;
+ case EMULATE:
+ return NBDKIT_ZERO_EMULATE;
+ default:
+ return next_ops->can_zero (nxdata);
+ }
}
static int
--
2.21.0
Eric Blake
2019-Aug-30 03:08 UTC
[Libguestfs] [nbdkit PATCH 8/9] server: Move fallbacks from protocol.c to backend.c
Our cache and extents fallbacks were hard-coded at the server layer,
which only benefits the filter furthest from the plugin. The
fallbacks also make sense if any intermediate filter wants to call in
to the plugin, regardless of how outer filters may further change what
the client sees.
Note that it is now possible to hit an assertion failure if a filter
overrides a given .can_FOO, and calls next_ops->FOO prior to
next_ops->can_FOO; however, all filters are in-tree, and an audit did
not reveal any in this situation (the cow filter came closest, by
overriding .can_cache, but it calls next_ops->can_cache prior to
next_ops->cache).
Signed-off-by: Eric Blake <eblake@redhat.com>
---
server/internal.h | 3 +++
server/backend.c | 32 ++++++++++++++++++++++++
server/protocol.c | 62 +++++------------------------------------------
3 files changed, 41 insertions(+), 56 deletions(-)
diff --git a/server/internal.h b/server/internal.h
index ddb79623..6d0709c8 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -65,6 +65,9 @@
(type *) ((char *) __mptr - offsetof(type, member)); \
})
+/* Maximum read or write request that we will handle. */
+#define MAX_REQUEST_SIZE (64 * 1024 * 1024)
+
/* main.c */
struct debug_flag {
struct debug_flag *next;
diff --git a/server/backend.c b/server/backend.c
index 1e3a0109..e785a824 100644
--- a/server/backend.c
+++ b/server/backend.c
@@ -40,6 +40,8 @@
#include <string.h>
#include "internal.h"
+#include "minmax.h"
+#include "protocol.h"
/* Helpers for registering a new backend. */
@@ -359,12 +361,23 @@ backend_extents (struct backend *b, struct connection
*conn,
uint32_t count, uint64_t offset, uint32_t flags,
struct nbdkit_extents *extents, int *err)
{
+ struct b_conn_handle *h = &conn->handles[b->i];
int r;
assert (!(flags & ~NBDKIT_FLAG_REQ_ONE));
debug ("%s: extents count=%" PRIu32 " offset=%" PRIu64
" req_one=%d",
b->name, count, offset, !!(flags & NBDKIT_FLAG_REQ_ONE));
+ assert (h->can_extents >= 0);
+ if (h->can_extents == 0) {
+ /* By default it is safe assume that everything in the range is
+ * allocated.
+ */
+ r = nbdkit_add_extent (extents, offset, count, 0 /* allocated data */);
+ if (r == -1)
+ *err = errno;
+ return r;
+ }
r = b->extents (b, conn, count, offset, flags, extents, err);
if (r == -1)
assert (*err);
@@ -376,12 +389,31 @@ backend_cache (struct backend *b, struct connection *conn,
uint32_t count, uint64_t offset,
uint32_t flags, int *err)
{
+ struct b_conn_handle *h = &conn->handles[b->i];
int r;
assert (flags == 0);
debug ("%s: cache count=%" PRIu32 " offset=%" PRIu64,
b->name, count, offset);
+ assert (h->can_cache >= 0);
+ if (h->can_cache == NBDKIT_CACHE_NONE) {
+ nbdkit_error ("invalid request: cache operation not supported");
+ *err = EINVAL;
+ return -1;
+ }
+ if (h->can_cache == NBDKIT_CACHE_EMULATE) {
+ static char buf[MAX_REQUEST_SIZE]; /* data sink, never read */
+ uint32_t limit;
+
+ while (count) {
+ limit = MIN (count, sizeof buf);
+ if (backend_pread (b, conn, buf, limit, offset, flags, err) == -1)
+ return -1;
+ count -= limit;
+ }
+ return 0;
+ }
r = b->cache (b, conn, count, offset, flags, err);
if (r == -1)
assert (*err);
diff --git a/server/protocol.c b/server/protocol.c
index 0ecf0b5c..03862f5f 100644
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -47,9 +47,6 @@
#include "minmax.h"
#include "protocol.h"
-/* Maximum read or write request that we will handle. */
-#define MAX_REQUEST_SIZE (64 * 1024 * 1024)
-
static bool
valid_range (struct connection *conn, uint64_t offset, uint32_t count)
{
@@ -203,18 +200,6 @@ validate_request (struct connection *conn,
}
}
- /* Cache allowed? */
- if (cmd == NBD_CMD_CACHE) {
- r = backend_can_cache (backend, conn);
- assert (r >= 0); /* Guaranteed by eflags computation */
- if (!r) {
- nbdkit_error ("invalid request: %s: cache operation not
supported",
- name_of_nbd_cmd (cmd));
- *error = EINVAL;
- return false;
- }
- }
-
/* Block status allowed? */
if (cmd == NBD_CMD_BLOCK_STATUS) {
if (!conn->structured_replies) {
@@ -258,7 +243,6 @@ handle_request (struct connection *conn,
{
uint32_t f = 0;
int err = 0;
- int r;
/* Clear the error, so that we know if the plugin calls
* nbdkit_set_error() or relied on errno. */
@@ -290,21 +274,7 @@ handle_request (struct connection *conn,
break;
case NBD_CMD_CACHE:
- r = backend_can_cache (backend, conn);
- assert (r > 0); /* Guaranteed by validate_request */
- if (r == NBDKIT_CACHE_EMULATE) {
- static char buf[MAX_REQUEST_SIZE]; /* data sink, never read */
- uint32_t limit;
-
- while (count) {
- limit = MIN (count, sizeof buf);
- if (backend_pread (backend, conn, buf, limit, offset, flags,
- &err) == -1)
- return err;
- count -= limit;
- }
- }
- else if (backend_cache (backend, conn, count, offset, 0, &err) == -1)
+ if (backend_cache (backend, conn, count, offset, 0, &err) == -1)
return err;
break;
@@ -318,31 +288,11 @@ handle_request (struct connection *conn,
break;
case NBD_CMD_BLOCK_STATUS:
- /* The other backend methods don't check can_*. That is because
- * those methods are implicitly suppressed by returning eflags to
- * the client (see validate_request), but there is no eflag for
- * extents. We did prime the cache earlier, but must check here
- * in order to perform a fallback when needed.
- */
- r = backend_can_extents (backend, conn);
- assert (r >= 0); /* Guaranteed during eflags computation */
- if (r) {
- if (flags & NBD_CMD_FLAG_REQ_ONE)
- f |= NBDKIT_FLAG_REQ_ONE;
- if (backend_extents (backend, conn, count, offset, f,
- extents, &err) == -1)
- return err;
- }
- else {
- /* By default it is safe assume that everything in the range is
- * allocated.
- */
- errno = 0;
- r = nbdkit_add_extent (extents, offset, count, 0 /* allocated data */);
- if (r == -1)
- return errno ? errno : EINVAL;
- return 0;
- }
+ if (flags & NBD_CMD_FLAG_REQ_ONE)
+ f |= NBDKIT_FLAG_REQ_ONE;
+ if (backend_extents (backend, conn, count, offset, f,
+ extents, &err) == -1)
+ return err;
break;
default:
--
2.21.0
Eric Blake
2019-Aug-30 03:08 UTC
[Libguestfs] [nbdkit PATCH 9/9] server: Move command validation from protocol.c to backend.c
Now instead of validating just the client's request, we are validating
that all filters are passing valid requests on down.
In protocol.c, we were able to assert that our computation of eflags
populated all of the flags, and thus calls such as backend_can_write
would not fail; however, with filters, keeping those assertions mean
the burden is now on the filter to avoid calling into next_ops->FOO
without first priming the cache of next_ops->can_FOO. An audit of
existing filters did not find any more culprits.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
server/backend.c | 94 ++++++++++++++++++++++++++++++++
server/protocol.c | 134 ++++++++++------------------------------------
2 files changed, 121 insertions(+), 107 deletions(-)
diff --git a/server/backend.c b/server/backend.c
index e785a824..cd678f96 100644
--- a/server/backend.c
+++ b/server/backend.c
@@ -124,6 +124,43 @@ backend_unload (struct backend *b, void (*unload) (void))
free (b->name);
}
+static bool
+writes_blocked (struct backend *b, struct connection *conn, const char *cmd,
+ uint32_t flags)
+{
+ struct b_conn_handle *h = &conn->handles[b->i];
+
+ assert (h->can_write >= 0);
+ if (h->can_write == 0) {
+ nbdkit_error ("invalid request: %s: write request on readonly
connection",
+ cmd);
+ return true;
+ }
+ if (flags & NBDKIT_FLAG_FUA) {
+ assert (h->can_fua >= 0);
+ if (h->can_fua == 0) {
+ nbdkit_error ("invalid request: %s: FUA flag not supported",
cmd);
+ return true;
+ }
+ }
+ return false;
+}
+
+static bool
+invalid_range (struct backend *b, struct connection *conn, const char *cmd,
+ uint64_t offset, uint32_t count)
+{
+ struct b_conn_handle *h = &conn->handles[b->i];
+
+ assert (h->exportsize >= 0);
+ if (!count || offset > h->exportsize || offset + count >
h->exportsize) {
+ nbdkit_error ("invalid request: %s: offset and count are out of range:
"
+ "offset=%" PRIu64 " count=%" PRIu32, cmd,
offset, count);
+ return true;
+ }
+ return false;
+}
+
void
backend_set_handle (struct backend *b, struct connection *conn, void *handle)
{
@@ -283,6 +320,10 @@ backend_pread (struct backend *b, struct connection *conn,
debug ("%s: pread count=%" PRIu32 " offset=%" PRIu64,
b->name, count, offset);
+ if (invalid_range (b, conn, "pread", offset, count)) {
+ *err = EINVAL;
+ return -1;
+ }
r = b->pread (b, conn, buf, count, offset, flags, err);
if (r == -1)
assert (*err);
@@ -300,6 +341,14 @@ backend_pwrite (struct backend *b, struct connection *conn,
debug ("%s: pwrite count=%" PRIu32 " offset=%" PRIu64
" fua=%d",
b->name, count, offset, !!(flags & NBDKIT_FLAG_FUA));
+ if (writes_blocked (b, conn, "pwrite", flags)) {
+ *err = EROFS;
+ return -1;
+ }
+ if (invalid_range (b, conn, "pwrite", offset, count)) {
+ *err = ENOSPC;
+ return -1;
+ }
r = b->pwrite (b, conn, buf, count, offset, flags, err);
if (r == -1)
assert (*err);
@@ -310,11 +359,18 @@ int
backend_flush (struct backend *b, struct connection *conn,
uint32_t flags, int *err)
{
+ struct b_conn_handle *h = &conn->handles[b->i];
int r;
assert (flags == 0);
debug ("%s: flush", b->name);
+ assert (h->can_flush >= 0);
+ if (h->can_flush == 0) {
+ nbdkit_error ("invalid request: flush operation not supported");
+ *err = EINVAL;
+ return -1;
+ }
r = b->flush (b, conn, flags, err);
if (r == -1)
assert (*err);
@@ -326,12 +382,27 @@ backend_trim (struct backend *b, struct connection *conn,
uint32_t count, uint64_t offset, uint32_t flags,
int *err)
{
+ struct b_conn_handle *h = &conn->handles[b->i];
int r;
assert (flags == 0);
debug ("%s: trim count=%" PRIu32 " offset=%" PRIu64
" fua=%d",
b->name, count, offset, !!(flags & NBDKIT_FLAG_FUA));
+ if (writes_blocked (b, conn, "trim", flags)) {
+ *err = EROFS;
+ return -1;
+ }
+ if (invalid_range (b, conn, "trim", offset, count)) {
+ *err = EINVAL;
+ return -1;
+ }
+ assert (h->can_trim >= 0);
+ if (h->can_trim == 0) {
+ nbdkit_error ("invalid request: trim operation not supported");
+ *err = EINVAL;
+ return -1;
+ }
r = b->trim (b, conn, count, offset, flags, err);
if (r == -1)
assert (*err);
@@ -343,6 +414,7 @@ backend_zero (struct backend *b, struct connection *conn,
uint32_t count, uint64_t offset, uint32_t flags,
int *err)
{
+ struct b_conn_handle *h = &conn->handles[b->i];
int r;
assert (!(flags & ~(NBDKIT_FLAG_MAY_TRIM | NBDKIT_FLAG_FUA)));
@@ -350,6 +422,20 @@ backend_zero (struct backend *b, struct connection *conn,
b->name, count, offset, !!(flags & NBDKIT_FLAG_MAY_TRIM),
!!(flags & NBDKIT_FLAG_FUA));
+ if (writes_blocked (b, conn, "zero", flags)) {
+ *err = EROFS;
+ return -1;
+ }
+ if (invalid_range (b, conn, "zero", offset, count)) {
+ *err = ENOSPC;
+ return -1;
+ }
+ assert (h->can_zero >= 0);
+ if (h->can_zero == NBDKIT_ZERO_NONE) {
+ nbdkit_error ("invalid request: write zeroes operation not
supported");
+ *err = EINVAL;
+ return -1;
+ }
r = b->zero (b, conn, count, offset, flags, err);
if (r == -1)
assert (*err && *err != ENOTSUP && *err != EOPNOTSUPP);
@@ -368,6 +454,10 @@ backend_extents (struct backend *b, struct connection
*conn,
debug ("%s: extents count=%" PRIu32 " offset=%" PRIu64
" req_one=%d",
b->name, count, offset, !!(flags & NBDKIT_FLAG_REQ_ONE));
+ if (invalid_range (b, conn, "extents", offset, count)) {
+ *err = EINVAL;
+ return -1;
+ }
assert (h->can_extents >= 0);
if (h->can_extents == 0) {
/* By default it is safe assume that everything in the range is
@@ -396,6 +486,10 @@ backend_cache (struct backend *b, struct connection *conn,
debug ("%s: cache count=%" PRIu32 " offset=%" PRIu64,
b->name, count, offset);
+ if (invalid_range (b, conn, "cache", offset, count)) {
+ *err = EINVAL;
+ return -1;
+ }
assert (h->can_cache >= 0);
if (h->can_cache == NBDKIT_CACHE_NONE) {
nbdkit_error ("invalid request: cache operation not supported");
diff --git a/server/protocol.c b/server/protocol.c
index 03862f5f..db40cca3 100644
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -47,50 +47,43 @@
#include "minmax.h"
#include "protocol.h"
-static bool
-valid_range (struct connection *conn, uint64_t offset, uint32_t count)
-{
- uint64_t exportsize = backend_get_size (backend, conn);
-
- assert (exportsize <= INT64_MAX); /* Guaranteed by negotiation phase */
- return count > 0 && offset <= exportsize && offset +
count <= exportsize;
-}
-
static bool
validate_request (struct connection *conn,
uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t
count,
uint32_t *error)
{
- int r;
-
- /* Readonly connection? */
- if (cmd == NBD_CMD_WRITE || cmd == NBD_CMD_TRIM ||
- cmd == NBD_CMD_WRITE_ZEROES) {
- r = backend_can_write (backend, conn);
- assert (r >= 0); /* Guaranteed by eflags computation */
- if (!r) {
- nbdkit_error ("invalid request: %s: write request on readonly
connection",
- name_of_nbd_cmd (cmd));
- *error = EROFS;
- return false;
- }
- }
-
- /* Validate cmd, offset, count. */
+ /* Validate command arguments (more checks will be done later in backend) */
switch (cmd) {
- case NBD_CMD_READ:
case NBD_CMD_CACHE:
- case NBD_CMD_WRITE:
case NBD_CMD_TRIM:
case NBD_CMD_WRITE_ZEROES:
+ break;
+
+ case NBD_CMD_READ:
+ case NBD_CMD_WRITE:
+ /* Refuse over-large read and write requests. */
+ if (count > MAX_REQUEST_SIZE) {
+ nbdkit_error ("invalid request: %s: data request is too large
(%" PRIu32
+ " > %d)",
+ name_of_nbd_cmd (cmd), count, MAX_REQUEST_SIZE);
+ *error = ENOMEM;
+ return false;
+ }
+ break;
+
case NBD_CMD_BLOCK_STATUS:
- if (!valid_range (conn, offset, count)) {
- /* XXX Allow writes to extend the disk? */
- nbdkit_error ("invalid request: %s: offset and count are out of
range: "
- "offset=%" PRIu64 " count=%" PRIu32,
- name_of_nbd_cmd (cmd), offset, count);
- *error = (cmd == NBD_CMD_WRITE ||
- cmd == NBD_CMD_WRITE_ZEROES) ? ENOSPC : EINVAL;
+ if (!conn->structured_replies) {
+ nbdkit_error ("invalid request: "
+ "%s: structured replies was not negotiated",
+ name_of_nbd_cmd (cmd));
+ *error = EINVAL;
+ return false;
+ }
+ if (!conn->meta_context_base_allocation) {
+ nbdkit_error ("invalid request: "
+ "%s: base:allocation was not negotiated",
+ name_of_nbd_cmd (cmd));
+ *error = EINVAL;
return false;
}
break;
@@ -144,79 +137,6 @@ validate_request (struct connection *conn,
*error = EINVAL;
return false;
}
- if (flags & NBD_CMD_FLAG_FUA) {
- r = backend_can_fua (backend, conn);
- assert (r >= 0); /* Guaranteed by eflags computation */
- if (!r) {
- nbdkit_error ("invalid request: FUA flag not supported");
- *error = EINVAL;
- return false;
- }
- }
-
- /* Refuse over-large read and write requests. */
- if ((cmd == NBD_CMD_WRITE || cmd == NBD_CMD_READ) &&
- count > MAX_REQUEST_SIZE) {
- nbdkit_error ("invalid request: %s: data request is too large (%"
PRIu32
- " > %d)",
- name_of_nbd_cmd (cmd), count, MAX_REQUEST_SIZE);
- *error = ENOMEM;
- return false;
- }
-
- /* Flush allowed? */
- if (cmd == NBD_CMD_FLUSH) {
- r = backend_can_flush (backend, conn);
- assert (r >= 0); /* Guaranteed by eflags computation */
- if (!r) {
- nbdkit_error ("invalid request: %s: flush operation not
supported",
- name_of_nbd_cmd (cmd));
- *error = EINVAL;
- return false;
- }
- }
-
- /* Trim allowed? */
- if (cmd == NBD_CMD_TRIM) {
- r = backend_can_trim (backend, conn);
- assert (r >= 0); /* Guaranteed by eflags computation */
- if (!r) {
- nbdkit_error ("invalid request: %s: trim operation not
supported",
- name_of_nbd_cmd (cmd));
- *error = EINVAL;
- return false;
- }
- }
-
- /* Zero allowed? */
- if (cmd == NBD_CMD_WRITE_ZEROES) {
- r = backend_can_zero (backend, conn);
- assert (r >= 0); /* Guaranteed by eflags computation */
- if (!r) {
- nbdkit_error ("invalid request: %s: write zeroes operation not
supported",
- name_of_nbd_cmd (cmd));
- *error = EINVAL;
- return false;
- }
- }
-
- /* Block status allowed? */
- if (cmd == NBD_CMD_BLOCK_STATUS) {
- if (!conn->structured_replies) {
- nbdkit_error ("invalid request: "
- "%s: structured replies was not negotiated",
- name_of_nbd_cmd (cmd));
- *error = EINVAL;
- return false;
- }
- if (!conn->meta_context_base_allocation) {
- nbdkit_error ("invalid request: "
- "%s: base:allocation was not negotiated",
- name_of_nbd_cmd (cmd));
- *error = EINVAL;
- return false;
- }
- }
return true; /* Command validates. */
}
--
2.21.0
Richard W.M. Jones
2019-Aug-30 13:03 UTC
Re: [Libguestfs] [nbdkit PATCH 1/9] server: Fewer dereferences in filter
On Thu, Aug 29, 2019 at 10:08:21PM -0500, Eric Blake wrote:> Anywhere that we compute 'f = container_of(b)' then use 'f->backend.', > we can instead just use 'b->'. Similarly, during registration, we can > use the 'filename' input rather than our just-copied 'f->filename'.This essentially follows from the definition of container_of. I couldn't spot any mistakes in the changes below, and in any case strong typing should prevent mistakes. So yes, ACK. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Richard W.M. Jones
2019-Aug-30 13:07 UTC
Re: [Libguestfs] [nbdkit PATCH 2/9] server: Consolidate common backend tasks into new backend.c
On Thu, Aug 29, 2019 at 10:08:22PM -0500, Eric Blake wrote:> Both plugin.c and filter.c add the same three fields on top of struct > backend, and use very similar code in initializing them. Let's stop > the duplication, by moving those three fields into struct backend, and > creating a new backend.c for manipulating them. In turn, we can drop > the backend->name() accessor in favor of just directly accessing the > name field. This is a net reduction in lines of code (remember, the > diffstat is also counting comment additions, including license > boilerplate).Yes, a quite reasonable simplification, ACK. I might have been tempted to save the static string "plugin" or "filter" in a new backend->type field on the basis that it would help a little with debugging and costs virtually nothing, but this is not essential. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
Richard W.M. Jones
2019-Aug-30 13:19 UTC
Re: [Libguestfs] [nbdkit PATCH 5/9] server: Cache per-connection size
ACK up to here. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
Eric Blake
2019-Aug-30 13:20 UTC
Re: [Libguestfs] [nbdkit PATCH 6/9] server: Cache per-connection can_FOO flags
On 8/29/19 10:08 PM, Eric Blake wrote:> Similar to the previous patch in caching size, we want to avoid > calling into the plugin more than once per connection on any of the > flag determination callbacks. >> +++ b/server/protocol-handshake.c > @@ -51,87 +51,72 @@ protocol_compute_eflags (struct connection *conn, uint16_t *flags) > { > uint16_t eflags = NBD_FLAG_HAS_FLAGS; > int fl; > + bool can_write = true; > > fl = backend_can_write (backend, conn); > if (fl == -1) > return -1; > if (readonly || !fl) { > eflags |= NBD_FLAG_READ_ONLY; > - conn->readonly = true; > + can_write = false;The old code set conn->readonly=false if either the command line -r was present or if the backend failed .can_write...> +++ b/server/protocol.c > @@ -64,14 +64,19 @@ validate_request (struct connection *conn, > uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count, > uint32_t *error) > { > + int r; > + > /* Readonly connection? */ > - if (conn->readonly && > - (cmd == NBD_CMD_WRITE || cmd == NBD_CMD_TRIM || > - cmd == NBD_CMD_WRITE_ZEROES)) { > - nbdkit_error ("invalid request: %s: write request on readonly connection", > - name_of_nbd_cmd (cmd)); > - *error = EROFS; > - return false; > + if (cmd == NBD_CMD_WRITE || cmd == NBD_CMD_TRIM || > + cmd == NBD_CMD_WRITE_ZEROES) { > + r = backend_can_write (backend, conn);...but the new code is only checking if the backend supports .can_write (if the backend says yes, then this permits a broken client to write in spite of the -r command line flag). I'll fix that. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Richard W.M. Jones
2019-Aug-30 13:24 UTC
Re: [Libguestfs] [nbdkit PATCH 6/9] server: Cache per-connection can_FOO flags
On Thu, Aug 29, 2019 at 10:08:26PM -0500, Eric Blake wrote:> @@ -232,8 +257,8 @@ handle_request (struct connection *conn, > void *buf, struct nbdkit_extents *extents) > { > uint32_t f = 0; > - bool fua = conn->can_fua && (flags & NBD_CMD_FLAG_FUA); > int err = 0; > + int r; > > /* Clear the error, so that we know if the plugin calls > * nbdkit_set_error() or relied on errno. */ > @@ -246,7 +271,7 @@ handle_request (struct connection *conn, > break; > > case NBD_CMD_WRITE: > - if (fua) > + if (flags & NBD_CMD_FLAG_FUA) > f |= NBDKIT_FLAG_FUA;So don't we need to keep the backend_can_fua() test here and later in this function? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
Richard W.M. Jones
2019-Aug-30 13:29 UTC
Re: [Libguestfs] [nbdkit PATCH 9/9] server: Move command validation from protocol.c to backend.c
Patches 7-9 are pretty complex. I guess the series passes something like: git rebase -i HEAD~9 -x "make && make check" ? It might be better to go with that as a review mechanism for these final patches ... Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Seemingly Similar Threads
- [PATCH nbdkit 2/2] server: Split out NBD protocol code from connections code.
- [PATCH nbdkit 3/3] server: Remove explicit connection parameter, use TLS instead.
- [PATCH nbdkit v2] protocol: Implement NBD_OPT_GO.
- [PATCH nbdkit 3/4] common/protocol: Update nbd-protocol.h so it matches libnbd’s copy.
- [nbdkit PATCH v2 7/7] server: Better newstyle .open failure handling