search for: nbd_internal_is_state_ready

...enum state state = get_state (h);\n"; let tests = List.map ( function diff --git a/lib/connect.c b/lib/connect.c index 63d2234..b889f80 100644 --- a/lib/connect.c +++ b/lib/connect.c @@ -38,16 +38,16 @@ static int error_unless_ready (struct nbd_handle *h) { - if (nbd_internal_is_state_ready (h->state)) + if (nbd_internal_is_state_ready (get_state (h))) return 0; /* Why did it fail? */ - if (nbd_internal_is_state_closed (h->state)) { + if (nbd_internal_is_state_closed (get_state (h))) { set_error (0, "connection is closed"); return -1; } - i...

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

...state (h);\n"; + pr "\n"; + let tests = + List.map ( + function + | Created -> "nbd_internal_is_state_created (state)" + | Connecting -> "nbd_internal_is_state_connecting (state)" + | Connected -> "nbd_internal_is_state_ready (state) || nbd_internal_is_state_processing (state)" + | Closed -> "nbd_internal_is_state_closed (state)" + | Dead -> "nbd_internal_is_state_dead (state)" + ) permitted_states in + pr " if (!(%s)) {\n" (String.concat " ||\...

...| Closed -> "nbd_aio_is_closed (h)" - | Dead -> "nbd_aio_is_dead (h)" + | Created -> "nbd_internal_is_state_created (state)" + | Connecting -> "nbd_internal_is_state_connecting (state)" + | Connected -> "nbd_internal_is_state_ready (state) || nbd_internal_is_state_processing (state)" + | Closed -> "nbd_internal_is_state_closed (state)" + | Dead -> "nbd_internal_is_state_dead (state)" ) permitted_states in pr " if (!(%s)) {\n" (String.concat " ||\...

...n" + ); pr " return ret;\n"; pr "}\n"; pr "\n"; diff --git a/lib/connect.c b/lib/connect.c index b889f80..4e3141f 100644 --- a/lib/connect.c +++ b/lib/connect.c @@ -38,16 +38,16 @@ static int error_unless_ready (struct nbd_handle *h) { - if (nbd_internal_is_state_ready (get_state (h))) + if (nbd_internal_is_state_ready (get_next_state (h))) return 0; /* Why did it fail? */ - if (nbd_internal_is_state_closed (get_state (h))) { + if (nbd_internal_is_state_closed (get_next_state (h))) { set_error (0, "connection is closed"); return -...

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

...n" + ); pr " return ret;\n"; pr "}\n"; pr "\n"; diff --git a/lib/connect.c b/lib/connect.c index b889f80..4e3141f 100644 --- a/lib/connect.c +++ b/lib/connect.c @@ -38,16 +38,16 @@ static int error_unless_ready (struct nbd_handle *h) { - if (nbd_internal_is_state_ready (get_state (h))) + if (nbd_internal_is_state_ready (get_next_state (h))) return 0; /* Why did it fail? */ - if (nbd_internal_is_state_closed (get_state (h))) { + if (nbd_internal_is_state_closed (get_next_state (h))) { set_error (0, "connection is closed"); return -...

...;nbd_aio_is_closed (h)" > - | Dead -> "nbd_aio_is_dead (h)" > + | Created -> "nbd_internal_is_state_created (state)" > + | Connecting -> "nbd_internal_is_state_connecting (state)" > + | Connected -> "nbd_internal_is_state_ready (state) || nbd_internal_is_state_processing (state)" Yes, this fixes the race: this code is executed outside the lock, so it must read h->state exactly once before using it in multiple spots. > +++ b/lib/connect.c > @@ -38,16 +38,16 @@ > static int > error_unless_ready (stru...

