search for: b_conn_handl

...or + * the i'th backend (if b->i >= 1 then for a filter). + */ enum { HANDLE_OPEN = 1, /* Set if .open passed, so .close is needed */ HANDLE_CONNECTED = 2, /* Set if .prepare passed, so .finalize is needed */ HANDLE_FAILED = 4, /* Set if .finalize failed */ }; -struct b_conn_handle { - void *handle; +struct handle { + void *handle; /* Plugin or filter handle. */ - unsigned char state; /* Bitmask of HANDLE_* values */ + unsigned char state; /* Bitmask of HANDLE_* values */ uint64_t exportsize; int can_write; @@ -195,7 +203,7 @@ struct b_conn_handle { }...

b_conn_handle fields exportsize and can_* have a special meaning when they are -1. It means that the value of the field has not been computed and cached yet. The struct was being initialized using memset (_, -1, _) which does indeed have the effect of setting all the fields to -1, although it's somewhat n...

...onn->handles); + conn->handles = calloc (top->i + 1, sizeof *conn->handles); if (conn->handles == NULL) { perror ("malloc"); goto error; } - conn->nr_handles = backend->i + 1; + conn->nr_handles = top->i + 1; for_each_backend (b) reset_b_conn_handle (&conn->handles[b->i]); @@ -277,7 +277,7 @@ new_connection (int sockin, int sockout, int nworkers) * we aren't accepting until the plugin is not running, making * non-atomicity okay. */ - assert (backend->thread_model (backend) <= + assert (top->thr...

