similar to: [nbdkit PATCH 0/2] Fix testsuite deadlocks during close

Hi Eric, There's a memory leak in the nbd client. The message (below) is not very useful because somehow debuginfo is missing in the plugin. However it's easily reproducible by doing: make check-valgrind TESTS=test-nbd I tried the attached patch to fix what I thought was the bug, but sadly the fix doesn't work for me :-( So I guess something else is going on, but it does look as

I want to make my nbd forwarding plugin fully parallel - but to do that, I first need to make nbdkit itself fully parallel ;) With this series, I was finally able to demonstrate out-of-order responses when using qemu-io (which is great at sending back-to-back requests prior to waiting for responses) coupled with the nbd file plugin (which has a great feature of rdelay and wdelay, to make it

I've posted some of these patches or ideas before; but now I'm confident enough with the series that it should be ready to push; at any rate, I can now run test-socket-activation in a tight loop without triggering any crashes or hangs. With this in place, I'm going back to work on making the nbd forwarder wort with the parallel thread model. Eric Blake (8): sockets: Use

ThreadSanitizer [1] pointed out that in the nbd plugin, nbd_close() can attempt close() in the main thread while the worker thread is still attempting to start a read(). Normally, if the read() loses the race, it will get a harmless EBADF that exits the worker thread (which is what we want, as we are closing the connection anyway); but if another connection happens to start in that window, we

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

nbdkit 1.2 as a server waits for read() to see EOF, even after the client has sent NBD_CMD_DISC. That was fixed in mbdkit 1.4; and most modern NBD servers are smarter than this (they close() the write end of their traffic soon after NBD_CMD_DISC). But it's easy enough to revert nbdkit commit c70616f8 to get back to a server with the same behavior as the older nbdkit, at which point both

According to the NBD spec, the server should close the connection rather than replying to NBD_CMD_DISC (after flushing any pending replies to earlier commands), and a compliant client should not send any data after that command. However, when nbdkit is running multithreaded, we already have multiple threads competing for a read lock, and each of those threads would try to read() from the socket,

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

With some crufty sed scripts we can generate functions that map from NBD protocol flags (eg. NBD_CMD_READ) to strings ("NBD_CMD_READ"). This works on GNU sed and with FreeBSD, also with GNU sed's --posix option, so I guess the sed code is POSIX-compatible. Rich.

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

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

With this, I am finally able to get the nbd plugin to do out-of-order responses to the client. Once this series goes in, we should be ready for Rich to cut a release. Eric Blake (4): nbd: Split reading into separate thread nbd: Protect writes with mutex nbd: Enable parallel handling tests: Test parallel nbd behavior plugins/nbd/nbd.c | 217

I'm fairly sure that these bugs which appear in the Python plugin: https://bugzilla.redhat.com/show_bug.cgi?id=1566516 https://bugzilla.redhat.com/show_bug.cgi?id=1566522 are really bugs in the SERIALIZE_ALL_REQUESTS thread model. See the first patch for the full explanation. The second patch is a fix for a race condition which is probably nudged into being by the first patch. Now this

I think I've addressed everything that was raised in review. Some of the highlights: - Callbacks should be freed reliably along all exit paths. - There's a simple test of closure lifetimes. - I've tried to use VALID|FREE in all the places where I'm confident that it's safe and correct to do. There may be more places. Note this is an optimization and shouldn't

I got annoyed by qemu-nbd's default of only allowing a single connection; combine that with nbdkit's nbd plugin, and even 'qemu-nbd --list' of nbdkit counts as the single connection and immediately hangs up. If we introduce a shared mode, then 'qemu-nbd --list' can connect as many times as it wants without killing the original qemu-nbd wrapped by nbdkit. But this in turn

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

I'm still working on the interleaving (and Rich reminded me on IRC that we still don't have THREAD_MODEL_PARALLEL working anywhere yet, anyways). Since nbdkit doesn't really have a parallel plugin yet, my testing on that front will have to use qemu-nbd as the original server, as well as qemu-io as the driver (qemu-io's aio_read and aio_write commands can be used to trigger

Tested via: term1$ qemu-nbd -k $PWD/sock -t -f raw -x foo junk --trace=nbd_\* term2$ ./nbdkit -f -v -e bar nbd socket=$PWD/sock export=foo term3$ qemu-io -t none -f raw nbd://localhost:10809/bar --trace=nbd_\* and checking the traces to see that 'w 0 1' vs. 'w -f 0 1' was able to influence whether the FUA flag showed up at the server in term1. Still to go: figure out how to

On Fri, Mar 27, 2020 at 05:33:28PM -0500, Eric Blake wrote: > We have been seeing sporadic hangs on test-nbd-tls-psk.sh, where even > though the client to the 'nbdkit nbd' process has cleanly exited, > things are stalled in .close where nbd is trying to pthread_join() the > reader thread, while the reader thread is itself blocked on a poll() > that will never make additional

One API change, some small documentation changes.