...| 5 +++++ tests/errors.c | 9 ++++++--- 4 files changed, 17 insertions(+), 5 deletions(-) diff --git a/lib/disconnect.c b/lib/disconnect.c index 95e9a37..53de386 100644 --- a/lib/disconnect.c +++ b/lib/disconnect.c @@ -29,8 +29,9 @@ int nbd_unlocked_shutdown (struct nbd_handle *h) { - if (nbd_internal_is_state_ready (get_next_state (h)) || - nbd_internal_is_state_processing (get_next_state (h))) { + if (!h->disconnect_request && + (nbd_internal_is_state_ready (get_next_state (h)) || + nbd_internal_is_state_processing (get_next_state (h)))) { if (nbd_unlocked_aio_disconnect (h,...

...ern int nbd_internal_set_block_size (struct nbd_handle *h, uint32_t min, /* is-state.c */ extern bool nbd_internal_is_state_created (enum state state); extern bool nbd_internal_is_state_connecting (enum state state); +extern bool nbd_internal_is_state_negotiating (enum state state); extern bool nbd_internal_is_state_ready (enum state state); extern bool nbd_internal_is_state_processing (enum state state); extern bool nbd_internal_is_state_dead (enum state state); diff --git a/generator/API.ml b/generator/API.ml index 82fdf75..ff7f4c1 100644 --- a/generator/API.ml +++ b/generator/API.ml @@ -90,6 +90,7 @@ and flags...

...; PRIu32, flags); return -1; } + /* If IMMEDIATE, abort any commands that have not yet had any bytes + * sent to the server, so that NBD_CMD_DISC will be first in line. + */ + if (flags & LIBNBD_SHUTDOWN_IMMEDIATE) { + struct command **cmd = &h->cmds_to_issue; + if (!nbd_internal_is_state_ready (get_next_state (h))) { + assert (*cmd); + h->cmds_to_issue_tail = *cmd; + cmd = &((*cmd)->next); + } + nbd_internal_abort_commands (h, cmd); + } + if (!h->disconnect_request && (nbd_internal_is_state_ready (get_next_state (h)) || nbd_int...

...results, and will eventually learn that the machine has + * moved on to DEAD at that time. */ h->in_flight++; if (h->cmds_to_issue != NULL) { @@ -240,7 +246,7 @@ nbd_internal_command_common (struct nbd_handle *h, h->cmds_to_issue = h->cmds_to_issue_tail = cmd; if (nbd_internal_is_state_ready (get_next_state (h)) && nbd_internal_run (h, cmd_issue) == -1) - return -1; + nbd_internal_debug (h, "command queued, ignoring state machine failure"); } return cmd->cookie; -- 2.20.1

...} > > + /* If IMMEDIATE, abort any commands that have not yet had any bytes > + * sent to the server, so that NBD_CMD_DISC will be first in line. > + */ > + if (flags & LIBNBD_SHUTDOWN_IMMEDIATE) { > + struct command **cmd = &h->cmds_to_issue; > + if (!nbd_internal_is_state_ready (get_next_state (h))) { > + assert (*cmd); > + h->cmds_to_issue_tail = *cmd; > + cmd = &((*cmd)->next); > + } > + nbd_internal_abort_commands (h, cmd); > + } > + > if (!h->disconnect_request && > (nbd_internal_is_state_...

...neral Public > @@ -58,6 +58,12 @@ nbd_internal_is_state_connecting (enum state state) > return is_connecting_group (group); > } > > +bool > +nbd_internal_is_state_negotiating (enum state state) > +{ > + return state == STATE_NEGOTIATING; > +} > + > bool > nbd_internal_is_state_ready (enum state state) > { When I was reading the rest of the patch I thought it would have been easier to review if the addition of the new state was in a separate commit. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualizat...

...prev_cmd = prev_cmd->next; - prev_cmd->next = cmd; + h->cmds_to_issue_tail = h->cmds_to_issue_tail->next = cmd; } else { - h->cmds_to_issue = cmd; + assert (h->cmds_to_issue_tail == NULL); + h->cmds_to_issue = h->cmds_to_issue_tail = cmd; if (nbd_internal_is_state_ready (get_next_state (h)) && nbd_internal_run (h, cmd_issue) == -1) return -1; -- 2.20.1

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 |

...otocol. Doable. Probably even something we could separate out, to keep the meat of this patch more focused. >> +bool >> +nbd_internal_is_state_negotiating (enum state state) >> +{ >> + return state == STATE_NEGOTIATING; >> +} >> + >> bool >> nbd_internal_is_state_ready (enum state state) >> { > > When I was reading the rest of the patch I thought it would have been > easier to review if the addition of the new state was in a separate > commit. Sure; I'll try and make that split for v2, as well as adding aio_opt_*. -- Eric Blake, Princi...

Well, I'm not quite done (I still want to get nbdinfo to work on a single nbd connection for all cases when reading the heads of the file is not required), but I'm happy with patches 1-11, and 12-13 show where I'm headed for getting NBD_OPT_INFO to work. Posting now to see if some of the earlier patches are ready to commit while I continue working on the latter half. Eric Blake (13):