...re called at most once and cached by nbdkit for that connection. =item C<.pread>, C<.pwrite> and other data serving callbacks diff --git a/server/internal.h b/server/internal.h index ec8a894c..ddb79623 100644 --- a/server/internal.h +++ b/server/internal.h @@ -152,6 +152,15 @@ struct b_conn_handle { void *handle; uint64_t exportsize; + int can_write; + int can_flush; + int is_rotational; + int can_trim; + int can_zero; + int can_fua; + int can_multi_conn; + int can_cache; + int can_extents; }; struct connection { @@ -169,16 +178,6 @@ struct connection { uint32_t cflag...

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

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

...se_function) (struct connection *) __attribute__((__nonnull__ (1))); +enum { + HANDLE_OPEN = 1, /* Set if .open passed, so .close is needed */ + HANDLE_CONNECTED = 2, /* Set if .prepare passed, so .finalize is needed */ + HANDLE_FAILED = 4, /* Set if .finalize failed */ +}; + struct b_conn_handle { void *handle; + unsigned char state; /* Bitmask of HANDLE_* values */ + uint64_t exportsize; int can_write; int can_flush; @@ -173,6 +181,7 @@ static inline void reset_b_conn_handle (struct b_conn_handle *h) { h->handle = NULL; + h->state = 0; h->exportsize = -1;...

...8f96 100644 --- a/server/backend.c +++ b/server/backend.c @@ -124,6 +124,43 @@ backend_unload (struct backend *b, void (*unload) (void)) free (b->name); } +static bool +writes_blocked (struct backend *b, struct connection *conn, const char *cmd, + uint32_t flags) +{ + struct b_conn_handle *h = &conn->handles[b->i]; + + assert (h->can_write >= 0); + if (h->can_write == 0) { + nbdkit_error ("invalid request: %s: write request on readonly connection", + cmd); + return true; + } + if (flags & NBDKIT_FLAG_FUA) { + assert (h-...

...r use by the forthcoming retry filter to close and > reopen the backend chain. It is handled entirely by server/backend.c > as no cooperation is needed with the plugin. > > Note the explicit readonly parameter: An alternative would be to store > the previous readonly setting in the b_conn_handle struct. However > passing it explicitly allows the retry filter to retry as readonly, > which might be useful. This design does however require any filter > which might call .reopen to save the original readonly parameter from > the .open call. > --- > include/nbdkit-filter.h...

...erver/backend.c +++ b/server/backend.c @@ -151,8 +151,9 @@ backend_unload (struct backend *b, void (*unload) (void)) } int -backend_open (struct backend *b, struct connection *conn, int readonly) +backend_open (struct backend *b, int readonly) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; controlpath_debug ("%s: open readonly=%d", b->name, readonly); @@ -166,12 +167,12 @@ backend_open (struct backend *b, struct connection *conn, int readonly) /* Most filters will call next_open first, resulting in * inner-to-outer orderi...

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.

...nd.c > index 8a434bd..b8c5742 100644 > --- a/server/backend.c > +++ b/server/backend.c > @@ -224,9 +224,17 @@ backend_valid_range (struct backend *b, struct connection *conn, > int > backend_reopen (struct backend *b, struct connection *conn, int readonly) > { > + struct b_conn_handle *h = &conn->handles[b->i]; > + > debug ("%s: reopen", b->name); > > b->close (b, conn); > + > + /* This forces .open to recalculate h->can_write, which might have > + * changed since we may have a new readonly value. > + */ > +...

This is a subset of the last half of the larger 9-patch series. The uncontroversial first half of that series is pushed, but here, I tried to reduce the size of the patches by splitting out some of the more complex changes, so that the rest of the changes remaining in the series are more mechanical. In turn, it forced me to write timing tests, which let me spot another spot where we are wasting

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 intended for use by the forthcoming retry filter to close and reopen the backend chain. It is handled entirely by server/backend.c as no cooperation is needed with the plugin. Note the explicit readonly parameter: An alternative would be to store the previous readonly setting in the b_conn_handle struct. However passing it explicitly allows the retry filter to retry as readonly, which might be useful. This design does however require any filter which might call .reopen to save the original readonly parameter from the .open call. --- include/nbdkit-filter.h | 6 ++++++ server/backend.c...

...r use by the forthcoming retry filter to close and > reopen the backend chain. It is handled entirely by server/backend.c > as no cooperation is needed with the plugin. > > Note the explicit readonly parameter: An alternative would be to store > the previous readonly setting in the b_conn_handle struct. However > passing it explicitly allows the retry filter to retry as readonly, > which might be useful. This design does however require any filter > which might call .reopen to save the original readonly parameter from > the .open call. > --- > include/nbdkit-filter.h...

...++++++---- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/server/backend.c b/server/backend.c index a637f754..b918adff 100644 --- a/server/backend.c +++ b/server/backend.c @@ -172,6 +172,7 @@ int backend_open (struct backend *b, struct connection *conn, int readonly) { struct b_conn_handle *h = &conn->handles[b->i]; + int r; debug ("%s: open readonly=%d", b->name, readonly); @@ -179,7 +180,13 @@ backend_open (struct backend *b, struct connection *conn, int readonly) assert (h->can_write == -1); if (readonly) h->can_write = 0; - return b...

It's easy to use the sh script to demonstrate that nbdkit is inefficiently calling into .get_size, .can_fua, and friends more than necessary. We've also commented on the list in the past that it would be nice to ensure that when filters call into next_ops, they are not violating constraints (as we've have to fix several bugs in the past where we did not have such checking to protect

...a commands like <.pread> will not +fail. =head2 C<.can_write> diff --git a/server/internal.h b/server/internal.h index 9bf84022..ec8a894c 100644 --- a/server/internal.h +++ b/server/internal.h @@ -151,7 +151,7 @@ typedef void (*connection_close_function) (struct connection *) struct b_conn_handle { void *handle; - // TODO add per-backend caching + uint64_t exportsize; }; struct connection { @@ -168,7 +168,6 @@ struct connection { size_t nr_handles; uint32_t cflags; - uint64_t exportsize; uint16_t eflags; bool readonly; bool can_flush; diff --git a/server/backend.c...

v1 was here: https://www.redhat.com/archives/libguestfs/2019-September/msg00199.html v2: - Adds a fairly simple yet comprehensive test using sh plugin. - Rebase and retest. Patch 1 is a misc patch not really related to the series. Rich.