[Libguestfs] [libnbd] How close are we to declaring a stable API?

[Richard W.M. Jones]

On Tue, Jul 02, 2019 at 08:48:15PM -0500, Eric Blake wrote:
> On 6/29/19 5:25 AM, Richard W.M. Jones wrote:
> > As the subject says, how close are we to being able to declare a
> > stable API for libnbd?
> > 
> > I believe these are the main topics:
> > 
> > * Do we need to have an extra thread for writing?  I'm unclear about
> >   whether b92392b717 (which allows the state machine to break during
> >   reply processing) means we definitely don't need threads.  I imagine
> >   that two threads doing simultaneous send(2) and recv(2) calls could
> >   still improve performance (eg. having two cores copying skbs from
> >   userspace to and from kernel).
> 
> It's hard to see how simultaneous send() and recv() with nonblocking
> sockets will do any better across two threads than one. If it was
> blocking, then it makes total sense, but since we have non-blocking I/O,
> I'm not seeing that it will make any noticeable difference.  That said,
> I do know that you were experimenting at one point about adding a way to
> offload writing to a user-controlled thread, and maybe it's still worth
> playing with that a bit more.

The POSIX send and recv APIs involve copying.  It does seem plausible
that two cores could perform better than one by doing the copies
between userspace and kernel space in parallel.  However it would
certainly depend on the specifics of how Linux works - for example
does it hold a lock per socket?

I had a patch to enable a concurrent writer thread:
https://www.redhat.com/archives/libguestfs/2019-June/msg00030.html
However it had multiple unresolved design problems:
https://www.redhat.com/archives/libguestfs/2019-June/msg00036.html

A better way to solve it might be to use an alternate set of APIs.
uring is one obvious choice.

> > 
> > * Should ‘nbd_shutdown’ take an extra parameter to indicate that it
> >   should be delayed until all commands in the queue are retired?
> 
> That may still be worthwhile to pursue.
> 
> > 
> > Is there anything else?
> 
> Do we like the signature of all the callbacks? Right now, there is a
> slight inconsistency in that the 'int *error' parameter is last for
> block_status and notify callbacks, but comes before 'int status' for
> pread_structured. It would be a simple API switch to pread_structured to
> put it last there as well, but something we can't do after declaring
> stability.

Yes we ought to fix these kinds of things before making the API
stable.

> > 
> > We could also consider doing a "soft stable API" release where we bump
> > the version up to 0.9.x, announce that we're going to make the API
> > stable soon, have a much higher bar for breaking the API, but don't
> > actually prevent API breaks in cases where it's necessary.
> 
> The notify APIs are in now, and I'm trying to squash a valgrind failure
> where tests/errors sometimes fails under load (if the server can read
> data fast enough that a large NBD_CMD_WRITE doesn't block after all).

OK, thanks.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v