[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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

Attachment: signature.asc
Description: Digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]