search for: backend_valid_range

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

On Thu, Sep 19, 2019 at 12:34:17PM +0100, Richard W.M. Jones wrote: > diff --git a/server/backend.c b/server/backend.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,...

...+ /* The rest of the next ops are the same as normal plugin operations. */ int64_t (*get_size) (void *nxdata); int (*can_write) (void *nxdata); diff --git a/server/backend.c b/server/backend.c index 6c102f9..69a07d7 100644 --- a/server/backend.c +++ b/server/backend.c @@ -233,6 +233,15 @@ backend_valid_range (struct backend *b, struct connection *conn, /* Wrappers for all callbacks in a filter's struct nbdkit_next_ops. */ +int +backend_reopen (struct backend *b, struct connection *conn, int readonly) +{ + debug ("%s: reopen", b->name); + + backend_close (b, conn); + return back...

...+ /* The rest of the next ops are the same as normal plugin operations. */ int64_t (*get_size) (void *nxdata); int (*can_write) (void *nxdata); diff --git a/server/backend.c b/server/backend.c index 6c102f9..52b1734 100644 --- a/server/backend.c +++ b/server/backend.c @@ -233,6 +233,18 @@ backend_valid_range (struct backend *b, struct connection *conn, /* Wrappers for all callbacks in a filter's struct nbdkit_next_ops. */ +int +backend_reopen (struct backend *b, struct connection *conn, int readonly) +{ + struct b_conn_handle *h = &conn->handles[b->i]; + + debug ("%s: reopen&q...

...ct connection *conn) - __attribute__((__nonnull__ (1, 2))); -extern int backend_finalize (struct backend *b, struct connection *conn) - __attribute__((__nonnull__ (1, 2))); -extern void backend_close (struct backend *b, struct connection *conn) - __attribute__((__nonnull__ (1, 2))); -extern bool backend_valid_range (struct backend *b, struct connection *conn, +extern int backend_open (struct backend *b, int readonly) + __attribute__((__nonnull__ (1))); +extern int backend_prepare (struct backend *b) + __attribute__((__nonnull__ (1))); +extern int backend_finalize (struct backend *b) + __attribute__((__nonn...

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.

This is a retry filter implementation as outlined here: https://www.redhat.com/archives/libguestfs/2019-September/msg00167.html It is only lightly tested. One way to test it is to try an SSH copy (see the commit message for patch 2/2), and in the middle of the copy kill the per-connection sshd on the remote machine. You will see that the copy recovers after a few seconds. Add the nbdkit -v

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.

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

...n the underlying chain. > + * Used by the retry filter. > + */ > + int (*reopen) (void *nxdata, int readonly); > + ABI change - but we've solved that with our version match check. No further header version bumps needed :) > +++ b/server/backend.c > @@ -233,6 +233,15 @@ backend_valid_range (struct backend *b, struct connection *conn, > > /* Wrappers for all callbacks in a filter's struct nbdkit_next_ops. */ > > +int > +backend_reopen (struct backend *b, struct connection *conn, int readonly) > +{ > + debug ("%s: reopen", b->name); I'd...

..., conn, h->handle); + if (h->handle) { + assert (h->state & HANDLE_OPEN); + b->close (b, conn, h->handle); + } + else + assert (! (h->state & HANDLE_OPEN)); reset_b_conn_handle (h); if (b->i) backend_close (b->next, conn); @@ -273,13 +294,21 @@ 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 readonly=%d", b->name, readonly); - if (h->handle != NULL) + if (ba...

...b, struct connection *conn) + __attribute__((__nonnull__ (1, 2))); extern void backend_set_handle (struct backend *b, struct connection *conn, void *handle) - __attribute__((__nonnull__ (1, 2 /* not 3 */))); + __attribute__((__nonnull__ (1, 2, 3))); extern bool backend_valid_range (struct backend *b, struct connection *conn, uint64_t offset, uint32_t count) __attribute__((__nonnull__ (1, 2))); diff --git a/server/backend.c b/server/backend.c index 3b213bfb..64dbf7db 100644 --- a/server/backend.c +++ b/server/backend.c @@ -201,10 +201,23 @...

...controlpath_debug ("%s: close", b->name); @@ -249,7 +249,7 @@ backend_close (struct backend *b) } else assert (! (h->state & HANDLE_OPEN)); - reset_b_conn_handle (h); + reset_handle (h); if (b->i) backend_close (b->next); } @@ -258,7 +258,7 @@ bool backend_valid_range (struct backend *b, uint64_t offset, uint32_t count) { GET_CONN; - struct b_conn_handle *h = &conn->handles[b->i]; + struct handle *h = get_handle (conn, b->i); assert (h->exportsize <= INT64_MAX); /* Guaranteed by negotiation phase */ return count > 0 &&...

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

...otocol.c b/server/protocol.c index d41ad569..b56d16bd 100644 --- a/server/protocol.c +++ b/server/protocol.c @@ -72,7 +72,7 @@ validate_request (uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count, case NBD_CMD_TRIM: case NBD_CMD_WRITE_ZEROES: case NBD_CMD_BLOCK_STATUS: - if (!backend_valid_range (backend, offset, count)) { + if (!backend_valid_range (top, offset, count)) { /* XXX Allow writes to extend the disk? */ nbdkit_error ("invalid request: %s: offset and count are out of range: " "offset=%" PRIu64 " count=%" PRIu32,...

v2 was here: https://www.redhat.com/archives/libguestfs/2019-September/msg00221.html I think this is more like "the one". It handles reopen failing correctly, and there is a second test for that. I also ran my sshd tests locally and it worked in all scenarios I could think up (except of course sshd not being available at the start, but we want that to fail). Rich.

...;, +L<nbdkit-readahead-filter(1)>, +L<nbdkit-retry-filter(1)>, L<virsh(1)>, L<https://www.vmware.com/support/developer/vddk/> diff --git a/server/backend.c b/server/backend.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 reca...

...kend_open (struct backend *b, + int readonly, const char *exportname) + __attribute__((__nonnull__ (1, 3))); extern int backend_prepare (struct backend *b) __attribute__((__nonnull__ (1))); extern int backend_finalize (struct backend *b) @@ -414,8 +421,9 @@ extern bool backend_valid_range (struct backend *b, uint64_t offset, uint32_t count) __attribute__((__nonnull__ (1))); -extern int backend_reopen (struct backend *b, int readonly) - __attribute__((__nonnull__ (1))); +extern int backend_reopen (struct backend *b, +...

I still don't have .prepare/.finalize working cleanly across reopen, but did find a nasty bug where a botched assertion means we failed to notice reads beyond EOF in both the xz and retry filter. Refactoring backend.c will make .finalize work easier. Eric Blake (5): xz: Avoid reading beyond EOF retry: Check size before transactions tests: Test retry when get_size values change