search for: b_conn

Another couple things I noticed that are worth improving, but aren't strictly related to implementing fast zero support. Eric Blake (2): server: Assert sane error responses nozero: More efficient FUA handling filters/nozero/nozero.c | 17 +++++++++++-- server/filters.c | 56 +++++++++++++++++++++++++++++++++-------- server/protocol.c | 32 +++++++++++++++++------ 3 files

...hread, which requires that nxdata be stable for the entire life of the connection between .open and .close. Our current approach of creating a stack-allocated nxdata for every call does not play nicely with that scheme, so rework things into using a malloc'd pointer. In fact, if we use struct b_conn as our filter handle, and merely add an additional field to track the user's handle, then we get the long-term persistance we desire. Signed-off-by: Eric Blake <eblake@redhat.com> --- docs/nbdkit-filter.pod | 7 +- server/filters.c | 141 ++++++++++++++++++++++++++---------------...

..._functions structure contains pointers to backend + * functions. However because these functions are all expecting a + * backend and a connection, we cannot call them directly, but must + * write some next_* functions that unpack the two parameters from a + * single ‘void *nxdata’ struct pointer (‘b_conn’). + */ + +/* Literally a backend + a connection pointer. This is the + * implementation if ‘void *nxdata’ in the filter API. + */ +struct b_conn { + struct backend *b; + struct connection *conn; +}; + +static int64_t +next_get_size (void *nxdata) +{ + struct b_conn *b_conn = nxdata; + return...

...0 100644 --- a/src/filters.c +++ b/src/filters.c @@ -280,43 +280,40 @@ next_can_trim (void *nxdata) } static int -next_pread (void *nxdata, void *buf, uint32_t count, uint64_t offset) +next_pread (void *nxdata, void *buf, uint32_t count, uint64_t offset, + uint32_t flags) { struct b_conn *b_conn = nxdata; - return b_conn->b->pread (b_conn->b, b_conn->conn, buf, count, offset, 0); + return b_conn->b->pread (b_conn->b, b_conn->conn, buf, count, offset, flags); } static int -next_pwrite (void *nxdata, const void *buf, uint32_t count, uint64_t offset) +next...

...onfig_key (f->backend.next); + return b->next->magic_config_key (b->next); } static int @@ -236,7 +230,7 @@ static int filter_open (struct backend *b, struct connection *conn, int readonly) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn nxdata = { .b = f->backend.next, .conn = conn }; + struct b_conn nxdata = { .b = b->next, .conn = conn }; void *handle; debug ("%s: open readonly=%d", f->name, readonly); @@ -245,24 +239,24 @@ filter_open (struct backend *b, struct connection *conn, int readonly) han...

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

