Re: [PATCH] tests/socket-helpers: Only fail socket protocol check if it is really necessary

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

On Tue, Sep 08, 2020 at 01:27:46PM +0200, Thomas Huth wrote:
> On 08/09/2020 13.07, Daniel P. Berrangé wrote:
> > On Tue, Sep 08, 2020 at 12:54:35PM +0200, Thomas Huth wrote:
> >> The tests/test-char test is currently always failing on my system since
> >> socket_can_bind_connect("::1", PF_INET6) fails with EINVAL and thus
> >> socket_check_protocol_support() is returning -1 for an error. But IPv4
> >> is working fine. The logic in socket_check_protocol_support() seems to
> >> be wrong here, if either IPv6 or IPv4 is working, we should not return
> >> an error here. Thus rework the function to only return errors if both
> >> checks failed.
> > 
> > Can you tell me which exact syscall is giving EINVAL in this scenario ?
> 
> getaddrinfo() fails with -2 (EAI_NONAME ?). The logic in
> socket_can_bind_connect() then translates this into EINVAL.

Ok, lets just translate EAI_NONAME into  EADDRNOTAVAIL as we do for
the other getaddrinfo errors we want to treat as soft-failures

> 
> [...]
> >> -    if (socket_can_bind_connect("::1", PF_INET6) < 0) {
> >> -        if (errno != EADDRNOTAVAIL) {
> >> -            return -1;
> >> -        }
> >> -    } else {
> >> -        *has_ipv6 = true;
> >> +    errv6 = socket_can_bind_connect("::1", PF_INET6);
> >> +    *has_ipv6 = (errv6 == 0);
> >> +
> >> +    if (!*has_ipv4 && !*has_ipv6 &&
> >> +        (errv4 != EADDRNOTAVAIL || errv6 != EADDRNOTAVAIL)) {
> >> +        return -1;
> >>      }
> > 
> > The return value of socket_can_bind_connect is either 0 or -1,
> > but you're treating it an errno which isn't right.
> 
> Uh, where's my brown paperbag? ... looks like I need more coffee today...
> 
>  Thomas
> 

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