[Qemu-devel] [PATCH] sockets: Fix test for DragonFly BSD

[Qemu-devel] [PATCH] sockets: Fix test for DragonFly BSD

[Antonio Huete Jiménez]

 From f57cdc7ec2d5a5e906fa8b795eeede2d7b66aa56 Mon Sep 17 00:00:00 2001
From: Antonio Huete Jimenez <tuxillo@quantumachine.net>
Date: Fri, 15 Dec 2017 01:08:10 +0100
Subject: [PATCH] sockets: Fix test for DragonFly BSD

DragonFly BSD does not implement AI_V4MAPPED for its getaddrinfo() so
probe and discard that flag instead of aborting the test.

Test that fails:
     ERROR:tests/test-char.c:448:char_udp_test_internal: 'chr' should  
not be NULL

Signed-off-by: Antonio Huete Jimenez <tuxillo@quantumachine.net>
---
  util/qemu-sockets.c | 25 ++++++++++++++++++++++---
  1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index af4f01211a..5a9c55c303 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -43,6 +43,8 @@
  # define AI_NUMERICSERV 0
  #endif

+int useV4Mapped = 1;
+

  static int inet_getport(struct addrinfo *e)
  {
@@ -383,7 +385,6 @@ static struct addrinfo  
*inet_parse_connect_saddr(InetSocketAddress *saddr,
      struct addrinfo ai, *res;
      int rc;
      Error *err = NULL;
-    static int useV4Mapped = 1;

      memset(&ai, 0, sizeof(ai));

@@ -474,7 +475,11 @@ static int inet_dgram_saddr(InetSocketAddress *sraddr,

      /* lookup peer addr */
      memset(&ai,0, sizeof(ai));
-    ai.ai_flags = AI_CANONNAME | AI_V4MAPPED | AI_ADDRCONFIG;
+    ai.ai_flags = AI_CANONNAME | AI_ADDRCONFIG;
+    if (atomic_read(&useV4Mapped)) {
+        ai.ai_flags |= AI_V4MAPPED;
+    }
+
      ai.ai_family = inet_ai_family_from_address(sraddr, &err);
      ai.ai_socktype = SOCK_DGRAM;

@@ -493,7 +498,21 @@ static int inet_dgram_saddr(InetSocketAddress *sraddr,
          goto err;
      }

-    if ((rc = getaddrinfo(addr, port, &ai, &peer)) != 0) {
+    /* lookup */
+    rc = getaddrinfo(addr, port, &ai, &peer);
+
+    /* At least FreeBSD and OS-X 10.6 declare AI_V4MAPPED but
+     * then don't implement it in their getaddrinfo(). Detect
+     * this and retry without the flag since that's preferrable
+     * to a fatal error
+     */
+    if (rc == EAI_BADFLAGS &&
+        (ai.ai_flags & AI_V4MAPPED)) {
+        atomic_set(&useV4Mapped, 0);
+        ai.ai_flags &= ~AI_V4MAPPED;
+        rc = getaddrinfo(addr, port, &ai, &peer);
+    }
+    if (rc != 0) {
          error_setg(errp, "address resolution failed for %s:%s: %s",  
addr, port,
                     gai_strerror(rc));
          goto err;
-- 
2.15.0

Re: [Qemu-devel] [PATCH] sockets: Fix test for DragonFly BSD

[Daniel P. Berrange]

On Tue, Dec 19, 2017 at 12:21:05AM +0000, Antonio Huete Jiménez wrote:
> From f57cdc7ec2d5a5e906fa8b795eeede2d7b66aa56 Mon Sep 17 00:00:00 2001
> From: Antonio Huete Jimenez <tuxillo@quantumachine.net>
> Date: Fri, 15 Dec 2017 01:08:10 +0100
> Subject: [PATCH] sockets: Fix test for DragonFly BSD
> 
> DragonFly BSD does not implement AI_V4MAPPED for its getaddrinfo() so
> probe and discard that flag instead of aborting the test.
> 
> Test that fails:
>     ERROR:tests/test-char.c:448:char_udp_test_internal: 'chr' should not be
> NULL
> 
> Signed-off-by: Antonio Huete Jimenez <tuxillo@quantumachine.net>
> ---
>  util/qemu-sockets.c | 25 ++++++++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index af4f01211a..5a9c55c303 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -43,6 +43,8 @@
>  # define AI_NUMERICSERV 0
>  #endif
> 
> +int useV4Mapped = 1;
> +
> 
>  static int inet_getport(struct addrinfo *e)
>  {
> @@ -383,7 +385,6 @@ static struct addrinfo
> *inet_parse_connect_saddr(InetSocketAddress *saddr,
>      struct addrinfo ai, *res;
>      int rc;
>      Error *err = NULL;
> -    static int useV4Mapped = 1;
> 
>      memset(&ai, 0, sizeof(ai));
> 
> @@ -474,7 +475,11 @@ static int inet_dgram_saddr(InetSocketAddress *sraddr,
> 
>      /* lookup peer addr */
>      memset(&ai,0, sizeof(ai));
> -    ai.ai_flags = AI_CANONNAME | AI_V4MAPPED | AI_ADDRCONFIG;
> +    ai.ai_flags = AI_CANONNAME | AI_ADDRCONFIG;
> +    if (atomic_read(&useV4Mapped)) {
> +        ai.ai_flags |= AI_V4MAPPED;
> +    }
> +
>      ai.ai_family = inet_ai_family_from_address(sraddr, &err);
>      ai.ai_socktype = SOCK_DGRAM;
> 
> @@ -493,7 +498,21 @@ static int inet_dgram_saddr(InetSocketAddress *sraddr,
>          goto err;
>      }
> 
> -    if ((rc = getaddrinfo(addr, port, &ai, &peer)) != 0) {
> +    /* lookup */
> +    rc = getaddrinfo(addr, port, &ai, &peer);
> +
> +    /* At least FreeBSD and OS-X 10.6 declare AI_V4MAPPED but
> +     * then don't implement it in their getaddrinfo(). Detect
> +     * this and retry without the flag since that's preferrable
> +     * to a fatal error
> +     */
> +    if (rc == EAI_BADFLAGS &&
> +        (ai.ai_flags & AI_V4MAPPED)) {
> +        atomic_set(&useV4Mapped, 0);
> +        ai.ai_flags &= ~AI_V4MAPPED;
> +        rc = getaddrinfo(addr, port, &ai, &peer);
> +    }

Rather than duplicating this repeated calls to getaddrinfo in multiple
methods, I think it would be better to define a wrapper method. ie
create a  'qemu_getaddrinfo()' method in this file that does the magic
re-try without AI_V4MAPPED. Export it in include/qemu/sockets.h too.

Then call that from all places in QEMU that currently use getaddrinfo()
directly - there are 5 in this file, another in io/dns-resolver.c,
and some more in net/ & test/. That way we are't going to mistakely
reintroduce the bug in other places later.

We should probably also move the #ifdef AI_*   code into qemu/sockets.h
too, so we can remove the duplication in test suites. 

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