[Libguestfs] [nbdkit PATCH 1/9] server: Internal hooks for implementing NBD_CMD_CACHE
Eric Blake
eblake at redhat.com
Fri May 10 03:03:34 UTC 2019
Until the next few patches expose and implement new callbacks for
filters and plugins, our initial implementation for NBD_CMD_CACHE is
to just blindly advertise the feature, and call into .pread with an
ignored buffer.
Note that for bisection reasons, this patch treats any use of a filter
as a forced .can_cache of false to bypass the filter's caching; once
all affected filters are patched to handle cache requests correctly, a
later patch will then switch the filter default to passthrough for the
sake of remaining filters.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
Note: we could also choose to always advertise caching, but make the
nbdkit default version be a no-op rather than call out to .pread. For
that matter, maybe a plugin's .can_cache should allow the plugin to
choose between a tri-state of no-op, fallback to .pread, or call
.cache; I'm not sure which is the saner default for a random plugin
compiled against older nbdkit. At least it's fairly easy to write a
filter that changes the default from no-op to .pread or from .pread to
no-op. Be thinking about that as you review the rest of the series.
---
docs/nbdkit-protocol.pod | 10 +++++++
common/protocol/protocol.h | 2 ++
server/internal.h | 4 +++
server/filters.c | 33 ++++++++++++++++++++++-
server/plugins.c | 34 ++++++++++++++++++++++++
server/protocol-handshake.c | 10 ++++++-
server/protocol.c | 25 +++++++++++++++++
tests/test-eflags.sh | 53 ++++++++++++++++++++-----------------
8 files changed, 144 insertions(+), 27 deletions(-)
diff --git a/docs/nbdkit-protocol.pod b/docs/nbdkit-protocol.pod
index f706cfd..6b85c14 100644
--- a/docs/nbdkit-protocol.pod
+++ b/docs/nbdkit-protocol.pod
@@ -152,6 +152,16 @@ when structured replies are in effect. However, the flag is a no-op
until we extend the plugin API to allow a fragmented read in the first
place.
+=item C<NBD_CMD_CACHE>
+
+Supported in nbdkit E<ge> 1.13.4.
+
+This protocol extension allows a client to inform the server about
+intent to access a portion of the export, to allow the server an
+opportunity to cache things appropriately. For plugins that do not
+support a particular form of caching, nbdkit performs pread then
+ignores the results.
+
=item Resize Extension
I<Not supported>.
diff --git a/common/protocol/protocol.h b/common/protocol/protocol.h
index c27104c..e938643 100644
--- a/common/protocol/protocol.h
+++ b/common/protocol/protocol.h
@@ -95,6 +95,7 @@ extern const char *name_of_nbd_flag (int);
#define NBD_FLAG_SEND_WRITE_ZEROES (1 << 6)
#define NBD_FLAG_SEND_DF (1 << 7)
#define NBD_FLAG_CAN_MULTI_CONN (1 << 8)
+#define NBD_FLAG_SEND_CACHE (1 << 10)
/* NBD options (new style handshake only). */
extern const char *name_of_nbd_opt (int);
@@ -217,6 +218,7 @@ extern const char *name_of_nbd_cmd (int);
#define NBD_CMD_DISC 2 /* Disconnect. */
#define NBD_CMD_FLUSH 3
#define NBD_CMD_TRIM 4
+#define NBD_CMD_CACHE 5
#define NBD_CMD_WRITE_ZEROES 6
#define NBD_CMD_BLOCK_STATUS 7
diff --git a/server/internal.h b/server/internal.h
index 67fccfc..6414a78 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -170,6 +170,7 @@ struct connection {
bool can_zero;
bool can_fua;
bool can_multi_conn;
+ bool can_cache;
bool can_extents;
bool using_tls;
bool structured_replies;
@@ -276,6 +277,7 @@ struct backend {
int (*can_extents) (struct backend *, struct connection *conn);
int (*can_fua) (struct backend *, struct connection *conn);
int (*can_multi_conn) (struct backend *, struct connection *conn);
+ int (*can_cache) (struct backend *, struct connection *conn);
int (*pread) (struct backend *, struct connection *conn, void *buf,
uint32_t count, uint64_t offset, uint32_t flags, int *err);
@@ -290,6 +292,8 @@ struct backend {
int (*extents) (struct backend *, struct connection *conn, uint32_t count,
uint64_t offset, uint32_t flags,
struct nbdkit_extents *extents, int *err);
+ int (*cache) (struct backend *, struct connection *conn, uint32_t count,
+ uint64_t offset, uint32_t flags, int *err);
};
/* plugins.c */
diff --git a/server/filters.c b/server/filters.c
index b73e74f..c619fd6 100644
--- a/server/filters.c
+++ b/server/filters.c
@@ -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
@@ -573,6 +573,18 @@ filter_can_multi_conn (struct backend *b, struct connection *conn)
return f->backend.next->can_multi_conn (f->backend.next, conn);
}
+static int
+filter_can_cache (struct backend *b, struct connection *conn)
+{
+ struct backend_filter *f = container_of (b, struct backend_filter, backend);
+
+ debug ("%s: can_cache", f->name);
+
+ /* FIXME: Default to f->backend.next->can_cache, once all filters
+ have been audited */
+ return 0;
+}
+
static int
filter_pread (struct backend *b, struct connection *conn,
void *buf, uint32_t count, uint64_t offset,
@@ -702,6 +714,23 @@ filter_extents (struct backend *b, struct connection *conn,
extents, err);
}
+static int
+filter_cache (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);
+
+ assert (flags == 0);
+
+ debug ("%s: cache count=%" PRIu32 " offset=%" PRIu64 " flags=0x%" PRIx32,
+ f->name, count, offset, flags);
+
+ /* FIXME: Allow filter to rewrite request */
+ return f->backend.next->cache (f->backend.next, conn,
+ count, offset, flags, err);
+}
+
static struct backend filter_functions = {
.free = filter_free,
.thread_model = filter_thread_model,
@@ -726,12 +755,14 @@ static struct backend filter_functions = {
.can_extents = filter_can_extents,
.can_fua = filter_can_fua,
.can_multi_conn = filter_can_multi_conn,
+ .can_cache = filter_can_cache,
.pread = filter_pread,
.pwrite = filter_pwrite,
.flush = filter_flush,
.trim = filter_trim,
.zero = filter_zero,
.extents = filter_extents,
+ .cache = filter_cache,
};
/* Register and load a filter. */
diff --git a/server/plugins.c b/server/plugins.c
index e26d133..cabf543 100644
--- a/server/plugins.c
+++ b/server/plugins.c
@@ -448,6 +448,17 @@ plugin_can_multi_conn (struct backend *b, struct connection *conn)
return 0; /* assume false */
}
+static int
+plugin_can_cache (struct backend *b, struct connection *conn)
+{
+ assert (connection_get_handle (conn, 0));
+
+ debug ("can_cache");
+
+ /* FIXME: return default based on plugin->cache */
+ return 0;
+}
+
/* Plugins and filters can call this to set the true errno, in cases
* where !errno_is_preserved.
*/
@@ -693,6 +704,27 @@ plugin_extents (struct backend *b, struct connection *conn,
return r;
}
+static int
+plugin_cache (struct backend *b, struct connection *conn,
+ uint32_t count, uint64_t offset, uint32_t flags,
+ int *err)
+{
+ struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
+ int r = -1;
+
+ assert (connection_get_handle (conn, 0));
+ assert (!flags);
+
+ debug ("cache count=%" PRIu32 " offset=%" PRIu64, count, offset);
+
+ /* FIXME: assert plugin->cache and call it */
+ assert (false);
+
+ if (r == -1)
+ *err = get_error (p);
+ return r;
+}
+
static struct backend plugin_functions = {
.free = plugin_free,
.thread_model = plugin_thread_model,
@@ -717,12 +749,14 @@ static struct backend plugin_functions = {
.can_extents = plugin_can_extents,
.can_fua = plugin_can_fua,
.can_multi_conn = plugin_can_multi_conn,
+ .can_cache = plugin_can_cache,
.pread = plugin_pread,
.pwrite = plugin_pwrite,
.flush = plugin_flush,
.trim = plugin_trim,
.zero = plugin_zero,
.extents = plugin_extents,
+ .cache = plugin_cache,
};
/* Register and load a plugin. */
diff --git a/server/protocol-handshake.c b/server/protocol-handshake.c
index 03377a9..af1cd18 100644
--- a/server/protocol-handshake.c
+++ b/server/protocol-handshake.c
@@ -109,7 +109,7 @@ protocol_compute_eflags (struct connection *conn, uint16_t *flags)
conn->can_multi_conn = true;
}
- /* The result of this is not returned to callers here (or at any
+ /* The results of the next two are 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.
@@ -120,6 +120,14 @@ protocol_compute_eflags (struct connection *conn, uint16_t *flags)
if (fl)
conn->can_extents = true;
+ fl = backend->can_cache (backend, conn);
+ if (fl == -1)
+ return -1;
+ if (fl)
+ conn->can_cache = true;
+ /* We emulate cache even when plugin doesn't support it */
+ eflags |= NBD_FLAG_SEND_CACHE;
+
if (conn->structured_replies)
eflags |= NBD_FLAG_SEND_DF;
diff --git a/server/protocol.c b/server/protocol.c
index 01d4c71..69227e5 100644
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -76,6 +76,7 @@ validate_request (struct connection *conn,
/* Validate cmd, offset, count. */
switch (cmd) {
case NBD_CMD_READ:
+ case NBD_CMD_CACHE:
case NBD_CMD_WRITE:
case NBD_CMD_TRIM:
case NBD_CMD_WRITE_ZEROES:
@@ -254,6 +255,30 @@ handle_request (struct connection *conn,
return err;
break;
+ case NBD_CMD_CACHE:
+ /* The other backend methods don't check can_*. That is because
+ * those methods are implicitly suppressed by returning fewer
+ * eflags to the client. However the eflag for cache is always
+ * sent, because we emulate it here when the plugin lacks it.
+ */
+ if (conn->can_cache) {
+ if (backend->cache (backend, conn, count, offset, 0, &err) == -1)
+ return err;
+ }
+ else {
+ 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;
+ }
+ }
+ break;
+
case NBD_CMD_WRITE_ZEROES:
if (!(flags & NBD_CMD_FLAG_NO_HOLE))
f |= NBDKIT_FLAG_MAY_TRIM;
diff --git a/tests/test-eflags.sh b/tests/test-eflags.sh
index 84dcfe8..0234aca 100755
--- a/tests/test-eflags.sh
+++ b/tests/test-eflags.sh
@@ -1,6 +1,6 @@
#!/usr/bin/env bash
# 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
@@ -94,8 +94,8 @@ case "$1" in
esac
EOF
-[ $eflags -eq $(( HAS_FLAGS|READ_ONLY|SEND_DF )) ] ||
- fail "expected HAS_FLAGS|READ_ONLY|SEND_DF"
+[ $eflags -eq $(( HAS_FLAGS|READ_ONLY|SEND_DF|SEND_CACHE )) ] ||
+ fail "expected HAS_FLAGS|READ_ONLY|SEND_DF|SEND_CACHE"
#----------------------------------------------------------------------
# -r
@@ -108,8 +108,8 @@ case "$1" in
esac
EOF
-[ $eflags -eq $(( HAS_FLAGS|READ_ONLY|SEND_DF )) ] ||
- fail "expected HAS_FLAGS|READ_ONLY|SEND_DF"
+[ $eflags -eq $(( HAS_FLAGS|READ_ONLY|SEND_DF|SEND_CACHE )) ] ||
+ fail "expected HAS_FLAGS|READ_ONLY|SEND_DF|SEND_CACHE"
#----------------------------------------------------------------------
# can_write=true
@@ -126,8 +126,8 @@ case "$1" in
esac
EOF
-[ $eflags -eq $(( HAS_FLAGS|SEND_WRITE_ZEROES|SEND_DF )) ] ||
- fail "expected HAS_FLAGS|SEND_WRITE_ZEROES|SEND_DF"
+[ $eflags -eq $(( HAS_FLAGS|SEND_WRITE_ZEROES|SEND_DF|SEND_CACHE )) ] ||
+ fail "expected HAS_FLAGS|SEND_WRITE_ZEROES|SEND_DF|SEND_CACHE"
#----------------------------------------------------------------------
# -r
@@ -144,8 +144,8 @@ case "$1" in
esac
EOF
-[ $eflags -eq $(( HAS_FLAGS|READ_ONLY|SEND_DF )) ] ||
- fail "expected HAS_FLAGS|READ_ONLY|SEND_DF"
+[ $eflags -eq $(( HAS_FLAGS|READ_ONLY|SEND_DF|SEND_CACHE )) ] ||
+ fail "expected HAS_FLAGS|READ_ONLY|SEND_DF|SEND_CACHE"
#----------------------------------------------------------------------
# can_write=false
@@ -164,8 +164,8 @@ case "$1" in
esac
EOF
-[ $eflags -eq $(( HAS_FLAGS|READ_ONLY|SEND_DF )) ] ||
- fail "expected HAS_FLAGS|READ_ONLY|SEND_DF"
+[ $eflags -eq $(( HAS_FLAGS|READ_ONLY|SEND_DF|SEND_CACHE )) ] ||
+ fail "expected HAS_FLAGS|READ_ONLY|SEND_DF|SEND_CACHE"
#----------------------------------------------------------------------
# -r
@@ -185,8 +185,8 @@ case "$1" in
esac
EOF
-[ $eflags -eq $(( HAS_FLAGS|READ_ONLY|SEND_DF )) ] ||
- fail "expected HAS_FLAGS|READ_ONLY|SEND_DF"
+[ $eflags -eq $(( HAS_FLAGS|READ_ONLY|SEND_DF|SEND_CACHE )) ] ||
+ fail "expected HAS_FLAGS|READ_ONLY|SEND_DF|SEND_CACHE"
#----------------------------------------------------------------------
# can_write=true
@@ -201,8 +201,8 @@ case "$1" in
esac
EOF
-[ $eflags -eq $(( HAS_FLAGS|SEND_TRIM|SEND_WRITE_ZEROES|SEND_DF )) ] ||
- fail "expected HAS_FLAGS|SEND_TRIM|SEND_WRITE_ZEROES|SEND_DF"
+[ $eflags -eq $(( HAS_FLAGS|SEND_TRIM|SEND_WRITE_ZEROES|SEND_DF|SEND_CACHE )) ] ||
+ fail "expected HAS_FLAGS|SEND_TRIM|SEND_WRITE_ZEROES|SEND_DF|SEND_CACHE"
#----------------------------------------------------------------------
# can_write=true
@@ -217,8 +217,8 @@ case "$1" in
esac
EOF
-[ $eflags -eq $(( HAS_FLAGS|ROTATIONAL|SEND_WRITE_ZEROES|SEND_DF )) ] ||
- fail "expected HAS_FLAGS|ROTATIONAL|SEND_WRITE_ZEROES|SEND_DF"
+[ $eflags -eq $(( HAS_FLAGS|ROTATIONAL|SEND_WRITE_ZEROES|SEND_DF|SEND_CACHE )) ] ||
+ fail "expected HAS_FLAGS|ROTATIONAL|SEND_WRITE_ZEROES|SEND_DF|SEND_CACHE"
#----------------------------------------------------------------------
# -r
@@ -234,8 +234,8 @@ case "$1" in
esac
EOF
-[ $eflags -eq $(( HAS_FLAGS|READ_ONLY|ROTATIONAL|SEND_DF )) ] ||
- fail "expected HAS_FLAGS|READ_ONLY|ROTATIONAL|SEND_DF"
+[ $eflags -eq $(( HAS_FLAGS|READ_ONLY|ROTATIONAL|SEND_DF|SEND_CACHE )) ] ||
+ fail "expected HAS_FLAGS|READ_ONLY|ROTATIONAL|SEND_DF|SEND_CACHE"
#----------------------------------------------------------------------
# can_write=true
@@ -250,8 +250,8 @@ case "$1" in
esac
EOF
-[ $eflags -eq $(( HAS_FLAGS|SEND_FUA|SEND_WRITE_ZEROES|SEND_DF )) ] ||
- fail "expected HAS_FLAGS|SEND_FUA|SEND_WRITE_ZEROES|SEND_DF"
+[ $eflags -eq $(( HAS_FLAGS|SEND_FUA|SEND_WRITE_ZEROES|SEND_DF|SEND_CACHE )) ] ||
+ fail "expected HAS_FLAGS|SEND_FUA|SEND_WRITE_ZEROES|SEND_DF|SEND_CACHE"
#----------------------------------------------------------------------
# -r
@@ -269,8 +269,8 @@ case "$1" in
esac
EOF
-[ $eflags -eq $(( HAS_FLAGS|READ_ONLY|SEND_DF )) ] ||
- fail "expected HAS_FLAGS|READ_ONLY|SEND_DF"
+[ $eflags -eq $(( HAS_FLAGS|READ_ONLY|SEND_DF|SEND_CACHE )) ] ||
+ fail "expected HAS_FLAGS|READ_ONLY|SEND_DF|SEND_CACHE"
#----------------------------------------------------------------------
# -r
@@ -284,5 +284,8 @@ case "$1" in
esac
EOF
-[ $eflags -eq $(( HAS_FLAGS|READ_ONLY|SEND_DF|CAN_MULTI_CONN )) ] ||
- fail "expected HAS_FLAGS|READ_ONLY|SEND_DF|CAN_MULTI_CONN"
+[ $eflags -eq $(( HAS_FLAGS|READ_ONLY|SEND_DF|CAN_MULTI_CONN|SEND_CACHE )) ] ||
+ fail "expected HAS_FLAGS|READ_ONLY|SEND_DF|CAN_MULTI_CONN|SEND_CACHE"
+
+# NBD_FLAG_SEND_CACHE is always set even if can_cache returns false, because
+# nbdkit reckons it can emulate caching using pread.
--
2.20.1
More information about the Libguestfs
mailing list