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

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.