search for: nbd_cmd_cache

On Thu, May 09, 2019 at 10:03:34PM -0500, Eric Blake wrote: > 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 h...

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...

I'm still working my way through the filters before this series will be complete, but this is enough of a start to at least get some feedback on the idea of implementing another NBD protocol extension. Eric Blake (9): server: Internal hooks for implementing NBD_CMD_CACHE plugins: Add .cache callback file, split: Implement .cache with posix_fadvise nbd: Implement NBD_CMD_CACHE passthrough plugins: Implement no-op .cache for in-memory plugins sh: Implement .cache script callback filters: Add .cache callback test-layers: Test .cache usage blocksize: Im...

...caching a > pread from one dd invocation may indeed speed up the next access), but > for the sake of the example, I demonstrated advertising a no-op > handler. > > The shell plugin, coupled with Rich's work on libnbd as a client-side > library for actually exercising calls to NBD_CMD_CACHE, will be a > useful way to prove that cache commands even make it through the > stack. (Remember, qemu 3.0 was released with a fatally flawed > NBD_CMD_CACHE server implementation, because there were no open source > clients at the time that could actually send the command to test the &...

Since v1: - rework .can_cache to be tri-state, with default of no advertisement (ripple effect through other patches) - add a lot more patches in order to round out filter support And in the meantime, Rich pushed NBD_CMD_CACHE support into libnbd, so in theory we now have a way to test cache commands through the entire stack. Eric Blake (24): server: Internal hooks for implementing NBD_CMD_CACHE plugins: Add .cache callback file, split: Implement .cache with posix_fadvise plugins: Implement .pread emulation cach...

