10:08 p.m.
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(a)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