search for: can_foo

On 3/18/20 8:21 PM, Eric Blake wrote: > In commit c306fa93ab and neighbors (v1.15.1), a concerted effort went > into caching the results of .can_FOO callbacks, with commit messages > demonstrating that a plugin with a slow callback should not have that > delay magnified multiple times. But nothing was added to the > testsuite at the time, and with the sh and eval plugins, we still have > two scenarios where we did not take advantag...

Recent patches clarified documentation to point out that within the life of a single connection, the .can_FOO helpers should return consistent results, and that callers may cache those results. But at least in the case of .can_fua, we aren't really caching things; depending on the overhead involved, calling out to the plugin's .can_fua on every .pwrite with FUA requested may be noticeable overhead...

...> Thinking about this more, maybe the real problem is that all > language bindings that have to wrap scripts (OCaml and Rust are > exceptions as they directly call into the C code; but lua, perl, > python, ruby, tcl, and sh all fit into the category I'm describing) > MUST define .can_FOO at the C level because they don't know a priori > whether the script they will be loading will also provide a .can_FOO > callback, so we have a lot of duplicated code in each language > binding. > > Life might be easier if we change the C contract for the .can_FOO > callbacks...

On Thu, Aug 29, 2019 at 10:08:26PM -0500, Eric Blake wrote: > @@ -232,8 +257,8 @@ handle_request (struct connection *conn, > void *buf, struct nbdkit_extents *extents) > { > uint32_t f = 0; > - bool fua = conn->can_fua && (flags & NBD_CMD_FLAG_FUA); > int err = 0; > + int r; > > /* Clear the error, so that we know if the

After a retry, if the second connection has fewer permissions than the first, but we blindly call next_ops->FOO, we end up triggering an assertion failure in backend.c. This is particularly noticeable when the force_readonly flag is in effect, as that makes it much easier for there to be fewer permissions than before. Add testsuite coverage of pwrite to demonstrate. Signed-off-by: Eric Blake

When I added .can_FOO caching in 1.16, I missed the case that the sh plugin itself was calling .can_flush twice in some situations (in order to default .can_fua). Then right after, I regressed it to call .can_zero twice (in order to default .can_fast_zero). I also missed that .thread_model could use better caching, be...

...n eflags) and not the plugin semantics (whether we should attempt .zero or just automatically fall back to .pwrite); that will be improved in the next patch. In the cow filter, adding a call to next_ops->can_cache in .prepare means that .cache no longer has to worry about can_cache failing (the can_FOO methods do not guarantee easy access to a sane errno value). Signed-off-by: Eric Blake <eblake@redhat.com> --- docs/nbdkit-filter.pod | 9 +-- docs/nbdkit-plugin.pod | 6 +- server/internal.h | 19 +++--- server/backend.c | 75 +++++++++++++++++++...

On 11/22/19 3:46 PM, Nir Soffer wrote: >>>> and for zero (once fast zero is supported later in the series), it could >>>> look like: >>>> >>>> def zero(h, count, offset, may_trim=False, fua=False, fast=False): >>> >>> This is nicer - but not extensible. >> >> Why not? Any future flag additions would still appear as new

...g with the idea of having some sort of witness whether a connection has ever reached the READY state (even if it is not there at the moment). Prior to that point, we don't want to accept commands (whether sync or async) because we aren't finished negotiating and thus cannot guarantee if the can_FOO() functions are accurate; after that point, it may be nice to queue up the commands for later issuance when we DO come back around to READY, rather than insisting that we be exactly in the READY state when issuing a command. > > For ‘nbd_connect_command’, it essentially ignored multi-conn a...

...client request that violates that limit will fail with EINVAL for being unaligned, without even asking the plugin to try the response. nbdkit calls the plugin callback once per connection per reasonable 'which' (caching the results like it does for .get_size, and skipping limits where .can_FOO fails). Returning int64_t allows us to reuse this callback without change when we add v3 callbacks, although values larger than 4G make no difference at present. I thought the use of an enum was nicer than filling in a struct whose size might change, or adding one callback per limit. Filters...

...is less nice, but easy to extend without adding v3. But I would call it **options instead. > > and manually extract key/value pairs out of the flags, to be robust to > unknown flags (although our API somewhat promises that we never pass > flags to the data-handling callbacks unless the can_FOO callbacks > already indicated that the plugin was willing to support the flag) ... Nir

Fairly straightforward, as the code in plugins.c already takes care of necessary .pwrite fallbacks. Simplify similar can_FOO callbacks that were copy-and-pasted while implementing py_can_zero. Signed-off-by: Eric Blake <eblake@redhat.com> --- plugins/python/nbdkit-python-plugin.pod | 7 +++++ plugins/python/python.c | 51 +++++++++++++++++++-------------- tests/test.py |...

...t; +=item C<can_flush> > > (Optional) > > - def is_rotational(h): > + def can_flush(h): > # return a boolean Why the shuffle? To match ordering in other docs? But not the end of the world. Otherwise, nice reduction in lines of code (it may get trickier with can_FOO that return tristate, but for this patch you really did simplify true bool/error return functions). > @@ -812,9 +758,9 @@ static struct nbdkit_plugin plugin = { > .close = py_close, > > .get_size = py_get_size, > + .is_rotational = py_is_rotatio...

...aving some sort of witness whether a > connection has ever reached the READY state (even if it is not there at > the moment). Prior to that point, we don't want to accept commands > (whether sync or async) because we aren't finished negotiating and thus > cannot guarantee if the can_FOO() functions are accurate; after that > point, it may be nice to queue up the commands for later issuance when > we DO come back around to READY, rather than insisting that we be > exactly in the READY state when issuing a command. > > > > > For ‘nbd_connect_command’, it es...

...it having value for clients which either ignore the block size information, or don't implement it correctly/completely. > nbdkit calls the plugin callback once per connection per reasonable > 'which' (caching the results like it does for .get_size, and > skipping limits where .can_FOO fails). Returning int64_t allows us > to reuse this callback without change when we add v3 callbacks, > although values larger than 4G make no difference at present. I > thought the use of an enum was nicer than filling in a struct whose > size might change, or adding one callback per...

Hi Eric, Nir. $SUBJECT - it's complicated! Imagine a scenario where we have extended the file plugin so you can point it at a directory and it will use the client-supplied export name to select a file to serve. Also we have extended the tar filter so that you can map tar file components into export names. You may be able to see where this is going ... Now we point the file plugin at a

nbd_connect_unix|tcp had a tricky failure case. This is a consequence of allowing callers to mix synchronous and asynchronous calls, with multi-conn thrown into the mix. I think the new behaviour proposed here is better. We could do with a better way of classifying the state of connections, such as are they connectING. Rich.

...ero). Eric Blake (9): server: Fewer dereferences in filter server: Consolidate common backend tasks into new backend.c server: Create new backend_* functions for central handling server: Rework storage of per-backend handle server: Cache per-connection size server: Cache per-connection can_FOO flags filters: Change semantics of .can_zero server: Move fallbacks from protocol.c to backend.c server: Move command validation from protocol.c to backend.c docs/nbdkit-filter.pod | 62 +++- docs/nbdkit-plugin.pod | 6 +- server/internal.h...

...e to assert that our computation of eflags populated all of the flags, and thus calls such as backend_can_write would not fail; however, with filters, keeping those assertions mean the burden is now on the filter to avoid calling into next_ops->FOO without first priming the cache of next_ops->can_FOO. An audit of existing filters did not find any more culprits. Signed-off-by: Eric Blake <eblake@redhat.com> --- server/backend.c | 94 ++++++++++++++++++++++++++++++++ server/protocol.c | 134 ++++++++++------------------------------------ 2 files changed, 121 insertions(+), 107 deletion...

On 7/9/20 12:08 PM, Richard W.M. Jones wrote: > On Thu, Jul 09, 2020 at 11:37:53AM -0500, Eric Blake wrote: >> Since .prepare is called before client negotiation has completed, >> filters have an additional burden to ensure prerequisite functions are >> called in order to avoid triggering assertions in backend.c. >> >> See also: