similar to: nbdkit Assertion `h->can_flush == 1' failed on Arch

Either patch 1 or patch 2 in isolation is sufficient to fix the problem that Rich forwarded on from an archlinux tester (name so I can credit them?). But both patches should be applied, as well as backported to appropriate stable branches, to maximize cross-version interoperability of nbdkit vs. plugins. Patch 3 will let us detect future similar bugs much faster. I want patch 4 to ensure that

On 3/18/20 8:21 PM, Eric Blake wrote: > In commit c306fa93ab and neighbors (v1.15.1), a concerted effort went > into caching the results of .can_FOO callbacks, with commit messages > demonstrating that a plugin with a slow callback should not have that > delay magnified multiple times. But nothing was added to the > testsuite at the time, and with the sh and eval plugins, we still

On Fri, Nov 22, 2019 at 9:55 PM Richard W.M. Jones <rjones@redhat.com> wrote: > > This tests the Python plugin thoroughly by issuing client commands > through libnbd and checking we get the expected results. > --- > tests/Makefile.am | 13 +-- > tests/test-python-plugin.py | 134 ++++++++++++++++++++++++++++ > tests/test-python.py | 172

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

We want some testsuite coverage of handling non-1 .can_flush values. The only in-tree plugin with this property is nbd-standalone, but it doesn't get frequently tested, not to mention that the next patch will change it to work with older nbdkit. Signed-off-by: Eric Blake <eblake@redhat.com> --- work in progress: currently compiles but fails during test; posting it now to show my

With our recent cleanups to nxdata, the only remaining difference between functions like backend_open() and next_open() was the signature (one used void*, the other struct backend *); the API is compatible. All of our filters are in-tree, and we don't promise API/ABI stability, but it is still a lot of files to touch, so the simplest solution to avoid the redundant hop through wrapper

In commit c306fa93ab and neighbors (v1.15.1), a concerted effort went into caching the results of .can_FOO callbacks, with commit messages demonstrating that a plugin with a slow callback should not have that delay magnified multiple times. But nothing was added to the testsuite at the time, and with the sh and eval plugins, we still have two scenarios where we did not take advantage of the

This was previously used as ‘nxdata’ and stored a tuple of ’b->next’ and the real filter handle. However after recent changes we don't need it. We can use ‘b->next’ as nxdata, and the handle is passed to us by the calling functions. Inspired by Eric Blakes observations in this email: https://www.redhat.com/archives/libguestfs/2020-February/msg00092.html --- server/filters.c | 217

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.

One API change, some small documentation changes.

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.

On 11/22/19 1:54 PM, Richard W.M. Jones wrote: > This change is pure refactoring and should have no effect. > --- > plugins/python/nbdkit-python-plugin.pod | 12 ++-- > plugins/python/python.c | 90 +++++-------------------- > 2 files changed, 24 insertions(+), 78 deletions(-) > > diff --git a/plugins/python/nbdkit-python-plugin.pod

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.

v1: https://www.redhat.com/archives/libguestfs/2019-November/msg00153.html v2: - Fix implementation of can_cache. - Add implementation of can_fua. - Add a very thorough test suite which tests every command + flag combination.

On Mon, Mar 16, 2020 at 10:36:14PM -0500, Eric Blake wrote: > The documentation for .can_write and friends mentioned merely that the > callback must return a boolean value, or -1 on failure. Historically, > we have treated any non-zero value other than -1 as true, and some > plugins have sloppily relied on this behavior, including the > standalone nbd plugin prior to the use of

Very much a work in progress as there are still many tests using qemu-io which are candidates for conversion. You'll notice at the end of test-full.sh that the new test has some duplicated code which looks as if it ought to be refactored into a Python function. When I tried to do that, I got loads of strange Python problems which may indicate bugs in nbdsh itself or problems with my

From: "Richard W.M. Jones" <rjones@redhat.com> This patch may be worth squashing? --- docs/nbdkit-filter.pod | 65 +++++++++++++++++++++++++++++++++++++++++++------ include/nbdkit-filter.h | 29 ++++++++++++---------- src/filters.c | 55 ++++++++++++++++++++--------------------- 3 files changed, 101 insertions(+), 48 deletions(-) diff --git a/docs/nbdkit-filter.pod

On Fri, Jan 19, 2018 at 10:45:51AM -0600, Eric Blake wrote: > I'm wondering if we're missing documentation here (and/or in the plugins > document) that if .can_write returns true, the plugin must supply a > .pwrite; likewise for .can_trim implying a .trim, and .can_flush > implying a .flush. It's a strange one. For example if a plugin returns .can_pwrite == 1 but

Now instead of validating just the client's request, we are validating that all filters are passing valid requests on down. In protocol.c, we were able to assert that our computation of eflags populated all of the flags, and thus calls such as backend_can_write would not fail; however, with filters, keeping those assertions mean the burden is now on the filter to avoid calling into

No need to duplicate maintenance of export flag computation between old and new style handshakes. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/connections.c | 120 +++++++++++++++++++++--------------------------------- 1 file changed, 47 insertions(+), 73 deletions(-) diff --git a/src/connections.c b/src/connections.c index 8dc1925..f9edea7 100644 --- a/src/connections.c +++