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

[Eric Blake]

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.

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.

> ---
>  generator/generator |  8 +++-----
>  lib/connect.c       | 48 +++++++++++++++++++++++++++++++++++----------
>  2 files changed, 41 insertions(+), 15 deletions(-)
> 
-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190521/a648f9d0/attachment.sig>