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