search for: nbd_aio_peek_command_complet

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 |

...t now our state machine remains blocked + * until we notify or otherwise poll it. + */ + if (nbd_kill_subprocess (nbd, SIGCONT) == -1) { + fprintf (stderr, "%s: test failed: nbd_kill_subprocess: %s\n", argv[0], + nbd_get_error ()); + exit (EXIT_FAILURE); + } + if (nbd_aio_peek_command_completed (nbd) != 0) { + fprintf (stderr, "%s: test failed: nbd_aio_peek_command_completed\n", + argv[0]); + exit (EXIT_FAILURE); + } + if (nbd_aio_command_completed (nbd, cookie) != 0) { + fprintf (stderr, "%s: test failed: nbd_aio_command_completed\n", argv[0])...

...> + * until we notify or otherwise poll it. > + */ > + if (nbd_kill_subprocess (nbd, SIGCONT) == -1) { > + fprintf (stderr, "%s: test failed: nbd_kill_subprocess: %s\n", argv[0], > + nbd_get_error ()); > + exit (EXIT_FAILURE); > + } > + if (nbd_aio_peek_command_completed (nbd) != 0) { > + fprintf (stderr, "%s: test failed: nbd_aio_peek_command_completed\n", > + argv[0]); > + exit (EXIT_FAILURE); > + } > + if (nbd_aio_command_completed (nbd, cookie) != 0) { > + fprintf (stderr, "%s: test failed: nbd_aio_comma...

Reverts commit 387cbe67c3db27e8a61117fedb6e7fad76e409ef. --- generator/generator | 18 +++++++++++++++++- lib/handle.c | 28 +++++++++++++++++++++++++++- tests/closure-lifetimes.c | 4 +++- 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/generator/generator b/generator/generator index 2cd83f1..25e4aa5 100755 --- a/generator/generator +++ b/generator/generator

...ble to parse server's pid\n", argv[0]); + goto fail; + } + + /* Issue a read that should not complete yet. */ + if ((handle = nbd_aio_pread (nbd, buf, sizeof buf, 0, 0)) == -1) { + fprintf (stderr, "%s: test failed: nbd_aio_pread\n", argv[0]); + goto fail; + } + if (nbd_aio_peek_command_completed (nbd) != 0) { + fprintf (stderr, "%s: test failed: nbd_aio_peek_command_completed\n", + argv[0]); + goto fail; + } + if (nbd_aio_command_completed (nbd, handle) != 0) { + fprintf (stderr, "%s: test failed: nbd_aio_command_completed\n", argv[0]); + got...

...r *argv[]) { struct nbd_handle *nbd; + int err; + const char *msg; char buf[512]; int64_t handle; char pidfile[] = "/tmp/libnbd-test-disconnectXXXXXX"; @@ -123,16 +125,25 @@ main (int argc, char *argv[]) goto fail; } - /* Proof that the read was stranded */ - if (nbd_aio_peek_command_completed (nbd) != 0) { + /* Detection of the dead server completes all remaining in-flight commands */ + if (nbd_aio_peek_command_completed (nbd) != handle) { fprintf (stderr, "%s: test failed: nbd_aio_peek_command_completed\n", argv[0]); goto fail; } - if (nbd_aio_...

...gt; + *) I like it. Other than the unintended semantic change in patch 4, I think this series is ready to go. We still had another potential API change to squeeze into 0.9.8: Right now, the API is overloading 'command': nbd_connect_command/nbd_kill_command vs. nbd_aio_command_completed/nbd_aio_peek_command_completed Keeping nbd_connect_command may be okay (it takes a command line to create a subprocess), but we may want to rename nbd_kill_command to nbd_kill_child or similar, to make it obvious that it is NOT associated with attempting an early abort of any synchronous nbd_pread or other commands issued to...

Rich recently patched things to generate one man page per function rather than libnbd-api.3 (nice), but in doing so got stumped by a problem with a fresh git clone (automake fails for any 'include' directive that does not already exist). I've figured out how to hack around it, but the hack requires GNU make. We already use GNU make constructs elsewhere (such as $(wildcard)), but

...r == 0) > + cmd->error = ENOTCONN; > + } > + if (prev_cmd) { > + prev_cmd->next = h->cmds_done; > + h->cmds_done = *list; > + *list = NULL; > + } This inserts the list to the head of cmds_done, which breaks its use as a FIFO queue for clients using nbd_aio_peek_command_completed to process messages in server order. I'll post a fix that keeps things in order. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

...a/examples/glib-main-loop.c b/examples/glib-main-loop.c index c633c1d..2230077 100644 --- a/examples/glib-main-loop.c +++ b/examples/glib-main-loop.c @@ -188,6 +188,8 @@ finalize (GSource *sp) DEBUG (source, "finalize"); + assert (nbd_aio_in_flight (source->nbd) == 0); + assert (nbd_aio_peek_command_completed (source->nbd) == -1); nbd_close (source->nbd); } @@ -418,7 +420,7 @@ finished_read (void *vp, int64_t rcookie, int *error) /* Create a writer idle handler. */ g_idle_add (write_data, NULL); - return 0; + return 1; } /* This idle callback schedules a write. */ @@ -507,5 +509...

...unbounded from our viewpoint. It's easy enough to track a tail pointer to keep insertion O(1), to match that these two lists are used as queues; cmds_to_issue is always consumed with O(1) lookup/deletion, and whether cmds_done is used via O(1) or O(n) lookup depends on whether the client uses nbd_aio_peek_command_completed. What's more, we documented that nbd_aio_peek_command_completed can be used to process commands strictly in the order that the server responded - but violated that promise if we strand any commands due to moving to DEAD when there are still outstanding successful commands waiting to be proce...

