similar to: [PATCH libnbd v2 0/6] Test connection states.

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

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

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.

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

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

On 5/22/19 4:50 AM, Richard W.M. Jones wrote: > If not using multi-conn then obviously the synchronous connection > calls ‘nbd_connect_unix’, ‘nbd_connect_tcp’ and ‘nbd_connect_command’ > should only return when the (one) connection object is connected. > > In the multi-conn case it's not very clear what these synchronous > calls should do. Previously I had it so that they

Simple code motion. --- lib/Makefile.am | 1 + lib/aio.c | 78 ----------------------------------- lib/is-state.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 78 deletions(-) diff --git a/lib/Makefile.am b/lib/Makefile.am index 312545e..72d2d0b 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -42,6 +42,7 @@ libnbd_la_SOURCES = \

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

Per recent discussion here: https://www.redhat.com/archives/libguestfs/2019-May/thread.html#00175

On Tue, May 21, 2019 at 10:15:49PM -0500, Eric Blake wrote: > Previously, our 'cmds_to_issue' list was at most 1 element long, > because we reject all commands except from state READY, but don't get > back to state READY until the issue-commands sequence has completed. > However, this is not as friendly on the client - once we are in > transmission phase, a client may

For each nbd_(unlocked_)?aio_is_* function, split it into an internal function that tests the state and a public visible API function. For calls within the library, use the internal function. This is simple refactoring. As a side effect this fixes a race condition identified by Eric Blake: Thread A Thread B (in a call that holds h->lock) (calling nbd_aio_pread)

If not using multi-conn then obviously the synchronous connection calls ‘nbd_connect_unix’, ‘nbd_connect_tcp’ and ‘nbd_connect_command’ should only return when the (one) connection object is connected. In the multi-conn case it's not very clear what these synchronous calls should do. Previously I had it so that they would return as soon as at least one connection was connected. However this

If not using multi-conn then obviously the synchronous connection calls ‘nbd_connect_unix’, ‘nbd_connect_tcp’ and ‘nbd_connect_command’ should only return when the (one) connection object is connected. In the multi-conn case it's not very clear what these synchronous calls should do. Previously I had it so that they would return as soon as at least one connection was connected. However this

For commands with long names and/or short descriptors, you can end up going over 72 characters in the first line of the man page (causing podwrapper to complain). Wrap these lines. --- generator/generator | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/generator/generator b/generator/generator index 7d3f656..ad1cb6b 100755 --- a/generator/generator +++ b/generator/generator

All accesses to the h->state field are redirected through these macros (except when we create the handle). This is simple refactoring. --- generator/generator | 18 +++++++++--------- lib/connect.c | 10 +++++----- lib/disconnect.c | 8 ++++---- lib/internal.h | 3 +++ lib/is-state.c | 12 ++++++------ lib/rw.c | 4 ++-- 6 files changed, 29 insertions(+), 26

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

On 7/20/20 9:43 AM, Richard W.M. Jones wrote: > A major missing feature of this library was the ability to list > exports from an NBD server. This implements the feature by adding a > new handle mode and additional functions for querying the list of > export names. > --- > .gitignore | 1 + > examples/Makefile.am | 14

Split h->state into: - h->public_state = the state on entry to the locked region This is also the atomically, publicly visible state. - h->state = the real current state of the handle When we leave the locked region we update h->public_state with h->state, so that from outside the lock the handle appears to move atomically from its previous state to the final state without

Previously, our 'cmds_to_issue' list was at most 1 element long, because we reject all commands except from state READY, but don't get back to state READY until the issue-commands sequence has completed. However, this is not as friendly on the client - once we are in transmission phase, a client may want to queue up another command whether or not the state machine is still tied up in