[libvirt] [PATCH v3] Loop through all resolved addresses in virNetSocketNewListenTCP

[Daniel P. Berrangé]

On Wed, Jul 04, 2018 at 01:48:56PM +0200, Olaf Hering wrote:
> Am Wed, 4 Jul 2018 11:57:58 +0100
> schrieb Daniel P. Berrangé <berrange at redhat.com>:
> 
> > On Mon, Jun 04, 2018 at 12:29:37PM +0200, Olaf Hering wrote:
> > > In this case the resolver returns not only the IPv4 addresses but also
> > > IPv6. Binding the IPv6 address will obviously fail. But this terminates
> > > the entire loop, even if binding to IPv4 succeeded.  
> > Could you elaborate on this a bit more, as on reflection I'm not sure
> > I understand the flaw here. Could you show the 'ip addr' output and
> > corresponding hostname resolution results, and say what errno you
> > are seeing from bind().
> 
> Not sure what is unclear about this statement.
> The hostname resolves to ipv4+ipv6. Binding to ipv4 works because this
> address is available on the interface. Binding to ipv6 fails because this
> address not not available on any interface.

So you've configured an IPv6 address in the hostname that doesn't
actually exist at all on any interface ?  In the text you cut out
in your reply it was saying you have NICs with IPv4 and NICs with
IPv6 link local, so it sounded like this problem was somehow
related to multiple NICs.

> Since bind() already succeeded (or will succeed) for one address, there is
> no reason to error out. Anyone who connects to an resolved ipv6 address will
> get an error, and moves on to try the resolved ipv4 address.
> libvirt is overdoing things here.

Many of the errnos reported by bind() indicate problems we should not be
ignoring IMHO.  So I'd rather handle the actual errnos that indicate
non-fatal problems.

> > This EDESTADDRREQ feels a bit odd to use - "Destination address required"
> > doesn't make much sense for something that has to be a local address.
> 
> Maybe that depends on the context. The whole function does not try hard
> to preserve all possible errors for the nsocks==0 case. This specific new
> case just assumes that bind() was the thing that resulted in nsocks==0.
> There are other error paths.
> 
> What errno value do you suggest for nsocks=0?

Well what errno are you getting from bind() when it fails ? It sounds like
it might be EADDRNOTAVAIL.

Preserving that looks like best option to me, rather than arbitrarily
picking a different/fixed errno.

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 :|