similar to: [PATCH libnbd v2 0/4] api: Implement concurrent writer.

This works, but there's no time saving and I'm still investigating whether it does what I think it does. Nevertheless I thought I would post it because it (probably) implements the idea I had last night outlined in: https://www.redhat.com/archives/libguestfs/2019-June/msg00010.html The meat of the change is patch 4. Patch 5 is an example which I would probably fold into patch 4 for

--- .gitignore | 1 + examples/Makefile.am | 12 + examples/concurrent-writer.c | 450 +++++++++++++++++++++++++++ examples/threaded-reads-and-writes.c | 2 +- 4 files changed, 464 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 30438c1..e4dad91 100644 --- a/.gitignore +++ b/.gitignore @@ -41,6 +41,7 @@ Makefile.in

--- .gitignore | 1 + examples/Makefile.am | 12 + examples/concurrent-writer.c | 450 +++++++++++++++++++++++++++++++++++ 3 files changed, 463 insertions(+) diff --git a/.gitignore b/.gitignore index 30438c1..e4dad91 100644 --- a/.gitignore +++ b/.gitignore @@ -41,6 +41,7 @@ Makefile.in /docs/libnbd-api.3 /docs/libnbd-api.pod /examples/batched-read-write

I still need to wire in the use of *_notify functions into nbdkit to prove whether it makes the code any faster or easier to maintain, but at least the added example shows one good use case for the new API. Eric Blake (6): api: Add nbd_aio_in_flight generator: Allow DEAD state actions to run generator: Allow Int64 in callbacks states: Prepare for aio notify callback api: Add new

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 Eric's improvements to the command queue in commit 6af72b8 (and following) there's now a queue of commands waiting to be issued stored in the handle, and there's no need to issue commands only from the ready state. We can therefore remove the want_to_send logic, queue as many commands as possible, and don't need to test if the socket is ready for POLLOUT. This commit also

I'm not sure if this is worth pursuing. On paper, it makes sense (if we know we have multiple commands batched to send over the wire, AND those commands are short in length, we might as well use MSG_MORE), but the measurement numbers with it applied might just be in the noise. Eric Blake (2): examples: Enhance access patterns of threaded-reads-and-writes states: Another use for MSG_MORE

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

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

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

Some clients need to know when it is safe to issue NBD_CMD_DISC, or to decide whether calling poll(POLLIN) will block indefinitely because the server isn't expected to respond. Make this easier to learn by tracking the count of commands we have queued up to send, as well as the count of commands where we are waiting on the server's response. Update tests/aio-parallel* and

Nir, BTW what are you using for performance testing? As far as I can tell it's not possible to make qemu-img convert use multi-conn when connecting to the source (which is going to be a problem if we want to use this stuff in virt-v2v). Instead I've hacked up a copy of this program from libnbd: https://github.com/libguestfs/libnbd/blob/master/examples/threaded-reads-and-writes.c so

As mentioned in commits 176fc4ea and 609c25f0, our original plan in adding a flags argument to nbd_shutdown was to let us specify different behaviors at the libnbd level, rather than NBD protocol flags (for that, the user has nbd_aio_disconnect). But when we later parameterized OFlags to accept various bitmasks (commit f891340b), we failed to mark nbd_shutdown as using a different bitmask than

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 |

I still need more soak time on testing, to see whether I can: a) reproduce the hang with patch 2 not applied b) feel confident that patch 2 is sufficient to fix the race, or else determine that I also need to augment the loop condition in the reader thread to additionally break out of the loop when the pipe-to-self sees EOF even when nbd_aio_is_dead() has not yet been satisfied I'm also

This is a subset of my v2 posting, but limited to just the NBD_OPT_LIST handling. The biggest change since v2 is the addition of added unit testing in all four language bindings (C, python, ocaml, golang). The tests require nbdkit built from git on PATH, and may not be entirely idiomatic, but I at least validated that they catch issues (for example, adding an exit statement near the end of the

I'm not observing any noticeable performance differences, but I'm liking the diffstat. I can't push this patch until we release a new libnbd version with the _notify API addition, but am posting it now for playing with things. Eric Blake (2): nbd: Move transaction info from heap to stack nbd: Use nbdkit aio_*_notify variants plugins/nbd/nbd.c | 217

RFC because this is an API break, but we haven't declared stable API yet. If we like it, I'm working on using libnbd to implement the nbdkit-nbd plugin; knowing whether it is API version 0.1 or 0.2 will be useful. I also dabbled with allowing optional parameters in python, although my OCaml is weak enough that there may be cleaner ways to approach that. Eric Blake (4): api: Add flags

I fixed the deadlock - turned out to be an actual bug in the nbd plugin (see patch 8). I changed the command syntax so it's now: nbdkit nbd command=qemu arg=-f arg=qcow2 arg=/path/to/disk.qcow2 Nir wrote: 18:08 < nsoffer> rwmjones: regarding the nbd proxy patches, did you have specific flow that help us? 18:08 < nsoffer> rwmjones: or this is just a way to support qcow2 in the

Patch 1 played with an early draft of Rich's Fedora 30 libnbd package: https://bugzilla.redhat.com/show_bug.cgi?id=1713767#c17 Note that comment 21 provides a newer package 0.1.1-1 with a different API; and that libnbd has more unreleased API changes in the pipeline (whether that will be called 0.2 or 0.1.2); so we'll have to tweak things based on what is actually available in distros.