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 <[email protected]> --- 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 _______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