...NULL, 0, NULL); } int64_t nbd_unlocked_aio_cache (struct nbd_connection *conn, uint64_t count, uint64_t offset) { - struct command_in_flight *cmd; - /* Actually according to the NBD protocol document, servers do exist * that support NBD_CMD_CACHE but don't advertize the * NBD_FLAG_SEND_CACHE bit, but we ignore those. @@ -421,13 +395,8 @@ nbd_unlocked_aio_cache (struct nbd_connection *conn, return -1; } - cmd = command_common (conn, 0, NBD_CMD_CACHE, offset, count, NULL); - if (!cmd) - return -1; - if (nbd_internal_run...

...ince a new shell process is started for each command); but if nothing else, this allows a user to implement .can_cache returning true to bypass nbdkit's normal fallback to .pread. The shell plugin, coupled with Rich's work on libnbd as a client-side library for actually exercising calls to NBD_CMD_CACHE, will be a useful way to prove that cache commands even make it through the stack. (Remember, qemu 3.0 was released with a fatally flawed NBD_CMD_CACHE server implementation, because there were no open source clients at the time that could actually send the command to test the server with). Signed...

...ed; instead MSG_MORE is used to control when outgoing packets are sent. Ramdisk plugins now support more efficient zeroing. (Eric Blake). Plugins can now select their thread model at run time. Thread model selection is not finalized until after the config stage (Eric Blake). The server supports NBD_CMD_CACHE for prefetching. Plugins and filters have been updated to implement this where it makes sense (Eric Blake). Low level pthread lock failures now call abort(). This should never happen in real situations (Eric Blake). The server will not advertize multi-conn support if the internal thread model i...

...example.sh script, the kernel caching a pread from one dd invocation may indeed speed up the next access), but for the sake of the example, I demonstrated advertising a no-op handler. The shell plugin, coupled with Rich's work on libnbd as a client-side library for actually exercising calls to NBD_CMD_CACHE, will be a useful way to prove that cache commands even make it through the stack. (Remember, qemu 3.0 was released with a fatally flawed NBD_CMD_CACHE server implementation, because there were no open source clients at the time that could actually send the command to test the server with). Signed...

...return s ? nbd_reply (h, s) : -1; } /* Cache a portion of the file. */ @@ -1260,11 +1260,11 @@ static int nbd_cache (void *handle, uint32_t count, uint64_t offset, uint32_t flags) { struct handle *h = handle; - int c; + struct transaction *s; assert (!flags); - c = nbd_request (h, 0, NBD_CMD_CACHE, offset, count); - return c < 0 ? c : nbd_reply (h, c); + s = nbd_request (h, 0, NBD_CMD_CACHE, offset, count); + return s ? nbd_reply (h, s) : -1; } static struct nbdkit_plugin plugin = { -- 2.20.1

...; + } } if (count == 0) { /* NBD protocol forbids this. */ @@ -399,18 +409,20 @@ nbd_unlocked_aio_cache (struct nbd_handle *h, { struct command_cb cb = { .completion = completion }; - /* Actually according to the NBD protocol document, servers do exist - * that support NBD_CMD_CACHE but don't advertise the - * NBD_FLAG_SEND_CACHE bit, but we ignore those. - */ - if (nbd_unlocked_can_cache (h) != 1) { - set_error (EINVAL, "server does not support cache operations"); - return -1; - } + if (h->strict & LIBNBD_STRICT_COMMANDS) { + /* Actually a...

...D_FLUSH, 0, 0, + return nbd_internal_command_common (h, flags, NBD_CMD_FLUSH, 0, 0, NULL, &cb); } @@ -409,7 +409,7 @@ nbd_unlocked_aio_cache (struct nbd_handle *h, } SET_CALLBACK_TO_NULL (*completion); - return nbd_internal_command_common (h, 0, NBD_CMD_CACHE, offset, count, + return nbd_internal_command_common (h, flags, NBD_CMD_CACHE, offset, count, NULL, &cb); } diff --git a/tests/errors.c b/tests/errors.c index 0c4151a..0cfcac5 100644 --- a/tests/errors.c +++ b/tests/errors.c @@ -84,7 +84,7 @@ main (int...

We have been inconsistent on how much we reject client-side without even consulting the server, vs. how much we depend on the server to detect failure (even if our request can be deemed undefined per NBD protocol). I'd like to change it so that by default, we reject as much as we can client-side for less traffic, but where the user can also change things on the fly for server-side integration

...flags 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 compu...

...#39;s cache implementation is worthwhile. A worthwhile test to add to the testsuite would connect the log filter both before and after the nocache filter, to prove how caching requests are altered. However, until we use libnbd in the nbdkit testsuite, there isn't really a clean way to trigger NBD_CMD_CACHE calls to test this (qemu-nbd can't do it). But I can at least exploit test-eflags to use 'qemu-nbd --list' to demonstrate that the advertised flag changes, with a little tweak to allow the test to run filters with arguments. Signed-off-by: Eric Blake <eblake@redhat.com> --- filt...

...mmand_common (h, flags, NBD_CMD_FLUSH, 0, 0, > NULL, &cb); > } > > @@ -409,7 +409,7 @@ nbd_unlocked_aio_cache (struct nbd_handle *h, > } > > SET_CALLBACK_TO_NULL (*completion); > - return nbd_internal_command_common (h, 0, NBD_CMD_CACHE, offset, count, > + return nbd_internal_command_common (h, flags, NBD_CMD_CACHE, offset, count, > NULL, &cb); > } > > diff --git a/tests/errors.c b/tests/errors.c > index 0c4151a..0cfcac5 100644 > --- a/tests/errors.c > +++ b/tes...

...common (conn, flags, NBD_CMD_TRIM, offset, count, NULL); if (!cmd) return -1; - if (nbd_internal_run (conn->h, conn, cmd_issue) == -1) - return -1; return cmd->handle; } @@ -424,8 +418,6 @@ nbd_unlocked_aio_cache (struct nbd_connection *conn, cmd = command_common (conn, 0, NBD_CMD_CACHE, offset, count, NULL); if (!cmd) return -1; - if (nbd_internal_run (conn->h, conn, cmd_issue) == -1) - return -1; return cmd->handle; } @@ -461,8 +453,6 @@ nbd_unlocked_aio_zero (struct nbd_connection *conn, cmd = command_common (conn, flags, NBD_CMD_WRITE_ZEROES, offset,...

...set_error (EINVAL, "count cannot be 0"); + return -1; + } + + if (offset > h->exportsize || offset + count > h->exportsize) { + set_error (EINVAL, "request out of bounds"); + return -1; + } + } + return nbd_internal_command_common (h, 0, NBD_CMD_CACHE, offset, count, NULL, &cb); } @@ -466,9 +521,16 @@ nbd_unlocked_aio_zero (struct nbd_handle *h, } } - if (count == 0) { /* NBD protocol forbids this. */ - set_error (EINVAL, "count cannot be 0"); - return -1; + if (h...

In v2: - now based on my proposal to add LIBNBD_SHUTDOWN_IMMEDIATE - four flags instead of two: STRICT_FLAGS is new (patch 4), and STRICT_BOUNDS is separate from STRICT_ZERO_SIZE (patch 5) - various refactorings for more shared code and less duplication Eric Blake (5): api: Add xxx_MASK constant for each Flags type generator: Refactor filtering of accepted OFlags api: Add

...ffset, count, - NULL, &cb); + ENOSPC, NULL, &cb); } int64_t @@ -410,7 +419,7 @@ nbd_unlocked_aio_cache (struct nbd_handle *h, SET_CALLBACK_TO_NULL (*completion); return nbd_internal_command_common (h, flags, NBD_CMD_CACHE, offset, count, - NULL, &cb); + EINVAL, NULL, &cb); } int64_t @@ -444,14 +453,9 @@ nbd_unlocked_aio_zero (struct nbd_handle *h, } } - if (count == 0) { /* NBD protocol forbids this. */ - se...