similar to: [PATCH nbdkit] server: Add nbdkit_export_name() to allow export name to be read.

The previous incomplete patch was here: https://www.redhat.com/archives/libguestfs/2019-September/msg00049.html based on earlier discussion here: https://www.redhat.com/archives/libguestfs/2019-September/msg00047.html In v2: - The previous patch was incomplete. This version completes it by adding tests and extending nbdkit-sh-plugin. - nbdkit_export_name now returns NULL for error,

On 9/10/19 6:20 AM, Richard W.M. Jones wrote: > This allows plugins (or filters) to read the export name which was > passed to the server from the client. > > UNFINISHED: > > - Needs tests > --- > TODO | 8 ++++++ > docs/nbdkit-plugin.pod | 39 ++++++++++++++++++++++++++++ > server/connections.c | 1 +

Since the v1 series (0/4, at [1]), I've applied patches 1 and 2, rewritten patch 3 [Forbid NUL in export and context names] into patch 4 here, patch 4 there turned into patch 6 here, and everything else here is new. [1]https://www.redhat.com/archives/libguestfs/2019-September/msg00180.html I don't know if there is a handy reusable function for checking whether a string contains valid

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.

The first one is the nastiest - it is an assertion failure caused by a spec-compliant client and introduced by our security fix that was released in 1.14.1. Eric Blake (4): server: Fix regression for NBD_OPT_INFO before NBD_OPT_GO server: Fix back-to-back SET_META_CONTEXT server: Forbid NUL in export and context names server: Fix OPT_GO on different export than SET_META_CONTEXT

This allows plugins (or filters) to read the export name which was passed to the server from the client. UNFINISHED: - Needs tests --- TODO | 8 ++++++ docs/nbdkit-plugin.pod | 39 ++++++++++++++++++++++++++++ server/connections.c | 1 + server/internal.h | 1 + server/protocol-handshake-newstyle.c | 30

There's no substantial difference over v1, I simply fixed a few whitespace issues, moved one struct around and tidied up the comments. Rich.

qemu is about to add 'qemu-nbd --list', which exercises NBD_OPT_LIST and NBD_OPT_INFO to give the user as much detail as possible about an export without actually connecting to it. For that to display more than the export name when nbdkit is the server, we need to implement NBD_OPT_INFO. Thankfully, the NBD spec intentionally made the command very similar to NBD_OPT_GO, to the point that

To allow filters to modify the export name as it passes through the layers this commit makes several changes: The filter .open callback now takes an extra parameter, the export name. This is always non-NULL (for oldstyle it is ""). This string has a short lifetime and filters that need to hang on to it must take a copy. The filter must pass the exportname parameter down to the next

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.

This is only lightly tested (against just qemu NBD client), and the code might be structured a little better as the _negotiate_handshake_newstyle_options function has now grown to be huge. Anyway works for me. Rich.

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.

None are supported at present, so this always returns an empty list. --- docs/nbdkit-protocol.pod | 4 ++ server/protocol.h | 2 + server/connections.c | 84 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+) diff --git a/docs/nbdkit-protocol.pod b/docs/nbdkit-protocol.pod index 68438fa..4c65e00 100644 --- a/docs/nbdkit-protocol.pod +++

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.

This takes Rich's API proposal and starts fleshing it out with enough meat that I was able to test 'nbdkit eval' advertising multiple exports with descriptions paired with 'qemu-nbd --list'. Eric Blake (3): server: Add exports list functions server: Prepare to use export list from plugin sh, eval: Add .list_exports support Richard W.M. Jones (1): server: Implement

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 ++++--

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

On Tue, Jul 21, 2020 at 07:41:21PM -0500, Eric Blake wrote: > On 7/21/20 3:47 PM, Richard W.M. Jones wrote: > >+++ b/server/plugins.c > >@@ -278,12 +278,30 @@ plugin_preconnect (struct backend *b, int readonly) > > } > > static void * > >-plugin_open (struct backend *b, int readonly) > >+plugin_open (struct backend *b, int readonly, const char *exportname)

More patches on the way for improving .list_exports signature and adding .export_description, but this is the promised code showing why nbdkit_string_intern is useful. Patch 4 is somewhat RFC: we could either add new API to take the boilerplate from: foo_config(const char *key, const char *value) { if (strcmp (key, "file") == 0) { CLEANUP_FREE char *tmp = nbdkit_realpath (value);

Since v1: - patch 1: check size limits - patch 2: better handling of default export name canonicalization - patch 3: support filters as well as plugins - patch 4: new - patch 5: rewrite sh parser, fix testsuite to actually work and cover more cases (now that libnbd.git is fixed) Eric Blake (4): server: Add exports list functions server: Prepare to use export list from plugin log: Add