Re: [Qemu-devel] [PATCH 2/2] util/qemu-sockets: shoot unix_nonblocking_connect()

[Daniel P. Berrange]

On Fri, Jul 22, 2016 at 06:43:51PM +0800, Cao jin wrote:
> 
> 
> On 07/22/2016 06:30 PM, Daniel P. Berrange wrote:
> > On Fri, Jul 22, 2016 at 06:34:11PM +0800, Cao jin wrote:
> > > Hi Daniel
> > > 
> > > On 07/21/2016 11:39 PM, Daniel P. Berrange wrote:
> > > > On Thu, Jul 21, 2016 at 08:42:25AM -0600, Eric Blake wrote:
> > > > > On 07/21/2016 04:33 AM, Cao jin wrote:
> > > > > > It is never used, and now all connect is nonblocking via
> > > > > > inet_connect_addr().
> > > > > > 
> > > > > 
> > > > > Could be squashed with 1/2.  In fact, if you squash it, I'd title the 
> > > > > patch:
> > > > > 
> > > > > util: Drop unused *_nonblocking_connect() functions
> > > > > 
> > > > > You may also want to call out which commit id rendered the functions 
> > > > > unused.
> > > > 
> > > > Well once those two functions are dropped the only other place accepting
> > > > NonBlockingConnectHandler is the socket_connect() method. Since nearly
> > > > everything is converted to QIOChannel now, there's only one caller of
> > > > socket_connect() left, and that's net/socket.c
> > > > 
> > > > Any newly written code which needs a non-blocking connect should use the
> > > > QIOChannel code, so I don't see any further usage of socket_connect()
> > > > being added.
> > > > 
> > > > IOW, we can rip out NonBlockingConnectHandler as a concept entirely, not
> > > > merely drop the *_nonblocking_connect() methods.
> > > > 
> > > 
> > > I don't quite follow the "rip out NonBlockingConnectHandler" thing.
> > > According what I learned from code, we offered non-blocking connection
> > > mechanism, but it seems nobody use it(all callers of socket_connect() set
> > > callback as NULL), so, do you mean removing this mechanism?
> > 
> > Yes, remove it all, as it is no longer needed.
> > 
> 
> Thanks for clarifying. Actually, I am still curious why nobody want to use
> this mechanism, is there any disadvantage? And why this mechanism is
> introduced in

As mentioned previously it is obsolete as all new code will use the QIOChannel
APIs which already provide non-blocking connect in a different manner. The
qemu-sockets non-blocking code never worked properly in the first place
because it calls getaddrinfo() which blocks on DNS lookups.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|