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

[Ján Tomko]

On Mon, Jun 04, 2018 at 12:29:37PM +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 some appropriate error.
>
>Signed-off-by: Olaf Hering <olaf at aepfle.de>
>---
>
>v3:
> more whitespace fixes, as suggested by Daniel P. Berrangé
>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..3a055acd39 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;

This skips all errors, not just EADDRNOTAVAIL. Saving the errno here...

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

...and using it here if needed would feel less random than picking one
at random.

Also, why the use of global errno variable to pass a value within a
single block?

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180604/3091d18d/attachment-0001.sig>