[Libguestfs] [PATCH libnbd] api: Synchronous connect waits til all connections are connected.

[Richard W.M. Jones]

On Tue, May 21, 2019 at 05:08:05PM -0500, Eric Blake wrote:
> On 5/21/19 3:44 PM, Richard W.M. Jones wrote:
> > If not using multi-conn then obviously the synchronous connection
> > calls ‘nbd_connect_unix’, ‘nbd_connect_tcp’ and ‘nbd_connect_command’
> > should only return when the (one) connection object is connected.
> > 
> > In the multi-conn case it's not very clear what these synchronous
> > calls should do.  Previously I had it so that they would return as
> > soon as at least one connection was connected.  However this is a
> > problem if you are using them as a convenient way to set up a
> > multi-threaded main loop, because it can be that some of them have not
> > finished connecting, but then you issue commands on those connections
> > and that will fail.  The failure is pernicious because most of the
> > time you won't see it, only if one connection is slow.  So it's
> > (probably) better that the synchronous ‘nbd_connect_unix’ and
> > ‘nbd_connect_tcp’ should connect every connection object before
> > returning.
> 
> Should this be a bool parameter for the caller to opt-in to?  In one
> direction, returning as soon as possible allows the caller to issue
> further synchronous commands which will pick the ready connections and
> skip the ones still initializing, to at least get lower latency on the
> first commands and better performance later as more connections come up;
> in the other direction, waiting to return until all connections are made
> makes it easier to issue async commands on any connection without
> worrying if the connection is up. The choice of the parameter to pass in
> depends, then, on whether the caller plans to make future sync calls or
> async calls.

It could be.  On the other hand if people want to do complicated stuff
then there is the AIO interface.

> Also, I'm playing with the idea of having some sort of witness whether a
> connection has ever reached the READY state (even if it is not there at
> the moment). Prior to that point, we don't want to accept commands
> (whether sync or async) because we aren't finished negotiating and thus
> cannot guarantee if the can_FOO() functions are accurate; after that
> point, it may be nice to queue up the commands for later issuance when
> we DO come back around to READY, rather than insisting that we be
> exactly in the READY state when issuing a command.
> 
> > 
> > For ‘nbd_connect_command’, it essentially ignored multi-conn anyway,
> > and I changed it so that it waits for conn[0] to get connected and
> > returns, the other connections (if they exist) being ignored.  It
> > should probably be an error for the user to enable multi-conn on the
> > handle and then use nbd_connect_command, but I did not make that
> > change yet.
> 
> Yes, that change (as a separate patch) makes sense.

Even more radical, what if we prevent the use of multi-conn at all
with the synchronous interface?  (I don't think this is a good idea,
just putting it out there for discussion).

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top