* [PATCH] tests/socket-helpers: Only fail socket protocol check if it is really necessary
@ 2020-09-08 10:54 Thomas Huth
2020-09-08 11:07 ` Daniel P. Berrangé
0 siblings, 1 reply; 4+ messages in thread
From: Thomas Huth @ 2020-09-08 10:54 UTC (permalink / raw)
To: qemu-devel, Daniel P . Berrange, Marc-André Lureau
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.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
tests/socket-helpers.c | 23 +++++++++--------------
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/tests/socket-helpers.c b/tests/socket-helpers.c
index 19a51e887e..62a0e0f2d9 100644
--- a/tests/socket-helpers.c
+++ b/tests/socket-helpers.c
@@ -136,22 +136,17 @@ static int socket_can_bind_connect(const char *hostname, int family)
int socket_check_protocol_support(bool *has_ipv4, bool *has_ipv6)
{
- *has_ipv4 = *has_ipv6 = false;
+ int errv4, errv6;
- if (socket_can_bind_connect("127.0.0.1", PF_INET) < 0) {
- if (errno != EADDRNOTAVAIL) {
- return -1;
- }
- } else {
- *has_ipv4 = true;
- }
+ errv4 = socket_can_bind_connect("127.0.0.1", PF_INET);
+ *has_ipv4 = (errv4 == 0);
- 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;
}
return 0;
--
2.18.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] tests/socket-helpers: Only fail socket protocol check if it is really necessary
2020-09-08 10:54 [PATCH] tests/socket-helpers: Only fail socket protocol check if it is really necessary Thomas Huth
@ 2020-09-08 11:07 ` Daniel P. Berrangé
2020-09-08 11:27 ` Thomas Huth
0 siblings, 1 reply; 4+ messages in thread
From: Daniel P. Berrangé @ 2020-09-08 11:07 UTC (permalink / raw)
To: Thomas Huth; +Cc: Marc-André Lureau, qemu-devel
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 ?
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> tests/socket-helpers.c | 23 +++++++++--------------
> 1 file changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/tests/socket-helpers.c b/tests/socket-helpers.c
> index 19a51e887e..62a0e0f2d9 100644
> --- a/tests/socket-helpers.c
> +++ b/tests/socket-helpers.c
> @@ -136,22 +136,17 @@ static int socket_can_bind_connect(const char *hostname, int family)
>
> int socket_check_protocol_support(bool *has_ipv4, bool *has_ipv6)
> {
> - *has_ipv4 = *has_ipv6 = false;
> + int errv4, errv6;
>
> - if (socket_can_bind_connect("127.0.0.1", PF_INET) < 0) {
> - if (errno != EADDRNOTAVAIL) {
> - return -1;
> - }
> - } else {
> - *has_ipv4 = true;
> - }
> + errv4 = socket_can_bind_connect("127.0.0.1", PF_INET);
> + *has_ipv4 = (errv4 == 0);
>
> - 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.
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 :|
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] tests/socket-helpers: Only fail socket protocol check if it is really necessary
2020-09-08 11:07 ` Daniel P. Berrangé
@ 2020-09-08 11:27 ` Thomas Huth
2020-09-08 11:41 ` Daniel P. Berrangé
0 siblings, 1 reply; 4+ messages in thread
From: Thomas Huth @ 2020-09-08 11:27 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Marc-André Lureau, qemu-devel
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.
[...]
>> - 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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] tests/socket-helpers: Only fail socket protocol check if it is really necessary
2020-09-08 11:27 ` Thomas Huth
@ 2020-09-08 11:41 ` Daniel P. Berrangé
0 siblings, 0 replies; 4+ messages in thread
From: Daniel P. Berrangé @ 2020-09-08 11:41 UTC (permalink / raw)
To: Thomas Huth; +Cc: Marc-André Lureau, qemu-devel
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 :|
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-09-08 11:44 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-08 10:54 [PATCH] tests/socket-helpers: Only fail socket protocol check if it is really necessary Thomas Huth
2020-09-08 11:07 ` Daniel P. Berrangé
2020-09-08 11:27 ` Thomas Huth
2020-09-08 11:41 ` Daniel P. Berrangé
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).