[libvirt] [PATCH v2] util: Loop through all resolved addresses in virNetSocketNewListenTCP

Olaf Hering posted 1 patch 5 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180524120006.2694-1-olaf@aepfle.de
Test syntax-check passed
src/rpc/virnetsocket.c | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)
[libvirt] [PATCH v2] util: Loop through all resolved addresses in virNetSocketNewListenTCP
Posted by Olaf Hering 5 years, 10 months ago
Currently virNetSocketNewListenTCP bails out early under the following
conditions:
- the hostname resolves to at least one IPv4 and at least one IPv6
  address
- the local interfaces have that one IPv4 address assigned, but not any
  of the IPv6 addresses
- the local interfaces have just IPv6 link-local addresses

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.

To fix this error, just keep going and loop through all returned
addresses. In case none of the attempts to bind to some address
succeeded, try to report the appropriate error.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
v2:
 whitespace fixes, as suggested by John Ferlan

 src/rpc/virnetsocket.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 7087abec9c..60a7187348 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -382,11 +382,8 @@ int virNetSocketNewListenTCP(const char *nodename,
 #endif
 
         if (bind(fd, runp->ai_addr, runp->ai_addrlen) < 0) {
-            if (errno != EADDRINUSE) {
-                virReportSystemError(errno, "%s", _("Unable to bind to port"));
-                goto error;
-            }
-            addrInUse = true;
+            if (errno == EADDRINUSE)
+                addrInUse = true;
             VIR_FORCE_CLOSE(fd);
             runp = runp->ai_next;
             continue;
@@ -409,14 +406,14 @@ int virNetSocketNewListenTCP(const char *nodename,
         fd = -1;
     }
 
-    if (nsocks == 0 && familyNotSupported) {
-        virReportSystemError(EAFNOSUPPORT, "%s", _("Unable to bind to port"));
-        goto error;
-    }
-
-    if (nsocks == 0 &&
-        addrInUse) {
-        virReportSystemError(EADDRINUSE, "%s", _("Unable to bind to port"));
+    if (nsocks == 0) {
+      if (familyNotSupported)
+        errno = EAFNOSUPPORT;
+      else if (addrInUse)
+        errno = EADDRINUSE;
+      else
+        errno = EDESTADDRREQ;
+        virReportSystemError(errno, "%s", _("Unable to bind to port"));
         goto error;
     }
 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] util: Loop through all resolved addresses in virNetSocketNewListenTCP
Posted by Daniel P. Berrangé 5 years, 10 months ago
On Thu, May 24, 2018 at 02:00:06PM +0200, Olaf Hering wrote:
> Currently virNetSocketNewListenTCP bails out early under the following
> conditions:
> - the hostname resolves to at least one IPv4 and at least one IPv6
>   address
> - the local interfaces have that one IPv4 address assigned, but not any
>   of the IPv6 addresses
> - the local interfaces have just IPv6 link-local addresses
> 
> 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.
> 
> To fix this error, just keep going and loop through all returned
> addresses. In case none of the attempts to bind to some address
> succeeded, try to report the appropriate error.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
> v2:
>  whitespace fixes, as suggested by John Ferlan
> 
>  src/rpc/virnetsocket.c | 23 ++++++++++-------------
>  1 file changed, 10 insertions(+), 13 deletions(-)
> 
> diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
> index 7087abec9c..60a7187348 100644
> --- a/src/rpc/virnetsocket.c
> +++ b/src/rpc/virnetsocket.c
> @@ -382,11 +382,8 @@ int virNetSocketNewListenTCP(const char *nodename,
>  #endif
>  
>          if (bind(fd, runp->ai_addr, runp->ai_addrlen) < 0) {
> -            if (errno != EADDRINUSE) {
> -                virReportSystemError(errno, "%s", _("Unable to bind to port"));
> -                goto error;
> -            }
> -            addrInUse = true;
> +            if (errno == EADDRINUSE)
> +                addrInUse = true;
>              VIR_FORCE_CLOSE(fd);
>              runp = runp->ai_next;
>              continue;
> @@ -409,14 +406,14 @@ int virNetSocketNewListenTCP(const char *nodename,
>          fd = -1;
>      }
>  
> -    if (nsocks == 0 && familyNotSupported) {
> -        virReportSystemError(EAFNOSUPPORT, "%s", _("Unable to bind to port"));
> -        goto error;
> -    }
> -
> -    if (nsocks == 0 &&
> -        addrInUse) {
> -        virReportSystemError(EADDRINUSE, "%s", _("Unable to bind to port"));
> +    if (nsocks == 0) {
> +      if (familyNotSupported)
> +        errno = EAFNOSUPPORT;
> +      else if (addrInUse)
> +        errno = EADDRINUSE;
> +      else
> +        errno = EDESTADDRREQ;
> +        virReportSystemError(errno, "%s", _("Unable to bind to port"));
>          goto error;
>      }

The indentation is still wrong here - libvirt uses 4 space indents.

Functionally the patch looks ok though.

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list