[Qemu-devel] [PATCH v5 0/5] Do not set SO_REUSEADDR on Windows

[Qemu-devel] [PATCH v5 0/5] Do not set SO_REUSEADDR on Windows

[Sebastian Ottlik]

This patchset disables most uses of SO_REUSEADDR on Windows and replaces it with
calls to the new function socket_set_fast_reuse. On Windows systems the default
behaviour is equivalent to SO_REUSEADDR on other operating systems. SO_REUSEADDR
can still be set but results in undesired behaviour in most cases. It may even
lead to situations were system behaviour is unspecified. More information on
this can be found at:
http://msdn.microsoft.com/en-us/library/windows/desktop/ms740621.aspx

I originally encountered this issue when accidentally launching two QEMU
instances with identical GDB ports at the same time. In which case QEMU won't
fail as one might expect.

Note that patch #4 fails checkpatch.pl. This is intentional (see v3 changes).

v5 Changes:
- Fixed inverted silent flag on all call sites of socket_set_fast_reuse

v4 Changes:
- Added the silent flag to socket_set_fast_reuse controlling error reporting
  One location where SO_REUSEADDR was set would report errors if setting the
  option failed. Keeping the reporting code there would be somewhat unclean, so
  I moved it to socket_set_fast_reuse. A side effect of this was that the error
  reporting was added for all locations that now use socket_set_fast_reuse. Here
  a new flag is added to control error reporting, which means this patchset
  won't change QEMU behaviour (except for not setting SO_REUSEADDR on Windows).

- Fixed a commit message typo

- Rebased to current master (2d1fe1873a984d1c2c89ffa3d12949cafc718551).

v3 Changes:
- Fixed coding style issues.
  According to checkpatch.pl patch #4 still introduces style errors as tabs are
  used instead of space for some indentation. I keept the tabs to stay
  consistent with the sourrounding code, as tabs seem to be used consitently in
  parts (all?) of the slirp code.

- Changed patch #3 to keep SO_REUSEADDR for multicast sockets on windows and
  added an explainatory comment.

- Rebased to current master (94c2b6aff43cdfcfdfb552773a6b6b973a72ef0b).

v2 Changes:

- Introduce a function with os specific implementation instead of using #ifdef
  I named it socket_set_fast_reuse instead of the suggested qemu_set_reuseaddr
  so the name better reflects what the function actually does.

gdbstub.c              |    6 ++----
include/qemu/sockets.h |    1 +
net/socket.c           |   18 ++++++++++--------
slirp/misc.c           |    3 +--
slirp/socket.c         |    4 +---
slirp/tcp_subr.c       |    6 ++----
slirp/udp.c            |    4 ++--
util/oslib-posix.c     |   14 ++++++++++++++
util/oslib-win32.c     |   10 ++++++++++
util/qemu-sockets.c    |    6 +++---
10 files changed, 46 insertions(+), 26 deletions(-)

util: add socket_set_fast_reuse function which will
gdbstub: call socket_set_fast_reuse instead of
net: call socket_set_fast_reuse instead of setting
slirp: call socket_set_fast_reuse instead of setting
util: call socket_set_fast_reuse instead of setting

[Qemu-devel] [PATCH v5 1/5] util: add socket_set_fast_reuse function which will replace setting SO_REUSEADDR

[Sebastian Ottlik]

If a socket is closed it remains in TIME_WAIT state for some time. On operating
systems using BSD sockets the endpoint of the socket may not be reused while in
this state unless SO_REUSEADDR was set on the socket. On windows on the other
hand the default behaviour is to allow reuse (i.e. identical to SO_REUSEADDR on
other operating systems) and setting SO_REUSEADDR on a socket allows it to be
bound to a endpoint even if the endpoint is already used by another socket
independently of the other sockets state. This can even result in undefined
behaviour.

