similar to: [libnbd PATCH] api: Allow completion callbacks to auto-retire

We have no control over the user piling up lots of commands faster than the server can accept (h->cmds_to_issue), or delaying retiring those batched commands (h->cmds_in_flight), hence our use of O(n) list insertion can be noticeable, since the growth of n can be unbounded from our viewpoint. It's easy enough to track a tail pointer to keep insertion O(1), to match that these two lists

Having the client polling thread perform an O(n) loop over all known in-flight commands after each time the poll woke up is somewhat inefficient, and in a multi-threaded setup requires additional locking beyond libnbd to track the set of known command handles. Better is a way for aio commands to call a notify callback the moment a specific command is ready to complete, and then a separate thread

On 6/29/19 8:28 AM, Eric Blake wrote: > > We also want the client to be aware of any issued/in-flight commands > that failed because they were stranded when the state machine moved to > CLOSED or DEAD. Previously, nbd_aio_command_completed() would never > locate such stranded commands, but adding a common point to fire the > notifier for such commands makes it also possible to

Some clients need to know when it is safe to issue NBD_CMD_DISC, or to decide whether calling poll(POLLIN) will block indefinitely because the server isn't expected to respond. Make this easier to learn by tracking the count of commands we have queued up to send, as well as the count of commands where we are waiting on the server's response. Update tests/aio-parallel* and

Noticed while thinking about the recent threads wondering if we need a more efficient lookup from cookie back to command. Both of these fix bugs, but are tricky enough that I'm posting for review. Eric Blake (2): lib: Decrement in_flight at response, not retirement lib: Do O(1) rather than O(n) queue insertion generator/states-issue-command.c | 2 ++ generator/states-reply.c |

Previously closures had a crude flag which tells if they are persistent or transient. Transient closures (flag = false) last for the lifetime of the currently called libnbd function. Persistent closures had an indefinite lifetime which could last for as long as the handle. In language bindings handling persistent closures was wasteful as we needed to register a "close callback" to

A generic client exploiting multiple in-flight commands should be prepared for out-of-order responses (and should probably ensure that there are no offset/count overlaps between parallel in-flight commands to avoid unspecified disk contents if the server acts on commands in an arbitrary order or even exposing non-atomic splicing effects). But a specific client aware of a specific server's

When retiring a command test for a free_callback associated with their buffer. If there is one call it. This allows language bindings to use this mechanism to automatically decrement a reference to the persistent buffer (note: this patch does not implement this). The vast majority of this change is simply passing around the handle so we have it when we call nbd_internal_free_callback in

On Fri, Sep 11, 2020 at 09:31:11AM -0500, Eric Blake wrote: > As mentioned in commits 176fc4ea and 609c25f0, our original plan in > adding a flags argument to nbd_shutdown was to let us specify > different behaviors at the libnbd level, rather than NBD protocol > flags (for that, the user has nbd_aio_disconnect). But when we later > parameterized OFlags to accept various bitmasks

As mentioned in commits 176fc4ea and 609c25f0, our original plan in adding a flags argument to nbd_shutdown was to let us specify different behaviors at the libnbd level, rather than NBD protocol flags (for that, the user has nbd_aio_disconnect). But when we later parameterized OFlags to accept various bitmasks (commit f891340b), we failed to mark nbd_shutdown as using a different bitmask than

On 7/24/19 7:17 AM, Richard W.M. Jones wrote: > Previously closures had a crude flag which tells if they are > persistent or transient. Transient closures (flag = false) last for > the lifetime of the currently called libnbd function. Persistent > closures had an indefinite lifetime which could last for as long as > the handle. In language bindings handling persistent closures

On 7/25/19 8:07 AM, Richard W.M. Jones wrote: > Previously closures had a crude flag which tells if they are > persistent or transient. Transient closures (flag = false) last for > the lifetime of the currently called libnbd function. Persistent > closures had an indefinite lifetime which could last for as long as > the handle. In language bindings handling persistent closures

The original nbd_aio_* (non-callback) functions are removed and replaced with the renamed callback variants. This is a simple mechanical change to the API: (1) Any existing call to nbd_aio_*_callback can simply be renamed to nbd_aio_* (2) Any existing call to nbd_aio_* must have two extra NULL parameters added before the final flags parameter. In non-C languages, only change (1) is

As discussed in this thread, the parameter is an invitation to write code with race conditions: https://www.redhat.com/archives/libguestfs/2019-July/thread.html#00309 --- docs/libnbd.pod | 6 +- examples/glib-main-loop.c | 10 ++-- examples/strict-structured-reads.c | 2 +- generator/generator | 57

The freeing feature can now be done by associating a free callback with a closure's user_data, so having the valid_flag is no longer necessary and it can be completely removed. This mostly reverts commit 2d9b98e96772e282d51dafac07f16387dadc8afa. --- TODO | 2 - docs/libnbd.pod | 64 ++-------------- examples/glib-main-loop.c |

The definition of functions that take a callback is changed so that the callback and user_data are combined into a single structure, eg: int64_t nbd_aio_pread (struct nbd_handle *h, void *buf, size_t count, uint64_t offset, - int (*completion_callback) (/*..*/), void *user_data, + nbd_completion_callback completion_callback, uint32_t flags); Several

Change the way that we deal with freeing closures in language bindings: * The valid_flag is removed (mostly reverting commit 2d9b98e96772e282d51dafac07f16387dadc8afa). * An extra ‘free’ parameter is added to all callback structures. This is called by the library whenever the closure won't be called again (so the user_data can be freed). This is analogous to valid_flag ==

On 7/15/19 1:38 PM, Richard W.M. Jones wrote: > --- > .gitignore | 1 + > README | 2 + > configure.ac | 9 + > examples/Makefile.am | 22 ++ > examples/glib-main-loop.c | 501 ++++++++++++++++++++++++++++++++++++++ > 5 files changed, 535 insertions(+) > Rough review (since I've never used a glib main loop

For example nbd_set_debug takes a callback function. Previously this was defined explicitly inside the function parameters. This commit defines a new public typedef: typedef int (*nbd_debug_callback) (unsigned valid_flag, void *user_data, const char *context, const char *msg); and then uses the typedef like this: extern int nbd_set_debug_callback

On 7/17/19 8:40 AM, Eric Blake wrote: > > It may be possible in rare situations that the libnbd state machine can > send() the command AND see data ready to recv() (perhaps from a previous > command still in flight), where it ends up read()ing until blocking and > happens to get the server's reply to this command and fire off the > callback, all prior to the