[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH nbdkit v3 2/3] Add new retry filter.

On 9/19/19 11:32 AM, Eric Blake wrote:
On 9/19/19 10:26 AM, Richard W.M. Jones wrote:
This filter can be used to transparently reopen/retry when a plugin
fails.  The connection is closed and reopened which for most plugins
causes them to attempt to reconnect to their source.

+struct retry_handle {
+  int readonly;                 /* Save original readonly setting. */
+static void *
+retry_open (nbdkit_next_open *next, void *nxdata, int readonly)

Out of review time today, I'll get back to the rest of this file later...

Resuming the review, I've found several issues, and have been pushing trivial patches as well as preparing more complex patches that I will post for review soon. Found so far:

test-retry.sh used ((++i)) in the 'sh -' scriptlet, but failed to request that the scriptlet be executed in bash (failure if /bin/sh doesn't recognize that bashism)

retry failed to implement .flush

nbdkit --run '...' exits with $? set to 0 even if nbdkit died from a coredump such as an assertion failure (oops). I've got a solution for that which involves using waitpid() (I was afraid I'd have to go with something more complex like a non-blocking pipe that disappears when the child exits, or even use the new Linux pidfd, but waitid() is reliable enough when spawning our own child process).

retrying anything other than .pread fails with an assertion (which was too easy to overlook due to the --run bug above...), because h->can_FOO was not consulted even though backend.c claimed it would. This is particularly noticeable with the retry-readonly=1 flag (where h->can_write changes from 1 to 0).

retrying .extents fails with an assertion if the first attempt made any progress before failure

we aren't calling .prepare/.finalize correctly during a reopen. Doesn't matter when retry is the filter closest to the plugin, but will matter if another filter can live in-between

Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]