Many sockets used by QEMU should not block the use of their endpoint after being
closed while they are still in TIME_WAIT state. Currently QEMU sets SO_REUSEADDR
for such sockets, which can lead to problems on Windows. This patch introduces
the function socket_set_fast_reuse that should be used instead of setting
SO_REUSEADDR when fast socket reuse is desired and behaves correctly on all
operating systems.

Leaving the existing error reporting to the call site of socket_set_fast_reuse
would result in bad code or a need to change the way errors are reported, so
error reporting has been moved to socket_set_fast_reuse. As some places setting
SO_REUSEADDR do not report errors, this behaviour can be controlled with the
silent flag.

Signed-off-by: Sebastian Ottlik <ottlik@fzi.de>
---
 include/qemu/sockets.h |    1 +
 util/oslib-posix.c     |   14 ++++++++++++++
 util/oslib-win32.c     |   10 ++++++++++
 3 files changed, 25 insertions(+)

diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index c5174d7..fcbdb2d 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -39,6 +39,7 @@ int socket_set_cork(int fd, int v);
 int socket_set_nodelay(int fd);
 void qemu_set_block(int fd);
 void qemu_set_nonblock(int fd);
+int socket_set_fast_reuse(int fd, bool silent);
 int send_all(int fd, const void *buf, int len1);
 int recv_all(int fd, void *buf, int len1, bool single_read);
 
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 3dc8b1b..3beb341 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -159,6 +159,20 @@ void qemu_set_nonblock(int fd)
     fcntl(fd, F_SETFL, f | O_NONBLOCK);
 }
 
+int socket_set_fast_reuse(int fd, bool silent)
+{
+    int val = 1, ret;
+
+    ret = setsockopt(fd, SOL_SOCKET, SO_REUSEADDR,
+                     (const char *)&val, sizeof(val));
+
+    if (ret < 0 && !silent) {
+        perror("setsockopt(SOL_SOCKET, SO_REUSEADDR)");
+    }
+
+    return ret;
+}
+
 void qemu_set_cloexec(int fd)
 {
     int f;
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 961fbf5..4e13aba 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -127,6 +127,16 @@ void qemu_set_nonblock(int fd)
     qemu_fd_register(fd);
 }
 
