search for: backend_open

...;eblake@redhat.com> --- server/backend.c | 9 ++++++++- server/filters.c | 12 ++++++++---- 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) a...

...file changed, 73 insertions(+), 144 deletions(-) > > @@ -216,201 +205,181 @@ plugin_magic_config_key (struct backend *b) > static int > next_open (void *nxdata, int readonly) > { > - struct b_h *b_h = nxdata; > + struct backend *b_next = nxdata; > > - return backend_open (b_h->b, readonly); > + return backend_open (b_next, readonly); > } With this change, 'next_open' and '(int (*)(void *, int))backend_open' now have identical semantics. I'm trying to see if there are further changes we could make that would alleviate the need for...

...rver/internal.h index a55d6406..fb197b9e 100644 --- a/server/internal.h +++ b/server/internal.h @@ -325,6 +325,9 @@ extern void backend_load (struct backend *b, const char *name, extern void backend_unload (struct backend *b, void (*unload) (void)) __attribute__((__nonnull__ (1))); +extern int backend_open (struct backend *b, struct connection *conn, + int readonly) + __attribute__((__nonnull__ (1, 2))); extern void backend_set_handle (struct backend *b, struct connection *conn, void *handle) __attribute__((__nonnull__ (1, 2 /* not 3 */)))...

...> > Hmm - we already pass the same 'readonly' state to filter's .prepare, > but not to backend_prepare(), which has to reconstruct it.  Would it be > easier to also change the signature of backend_prepare() to take both > the original readonly and exportname passed to backend_open(), rather > than making the filter have to save it off in the filter?  It looks like > protocol-handshake.c is the only caller, and still has everything in > scope at the time. Nope, we can't. It's because the sequence is: backend_open("clientname") outer_filter....

...inalize) (struct backend *, void *handle); void (*close) (struct backend *, void *handle); @@ -402,8 +404,13 @@ extern void backend_load (struct backend *b, const char *name, extern void backend_unload (struct backend *b, void (*unload) (void)) __attribute__((__nonnull__ (1))); -extern int backend_open (struct backend *b, int readonly) - __attribute__((__nonnull__ (1))); +/* exportname is only valid for this call and almost certainly will be + * freed on return of this function, so backends must save the + * exportname if they need to refer to it later. + */ +extern int backend_open (struct back...

...), 144 deletions(-) > > > > >@@ -216,201 +205,181 @@ plugin_magic_config_key (struct backend *b) > > static int > > next_open (void *nxdata, int readonly) > > { > >- struct b_h *b_h = nxdata; > >+ struct backend *b_next = nxdata; > >- return backend_open (b_h->b, readonly); > >+ return backend_open (b_next, readonly); > > } > > With this change, 'next_open' and '(int (*)(void *, > int))backend_open' now have identical semantics. I'm trying to see > if there are further changes we could make that wo...

...dle (struct b_conn_handle *h) { h->handle = NULL; + h->state = 0; h->exportsize = -1; h->can_write = -1; h->can_flush = -1; diff --git a/server/backend.c b/server/backend.c index 702c9b96..bdc5bbfd 100644 --- a/server/backend.c +++ b/server/backend.c @@ -176,6 +176,7 @@ backend_open (struct backend *b, struct connection *conn, int readonly) debug ("%s: open readonly=%d", b->name, readonly); assert (h->handle == NULL); + assert ((h->state & HANDLE_OPEN) == 0); assert (h->can_write == -1); if (readonly) h->can_write = 0; @@ -192,6...

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

...+ > 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. > + */ > + h->can_write = -1; > + > return backend_open (b, conn, readonly); > } Obviously this hunk should be in the first patch ... Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports sh...

...server/protocol-handshake.c b/server/protocol-handshake.c index a32fcde0..70ea4933 100644 --- a/server/protocol-handshake.c +++ b/server/protocol-handshake.c @@ -79,14 +79,14 @@ protocol_common_open (uint64_t *exportsize, uint16_t *flags) uint16_t eflags = NBD_FLAG_HAS_FLAGS; int fl; - if (backend_open (backend, read_only) == -1) + if (backend_open (top, read_only) == -1) return -1; /* Prepare (for filters), called just after open. */ - if (backend_prepare (backend) == -1) + if (backend_prepare (top) == -1) return -1; - size = backend_get_size (backend); + size = backend_get...

...+backend_reopen (struct backend *b, struct connection *conn, int readonly) > +{ > + struct b_conn_handle *h = &conn->handles[b->i]; > + > + debug ("%s: reopen", b->name); > + > + if (h->handle != NULL) > + backend_close (b, conn); > + return backend_open (b, conn, readonly); Do we want to grab a resource lock at this point? Should we be trying hard to prevent further client calls from going to the retry filter while the retry filter is reopening the real plugin? Or do those locks belong in the retry filter itself? If nothing else, we probably w...

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

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

...(struct backend *, void *handle, int readonly); int (*finalize) (struct backend *, void *handle); void (*close) (struct backend *, void *handle); diff --git a/server/backend.c b/server/backend.c index 75ca53be..8f4fed9d 100644 --- a/server/backend.c +++ b/server/backend.c @@ -186,8 +186,8 @@ backend_open (struct backend *b, int readonly, const char *exportname) GET_CONN; struct handle *h = get_handle (conn, b->i); - controlpath_debug ("%s: open readonly=%d exportname=\"%s\"", - b->name, readonly, exportname); + controlpath_debug ("%s: open...

...handle), but in 1.14.1 it regressed into causing an assertion failure: we are now calling .open a second time on a connection that is already opened. $ nbdkit -rfv null & $ hacked-qemu-io -f raw -r nbd://localhost -c quit ... nbdkit: null[1]: debug: null: open readonly=1 nbdkit: backend.c:179: backend_open: Assertion `h->handle == NULL' failed. Worse, on the mainline development, we have recently made it possible for plugins to actively report different information for different export names; for example, a plugin may choose to report different answers for .can_write on export A than for expo...

...gt; > Hmm - we already pass the same 'readonly' state to filter's > .prepare, but not to backend_prepare(), which has to reconstruct it. > Would it be easier to also change the signature of backend_prepare() > to take both the original readonly and exportname passed to > backend_open(), rather than making the filter have to save it off in > the filter? It looks like protocol-handshake.c is the only caller, > and still has everything in scope at the time. Possibly, although this would be a seperate change. Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat...

...*handle, uint32_t count, uint64_t offset, uint32_t flags, int *err); }; @@ -382,72 +365,70 @@ extern void backend_load (struct backend *b, const char *name, extern void backend_unload (struct backend *b, void (*unload) (void)) __attribute__((__nonnull__ (1))); -extern int backend_open (struct backend *b, struct connection *conn, - int readonly) - __attribute__((__nonnull__ (1, 2))); -extern int backend_prepare (struct backend *b, struct connection *conn) - __attribute__((__nonnull__ (1, 2))); -extern int backend_finalize (struct backend *b, struct conne...

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.

...n void handle_single_connection (int sockin, int sockout); extern int connection_get_status (void); extern int connection_set_status (int value); diff --git a/server/backend.c b/server/backend.c index 616c24d8..9669ada1 100644 --- a/server/backend.c +++ b/server/backend.c @@ -154,7 +154,7 @@ int backend_open (struct backend *b, int readonly) { GET_CONN; - struct b_conn_handle *h = &conn->handles[b->i]; + struct handle *h = get_handle (conn, b->i); controlpath_debug ("%s: open readonly=%d", b->name, readonly); @@ -178,7 +178,7 @@ backend_open (struct backend *b, in...

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