search for: nbd_internal_run

...>cmds_to_issue; - conn->cmds_to_issue = cmd; /* This will leave the command on the in-flight list. Is this a * problem? Probably it isn't. If it is, we could add a flag to * the command struct to tell SEND_REQUEST not to add it to the * in-flight list. */ - return nbd_internal_run (conn->h, conn, cmd_issue); + return 0; } diff --git a/lib/internal.h b/lib/internal.h index 3f2b729..67bd52a 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -265,6 +265,12 @@ extern void nbd_internal_set_last_error (int errnum, char *error); extern int nbd_internal_errno_of_nbd_error (ui...

Our construction of struct command_in_flight was ad hoc; most parameters were set in command_common(), but extent callbacks were done after the fact, and NBD_CMD_DISC was open-coding things. Furthermore, every caller was triggering nbd_internal_run() for the cmd_issue event; doing that in a central place makes it easier for the next patch to improve that logic without duplicating the fix at each caller. Fix these things by renaming the function to nbd_internal_command_common, which is necessary for exporting it, and by adding parameters and...

...ter this morning. I believe they will let you get rid of the reached_ready flag, as well as making this patch more correct because you probably want to avoid issuing commands on a connection which is closed or dead. You'd use a test like: if (nbd_unlocked_aio_is_ready (conn)) { // call nbd_internal_run } else if (nbd_unlocked_aio_is_processing (conn)) { // can't call nbd_internal_run because we're in the // wrong state, but we can enqueue the command and it // will be processed next time we get to READY } else { // this is an error } > @@ -296,9 +297,24 @@ co...

On v1, we discussed whether cmds_to_issue needed to be a list, since it never had more than one element. I played with the idea of making it a list, and allowing the client to queue up new commands regardless of whether the state machine is currently in READY. I also polished up the tmp demo into a bit more full-fledged example file, worth including since it also let me discover a hard-to-hit race

On Tue, Jul 30, 2019 at 11:21:25AM -0500, Eric Blake wrote: > On 7/30/19 10:36 AM, Richard W.M. Jones wrote: > > +When the command completes, C<callback> > > will be invoked as described in L<libnbd(3)/Completion callbacks>. > > + > > Do we need wording anywhere (probably in the .pod, so we only state it > once) that mentions that the callback routine is

Since v2: - rebase to Rich's new API calls - more refactoring in patch 1 (retitled) - new patches 3 and 4 - fix data corruption in patch 6 (was 4) - more tweaks to the reproducer example (including using new API from 3) Eric Blake (7): lib: Refactor command_common() to do more common work commands: Allow for a command queue commands: Expose FIFO ordering of server completions

Although rare, it is possible that nbd_internal_run(cmd_issue) will report failure (perhaps because the server disconnected at the wrong time), even though we have queued the user's command. If we have a valid cookie, we MUST return it for the sake of users that will be calling nbd_aio_command_complete, as otherwise the user has no idea what co...

...@ -299,10 +308,23 @@ nbd_internal_command_common (struct nbd_connection *conn, if (conn->structured_replies && cmd->data && type == NBD_CMD_READ) memset (cmd->data, 0, cmd->count); - cmd->next = conn->cmds_to_issue; - conn->cmds_to_issue = cmd; - if (nbd_internal_run (conn->h, conn, cmd_issue) == -1) - return -1; + /* Add the command to the end of the queue. Kick the state machine + * if there is no other command being processed, otherwise, it will + * be handled automatically on a future cycle around to READY. + */ + if (conn->cmds_to_issue !=...

...EST_SIZE. */ @@ -296,9 +297,24 @@ command_common (struct nbd_connection *conn, if (conn->structured_replies && cmd->data && type == NBD_CMD_READ) memset (cmd->data, 0, cmd->count); - cmd->next = conn->cmds_to_issue; - conn->cmds_to_issue = cmd; - if (nbd_internal_run (conn->h, conn, cmd_issue) == -1) + /* If we have not reached READY yet, sending the event lets the + * state machine fail for requesting a command too early. If there + * are no queued commands and we are already in state READY, send an + * event to kick the state machine. In all other c...

As promised in my earlier thread on libnbd completion callback question. Eric Blake (2): generator: Refactor handling of closures in unlocked functions generator: Free closures on failure docs/libnbd.pod | 2 +- generator/C.ml | 48 +++++++++++------ generator/C.mli | 1 + lib/debug.c | 7 +-- lib/opt.c | 31 ++++++-----

...eturn -1; + SET_CALLBACK_TO_NULL (*list); if (wait_for_option (h) == -1) return -1; if (s.err) { @@ -172,6 +173,7 @@ nbd_unlocked_aio_opt_go (struct nbd_handle *h, { h->opt_current = NBD_OPT_GO; h->opt_cb.completion = *complete; + SET_CALLBACK_TO_NULL (*complete); if (nbd_internal_run (h, cmd_issue) == -1) debug (h, "option queued, ignoring state machine failure"); @@ -190,6 +192,7 @@ nbd_unlocked_aio_opt_info (struct nbd_handle *h, h->opt_current = NBD_OPT_INFO; h->opt_cb.completion = *complete; + SET_CALLBACK_TO_NULL (*complete); if (nbd_intern...

...int nbd_unlocked_aio_opt_go (struct nbd_handle *h, - nbd_completion_callback complete) + nbd_completion_callback *complete) { h->opt_current = NBD_OPT_GO; - h->opt_cb.completion = complete; + h->opt_cb.completion = *complete; if (nbd_internal_run (h, cmd_issue) == -1) debug (h, "option queued, ignoring state machine failure"); @@ -181,7 +181,7 @@ nbd_unlocked_aio_opt_go (struct nbd_handle *h, /* Issue NBD_OPT_INFO without waiting. */ int nbd_unlocked_aio_opt_info (struct nbd_handle *h, - nbd_compl...

This isn't quite finished because not all of the tests or examples have been updated, but it demonstrates an idea: Should we forget about the concept of having multiple connections managed under a single handle? In this patch there is a single ‘struct nbd_handle *’ which manages a single state machine and connection (and therefore no nbd_connection). To connect to a multi-conn server you must

...unlocked_aio_connect_vsock (h, cid, port) == -1) + return -1; + + return wait_until_connected (h); +} + /* Connect to a TCP port. */ int nbd_unlocked_connect_tcp (struct nbd_handle *h, @@ -388,6 +398,16 @@ nbd_unlocked_aio_connect_unix (struct nbd_handle *h, const char *unixsocket) return nbd_internal_run (h, cmd_connect_unix); } +int +nbd_unlocked_aio_connect_vsock (struct nbd_handle *h, + uint32_t cid, uint32_t port) +{ + h->svm_cid = cid; + h->svm_port = port; + + return nbd_internal_run (h, cmd_connect_vsock); +} + int nbd_unlocked_aio_connect_tcp (st...

...f (nbd_unlocked_aio_connect_socket (h, sock) == -1) + return -1; + + return wait_until_connected (h); +} + /* Connect to a local command. */ int nbd_unlocked_connect_command (struct nbd_handle *h, char **argv) @@ -399,6 +410,42 @@ nbd_unlocked_aio_connect_tcp (struct nbd_handle *h, return nbd_internal_run (h, cmd_connect_tcp); } +int +nbd_unlocked_aio_connect_socket (struct nbd_handle *h, int sock) +{ + int flags; + + /* Set O_NONBLOCK on the file and FD_CLOEXEC on the file descriptor. + * We can't trust that the calling process did either of these. + */ + flags = fcntl (sock, F_GETFL,...

This works. I'm in the middle of testing whether there is any noticable benefit. Rich.

...argv) == -1) + return -1; + + return wait_until_connected (h); +} + int nbd_unlocked_aio_connect (struct nbd_handle *h, const struct sockaddr *addr, socklen_t len) @@ -405,3 +415,21 @@ nbd_unlocked_aio_connect_command (struct nbd_handle *h, char **argv) return nbd_internal_run (h, cmd_connect_command); } + +int +nbd_unlocked_aio_connect_socket_activation (struct nbd_handle *h, char **argv) +{ + char **copy; + + copy = nbd_internal_copy_string_list (argv); + if (!copy) { + set_error (errno, "copy_string_list"); + return -1; + } + + if (h->argv) +...

I need to think about this patch series a bit more, but it does at least pass the tests. Rich.

...ibnbd_la_SOURCES = \ flags.c \ handle.c \ internal.h \ + is-state.c \ nbd-protocol.h \ poll.c \ protocol.c \ diff --git a/lib/aio.c b/lib/aio.c index a129af2..38e0318 100644 --- a/lib/aio.c +++ b/lib/aio.c @@ -48,84 +48,6 @@ nbd_unlocked_aio_notify_write (struct nbd_handle *h) return nbd_internal_run (h, notify_write); } -/* NB: is_locked = false, may_set_error = false. */ -int -nbd_unlocked_aio_is_created (struct nbd_handle *h) -{ - return h->state == STATE_START; -} - -static int -is_connecting_group (enum state_group group) -{ - switch (group) { - case GROUP_TOP: - return 0; - c...

...D_OPT_GO (or NBD_OPT_EXPORT_NAME) without waiting. */ int -nbd_unlocked_aio_opt_go (struct nbd_handle *h) +nbd_unlocked_aio_opt_go (struct nbd_handle *h, + nbd_completion_callback complete) { h->current_opt = NBD_OPT_GO; + h->opt_cb.completion = complete; if (nbd_internal_run (h, cmd_issue) == -1) debug (h, "option queued, ignoring state machine failure"); -- 2.28.0