+int socket_set_fast_reuse(int fd, bool silent)
+{
+    /* Enabling the reuse of an endpoint that was used by a socket still in
+     * TIME_WAIT state is usually performed by setting SO_REUSEADDR. On Windows
+     * fast reuse is the default and SO_REUSEADDR does strange things. So we
+     * don't have to do anything here. More info can be found at:
+     * http://msdn.microsoft.com/en-us/library/windows/desktop/ms740621.aspx */
+    return 0;
+}
+
 int inet_aton(const char *cp, struct in_addr *ia)
 {
     uint32_t addr = inet_addr(cp);
-- 
1.7.9.5

[Qemu-devel] [PATCH v5 2/5] gdbstub: call socket_set_fast_reuse instead of setting SO_REUSEADDR

[Sebastian Ottlik]

SO_REUSEADDR should be avoided on Windows but is desired on other operating
systems. So instead of setting it we call socket_set_fast_reuse that will result
in the appropriate behaviour on all operating systems.

Signed-off-by: Sebastian Ottlik <ottlik@fzi.de>
---
 gdbstub.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 2b7f22b..1bab205 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1553,7 +1553,7 @@ static void gdb_accept(void)
 static int gdbserver_open(int port)
 {
     struct sockaddr_in sockaddr;
-    int fd, val, ret;
+    int fd, ret;
 
     fd = socket(PF_INET, SOCK_STREAM, 0);
     if (fd < 0) {
@@ -1564,9 +1564,7 @@ static int gdbserver_open(int port)
     fcntl(fd, F_SETFD, FD_CLOEXEC);
 #endif
 
-    /* allow fast reuse */
-    val = 1;
-    qemu_setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
+    socket_set_fast_reuse(fd, true);
 
     sockaddr.sin_family = AF_INET;
     sockaddr.sin_port = htons(port);
-- 
1.7.9.5

[Qemu-devel] [PATCH v5 3/5] net: call socket_set_fast_reuse instead of setting SO_REUSEADDR

[Sebastian Ottlik]

SO_REUSEADDR should be avoided on Windows but is desired on other operating
systems. So instead of setting it we call socket_set_fast_reuse that will result
in the appropriate behaviour on all operating systems.

An exception to this rule are multicast sockets where it is sensible to have
multiple sockets listen on the same ip and port and we should set SO_REUSEADDR
on windows.

Signed-off-by: Sebastian Ottlik <ottlik@fzi.de>
---
 net/socket.c |   18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index e61309d..e1c7b27 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -262,6 +262,11 @@ static int net_socket_mcast_create(struct sockaddr_in *mcastaddr, struct in_addr
         return -1;
     }
 
+    /* Allow multiple sockets to bind the same multicast ip and port by setting
+     * SO_REUSEADDR. This is the only situation where SO_REUSEADDR should be set
+     * on windows. Use socket_set_fast_reuse otherwise as it sets SO_REUSEADDR
+     * only on posix systems.
+     */
     val = 1;
     ret = qemu_setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
     if (ret < 0) {
@@ -510,7 +515,7 @@ static int net_socket_listen_init(NetClientState *peer,
     NetClientState *nc;
     NetSocketState *s;
     struct sockaddr_in saddr;
-    int fd, val, ret;
+    int fd, ret;
 
     if (parse_host_port(&saddr, host_str) < 0)
         return -1;
@@ -523,8 +528,7 @@ static int net_socket_listen_init(NetClientState *peer,
     qemu_set_nonblock(fd);
 
     /* allow fast reuse */
-    val = 1;
-    qemu_setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
+    socket_set_fast_reuse(fd, true);
 
     ret = bind(fd, (struct sockaddr *)&saddr, sizeof(saddr));
     if (ret < 0) {
@@ -645,7 +649,7 @@ static int net_socket_udp_init(NetClientState *peer,
                                  const char *lhost)
 {
     NetSocketState *s;
-    int fd, val, ret;
+    int fd, ret;
     struct sockaddr_in laddr, raddr;
 
     if (parse_host_port(&laddr, lhost) < 0) {
@@ -661,11 +665,9 @@ static int net_socket_udp_init(NetClientState *peer,
         perror("socket(PF_INET, SOCK_DGRAM)");
         return -1;
     }
-    val = 1;
-    ret = qemu_setsockopt(fd, SOL_SOCKET, SO_REUSEADDR,
-                          &val, sizeof(val));
+
+    ret = socket_set_fast_reuse(fd, false);
     if (ret < 0) {
-        perror("setsockopt(SOL_SOCKET, SO_REUSEADDR)");
         closesocket(fd);
         return -1;
     }
-- 
1.7.9.5

[Qemu-devel] [PATCH v5 4/5] slirp: call socket_set_fast_reuse instead of setting SO_REUSEADDR

[Sebastian Ottlik]

SO_REUSEADDR should be avoided on Windows but is desired on other operating
systems. So instead of setting it we call socket_set_fast_reuse that will result
in the appropriate behaviour on all operating systems.

Signed-off-by: Sebastian Ottlik <ottlik@fzi.de>
---
 slirp/misc.c     |    3 +--
 slirp/socket.c   |    4 +---
 slirp/tcp_subr.c |    6 ++----
 slirp/udp.c      |    4 ++--
 4 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/slirp/misc.c b/slirp/misc.c
index c0d4899..86c2e08 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -212,8 +212,7 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
                     so->s = accept(s, (struct sockaddr *)&addr, &addrlen);
                 } while (so->s < 0 && errno == EINTR);
                 closesocket(s);
-                opt = 1;
-                qemu_setsockopt(so->s, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(int));
+                socket_set_fast_reuse(so->s, true);
                 opt = 1;
                 qemu_setsockopt(so->s, SOL_SOCKET, SO_OOBINLINE, &opt, sizeof(int));
 		qemu_set_nonblock(so->s);
diff --git a/slirp/socket.c b/slirp/socket.c
index 25d60e7..e67d0c5 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -627,9 +627,7 @@ tcp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr,
 	addr.sin_port = hport;
 
 	if (((s = qemu_socket(AF_INET,SOCK_STREAM,0)) < 0) ||
-#ifndef _WIN32
-	    (qemu_setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(int)) < 0) ||
-#endif
+	    (socket_set_fast_reuse(s, true) < 0) ||
 	    (bind(s,(struct sockaddr *)&addr, sizeof(addr)) < 0) ||
 	    (listen(s,1) < 0)) {
 		int tmperrno = errno; /* Don't clobber the real reason we failed */
diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
index 043f28f..065750c 100644
--- a/slirp/tcp_subr.c
+++ b/slirp/tcp_subr.c
@@ -337,8 +337,7 @@ int tcp_fconnect(struct socket *so)
     struct sockaddr_in addr;
 
     qemu_set_nonblock(s);
-    opt = 1;
-    qemu_setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt));
+    socket_set_fast_reuse(s, true);
     opt = 1;
     qemu_setsockopt(s, SOL_SOCKET, SO_OOBINLINE, &opt, sizeof(opt));
 
@@ -426,8 +425,7 @@ void tcp_connect(struct socket *inso)
         return;
     }
     qemu_set_nonblock(s);
-    opt = 1;
-    qemu_setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(int));
+    socket_set_fast_reuse(s, true);
     opt = 1;
     qemu_setsockopt(s, SOL_SOCKET, SO_OOBINLINE, &opt, sizeof(int));
     socket_set_nodelay(s);
diff --git a/slirp/udp.c b/slirp/udp.c
index b105f87..2c8ed2a 100644
--- a/slirp/udp.c
+++ b/slirp/udp.c
@@ -354,7 +354,7 @@ udp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr,
 {
 	struct sockaddr_in addr;
 	struct socket *so;
-	socklen_t addrlen = sizeof(struct sockaddr_in), opt = 1;
+	socklen_t addrlen = sizeof(struct sockaddr_in);
 
 	so = socreate(slirp);
 	if (!so) {
@@ -372,7 +372,7 @@ udp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr,
 		udp_detach(so);
 		return NULL;
 	}
-	qemu_setsockopt(so->s, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(int));
+	socket_set_fast_reuse(so->s, true);
 
 	getsockname(so->s,(struct sockaddr *)&addr,&addrlen);
 	so->so_fport = addr.sin_port;
-- 
1.7.9.5

[Qemu-devel] [PATCH v5 5/5] util: call socket_set_fast_reuse instead of setting SO_REUSEADDR

[Sebastian Ottlik]

SO_REUSEADDR should be avoided on Windows but is desired on other operating
systems. So instead of setting it we call socket_set_fast_reuse that will result
in the appropriate behaviour on all operating systems.

Signed-off-by: Sebastian Ottlik <ottlik@fzi.de>
---
 util/qemu-sockets.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 095716e..5d80e52 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -155,7 +155,7 @@ int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp)
             continue;
         }
 
-        qemu_setsockopt(slisten, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
+        socket_set_fast_reuse(slisten, true);
 #ifdef IPV6_V6ONLY
         if (e->ai_family == PF_INET6) {
             /* listen on both ipv4 and ipv6 */
@@ -274,7 +274,7 @@ static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
         error_set_errno(errp, errno, QERR_SOCKET_CREATE_FAILED);
         return -1;
     }
-    qemu_setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
+    socket_set_fast_reuse(sock, true);
     if (connect_state != NULL) {
         qemu_set_nonblock(sock);
     }
@@ -455,7 +455,7 @@ int inet_dgram_opts(QemuOpts *opts, Error **errp)
         error_set_errno(errp, errno, QERR_SOCKET_CREATE_FAILED);
         goto err;
     }
-    qemu_setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
+    socket_set_fast_reuse(sock, true);
 
     /* bind socket */
     if (bind(sock, local->ai_addr, local->ai_addrlen) < 0) {
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH v5 0/5] Do not set SO_REUSEADDR on Windows

[Paolo Bonzini]

Il 16/09/2013 16:23, Sebastian Ottlik ha scritto:
> - Added the silent flag to socket_set_fast_reuse controlling error reporting
>   One location where SO_REUSEADDR was set would report errors if setting the
>   option failed. Keeping the reporting code there would be somewhat unclean, so
>   I moved it to socket_set_fast_reuse. A side effect of this was that the error
>   reporting was added for all locations that now use socket_set_fast_reuse. Here
>   a new flag is added to control error reporting, which means this patchset
>   won't change QEMU behaviour (except for not setting SO_REUSEADDR on Windows).

Is there actually a case where setting SO_REUSEADDR could fail?

Paolo

Re: [Qemu-devel] [PATCH v5 0/5] Do not set SO_REUSEADDR on Windows

[Sebastian Ottlik]

On 16.09.2013 16:55, Paolo Bonzini wrote:
> Il 16/09/2013 16:23, Sebastian Ottlik ha scritto:
>> - Added the silent flag to socket_set_fast_reuse controlling error reporting
>>    One location where SO_REUSEADDR was set would report errors if setting the
>>    option failed. Keeping the reporting code there would be somewhat unclean, so
>>    I moved it to socket_set_fast_reuse. A side effect of this was that the error
>>    reporting was added for all locations that now use socket_set_fast_reuse. Here
>>    a new flag is added to control error reporting, which means this patchset
>>    won't change QEMU behaviour (except for not setting SO_REUSEADDR on Windows).
> Is there actually a case where setting SO_REUSEADDR could fail?
>
> Paolo
Yes, but its very unlikely. E.g. the first parameter is not a valid socket.

Re: [Qemu-devel] [PATCH v5 0/5] Do not set SO_REUSEADDR on Windows

[Stefan Hajnoczi]

On Mon, Sep 16, 2013 at 04:23:44PM +0200, Sebastian Ottlik wrote:
> This patchset disables most uses of SO_REUSEADDR on Windows and replaces it with
> calls to the new function socket_set_fast_reuse. On Windows systems the default
> behaviour is equivalent to SO_REUSEADDR on other operating systems. SO_REUSEADDR
> can still be set but results in undesired behaviour in most cases. It may even
> lead to situations were system behaviour is unspecified. More information on
> this can be found at:
> http://msdn.microsoft.com/en-us/library/windows/desktop/ms740621.aspx
> 
> I originally encountered this issue when accidentally launching two QEMU
> instances with identical GDB ports at the same time. In which case QEMU won't
> fail as one might expect.
> 
> Note that patch #4 fails checkpatch.pl. This is intentional (see v3 changes).
> 
> v5 Changes:
> - Fixed inverted silent flag on all call sites of socket_set_fast_reuse
> 
> v4 Changes:
> - Added the silent flag to socket_set_fast_reuse controlling error reporting
>   One location where SO_REUSEADDR was set would report errors if setting the
>   option failed. Keeping the reporting code there would be somewhat unclean, so
>   I moved it to socket_set_fast_reuse. A side effect of this was that the error
>   reporting was added for all locations that now use socket_set_fast_reuse. Here
>   a new flag is added to control error reporting, which means this patchset
>   won't change QEMU behaviour (except for not setting SO_REUSEADDR on Windows).
> 
> - Fixed a commit message typo
> 
> - Rebased to current master (2d1fe1873a984d1c2c89ffa3d12949cafc718551).
> 
> v3 Changes:
> - Fixed coding style issues.
>   According to checkpatch.pl patch #4 still introduces style errors as tabs are
>   used instead of space for some indentation. I keept the tabs to stay
>   consistent with the sourrounding code, as tabs seem to be used consitently in
>   parts (all?) of the slirp code.
> 
> - Changed patch #3 to keep SO_REUSEADDR for multicast sockets on windows and
>   added an explainatory comment.
> 
> - Rebased to current master (94c2b6aff43cdfcfdfb552773a6b6b973a72ef0b).
> 
> v2 Changes:
> 
> - Introduce a function with os specific implementation instead of using #ifdef
>   I named it socket_set_fast_reuse instead of the suggested qemu_set_reuseaddr
>   so the name better reflects what the function actually does.
> 
> gdbstub.c              |    6 ++----
> include/qemu/sockets.h |    1 +
> net/socket.c           |   18 ++++++++++--------
> slirp/misc.c           |    3 +--
> slirp/socket.c         |    4 +---
> slirp/tcp_subr.c       |    6 ++----
> slirp/udp.c            |    4 ++--
> util/oslib-posix.c     |   14 ++++++++++++++
> util/oslib-win32.c     |   10 ++++++++++
> util/qemu-sockets.c    |    6 +++---
> 10 files changed, 46 insertions(+), 26 deletions(-)
> 
> util: add socket_set_fast_reuse function which will
> gdbstub: call socket_set_fast_reuse instead of
> net: call socket_set_fast_reuse instead of setting
> slirp: call socket_set_fast_reuse instead of setting
> util: call socket_set_fast_reuse instead of setting
> 

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Re: [Qemu-devel] [PATCH v5 0/5] Do not set SO_REUSEADDR on Windows

[Stefan Weil]

Am 16.09.2013 17:10, schrieb Sebastian Ottlik:
> On 16.09.2013 16:55, Paolo Bonzini wrote:
>> Il 16/09/2013 16:23, Sebastian Ottlik ha scritto:
>>> - Added the silent flag to socket_set_fast_reuse controlling error
>>> reporting
>>>    One location where SO_REUSEADDR was set would report errors if
>>> setting the
>>>    option failed. Keeping the reporting code there would be somewhat
>>> unclean, so
>>>    I moved it to socket_set_fast_reuse. A side effect of this was
>>> that the error
>>>    reporting was added for all locations that now use
>>> socket_set_fast_reuse. Here
>>>    a new flag is added to control error reporting, which means this
>>> patchset
>>>    won't change QEMU behaviour (except for not setting SO_REUSEADDR
>>> on Windows).
>> Is there actually a case where setting SO_REUSEADDR could fail?
>>
>> Paolo
> Yes, but its very unlikely. E.g. the first parameter is not a valid
> socket.
>

If failures only happen when something is very wrong (like an invalid
socket id),
an assertion might be better, and we could remove the 'silent' parameter.

Stefan

Re: [Qemu-devel] [PATCH v5 0/5] Do not set SO_REUSEADDR on Windows

[Sebastian Ottlik]

On 18.09.2013 18:58, Stefan Weil wrote:
> Am 16.09.2013 17:10, schrieb Sebastian Ottlik:
>> On 16.09.2013 16:55, Paolo Bonzini wrote:
>>> Il 16/09/2013 16:23, Sebastian Ottlik ha scritto:
>>>> - Added the silent flag to socket_set_fast_reuse controlling error
>>>> reporting
>>>>     One location where SO_REUSEADDR was set would report errors if
>>>> setting the
>>>>     option failed. Keeping the reporting code there would be somewhat
>>>> unclean, so
>>>>     I moved it to socket_set_fast_reuse. A side effect of this was
>>>> that the error
>>>>     reporting was added for all locations that now use
>>>> socket_set_fast_reuse. Here
>>>>     a new flag is added to control error reporting, which means this
>>>> patchset
>>>>     won't change QEMU behaviour (except for not setting SO_REUSEADDR
>>>> on Windows).
>>> Is there actually a case where setting SO_REUSEADDR could fail?
>>>
>>> Paolo
>> Yes, but its very unlikely. E.g. the first parameter is not a valid
>> socket.
>>
> If failures only happen when something is very wrong (like an invalid
> socket id),
> an assertion might be better, and we could remove the 'silent' parameter.
>
> Stefan
>
IMO for debug builds this is a good idea. However, in production use it 
is probably preferable to keep QEMU running, as a failure won't be too 
critical. From a quick grep it looks like NDEBUG is not set so 
assertions wont be removed for non-debug builds. I don't feel acquainted 
enough with the source code to decide about this kind of change in 
functionality, which is why I was waiting so long to reply.

Re: [Qemu-devel] [PATCH v5 0/5] Do not set SO_REUSEADDR on Windows

[Sebastian Ottlik]

On 23.09.2013 12:33, Sebastian Ottlik wrote:
> On 18.09.2013 18:58, Stefan Weil wrote:
>> Am 16.09.2013 17:10, schrieb Sebastian Ottlik:
>>> On 16.09.2013 16:55, Paolo Bonzini wrote:
>>>> Il 16/09/2013 16:23, Sebastian Ottlik ha scritto:
>>>>> - Added the silent flag to socket_set_fast_reuse controlling error
>>>>> reporting
>>>>>     One location where SO_REUSEADDR was set would report errors if
>>>>> setting the
>>>>>     option failed. Keeping the reporting code there would be somewhat
>>>>> unclean, so
>>>>>     I moved it to socket_set_fast_reuse. A side effect of this was
>>>>> that the error
>>>>>     reporting was added for all locations that now use
>>>>> socket_set_fast_reuse. Here
>>>>>     a new flag is added to control error reporting, which means this
>>>>> patchset
>>>>>     won't change QEMU behaviour (except for not setting SO_REUSEADDR
>>>>> on Windows).
>>>> Is there actually a case where setting SO_REUSEADDR could fail?
>>>>
>>>> Paolo
>>> Yes, but its very unlikely. E.g. the first parameter is not a valid
>>> socket.
>>>
>> If failures only happen when something is very wrong (like an invalid
>> socket id),
>> an assertion might be better, and we could remove the 'silent' 
>> parameter.
>>
>> Stefan
>>
> IMO for debug builds this is a good idea. However, in production use 
> it is probably preferable to keep QEMU running, as a failure won't be 
> too critical. From a quick grep it looks like NDEBUG is not set so 
> assertions wont be removed for non-debug builds. I don't feel 
> acquainted enough with the source code to decide about this kind of 
> change in functionality, which is why I was waiting so long to reply.
ping

