search for: handle_open

....h b/server/internal.h index eb0e30c1..167da59a 100644 --- a/server/internal.h +++ b/server/internal.h @@ -153,9 +153,17 @@ typedef int (*connection_send_function) (struct connection *, typedef void (*connection_close_function) (struct connection *) __attribute__((__nonnull__ (1))); +enum { + HANDLE_OPEN = 1, /* Set if .open passed, so .close is needed */ + HANDLE_CONNECTED = 2, /* Set if .prepare passed, so .finalize is needed */ + HANDLE_FAILED = 4, /* Set if .finalize failed */ +}; + struct b_conn_handle { void *handle; + unsigned char state; /* Bitmask of HANDLE_* values */ +...

...c. * * 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 from the filter closest to the * plugin, similar to typical .open order. But remember that diff --git a/filters/retry...

...s is the filter or plugin handle, but other state is also stored + * here. + * + * Use get_handle (conn, 0) to return the struct handle for the + * plugin, and get_handle (conn, b->i) to return the struct handle for + * the i'th backend (if b->i >= 1 then for a filter). + */ enum { HANDLE_OPEN = 1, /* Set if .open passed, so .close is needed */ HANDLE_CONNECTED = 2, /* Set if .prepare passed, so .finalize is needed */ HANDLE_FAILED = 4, /* Set if .finalize failed */ }; -struct b_conn_handle { - void *handle; +struct handle { + void *handle; /* Plugin or filter...

...ackend.c +++ b/server/backend.c @@ -241,10 +241,10 @@ backend_close (struct backend *b) struct handle *h = get_handle (conn, b->i); /* outer-to-inner order, opposite .open */ - controlpath_debug ("%s: close", b->name); if (h->handle) { assert (h->state & HANDLE_OPEN); + controlpath_debug ("%s: close", b->name); b->close (b, h->handle); } else -- 2.25.0

...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; OK because CONNECTED means the handle has been opened & prepared already. > /* Call these in order starting from the filter cl...

...d.c index 8bfa8525..753f5cca 100644 --- a/server/backend.c +++ b/server/backend.c @@ -221,15 +221,13 @@ backend_finalize (struct backend *b) if (h->state & HANDLE_FAILED) return -1; - if (h->handle) { - assert (h->state & HANDLE_CONNECTED); + assert (h->state & HANDLE_OPEN); + if (h->state & HANDLE_CONNECTED) { if (b->finalize (b, h->handle) == -1) { h->state |= HANDLE_FAILED; return -1; } } - else - assert (! (h->state & HANDLE_CONNECTED)); if (b->i) return backend_finalize (b->next); -- 2.24.1

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

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

...0 +241,10 @@ backend_close (struct backend *b) > struct handle *h = get_handle (conn, b->i); > > /* outer-to-inner order, opposite .open */ > - controlpath_debug ("%s: close", b->name); > > if (h->handle) { > assert (h->state & HANDLE_OPEN); > + controlpath_debug ("%s: close", b->name); > b->close (b, h->handle); > } > else > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

It's confusing to use the same terminology for a single backend as for the linked list of backends. In particular it's often not clear if we're calling the next backend or the whole chain of backends. --- server/internal.h | 14 ++++++++++-- server/connections.c | 20 ++++++++--------- server/locks.c | 2 +- server/main.c

...get_handle (conn, b->i); int r; + assert (!default_only); /* XXX Switch to is_tls... */ controlpath_debug ("%s: list_exports readonly=%d default_only=%d", b->name, readonly, default_only); assert (h->handle == NULL); assert ((h->state & HANDLE_OPEN) == 0); - if (default_only && h->default_exportname) - return nbdkit_add_export (exports, h->default_exportname, NULL); r = b->list_exports (b, readonly, default_only, exports); if (r == -1) @@ -180,13 +179,34 @@ backend_list_exports (struct backend *b, int readonly, int...

...t; assert (!default_only); /* XXX Switch to is_tls... */ controlpath_debug ("%s: list_exports readonly=%d default_only=%d", @@ -173,14 +173,15 @@ backend_list_exports (struct backend *b, int readonly, int default_only, assert (h->handle == NULL); assert ((h->state & HANDLE_OPEN) == 0); - r = b->list_exports (b, readonly, default_only, exports); - if (r == -1) + if (b->list_exports (b, readonly, default_only, exports) == -1 || + exports_resolve_default (exports, b, readonly) == -1) { controlpath_debug ("%s: list_exports failed", b->name); -...

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

...ult_only); /* XXX Switch to is_tls... */ > controlpath_debug ("%s: list_exports readonly=%d default_only=%d", > @@ -173,14 +173,15 @@ backend_list_exports (struct backend *b, int readonly, int default_only, > assert (h->handle == NULL); > assert ((h->state & HANDLE_OPEN) == 0); > > - r = b->list_exports (b, readonly, default_only, exports); > - if (r == -1) > + if (b->list_exports (b, readonly, default_only, exports) == -1 || > + exports_resolve_default (exports, b, readonly) == -1) { > controlpath_debug ("%s: list_export...

...t;", - b->name, readonly, exportname); + controlpath_debug ("%s: open readonly=%d exportname=\"%s\" tls=%d", + b->name, readonly, exportname, conn->using_tls); assert (h->handle == NULL); assert ((h->state & HANDLE_OPEN) == 0); @@ -212,7 +212,7 @@ backend_open (struct backend *b, int readonly, const char *exportname) /* Most filters will call next_open first, resulting in * inner-to-outer ordering. */ - h->handle = b->open (b, readonly, exportname); + h->handle = b->open (b, readonly, expo...

This is a revision of my .default_export work, plus new work on .export_descriptions and a new exportname filter. I think it is now ready to check in. Things I'd still like in 1.22: - the file plugin should implement .list_exports (patch already posted, but it needs rebasing on this series) - the ext2 filter should override .list_exports when in exportname mode - the nbd plugin should be

..._function) (const void *buf, size_t len, int flags) - __attribute__((__nonnull__ (1, 2))); -typedef void (*connection_close_function) (struct connection *) __attribute__((__nonnull__ (1))); +typedef void (*connection_close_function) (void); enum { HANDLE_OPEN = 1, /* Set if .open passed, so .close is needed */ @@ -234,29 +231,22 @@ struct connection { }; extern void handle_single_connection (int sockin, int sockout); -extern int connection_get_status (struct connection *conn) - __attribute__((__nonnull__ (1))); -extern int connection_set_statu...

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.

More patches on the way for improving .list_exports signature and adding .export_description, but this is the promised code showing why nbdkit_string_intern is useful. Patch 4 is somewhat RFC: we could either add new API to take the boilerplate from: foo_config(const char *key, const char *value) { if (strcmp (key, "file") == 0) { CLEANUP_FREE char *tmp = nbdkit_realpath (value);

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.