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

[Cao jin <caoj.fnst@cn.fujitsu.com>]

On 07/22/2016 06:38 PM, Daniel P. Berrange wrote:
> 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.
>

Aha! I see! And also I see the comment you left in the code:
     /* socket_connect() does a non-blocking connect(), but it
      * still blocks in DNS lookups, so we must use a thread */

Thanks very much, and I think maybe I can do this cleanup work:)

> Regards,
> Daniel
>

-- 
Yours Sincerely,

Cao jin