On Thu, Aug 15, 2019 at 07:01:49AM -0500, Eric Blake wrote: > We still had another potential API change to squeeze into 0.9.8: > Right now, the API is overloading 'command': > nbd_connect_command/nbd_kill_command > vs. > nbd_aio_command_completed/nbd_aio_peek_command_completed > > Keeping nbd_connect_command may be okay (it takes a command line to > create a subprocess), but we may want to rename nbd_kill_command to > nbd_kill_child or similar, to make it obvious that it is NOT associated > with attempting an early abort of any synchronous nbd_pread or...

...n to depend on fully serialized answers. For example, knowing whether a write response arrived prior to a flush response can be used to learn whether the flush covered the write, or whether another flush may be needed. We need both a way to let the client query which command completed first (a new nbd_aio_peek_command_completed) and to treat the completed list as a queue rather than a stack to preserve FIFO order. --- generator/generator | 12 ++++++++++++ generator/states-reply.c | 13 ++++++++++--- lib/aio.c | 13 +++++++++++++ 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/genera...

...aio_zero \ - nbd_aio_block_status \ - nbd_aio_get_fd \ - nbd_aio_get_direction \ - nbd_aio_notify_read \ - nbd_aio_notify_write \ - nbd_aio_is_created \ - nbd_aio_is_connecting \ - nbd_aio_is_ready \ - nbd_aio_is_processing \ - nbd_aio_is_dead \ - nbd_aio_is_closed \ - nbd_aio_command_completed \ - nbd_aio_peek_command_completed \ - nbd_aio_in_flight \ - nbd_connection_state \ - nbd_get_package_name \ - nbd_get_version \ - nbd_kill_command \ - nbd_supports_tls \ - nbd_supports_uri \ - $(NULL) diff --git a/generator/generator b/generator/generator index 257303f..9fe98f5 100755 --- a/generator/generator +++ b/generator/gen...

...aio_zero \ + nbd_aio_block_status \ + nbd_aio_get_fd \ + nbd_aio_get_direction \ + nbd_aio_notify_read \ + nbd_aio_notify_write \ + nbd_aio_is_created \ + nbd_aio_is_connecting \ + nbd_aio_is_ready \ + nbd_aio_is_processing \ + nbd_aio_is_dead \ + nbd_aio_is_closed \ + nbd_aio_command_completed \ + nbd_aio_peek_command_completed \ + nbd_aio_in_flight \ + nbd_connection_state \ + nbd_get_package_name \ + nbd_get_version \ + nbd_kill_subprocess \ + nbd_supports_tls \ + nbd_supports_uri \ + $(NULL) diff --git a/generator/generator b/generator/generator index 6cc06cc..437f432 100755 --- a/generator/generator +++ b/generator/...

...[0].revents & POLLIN) != 0) + nbd_aio_notify_read (conn); + else if ((dir & LIBNBD_AIO_DIRECTION_WRITE) != 0 && + (fds[0].revents & POLLOUT) != 0) + nbd_aio_notify_write (conn); + + /* If a command is ready to retire, retire it. */ + while ((done = nbd_aio_peek_command_completed (conn)) >= 0) { + for (i = 0; i < in_flight; ++i) { + if (handles[i] == done) { + r = nbd_aio_command_completed (conn, handles[i]); + if (r == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + goto error; + } +...

I still need to wire in the use of *_notify functions into nbdkit to prove whether it makes the code any faster or easier to maintain, but at least the added example shows one good use case for the new API. Eric Blake (6): api: Add nbd_aio_in_flight generator: Allow DEAD state actions to run generator: Allow Int64 in callbacks states: Prepare for aio notify callback api: Add new

...U nbd_aio_get_direction@LIBNBD_1.0 U nbd_aio_get_fd@LIBNBD_1.0 U nbd_aio_in_flight@LIBNBD_1.0 U nbd_aio_is_ready@LIBNBD_1.0 U nbd_aio_notify_read@LIBNBD_1.0 U nbd_aio_notify_write@LIBNBD_1.0 U nbd_aio_peek_command_completed@LIBNBD_1.0 U nbd_aio_pread_callback@LIBNBD_1.0 U nbd_aio_pwrite_callback@LIBNBD_1.0 U nbd_close@LIBNBD_1.0 U nbd_create@LIBNBD_1.0 U nbd_get_debug@LIBNBD_1.0 U nbd_get_error@LIBNBD_1.0 Rich.

...t, d, + read_chunk, read_verify, + flags) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + } + + while (nbd_aio_in_flight (nbd) > 0) { + int64_t handle = nbd_aio_peek_command_completed (nbd); + + if (handle == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if (handle == 0) { + if (nbd_poll (nbd, -1) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } +...

...aio_in_flight (nbd) > 0) { if (nbd_aio_is_dead (nbd) || nbd_aio_is_closed (nbd)) { fprintf (stderr, "connection is dead or closed\n"); goto error; @@ -96,23 +92,20 @@ try_deadlock (void *arg) /* If a command is ready to retire, retire it. */ while ((done = nbd_aio_peek_command_completed (nbd)) > 0) { - for (i = 0; i < in_flight; ++i) { + for (i = 0; i < sizeof handles / sizeof handles[0]; ++i) { if (handles[i] == done) { r = nbd_aio_command_completed (nbd, handles[i]); if (r == -1) { fprintf (stderr, "%s\n"...