search for: nbd_unlocked_poll

...it that external event in any other public state. > But that's a bigger patch. > --- > generator/generator | 18 +++++++++++++----- > lib/poll.c | 13 ++++++++++--- > 2 files changed, 23 insertions(+), 8 deletions(-) > > +++ b/lib/poll.c > @@ -57,21 +57,28 @@ nbd_unlocked_poll (struct nbd_handle *h, int timeout) > set_error (errno, "poll"); > return -1; > } I also need to squash in this, to fix our use of an uninitialized variable when nbd_aio_get_direction returns 0 (such as when we are already DEAD): diff --git i/lib/poll.c w/lib/poll....

While working on a new test of what happens when the server goes away while commands are in flight, I managed to hit a race where I hit death from SIGPIPE instead of a clean transition to the DEAD state. I also found myself wanting to use nbd_poll from the test, but with a way to distinguish between the state machine progressing vs. hanging. Eric Blake (2): socket: Avoid SIGPIPE where possible

...r here, keep the error set when + * the connection died. + */ + return -1; + } + if (!nbd_unlocked_aio_is_ready (h->conns[i])) { + all_connected = false; break; } } - if (connected) + if (all_connected) break; if (nbd_unlocked_poll (h, -1) == -1) @@ -59,7 +69,7 @@ wait_one_connected (struct nbd_handle *h) } /* For all connections in the CREATED state, start connecting them to - * a Unix domain socket. Wait until at least one is in the READY + * a Unix domain socket. Wait until all connections are in the READY * state....

...r here, keep the error set when + * the connection died. + */ + return -1; + } + if (!nbd_unlocked_aio_is_ready (h->conns[i])) { + all_connected = false; break; } } - if (connected) + if (all_connected) break; if (nbd_unlocked_poll (h, -1) == -1) @@ -59,7 +69,7 @@ wait_one_connected (struct nbd_handle *h) } /* For all connections in the CREATED state, start connecting them to - * a Unix domain socket. Wait until at least one is in the READY + * a Unix domain socket. Wait until all connections are in the READY * state....

...rt_string (h->state)); + nbd_internal_state_short_string (get_state (h))); return -1; } static int wait_until_connected (struct nbd_handle *h) { - while (nbd_internal_is_state_connecting (h->state)) { + while (nbd_internal_is_state_connecting (get_state (h))) { if (nbd_unlocked_poll (h, -1) == -1) return -1; } diff --git a/lib/disconnect.c b/lib/disconnect.c index 355a1c9..423edaf 100644 --- a/lib/disconnect.c +++ b/lib/disconnect.c @@ -29,14 +29,14 @@ int nbd_unlocked_shutdown (struct nbd_handle *h) { - if (nbd_internal_is_state_ready (h->state) || - nbd...

...keep the error set when * the connection died. */ @@ -62,7 +62,7 @@ error_unless_ready (struct nbd_handle *h) static int wait_until_connected (struct nbd_handle *h) { - while (nbd_unlocked_aio_is_connecting (h)) { + while (nbd_internal_is_state_connecting (h->state)) { if (nbd_unlocked_poll (h, -1) == -1) return -1; } diff --git a/lib/disconnect.c b/lib/disconnect.c index 6695e59..355a1c9 100644 --- a/lib/disconnect.c +++ b/lib/disconnect.c @@ -29,15 +29,14 @@ int nbd_unlocked_shutdown (struct nbd_handle *h) { - - if (nbd_unlocked_aio_is_ready (h) || - nbd_unlocked_...

..._state (h))); + nbd_internal_state_short_string (get_next_state (h))); return -1; } static int wait_until_connected (struct nbd_handle *h) { - while (nbd_internal_is_state_connecting (get_state (h))) { + while (nbd_internal_is_state_connecting (get_next_state (h))) { if (nbd_unlocked_poll (h, -1) == -1) return -1; } diff --git a/lib/disconnect.c b/lib/disconnect.c index 423edaf..95e9a37 100644 --- a/lib/disconnect.c +++ b/lib/disconnect.c @@ -29,14 +29,14 @@ int nbd_unlocked_shutdown (struct nbd_handle *h) { - if (nbd_internal_is_state_ready (get_state (h)) || - n...

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

Patch 1/6 was posted before and I didn't change it: https://www.redhat.com/archives/libguestfs/2019-May/thread.html#00134 That doesn't necessarily mean I shouldn't change it, I'm posting it again because the other patches depend on it. The main change in this series is we add three new API functions: nbd_aio_is_created - connection has just been created

nbd_connect_unix|tcp had a tricky failure case. This is a consequence of allowing callers to mix synchronous and asynchronous calls, with multi-conn thrown into the mix. I think the new behaviour proposed here is better. We could do with a better way of classifying the state of connections, such as are they connectING. Rich.

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

..._state (h))); + nbd_internal_state_short_string (get_next_state (h))); return -1; } static int wait_until_connected (struct nbd_handle *h) { - while (nbd_internal_is_state_connecting (get_state (h))) { + while (nbd_internal_is_state_connecting (get_next_state (h))) { if (nbd_unlocked_poll (h, -1) == -1) return -1; } diff --git a/lib/disconnect.c b/lib/disconnect.c index 423edaf..95e9a37 100644 --- a/lib/disconnect.c +++ b/lib/disconnect.c @@ -29,14 +29,14 @@ int nbd_unlocked_shutdown (struct nbd_handle *h) { - if (nbd_internal_is_state_ready (get_state (h)) || - n...

..._state (h))); + nbd_internal_state_short_string (get_next_state (h))); return -1; } static int wait_until_connected (struct nbd_handle *h) { - while (nbd_internal_is_state_connecting (get_state (h))) { + while (nbd_internal_is_state_connecting (get_next_state (h))) { if (nbd_unlocked_poll (h, -1) == -1) return -1; } diff --git a/lib/disconnect.c b/lib/disconnect.c index 423edaf..95e9a37 100644 --- a/lib/disconnect.c +++ b/lib/disconnect.c @@ -29,14 +29,14 @@ int nbd_unlocked_shutdown (struct nbd_handle *h) { - if (nbd_internal_is_state_ready (get_state (h)) || - n...

...lt;nbd_aio_notify_read>. Note that once libnbd reaches C<nbd_aio_is_ready>, this direction is returned even before a command is issued via C<nbd_aio_pwrite> and diff --git a/lib/poll.c b/lib/poll.c index 982b172..d356afe 100644 --- a/lib/poll.c +++ b/lib/poll.c @@ -57,21 +57,28 @@ nbd_unlocked_poll (struct nbd_handle *h, int timeout) set_error (errno, "poll"); return -1; } + if (r == 0) + return 0; /* POLLIN and POLLOUT might both be set. However we shouldn't call * both nbd_aio_notify_read and nbd_aio_notify_write at this time * since the first mig...

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

...ill didn't seem to get rid of the child process even though the poll() was definitely seeing the fd closing, until I remembered that kill(pid,0) to zombie processes succeeds until you wait() or alter SIGCHLD to specifically prevent zombies. And debugging the now-fixed uninitialized variable in nbd_unlocked_poll added to the mix. I'm not 100% sure the test is portable to non-Linux - I guess we'll eventually find out when someone worries about a BSD port of libnbd. --- Applies on top of my nbd_poll return value semantic change. I'm open to bike-shedding on the test name. .gitignore...

...pt.c index 306a2e9..6243553 100644 --- a/lib/opt.c +++ b/lib/opt.c @@ -39,3 +39,37 @@ nbd_unlocked_get_opt_mode (struct nbd_handle *h) { return h->opt_mode; } + +static int +wait_for_option (struct nbd_handle *h) +{ + while (nbd_internal_is_state_connecting (get_next_state (h))) { + if (nbd_unlocked_poll (h, -1) == -1) + return -1; + } + + return 0; +} + +/* Issue NBD_OPT_ABORT and wait for the state change. */ +int +nbd_unlocked_opt_abort (struct nbd_handle *h) +{ + int r = nbd_unlocked_aio_opt_abort (h); + + if (r == -1) + return r; + + return wait_for_option (h); +} + +/* Issue NBD_...

This is the bare minimum needed to allow the user to take control over the rest of option negotiating. This patch adds several new API: nbd_set_opt_mode() - called during Created to enable the new mode nbd_get_opt_mode() - query whether opt mode is enabled nbd_opt_go() - used in Negotiating state to attempt to use export nbd_opt_abort() - used in Negotiating state to skip Connected state

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):