Re: [Qemu-devel] [RFC 2/4] qemu-socket: Allow custom socket options in socket_connect

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Jan 30, 2018 at 03:13:42AM +0800, Zihan Yang wrote:
> Currently, socket_connect doesn't allow custom socket options,
> which is inconvenient when the caller wants a different kind of
> socket from that the socket_connect provides. This patch allows
> custom config in socket_connect by providing an extra parameter.
> Existing functions can just pass a NULL pointer.

Again adding options functionality hwich is not actually used
is bad.

The O_NONBLOCK functionality makes more sense in this context
but the implementation is really broken.

These functions do hostname lookups, so can never do non-blocking
mode correctly as the hostname lookup itself does blocking I/O
to the nameserver(s).  Ignoring that, the way you handle the
connect() is wrong too. You're iterating over many addresses
returned by getaddrinfo() and doing a non-blocking connect
on each of them. These will essentially all fail with EAGAIN
and you'll skip onto the next address which is wrong.

This code used to have support for O_NONBLOCK but it was removed
because the DNS problem means that any code relying on it was
already broken.  The rest of the QEMU codebase has been converted
to use QIOChannelSocket instead which can handle non-blocking
DNS properly

So overall I'm against this patch too.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|