similar to: [PATCH nbdkit] server: Only display "close: " debug message if callback is called.

On 3/4/20 9:15 AM, Richard W.M. Jones wrote: > For example if .preconnect fails, it printed the debug messages: > > debug: preconnect: ... > debug: close: ... > > This confused me into thinking that the close callback was actually > being called without a corresponding open. In fact it is not called > in this case. Suppress the debug message unless the callback is

It's confusing to use the same terminology for a single backend as for the linked list of backends. In particular it's often not clear if we're calling the next backend or the whole chain of backends. --- server/internal.h | 14 ++++++++++-- server/connections.c | 20 ++++++++--------- server/locks.c | 2 +- server/main.c

Also add an inline accessor function (get_handle). This also removes the unused function connection_get_handle. --- server/internal.h | 24 ++++++++++++++++----- server/backend.c | 50 ++++++++++++++++++++++---------------------- server/connections.c | 11 +--------- 3 files changed, 45 insertions(+), 40 deletions(-) diff --git a/server/internal.h b/server/internal.h index

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.

I'm about to add an 'exportname' filter, and in the process, I noticed a few shortcomings in our API. Time to fix those before the 1.22 release locks our API in stone. Overloading .list_exports in order to determine a canonical export name at .open time is awkward; the two uses (answering NBD_OPT_LIST for a full list, vs. remapping a client's "" into a canonical name

This is a revision of my .default_export work, plus new work on .export_descriptions and a new exportname filter. I think it is now ready to check in. Things I'd still like in 1.22: - the file plugin should implement .list_exports (patch already posted, but it needs rebasing on this series) - the ext2 filter should override .list_exports when in exportname mode - the nbd plugin should be

Since commit 86fdb48c6a5362d66865493d9d2172166f99722e we have stored the connection object in thread-local storage. In this very large, but mostly mechanical change we stop passing the connection pointer around everywhere, and instead use the value stored in thread-local storage. This assumes a 1-1 mapping between the connection and the current thread which is true in *most* places.

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

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);

The immediate reason for this patch is to reduce the amount of debugging in virt-v2v with using the virt-v2v -v option (because this implies running nbdkit in verbose mode too). Most of the messages are datapath ones about pread/pwrite requests, and in fact as we've added more filters on top of nbdkit these messages have got more and more verbose. However they are not particularly

I think this is my last round of patches for issues I identified with the retry filter. With this in place, it should be safe to interject another filter in between retry and the plugin. Eric Blake (5): retry: Don't call into closed plugin tests: Refactor test-retry-reopen-fail.sh tests: Enhance retry test to cover failed reopen server: Move prepare/finalize/close recursion to

It's been several weeks since I posted v2 (I got distracted by improving libnbd to better test things, which in turn surfaced some major memory leak problems in nbdsh that are now fixed). Many of the patches are minor rebases from v2, with the biggest changes being fallout from: - patch 2: rename nbdkit_add_default_export to nbdkit_use_default_export - overall: this missed 1.22, so update

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

On Wed, Aug 26, 2020 at 09:16:46PM -0500, Eric Blake wrote: > Rather than defaulting .list_exports to blindly returning "", it is > nicer to have it reflect the result of .default_export. Meanwhile, > language bindings will have a C callback for both .list_exports and > .default_export, even if the underlying script does not, which means > any logic in plugins.c for

The retry filter was originally written to be the closest filter to the plugin, with no other filters in between; as such, the reopen command did not have to worry about recursion or about .prepare or .finalize. But it is not that much harder to properly track everything needed to allow other filters to be retried, as long as we are careful to never call .finalize unless .prepare succeeded, never

Somewhat of a mishmash between .open (in that it takes a bool readonly parameter) and .config_complete (in that the C code returns an int, but the Ocaml code either throws an exception or completes with unit). I did not spot any existing testsuite coverage to modify for this, and am relying on the fact that it compiles cleanly. Signed-off-by: Eric Blake <eblake@redhat.com> ---

Rather than defaulting .list_exports to blindly returning "", it is nicer to have it reflect the result of .default_export. Meanwhile, language bindings will have a C callback for both .list_exports and .default_export, even if the underlying script does not, which means any logic in plugins.c for calling .default_export when .list_export is missing would have to be duplicated in each

Now that we can differentiate content based on export name, we also need to be able to differentiate content for a --tls=on server, since TLS is optional according to whether the client has authenticated. For internal code and filters, this means adding a new parameter; the sh plugin can do likewise. For plugins, we can't add a parameter until the V3 protocol, so in the meantime, we add

I'm about to add an 'exportname' filter, and in the process, I noticed a few shortcomings in our API. Having .default_export makes it easy to answer NBD_INFO_NAME in response to a client request during NBD_OPT_GO, but answering NBD_INFO_DESCRIPTION is awkward - there's no guarantee the export name was given with a description in .list_exports. Note, however, that while we map