search for: nbd_unlocked_aio_is_dead

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

...d_unlocked_aio_is_ready (h)) + if (nbd_internal_is_state_ready (h->state)) return 0; /* Why did it fail? */ - if (nbd_unlocked_aio_is_closed (h)) { + if (nbd_internal_is_state_closed (h->state)) { set_error (0, "connection is closed"); return -1; } - if (nbd_unlocked_aio_is_dead (h)) + if (nbd_internal_is_state_dead (h->state)) /* Don't set the error here, 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_ai...

...nected? */ for (i = 0; i < h->multi_conn; ++i) { - if (nbd_unlocked_aio_is_ready (h->conns[i])) { - connected = true; + if (nbd_unlocked_aio_is_closed (h->conns[i])) { + set_error (0, "connection is closed"); + return -1; + } + if (nbd_unlocked_aio_is_dead (h->conns[i])) { + /* Don't set the error 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 (conne...

...nected? */ for (i = 0; i < h->multi_conn; ++i) { - if (nbd_unlocked_aio_is_ready (h->conns[i])) { - connected = true; + if (nbd_unlocked_aio_is_closed (h->conns[i])) { + set_error (0, "connection is closed"); + return -1; + } + if (nbd_unlocked_aio_is_dead (h->conns[i])) { + /* Don't set the error 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 (conne...

...- } -} - -/* NB: is_locked = false, may_set_error = false. */ -int -nbd_unlocked_aio_is_processing (struct nbd_handle *h) -{ - enum state_group group = nbd_internal_state_group (h->state); - - return is_processing_group (group); -} - -/* NB: is_locked = false, may_set_error = false. */ -int -nbd_unlocked_aio_is_dead (struct nbd_handle *h) -{ - return h->state == STATE_DEAD; -} - -/* NB: is_locked = false, may_set_error = false. */ -int -nbd_unlocked_aio_is_closed (struct nbd_handle *h) -{ - return h->state == STATE_CLOSED; -} - int nbd_unlocked_aio_command_completed (struct nbd_handle *h,...

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

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.

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

...s_state_ready (h->state)) > return 0; > > /* Why did it fail? */ > - if (nbd_unlocked_aio_is_closed (h)) { > + if (nbd_internal_is_state_closed (h->state)) { > set_error (0, "connection is closed"); > return -1; > } > > - if (nbd_unlocked_aio_is_dead (h)) > + if (nbd_internal_is_state_dead (h->state)) error_unless_ready() is called while lock is held, so multiple reads of h->state are fine here. > +++ b/lib/disconnect.c > @@ -29,15 +29,14 @@ > int > nbd_unlocked_shutdown (struct nbd_handle *h) > { > - > - if...

.../* NB: is_locked = false, may_set_error = false. */ int nbd_unlocked_aio_is_processing (struct nbd_handle *h) { - return nbd_internal_is_state_processing (h->state); + return nbd_internal_is_state_processing (get_state (h)); } /* NB: is_locked = false, may_set_error = false. */ int nbd_unlocked_aio_is_dead (struct nbd_handle *h) { - return nbd_internal_is_state_dead (h->state); + return nbd_internal_is_state_dead (get_state (h)); } /* NB: is_locked = false, may_set_error = false. */ int nbd_unlocked_aio_is_closed (struct nbd_handle *h) { - return nbd_internal_is_state_closed (h->stat...

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

...is_locked = false, may_set_error = false. */ int nbd_unlocked_aio_is_processing (struct nbd_handle *h) { - return nbd_internal_is_state_processing (get_state (h)); + return nbd_internal_is_state_processing (get_public_state (h)); } -/* NB: is_locked = false, may_set_error = false. */ int nbd_unlocked_aio_is_dead (struct nbd_handle *h) { - return nbd_internal_is_state_dead (get_state (h)); + return nbd_internal_is_state_dead (get_public_state (h)); } -/* NB: is_locked = false, may_set_error = false. */ int nbd_unlocked_aio_is_closed (struct nbd_handle *h) { - return nbd_internal_is_state_closed (g...

...is_locked = false, may_set_error = false. */ int nbd_unlocked_aio_is_processing (struct nbd_handle *h) { - return nbd_internal_is_state_processing (get_state (h)); + return nbd_internal_is_state_processing (get_public_state (h)); } -/* NB: is_locked = false, may_set_error = false. */ int nbd_unlocked_aio_is_dead (struct nbd_handle *h) { - return nbd_internal_is_state_dead (get_state (h)); + return nbd_internal_is_state_dead (get_public_state (h)); } -/* NB: is_locked = false, may_set_error = false. */ int nbd_unlocked_aio_is_closed (struct nbd_handle *h) { - return nbd_internal_is_state_closed (g...

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