[libvirt] [PATCH 0/2] Allow for further expected failures when listening on sockets
Daniel P. Berrangé (2): socket: preserve real errno when socket/bind calls fail rpc: treat EADDRNOTAVAIL as non-fatal when listening src/rpc/virnetsocket.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) -- 2.17.1
When reporting socket/bind failures we want to ensure any fatal error reported is as accurate as possible. We'll prefer reporting a bind() errno over a socket() errno, because if socket() works but bind() fails that is a more significant event. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/rpc/virnetsocket.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index fee61ace60..8e04d61e98 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -310,8 +310,8 @@ int virNetSocketNewListenTCP(const char *nodename, struct addrinfo hints; int fd = -1; size_t i; - bool addrInUse = false; - bool familyNotSupported = false; + int socketErrno = 0; + int bindErrno = 0; virSocketAddr tmp_addr; *retsocks = NULL; @@ -351,7 +351,7 @@ int virNetSocketNewListenTCP(const char *nodename, if ((fd = socket(runp->ai_family, runp->ai_socktype, runp->ai_protocol)) < 0) { if (errno == EAFNOSUPPORT) { - familyNotSupported = true; + socketErrno = errno; runp = runp->ai_next; continue; } @@ -386,7 +386,7 @@ int virNetSocketNewListenTCP(const char *nodename, virReportSystemError(errno, "%s", _("Unable to bind to port")); goto error; } - addrInUse = true; + bindErrno = errno; VIR_FORCE_CLOSE(fd); runp = runp->ai_next; continue; @@ -409,14 +409,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 (bindErrno) { + virReportSystemError(bindErrno, "%s", _("Unable to bind to port")); + } else if (socketErrno) { + virReportSystemError(socketErrno, "%s", _("Unable to create socket")); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("No addresses to bind to")); + } goto error; } -- 2.17.1
On Tue, Jul 24, 2018 at 15:18:09 +0100, Daniel P. Berrangé wrote:
When reporting socket/bind failures we want to ensure any fatal error reported is as accurate as possible. We'll prefer reporting a bind() errno over a socket() errno, because if socket() works but bind() fails that is a more significant event.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/rpc/virnetsocket.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index fee61ace60..8e04d61e98 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -310,8 +310,8 @@ int virNetSocketNewListenTCP(const char *nodename, struct addrinfo hints; int fd = -1; size_t i; - bool addrInUse = false; - bool familyNotSupported = false; + int socketErrno = 0; + int bindErrno = 0; virSocketAddr tmp_addr;
*retsocks = NULL; @@ -351,7 +351,7 @@ int virNetSocketNewListenTCP(const char *nodename, if ((fd = socket(runp->ai_family, runp->ai_socktype, runp->ai_protocol)) < 0) { if (errno == EAFNOSUPPORT) { - familyNotSupported = true; + socketErrno = errno; runp = runp->ai_next; continue; } @@ -386,7 +386,7 @@ int virNetSocketNewListenTCP(const char *nodename, virReportSystemError(errno, "%s", _("Unable to bind to port")); goto error; } - addrInUse = true; + bindErrno = errno; VIR_FORCE_CLOSE(fd); runp = runp->ai_next; continue; @@ -409,14 +409,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 (bindErrno) { + virReportSystemError(bindErrno, "%s", _("Unable to bind to port")); + } else if (socketErrno) { + virReportSystemError(socketErrno, "%s", _("Unable to create socket")); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("No addresses to bind to")); + }
We usually don't use {} around single line bodies, you can remove all of them here.
goto error; }
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
On Tue, Jul 24, 2018 at 05:51:50PM +0200, Jiri Denemark wrote:
On Tue, Jul 24, 2018 at 15:18:09 +0100, Daniel P. Berrangé wrote:
When reporting socket/bind failures we want to ensure any fatal error reported is as accurate as possible. We'll prefer reporting a bind() errno over a socket() errno, because if socket() works but bind() fails that is a more significant event.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/rpc/virnetsocket.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index fee61ace60..8e04d61e98 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -310,8 +310,8 @@ int virNetSocketNewListenTCP(const char *nodename, struct addrinfo hints; int fd = -1; size_t i; - bool addrInUse = false; - bool familyNotSupported = false; + int socketErrno = 0; + int bindErrno = 0; virSocketAddr tmp_addr;
*retsocks = NULL; @@ -351,7 +351,7 @@ int virNetSocketNewListenTCP(const char *nodename, if ((fd = socket(runp->ai_family, runp->ai_socktype, runp->ai_protocol)) < 0) { if (errno == EAFNOSUPPORT) { - familyNotSupported = true; + socketErrno = errno; runp = runp->ai_next; continue; } @@ -386,7 +386,7 @@ int virNetSocketNewListenTCP(const char *nodename, virReportSystemError(errno, "%s", _("Unable to bind to port")); goto error; } - addrInUse = true; + bindErrno = errno; VIR_FORCE_CLOSE(fd); runp = runp->ai_next; continue; @@ -409,14 +409,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 (bindErrno) { + virReportSystemError(bindErrno, "%s", _("Unable to bind to port")); + } else if (socketErrno) { + virReportSystemError(socketErrno, "%s", _("Unable to create socket")); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("No addresses to bind to")); + }
We usually don't use {} around single line bodies, you can remove all of them here.
Oh yes, I always get this wrong since QEMU requires the opposite to libvirt - always {} no matter what :-)
goto error; }
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
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 :|
Consider creating a listener socket from a hostname that resolves to multiple addresses. It might be the case that the hostname resolves to both an IPv4 and IPv6 address because it is reachable over both protocols, but the IPv6 connectivity is provided off-host. In such a case no local NIC will have IPv6 and so bind() would fail with the EADDRNOTAVAIL errno. Thus it should be treated as non-fatal as long as at least one socket was succesfully bound. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/rpc/virnetsocket.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 8e04d61e98..044e6d8804 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -382,7 +382,7 @@ int virNetSocketNewListenTCP(const char *nodename, #endif if (bind(fd, runp->ai_addr, runp->ai_addrlen) < 0) { - if (errno != EADDRINUSE) { + if (errno != EADDRINUSE && errno != EADDRNOTAVAIL) { virReportSystemError(errno, "%s", _("Unable to bind to port")); goto error; } -- 2.17.1
On Tue, Jul 24, 2018 at 15:18:10 +0100, Daniel P. Berrangé wrote:
Consider creating a listener socket from a hostname that resolves to multiple addresses. It might be the case that the hostname resolves to both an IPv4 and IPv6 address because it is reachable over both protocols, but the IPv6 connectivity is provided off-host. In such a case no local NIC will have IPv6 and so bind() would fail with the EADDRNOTAVAIL errno. Thus it should be treated as non-fatal as long as at least one socket was succesfully bound.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/rpc/virnetsocket.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 8e04d61e98..044e6d8804 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -382,7 +382,7 @@ int virNetSocketNewListenTCP(const char *nodename, #endif
if (bind(fd, runp->ai_addr, runp->ai_addrlen) < 0) { - if (errno != EADDRINUSE) { + if (errno != EADDRINUSE && errno != EADDRNOTAVAIL) { virReportSystemError(errno, "%s", _("Unable to bind to port")); goto error; }
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
participants (2)
-
Daniel P. Berrangé -
Jiri Denemark