Re: [Qemu-devel] [PATCH v5 0/5] Do not set SO_REUSEADDR on Windows

[Stefan Weil]

Am 01.10.2013 10:11, schrieb Sebastian Ottlik:
> On 23.09.2013 12:33, Sebastian Ottlik wrote:
>> On 18.09.2013 18:58, Stefan Weil wrote:
>>> If failures only happen when something is very wrong (like an invalid
>>> socket id),
>>> an assertion might be better, and we could remove the 'silent'
>>> parameter.
>>>
>>> Stefan
>>>
>> IMO for debug builds this is a good idea. However, in production use
>> it is probably preferable to keep QEMU running, as a failure won't be
>> too critical. From a quick grep it looks like NDEBUG is not set so
>> assertions wont be removed for non-debug builds. I don't feel
>> acquainted enough with the source code to decide about this kind of
>> change in functionality, which is why I was waiting so long to reply.
> ping

Running QEMU with an invalid socket id is not preferable.
It's better to get a dozen of assertions in production and fix the code
than to have hundreds of production systems running with hidden bugs.

Those users who don't agree can build QEMU with -DNDEBUG and use the
close-your-eyes strategy "if error then no error".

Therefore I strongly suggest adding an assertion and removing the
'silent' parameter.

Regards,
Stefan