[Libguestfs] [nbdkit PATCH v2 1/3] backend: Rework internal/filter error return semantics

[Richard W.M. Jones]

On Tue, Feb 06, 2018 at 08:42:13AM -0600, Eric Blake wrote:
> On 02/06/2018 01:05 AM, Richard W.M. Jones wrote:
> >
> >I had a think about this.  How about (for the internal backend
> >interface only) returning an extra ‘int *err’ parameter?
> 
> For backend only, or for backend and filters?  The biggest reason
> for the change is so that filters can inspect/modify the error to
> push over the wire to the client.  If for the filters, would it be
> for both the callbacks and the next_ops functions, or just the
> next_ops functions?

Initially I meant for the backend interface, but yes this could be
added to the filter interface as well.  I believe for callbacks and
next_ops .. I think?

> >
> >   int (*pread) (struct backend *, struct connection *conn, void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err);
> >
> >This has some advantages:
> >
> >  - It's explicit.
> >
> >  - It means we preserve the convention that -1 = error
> >    used everywhere else in nbdkit.
> >
> >  - Better for APIs that actually return something like get_size.
> >
> >  - Can be both read and written.
> 
> Seems like it might be reasonable enough, even though it's a bit
> more verbose; I can respin my series along those lines for
> comparison.
> 
> Note that get_size() doesn't send an error over the wire to the
> client, so there, accessing errno in the filter doesn't buy much,
> and the filter really has no need to modify errno if failing its own
> .get_size callback.  It's only the transmission functions (.pread,
> .pwrite, .flush, .trim, .zero) where getting at the error value
> matters.
> 

OK.  But we could add another callback in future which does return a
significant value.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html