...> -/* Literally a backend, a connection pointer, and the filter's handle. > +/* Literally a backend and the filter's handle. > + * > * This is the implementation of our handle in .open, and serves as > * a stable ‘void *nxdata’ in the filter API. > */ > -struct b_conn { > +struct b_h { > struct backend *b; > - struct connection *conn; > void *handle; > }; > > @@ -186,22 +186,22 @@ filter_config_complete (struct backend *b) > static int > next_preconnect (void *nxdata, int readonly) > { > - struct b_conn *b_co...

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.

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

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.

...extra fields relating + * to this filter. + */ +struct backend_filter { + struct backend backend; + char *filename; + void *dl; + struct nbdkit_filter filter; +}; + +/* Literally a backend + a connection pointer. This is the + * implementation of ‘void *nxdata’ in the filter API. + */ +struct b_conn { + struct backend *b; + struct connection *conn; +}; + +/* Note this frees the whole chain. */ +static void +filter_free (struct backend *b) +{ + struct backend_filter *f = container_of (b, struct backend_filter, backend); + + f->backend.next->free (f->backend.next); + + /* Acquiring...

..._functions structure contains pointers to backend + * functions. However because these functions are all expecting a + * backend and a connection, we cannot call them directly, but must + * write some next_* functions that unpack the two parameters from a + * single ‘void *nxdata’ struct pointer (‘b_conn’). + */ + +/* Literally a backend + a connection pointer. This is the + * implementation of ‘void *nxdata’ in the filter API. + */ +struct b_conn { + struct backend *b; + struct connection *conn; +}; + +static int64_t +next_get_size (void *nxdata) +{ + struct b_conn *b_conn = nxdata; + return...

...orted"); + *error = EINVAL; + return false; + } + return true; /* Command validates. */ } diff --git a/src/filters.c b/src/filters.c index 2804cba..87c6e97 100644 --- a/src/filters.c +++ b/src/filters.c @@ -297,6 +297,20 @@ next_can_trim (void *nxdata) return b_conn->b->can_trim (b_conn->b, b_conn->conn); } +static int +next_can_zero (void *nxdata) +{ + struct b_conn *b_conn = nxdata; + return b_conn->b->can_zero (b_conn->b, b_conn->conn); +} + +static int +next_can_fua (void *nxdata) +{ + struct b_conn *b_conn = nxdata; + return...

...size_t *nr_extents, struct nbdkit_extent **extents, + int *err); }; /* plugins.c */ diff --git a/server/filters.c b/server/filters.c index 5b7abc4..d6cc00f 100644 --- a/server/filters.c +++ b/server/filters.c @@ -309,6 +309,13 @@ next_can_zero (void *nxdata) return b_conn->b->can_zero (b_conn->b, b_conn->conn); } +static int +next_can_extents (void *nxdata) +{ + struct b_conn *b_conn = nxdata; + return b_conn->b->can_extents (b_conn->b, b_conn->conn); +} + static int next_can_fua (void *nxdata) { @@ -364,6 +371,16 @@ next_zero (void *...

Rebased filters patch. Requires current git master + the locks / thread model fix (https://www.redhat.com/archives/libguestfs/2018-January/msg00128.html) So a few changes here since last time: The "introduce filters" and "implement filters" patches are squashed together. I introduced a concept of .prepare and .finalize. These run before and after the data serving phase

...kend.c b/server/backend.c index 749b1f15..ebdef63a 100644 --- a/server/backend.c +++ b/server/backend.c @@ -167,6 +167,20 @@ backend_unload (struct backend *b, void (*unload) (void)) free (b->name); } +int +backend_open (struct backend *b, struct connection *conn, int readonly) +{ + struct b_conn_handle *h = &conn->handles[b->i]; + + debug ("%s: open readonly=%d", b->name, readonly); + + assert (h->handle == NULL); + assert (h->can_write == -1); + if (readonly) + h->can_write = 0; + return b->open (b, conn, readonly); +} + void backend_set_handle...

...gt;filename); + debug ("%s: config_complete", f->name); if (f->filter.config_complete) { if (f->filter.config_complete (next_config_complete, f->backend.next) == -1) @@ -220,7 +220,7 @@ filter_open (struct backend *b, struct connection *conn, int readonly) struct b_conn nxdata = { .b = f->backend.next, .conn = conn }; void *handle; - debug ("%s: open readonly=%d", f->filename, readonly); + debug ("%s: open readonly=%d", f->name, readonly); if (f->filter.open) { handle = f->filter.open (next_open, &nxdata, rea...

...uint32_t type, void *opaque), + void *opaque); +#endif diff --git a/server/filters.c b/server/filters.c index 5b7abc4..e284080 100644 --- a/server/filters.c +++ b/server/filters.c @@ -309,6 +309,13 @@ next_can_zero (void *nxdata) return b_conn->b->can_zero (b_conn->b, b_conn->conn); } +static int +next_can_extents (void *nxdata) +{ + struct b_conn *b_conn = nxdata; + return b_conn->b->can_extents (b_conn->b, b_conn->conn); +} + static int next_can_fua (void *nxdata) { @@ -364,6 +371,16 @@ next_zero (void *...

...uint32_t type, void *opaque), + void *opaque); +#endif diff --git a/server/filters.c b/server/filters.c index 5b7abc4..e284080 100644 --- a/server/filters.c +++ b/server/filters.c @@ -309,6 +309,13 @@ next_can_zero (void *nxdata) return b_conn->b->can_zero (b_conn->b, b_conn->conn); } +static int +next_can_extents (void *nxdata) +{ + struct b_conn *b_conn = nxdata; + return b_conn->b->can_extents (b_conn->b, b_conn->conn); +} + static int next_can_fua (void *nxdata) { @@ -364,6 +371,16 @@ next_zero (void *...

This tentative commit implements extents/can_extents, roughly as discussed in the previous thread here: https://www.redhat.com/archives/libguestfs/2019-March/msg00017.html I can't say that I'm a big fan of having the plugin allocate an extents array. There are no other plugin callbacks currently where we require the plugin to allocate complex data structures (or indeed do any allocation