I've given some more thoughts to error handling in nbdkit filters, and
am writing this while it is still fresh on my mind (even though I won't
have time to code any of it until next week).
Right now, plugins are allowed to call either nbdkit_set_error OR to
leave a value in errno; the latter works only if errno_is_preserved was
set. If a plugin returns -1 but does not set an error through either
method, then nbdkit assumes EIO as the error value.
But filters may want/need to perform error recovery (perhaps recognizing
an ENOSPC on write, by resizing and then retrying the operation).
Filters have access to errno after a client call fails, but cannot rely
on errno having the right value - they have to be able to get at the
thread-local value that was stored during nbdkit_set_error.
Furthermore, if a filter does ANYTHING non-trivial after the client
returns an error, it is likely to clobber the errno value that the
plugin set (if the client relied on errno instead of nbdkit_set_error) -
which may be okay if the filter itself wants to return a new value, but
not so good if it was just logging things but wants to preserve the
plugin's error. At the same time, if the plugin set the error via
nbdkit_set_error, then changing errno won't make a difference; the way
nbdkit handles errors is that the thread-local value takes priority and
errno is only a fallback if the thread-local value was not set.
So, we can either mandate that filters that want to do non-trivial work
after calling into next_ops must check what the plugin set (hoist the
connections.c:get_error into nbdkit_get_error), and then MUST call
nbdkit_set_error to change the error (meaning that filters should not
rely on errno for setting the value) - or we can change the semantics of
the backend and filters to say that instead of returning -1, we return
the actual error code. Plugins still return -1 for backward
compatibility, but the conversion from plugin to backend semantics in
plugins.c will convert the errno-or-nbdkit_set_error value into its
return; and then the filter stack will see the return value directly,
and can then return that same value without having to mess with either
errno or nbdkit_set_error.
At this point, I'm leaning towards this semantic change as being less
boilerplate in filters.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
