search for: prev_cmd

...it a/generator/states-reply.c b/generator/states-reply.c index 93f6cda..45362d4 100644 --- a/generator/states-reply.c +++ b/generator/states-reply.c @@ -103,13 +103,20 @@ } assert (cmd != NULL); - /* Move it to the cmds_done list. */ + /* Move it to the end of the cmds_done list. */ if (prev_cmd != NULL) prev_cmd->next = cmd->next; else conn->cmds_in_flight = cmd->next; - cmd->next = conn->cmds_done; - conn->cmds_done = cmd; + cmd->next = NULL; + if (conn->cmds_done) { + prev_cmd = conn->cmds_done; + while (prev_cmd->next) + prev...

...tates-reply.c b/generator/states-reply.c index 4b22c39..1a0c149 100644 --- a/generator/states-reply.c +++ b/generator/states-reply.c @@ -171,14 +171,12 @@ save_reply_state (struct nbd_handle *h) else h->cmds_in_flight = cmd->next; cmd->next = NULL; - if (h->cmds_done) { - prev_cmd = h->cmds_done; - while (prev_cmd->next) - prev_cmd = prev_cmd->next; - prev_cmd->next = cmd; + if (h->cmds_done_tail != NULL) + h->cmds_done_tail = h->cmds_done_tail->next = cmd; + else { + assert (h->cmds_done == NULL); + h->cmds_done = h->cm...

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 |

...h) > return 0; /* move to next state */ > } > > +/* Forcefully fail any remaining in-flight commands in list */ > +void abort_commands (struct nbd_handle *h, > + struct command_in_flight **list) > +{ > + struct command_in_flight *prev_cmd, *cmd; > + > + for (cmd = *list, prev_cmd = NULL; > + cmd != NULL; > + prev_cmd = cmd, cmd = cmd->next) { > + if (cmd->cb.notify && cmd->type != NBD_CMD_DISC) { > + int error = cmd->error ? cmd->error : ENOTCONN; > + > + if (c...

...it a/generator/states-reply.c b/generator/states-reply.c index 93f6cda..45362d4 100644 --- a/generator/states-reply.c +++ b/generator/states-reply.c @@ -103,13 +103,20 @@ } assert (cmd != NULL); - /* Move it to the cmds_done list. */ + /* Move it to the end of the cmds_done list. */ if (prev_cmd != NULL) prev_cmd->next = cmd->next; else conn->cmds_in_flight = cmd->next; - cmd->next = conn->cmds_done; - conn->cmds_done = cmd; + cmd->next = NULL; + if (conn->cmds_done) { + prev_cmd = conn->cmds_done; + while (prev_cmd->next) + prev...

...rnal.h" @@ -247,7 +248,15 @@ nbd_internal_command_common (struct nbd_connection *conn, uint64_t offset, uint64_t count, void *data, int64_t id, extent_fn extent) { - struct command_in_flight *cmd; + struct command_in_flight *cmd, *prev_cmd; + + if (!nbd_unlocked_aio_is_ready (conn) && + !nbd_unlocked_aio_is_processing (conn)) { + set_error (0, "command request %s is invalid in state %s", + nbd_internal_name_of_nbd_cmd (type), + nbd_internal_state_short_string (conn->state)); +...

...7587e9..ebd4ff9 100644 --- a/lib/rw.c +++ b/lib/rw.c @@ -246,7 +246,8 @@ command_common (struct nbd_connection *conn, uint16_t flags, uint16_t type, uint64_t offset, uint64_t count, void *data) { - struct command_in_flight *cmd; + struct command_in_flight *cmd, *prev_cmd; + bool event = !conn->reached_ready; switch (type) { /* Commands which send or receive data are limited to MAX_REQUEST_SIZE. */ @@ -296,9 +297,24 @@ command_common (struct nbd_connection *conn, if (conn->structured_replies && cmd->data && type == NBD_CMD_READ)...

...1 @@ send_from_wbuf (struct nbd_handle *h) return 0; /* move to next state */ } +/* Forcefully fail any remaining in-flight commands in list */ +void abort_commands (struct nbd_handle *h, + struct command_in_flight **list) +{ + struct command_in_flight *prev_cmd, *cmd; + + for (cmd = *list, prev_cmd = NULL; + cmd != NULL; + prev_cmd = cmd, cmd = cmd->next) { + if (cmd->cb.notify && cmd->type != NBD_CMD_DISC) { + int error = cmd->error ? cmd->error : ENOTCONN; + + if (cmd->cb.notify (cmd->cb.opaque, cmd...

This might not be the final solution, but it certainly seems to solve a deadlock for me that I could trigger by using 'nbdkit --filter=noparallel memory 512k' and calling nbd_aio_pread for a request larger than 256k (enough for the Linux kernel to block the server until libnbd read()s), immediately followed by nbd_aio_pwrite for a request larger than 256k (enough to block libnbd until the

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

...1,7 @@ nbd_internal_command_common (struct nbd_handle *h, * be handled automatically on a future cycle around to READY. */ if (h->cmds_to_issue != NULL) { - assert (nbd_internal_is_state_processing (h->state)); + assert (nbd_internal_is_state_processing (get_state (h))); prev_cmd = h->cmds_to_issue; while (prev_cmd->next) prev_cmd = prev_cmd->next; @@ -209,7 +209,7 @@ nbd_internal_command_common (struct nbd_handle *h, } else { h->cmds_to_issue = cmd; - if (nbd_internal_is_state_ready (h->state) && + if (nbd_internal_is_sta...

...@@ -201,7 +201,7 @@ nbd_internal_command_common (struct nbd_handle *h, * be handled automatically on a future cycle around to READY. */ if (h->cmds_to_issue != NULL) { - assert (nbd_unlocked_aio_is_processing (h)); + assert (nbd_internal_is_state_processing (h->state)); prev_cmd = h->cmds_to_issue; while (prev_cmd->next) prev_cmd = prev_cmd->next; @@ -209,7 +209,7 @@ nbd_internal_command_common (struct nbd_handle *h, } else { h->cmds_to_issue = cmd; - if (nbd_unlocked_aio_is_ready (h) && + if (nbd_internal_is_state_ready (h-...

...nbd_internal_command_common (struct nbd_handle *h, * be handled automatically on a future cycle around to READY. */ if (h->cmds_to_issue != NULL) { - assert (nbd_internal_is_state_processing (get_state (h))); + assert (nbd_internal_is_state_processing (get_next_state (h))); prev_cmd = h->cmds_to_issue; while (prev_cmd->next) prev_cmd = prev_cmd->next; @@ -209,7 +209,7 @@ nbd_internal_command_common (struct nbd_handle *h, } else { h->cmds_to_issue = cmd; - if (nbd_internal_is_state_ready (get_state (h)) && + if (nbd_internal_is_s...

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

...THE LATEST ERROR MESSAGE IN THE THREAD diff --git a/generator/states-reply.c b/generator/states-reply.c index 1a0c149..6ea43d5 100644 --- a/generator/states-reply.c +++ b/generator/states-reply.c @@ -149,6 +149,7 @@ save_reply_state (struct nbd_handle *h) REPLY.FINISH_COMMAND: struct command *prev_cmd, *cmd; uint64_t cookie; + bool retire; /* NB: This works for both simple and structured replies because the * handle (our cookie) is stored at the same offset. @@ -164,6 +165,23 @@ save_reply_state (struct nbd_handle *h) assert (cmd != NULL); assert (h->reply_cmd == cmd); h-...

...nbd_internal_command_common (struct nbd_handle *h, * be handled automatically on a future cycle around to READY. */ if (h->cmds_to_issue != NULL) { - assert (nbd_internal_is_state_processing (get_state (h))); + assert (nbd_internal_is_state_processing (get_next_state (h))); prev_cmd = h->cmds_to_issue; while (prev_cmd->next) prev_cmd = prev_cmd->next; @@ -209,7 +209,7 @@ nbd_internal_command_common (struct nbd_handle *h, } else { h->cmds_to_issue = cmd; - if (nbd_internal_is_state_ready (get_state (h)) && + if (nbd_internal_is_s...

...nbd_internal_command_common (struct nbd_handle *h, * be handled automatically on a future cycle around to READY. */ if (h->cmds_to_issue != NULL) { - assert (nbd_internal_is_state_processing (get_state (h))); + assert (nbd_internal_is_state_processing (get_next_state (h))); prev_cmd = h->cmds_to_issue; while (prev_cmd->next) prev_cmd = prev_cmd->next; @@ -209,7 +209,7 @@ nbd_internal_command_common (struct nbd_handle *h, } else { h->cmds_to_issue = cmd; - if (nbd_internal_is_state_ready (get_state (h)) && + if (nbd_internal_is_s...

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

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

v1 was here: https://www.redhat.com/archives/libguestfs/2019-June/thread.html#00055 v2 was here: https://www.redhat.com/archives/libguestfs/2019-June/thread.html#00067 v3: - Fix atomicly -> atomically in commit message. - Fix a comment. - Fix TOCTTOU: There is now an inline function generated called <name>_is_permitted_state, and this is called twice, first outside the