similar to: [PATCH nbdkit 0/5] protocol: Generate map functions from NBD protocol flags to printable strings.

We should have only one NBD protocol file. Let's make nbdkit's version the canonical one, and use it in libnbd. Rich.

These are a couple of patches in preparation for the Block Status implementation. While the patches (especially the second one) are very large they are really just elementary code motion. Rich.

This time around, the numbers are indeed looking better than in v1; and I like the interface better. Eric Blake (2): server: Prefer send() over write() server: Group related transmission send()s server/internal.h | 7 +++- server/connections.c | 51 +++++++++++++++++++++++++--- server/crypto.c | 11 ++++--

On Fri, Dec 07, 2018 at 10:00:42AM -0600, Eric Blake wrote: > Use new 'name_of_*' functions to offer a bit more details about > messages being forwarded on to the remote server. > > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > plugins/nbd/nbd.c | 8 +++++--- > plugins/nbd/Makefile.am | 3 ++- > 2 files changed, 7 insertions(+), 4 deletions(-)

This is about the simplest implementation of NBD Structured Replies (SR) that we can do right now. It accepts NBD_OPT_STRUCTURED_REPLIES when negotiated by the client, but only sends back the simplest possible SR when required to by NBD_CMD_READ. The rest of the time it will send back simple replies as before. We do not modify the plugin API so plugins are unable to send complex SRs. Also we

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

The DF flag is only available to clients that negotiated structured replies, and even then, the spec does not require that it be implemented. However, since our current implementation can't fragment NBD_CMD_READ replies, we trivially implement the flag (by ignoring it); we don't even have to pass it on to the plugins. Enhance some documentation about sparse reads while at it (when we

This generates small functions which map from various integer NBD protocol flags to the string equivalent. eg: name_of_nbd_cmd (NBD_CMD_READ) ---> "NBD_CMD_READ" This commit uses some hairy sed scripting to ensure that we don't add any more dependencies to nbdkit. --- src/protocol.h | 9 +++++++ .gitignore | 1 + src/Makefile.am | 17

v1 was here: https://www.redhat.com/archives/libguestfs/2020-February/msg00081.html v2 replaces struct connection *conn = GET_CONN; with GET_CONN; which sets conn implicitly and asserts that it is non-NULL. If we actually want to test if conn is non-NULL or behave differently, then you must use threadlocal_get_conn() instead, and some existing uses do that. Rich.

The third patch is a large but mechanical change which gets rid of passing around struct connection * entirely within the server, preferring instead to reference the connection through thread-local storage. I hope this is a gateway to simplifying other parts of the code. Rich.

Slightly RFC, as I need more time to investigate why Unix sockets appeared to degrade with this patch. But as TCP sockets (over loopback to localhost) and TLS sessions (regardless of underlying Unix or TCP) both showed improvements, this looks like a worthwhile series. Eric Blake (2): server: Add support for corking server: Cork around grouped transmission send()s server/internal.h | 3

On 12/6/18 3:50 PM, Richard W.M. Jones wrote: > This generates small functions which map from various integer NBD > protocol flags to the string equivalent. > > eg: > name_of_nbd_cmd (NBD_CMD_READ) > ---> "NBD_CMD_READ" > > This commit uses some hairy sed scripting to ensure that we don't add > any more dependencies to nbdkit. Hairy, but

Fuzzing with afl found a bug where a 27 byte client sequence can cause nbdkit to report a strange error message: $ printf %s $'000\1IHAVEOPT000\6'$'000\7'$'000\1x00' | tr 0 '\0' | ./nbdkit -s memory size=1m >/dev/null nbdkit: memory: error: client exceeded maximum number of options (32) The culprit? The client is hanging up on a message boundary, so

Not sure what version we're up to, but this reimplements extents using the new simpler structure described in this thread: https://www.redhat.com/archives/libguestfs/2019-March/msg00077.html I also fixed most of the things that Eric pointed out in the previous review, although I need to go back over his replies and check I've got everything. This needs a bit more testing. However 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.

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:

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 not sure exactly which version we're up to, but let's say it's version 4. I'm a lot happier with this version: - all filters have been reviewed and changed where I think that's necessary - can_extents is properly defined and implemented now - NBD protocol is followed - I believe it addresses all previous review points where possible The "only" thing

Use a simple bash script to generate the protostrings.c functions. Remove the extern decls from the nbd-protocol.h file which were used previously in the generation of this file. They have been moved to a new internal header called "protostrings.h". --- common/protocol/Makefile.am | 8 ++- ...tostrings.sed => generate-protostrings.sh} | 56 +++++++++++--------