search for: nbd_reader

Nothing in the second loop of nbd_reader() was ever setting quit; the only way the loop could end was via SEGV for dereferencing NULL or abort() when attempting to write to an invalid fd. Finish the implementation to properly mop up stranded in-flight requests. The testsuite did not exercise the case of quitting nbdkit while a transacti...

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

...->cookie) + if (be32toh (rep.magic) != NBD_REPLY_MAGIC || rep.handle != trans->u.cookie) return nbd_mark_dead (h); switch (be32toh (rep.error)) { case NBD_SUCCESS: @@ -295,17 +329,52 @@ nbd_reply_raw (struct handle *h, struct transaction *trans) } } +/* Reader loop. */ +void *nbd_reader (void *handle) +{ + struct handle *h = handle; + bool done = false; + + while (!done) { + struct transaction trans; + int r; + int fd; + + r = nbd_reply_raw (h, &trans); + fd = trans.u.fds[1]; + trans.u.fds[1] = -1; + if (r >= 0) { + if (write (fd, &r, sizeof...

...any earlier chunk for replay during the final chunk. */ if (!more) { - *fd = trans->u.fds[1]; + *trans_out = trans; if (!error) error = trans->err; - free (trans); } else if (error && !trans->err) trans->err = error; @@ -616,19 +610,20 @@ nbd_reader (void *handle) int r; while (!done) { - int fd; + struct transaction *trans; - r = nbd_reply_raw (h, &fd); + r = nbd_reply_raw (h, &trans); if (r >= 0) { - if (fd < 0) + if (!trans) nbdkit_debug ("partial reply handled, waiting for fina...

...ugins/nbd/nbd.c @@ -334,6 +334,7 @@ nbd_reply_raw (struct handle *h, int *fd) } *fd = trans->u.fds[1]; + free (trans); switch (be32toh (rep.error)) { case NBD_SUCCESS: if (trans->buf && read_full (h->fd, trans->buf, trans->count) < 0) @@ -399,6 +400,7 @@ nbd_reader (void *handle) abort (); } close (trans->u.fds[1]); + free (trans); } return NULL; } -- 2.14.3

...switch (be32toh (rep.error)) { case NBD_SUCCESS: - if (trans->buf && read_full (h->fd, trans->buf, trans->count) < 0) + if (buf && read_full (h->fd, buf, count) < 0) return nbd_mark_dead (h); return 0; case NBD_EPERM: @@ -399,6 +404,7 @@ nbd_reader (void *handle) abort (); } close (trans->u.fds[1]); + free (trans); } return NULL; } -- 2.14.3

...reply_raw (struct handle *h, int *fd) > } > > *fd = trans->u.fds[1]; > + free (trans); > switch (be32toh (rep.error)) { > case NBD_SUCCESS: > if (trans->buf && read_full (h->fd, trans->buf, trans->count) < 0) > @@ -399,6 +400,7 @@ nbd_reader (void *handle) > abort (); > } > close (trans->u.fds[1]); > + free (trans); > } > return NULL; > } Can this be right? valgrind seems to be saying that there are double-free errors when I add this patch (see below). Rich. FAIL: test-nbd =======...

...l unwritten at the moment) would basically change a single transaction into a linked list, where nbd_request_full() adds another transaction to the list, nbd_read_raw() uses the cookie received from the server to look up a transaction to remove from the list, and where breaking the while(1) loop of nbd_reader() cleans up any remaining transactions still on the list. Eric Blake (2): nbd: Add new nbd forwarding plugin nbd: Split reading into separate thread configure.ac | 1 + plugins/Makefile.am | 1 + plugins/nbd/Makefile.am | 63 ++++ plugins/nbd...

...n ENOMEM; default: - nbdkit_debug ("unexpected error %d, squashing to EINVAL", - be32toh (rep.error)); + nbdkit_debug ("unexpected error %d, squashing to EINVAL", error); /* fallthrough */ case NBD_EINVAL: return EINVAL; @@ -405,7 +570,9 @@ nbd_reader (void *handle) r = nbd_reply_raw (h, &fd); if (r >= 0) { - if (write (fd, &r, sizeof r) != sizeof r) { + if (fd < 0) + nbdkit_debug ("partial reply handled, waiting for final reply"); + else if (write (fd, &r, sizeof r) != sizeof r) {...

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.

libnbd-0.1.2-1 is now available in Fedora 29/30 updates-testing, although it was not compiled against libxml2 so it lacks uri support (I ended up testing patch 4 with a self-built libnbd). Diffs since v1 - rebase to master, bump from libnbd 0.1 to 0.1.2, add URI support, better timing results Still not done - patch 5 needs associated tests Eric Blake (5): nbd: Check for libnbd nbd:

...ug ("unexpected error %d, squashing to EINVAL", error); - /* fallthrough */ - case NBD_EINVAL: - return EINVAL; - case NBD_ENOSPC: - return ENOSPC; - case NBD_EOVERFLOW: - return EOVERFLOW; - case NBD_ESHUTDOWN: - return ESHUTDOWN; - } -} - -/* Reader loop. */ -void * -nbd_reader (void *handle) -{ - struct handle *h = handle; - bool done = false; - int r; - - while (!done) { - struct transaction *trans; - - r = nbd_reply_raw (h, &trans); - if (r >= 0) { - if (!trans) - nbdkit_debug ("partial reply handled, waiting for final reply");...

libnbd-0.1.4-1 is now available in Fedora 29/30 updates testing. Diffs since v2 - rebase to master, bump from libnbd 0.1.2 to 0.1.3+, add tests to TLS usage which flushed out the need to turn relative pathnames into absolute, doc tweaks Now that the testsuite covers TLS and libnbd has been fixed to provide the things I found lacking when developing v2, I'm leaning towards pushing this on

Updated based on other changes that have happened in the meantime: - rely more on cleanup.h (throughout) - split structured read for easier review (patch 2 and 3 were combined in v1) - rely on nbdkit not leaking a server's partial answer (patch 3) - add tests (patch 5) - other bug fixes I found while testing it - drop EOVERFLOW patch for now; it will be separate once upstream NBD protocol

I'm hoping to implement .extents for the nbd plugin; this is a prerequisite. I'm not sure about patch 3 - if we like it, I'll squash it to 2, if we don't, I think we are okay just dropping it. I'm also wondering if we have to worry about malicious plugins that don't populate the entire .pread buffer in an effort to get nbdkit to expose portions of the heap; my patch 7 loses