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