search for: backend_fin

If .prepare fails, we do not want to call .finalize (similar to how if .open fails, we do not want to call .close). However, the logic in backend_finalize was checking on the wrong condition ('was the connection opened' rather than 'was the connection prepared'), which led to an assertion failure. Simple reproducer: $ nbdkit -U - -f --filter cache --run 'qemu-io -r -c quit $nbd' sh - <<\EOF > case $1 in get_siz...

...are readonly=%d", b->name, h->can_write == 0); - return b->prepare (b, conn, h->handle, h->can_write == 0); + if (b->prepare (b, conn, h->handle, h->can_write == 0) == -1) + return -1; + h->state |= HANDLE_CONNECTED; + return 0; } int @@ -224,12 +230,22 @@ backend_finalize (struct backend *b, struct connection *conn) * filter furthest away from the plugin, and matching .close order. */ - assert (h->handle); debug ("%s: finalize", b->name); - if (b->finalize (b, conn, h->handle) == -1) + /* Once finalize fails, we can do noth...

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

...#3 0x00007f12b86c1a66 in __GI___assert_fail (assertion=assertion at entry=0x5577156bb09d "h->state & HANDLE_CONNECTED", file=file at entry=0x5577156bb064 "backend.c", line=line at entry=240, function=function at entry=0x5577156bb880 <__PRETTY_FUNCTION__.6528> "backend_finalize") at assert.c:101 #4 0x00005577156ad04e in backend_finalize (b=<optimized out>, conn=conn at entry=0x55771f1ac710) at backend.c:240 #5 0x00005577156b6ea8 in negotiate_handshake_newstyle_options (conn=<optimized out>) at protocol-handshake-newstyle.c:484 #6 protocol_handshak...

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

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.

...econnect (backend, read_only) == -1) + if (top && top->preconnect (top, read_only) == -1) goto done; /* NBD handshake. @@ -225,7 +225,7 @@ handle_single_connection (int sockin, int sockout) /* Finalize (for filters), called just before close. */ lock_request (); - r = backend_finalize (backend); + r = backend_finalize (top); unlock_request (); if (r == -1) goto done; @@ -251,12 +251,12 @@ new_connection (int sockin, int sockout, int nworkers) conn->status_pipe[0] = conn->status_pipe[1] = -1; - conn->handles = calloc (backend->i + 1, sizeof *c...

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

...t backend_prepare (struct backend *b) { GET_CONN; - struct b_conn_handle *h = &conn->handles[b->i]; + struct handle *h = get_handle (conn, b->i); assert (h->handle); assert ((h->state & (HANDLE_OPEN | HANDLE_CONNECTED)) == HANDLE_OPEN); @@ -209,7 +209,7 @@ int backend_finalize (struct backend *b) { GET_CONN; - struct b_conn_handle *h = &conn->handles[b->i]; + struct handle *h = get_handle (conn, b->i); /* Call these in reverse order to .prepare above, starting from the * filter furthest away from the plugin, and matching .close order. @@...

...* exportname if they need to refer to it later. + */ +extern int backend_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)))...

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