search for: backend_prepare

...>>     int64_t r; >>     CLEANUP_FREE char *name = NULL; >> -  const char *fname = file ?: nbdkit_export_name (); >> +  const char *fname = file ?: h->exportname; > > 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-handshak...

...> > int64_t r; > > CLEANUP_FREE char *name = NULL; > >- const char *fname = file ?: nbdkit_export_name (); > >+ const char *fname = file ?: h->exportname; > > 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 i...

...2,6 +193,7 @@ backend_open (struct backend *b, struct connection *conn, int readonly) return -1; } + h->state |= HANDLE_OPEN; if (b->i) /* A filter must not succeed unless its backend did also */ assert (conn->handles[b->i - 1].handle); return 0; @@ -203,6 +205,7 @@ backend_prepare (struct backend *b, struct connection *conn) struct b_conn_handle *h = &conn->handles[b->i]; assert (h->handle); + assert ((h->state & (HANDLE_OPEN | HANDLE_CONNECTED)) == HANDLE_OPEN); /* Call these in order starting from the filter closest to the * plugin, simi...

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

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

...tions(+), 10 deletions(-) diff --git a/server/internal.h b/server/internal.h index c31bb340..da4fae19 100644 --- a/server/internal.h +++ b/server/internal.h @@ -334,9 +334,11 @@ extern int backend_open (struct backend *b, struct connection *conn, __attribute__((__nonnull__ (1, 2))); extern int backend_prepare (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 void backend_set_handle (struct backend *b, struct connection *conn,...

...; struct ext2_inode inode; > int64_t r; > CLEANUP_FREE char *name = NULL; > - const char *fname = file ?: nbdkit_export_name (); > + const char *fname = file ?: h->exportname; 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 call...

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

...b/server/backend.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2013-2021 Red Hat Inc. + * Copyright (C) 2013-2023 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -312,7 +312,10 @@ backend_prepare (struct context *c) struct backend *b = c->b; assert (c->handle); - assert ((c->state & (HANDLE_OPEN | HANDLE_CONNECTED)) == HANDLE_OPEN); + assert (c->state & HANDLE_OPEN); + + if (c->state & HANDLE_CONNECTED) + return 0; /* Call these in order starting...

...ckend *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 connection *conn) - __attribute__((__nonnull__ (1, 2))); -extern void backend_close (struct backend *b, struct connection *conn) - __attribute__((__nonnull_...

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.

...n); @@ -409,7 +413,7 @@ filter_prepare (struct backend *b, struct connection *conn, int readonly) struct b_conn nxdata = { .b = b->next, .conn = conn }; /* Call these in order starting from the filter closest to the - * plugin. + * plugin, similar to typical .open order. */ if (backend_prepare (b->next, conn) == -1) return -1; @@ -431,7 +435,7 @@ filter_finalize (struct backend *b, struct connection *conn) debug ("%s: finalize", b->name); /* Call these in reverse order to .prepare above, starting from the - * filter furthest away from the plugin. + * filte...

...> /* nbdkit > - * Copyright (C) 2013-2021 Red Hat Inc. > + * Copyright (C) 2013-2023 Red Hat Inc. > * > * Redistribution and use in source and binary forms, with or without > * modification, are permitted provided that the following conditions are > @@ -312,7 +312,10 @@ backend_prepare (struct context *c) > struct backend *b = c->b; > > assert (c->handle); > - assert ((c->state & (HANDLE_OPEN | HANDLE_CONNECTED)) == HANDLE_OPEN); > + assert (c->state & HANDLE_OPEN); > + > + if (c->state & HANDLE_CONNECTED) > + return...

...struct backend *b, int readonly) h->state |= HANDLE_OPEN; if (b->i) /* A filter must not succeed unless its backend did also */ - assert (conn->handles[b->i - 1].handle); + assert (get_handle (conn, b->i-1)->handle != NULL); return 0; } @@ -186,7 +186,7 @@ int 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 (...

In https://bugzilla.redhat.com/show_bug.cgi?id=1841820, it was pointed out that the retry filter not retrying .open means that an ssh connection (such as in a vmx+ssh v2v conversion) fails when the ssh connection itself cannot be retried. A year ago, this was an inherent limitation of our retry implementation; but in the meantime, my work to allow filters to open independent backends has made it

...4 +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_size (top); if (size == -1) return -1; if (size < 0) { @@ -98,57 +98,57 @@ protocol_common_open (uint64_t *exportsize, uint16_t *flags) /* Check all fl...

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

I'm still working on another set of patches to have reopen call .finalize/.prepare (so that another filter can safely appear between retry and the plugin), but for tonight, these are the patches I think are ready to go. Eric Blake (4): retry: Handle can_fua and can_fast_zero changes tests: Test retry with different fua/fast-zero flags server: Close backends if a filter's .open fails

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