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

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

[Sebastian Ottlik]

This patchset disabels all use of SO_REUSEADDR on Windows. 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
instead. 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.

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           |   19 +++++++------------
 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, 43 insertions(+), 30 deletions(-)


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

[Qemu-devel] [PATCH v2 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 and does the right thing on all operating systems.

Signed-off-by: Sebastian Ottlik <ottlik@fzi.de>

diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index c5174d7..45588d7 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);
 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..f071793 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)
+{
+    int val=1, ret;
+
+    ret = setsockopt(fd, SOL_SOCKET, SO_REUSEADDR,
+                     (const char *)&val, sizeof(val));
+
+    if (ret < 0) {
+      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..dd81f55 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)
+{
+    /* 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

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

[Stefan Hajnoczi]

On Wed, Sep 04, 2013 at 07:08:51PM +0200, Sebastian Ottlik wrote:
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 3dc8b1b..f071793 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)
> +{
> +    int val=1, ret;
> +
> +    ret = setsockopt(fd, SOL_SOCKET, SO_REUSEADDR,
> +                     (const char *)&val, sizeof(val));
> +
> +    if (ret < 0) {
> +      perror("setsockopt(SOL_SOCKET, SO_REUSEADDR)");
> +    }
> +
> +    return ret;
> +}

Please run scripts/checkpatch.pl before submitting patches to check
coding style (whitespace, indentation, etc).

[Qemu-devel] [PATCH v2 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>

diff --git a/gdbstub.c b/gdbstub.c
index 2b7f22b..0e5a3f5 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);
 
     sockaddr.sin_family = AF_INET;
     sockaddr.sin_port = htons(port);
-- 
1.7.9.5

[Qemu-devel] [PATCH v2 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.

Signed-off-by: Sebastian Ottlik <ottlik@fzi.de>

diff --git a/net/socket.c b/net/socket.c
index e61309d..0f26208 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -241,7 +241,7 @@ static int net_socket_mcast_create(struct sockaddr_in *mcastaddr, struct in_addr
 {
     struct ip_mreq imr;
     int fd;
-    int val, ret;
+    int ret;
 #ifdef __OpenBSD__
     unsigned char loop;
 #else
@@ -262,10 +262,8 @@ static int net_socket_mcast_create(struct sockaddr_in *mcastaddr, struct in_addr
         return -1;
     }
 
-    val = 1;
-    ret = qemu_setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
+    ret = socket_set_fast_reuse(fd);
     if (ret < 0) {
-        perror("setsockopt(SOL_SOCKET, SO_REUSEADDR)");
         goto fail;
     }
 
@@ -510,7 +508,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 +521,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);
 
     ret = bind(fd, (struct sockaddr *)&saddr, sizeof(saddr));
     if (ret < 0) {
@@ -645,7 +642,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 +658,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);
     if (ret < 0) {
-        perror("setsockopt(SOL_SOCKET, SO_REUSEADDR)");
         closesocket(fd);
         return -1;
     }
-- 
1.7.9.5

[Qemu-devel] [PATCH v2 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>

diff --git a/slirp/misc.c b/slirp/misc.c
index c0d4899..6c1636f 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);
                 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..37ac5cf 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) < 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..7571c5a 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);
     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);
     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..8cc6cb6 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);
 
 	getsockname(so->s,(struct sockaddr *)&addr,&addrlen);
 	so->so_fport = addr.sin_port;
-- 
1.7.9.5

[Qemu-devel] [PATCH v2 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>

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 095716e..6b97dc1 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);
 #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);
     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);
 
     /* bind socket */
     if (bind(sock, local->ai_addr, local->ai_addrlen) < 0) {
-- 
1.7.9.5

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

[Sebastian Ottlik]

On 04.09.2013 19:08, Sebastian Ottlik wrote:
> This patchset disabels all use of SO_REUSEADDR on Windows. 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
> instead. 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.
>
> 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           |   19 +++++++------------
>   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, 43 insertions(+), 30 deletions(-)
>
>
> util: add socket_set_fast_reuse function
> gdbstub: call socket_set_fast_reuse instead of setting SO_REUSEADDR
> net: call socket_set_fast_reuse instead of setting SO_REUSEADDR
> slirp: call socket_set_fast_reuse instead of setting SO_REUSEADDR
> util: call socket_set_fast_reuse instead of setting SO_REUSEADDR
>

Pinging this patch, as I think it is still an appropriate approach to 
the issue:

I did some research and apparently there is a valid use case for 
SO_REUSEADDR
on windows when multiple clients need to listen to the same port for the 
same
multicast group. IMHO making qemu_setsockopt ignore SO_REUSEADDR on windows
might be confusing for some use cases. Actually net_socket_mcast_create in
net/socket.c should probably set SO_REUSEADDR on windows. This is also 
an issue
with patch 3 I supplied that I will address in a new version of this 
patch set if there is
an agreement on a general approach.

It was also suggested to introduce a qemu_stream_listen_socket, but I do 
not see a
clear advantage of this approach over the one taken in my patches. To my
understanding there also may be situations where one would like to 
enable fast reuse
of client sockets (i.e. those used with connect()).

Best Regards,
    Sebastian

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

[Stefan Hajnoczi]

On Thu, Sep 05, 2013 at 03:48:16PM +0200, Sebastian Ottlik wrote:
> On 04.09.2013 19:08, Sebastian Ottlik wrote:
> >This patchset disabels all use of SO_REUSEADDR on Windows. 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
> >instead. 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.
> >
> >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           |   19 +++++++------------
> >  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, 43 insertions(+), 30 deletions(-)
> >
> >
> >util: add socket_set_fast_reuse function
> >gdbstub: call socket_set_fast_reuse instead of setting SO_REUSEADDR
> >net: call socket_set_fast_reuse instead of setting SO_REUSEADDR
> >slirp: call socket_set_fast_reuse instead of setting SO_REUSEADDR
> >util: call socket_set_fast_reuse instead of setting SO_REUSEADDR
> >
> 
> Pinging this patch, as I think it is still an appropriate approach
> to the issue:
> 
> I did some research and apparently there is a valid use case for
> SO_REUSEADDR
> on windows when multiple clients need to listen to the same port for
> the same
> multicast group. IMHO making qemu_setsockopt ignore SO_REUSEADDR on windows
> might be confusing for some use cases. Actually net_socket_mcast_create in
> net/socket.c should probably set SO_REUSEADDR on windows. This is
> also an issue
> with patch 3 I supplied that I will address in a new version of this
> patch set if there is
> an agreement on a general approach.

Sounds like a good idea.  The patch series overall looks good.

Stefan

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

[Sebastian Ottlik]

On 09.09.2013 14:05, Stefan Hajnoczi wrote:
> On Thu, Sep 05, 2013 at 03:48:16PM +0200, Sebastian Ottlik wrote:
>> On 04.09.2013 19:08, Sebastian Ottlik wrote:
>>> This patchset disabels all use of SO_REUSEADDR on Windows. 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
>>> instead. 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.
>>>
>>> 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           |   19 +++++++------------
>>>   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, 43 insertions(+), 30 deletions(-)
>>>
>>>
>>> util: add socket_set_fast_reuse function
>>> gdbstub: call socket_set_fast_reuse instead of setting SO_REUSEADDR
>>> net: call socket_set_fast_reuse instead of setting SO_REUSEADDR
>>> slirp: call socket_set_fast_reuse instead of setting SO_REUSEADDR
>>> util: call socket_set_fast_reuse instead of setting SO_REUSEADDR
>>>
>> Pinging this patch, as I think it is still an appropriate approach
>> to the issue:
>>
>> I did some research and apparently there is a valid use case for
>> SO_REUSEADDR
>> on windows when multiple clients need to listen to the same port for
>> the same
>> multicast group. IMHO making qemu_setsockopt ignore SO_REUSEADDR on windows
>> might be confusing for some use cases. Actually net_socket_mcast_create in
>> net/socket.c should probably set SO_REUSEADDR on windows. This is
>> also an issue
>> with patch 3 I supplied that I will address in a new version of this
>> patch set if there is
>> an agreement on a general approach.
> Sounds like a good idea.  The patch series overall looks good.
>
> Stefan
Thanks for the feedback. I will resubmit the patch series including the 
change for net_socket_mcast_create and fixes for the style issues you 
pointed out soon.

When I submitted this new version of the patch set I think I was a 
little early as there was still some discussion in the thread of the 
original version. In general, what is a good period to wait before 
submitting a new version?

Best Regards,
   Sebastian

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

[Stefan Hajnoczi]

On Mon, Sep 09, 2013 at 02:15:20PM +0200, Sebastian Ottlik wrote:
> On 09.09.2013 14:05, Stefan Hajnoczi wrote:
> >On Thu, Sep 05, 2013 at 03:48:16PM +0200, Sebastian Ottlik wrote:
> >>On 04.09.2013 19:08, Sebastian Ottlik wrote:
> >>>This patchset disabels all use of SO_REUSEADDR on Windows. 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
> >>>instead. 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.
> >>>
> >>>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           |   19 +++++++------------
> >>>  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, 43 insertions(+), 30 deletions(-)
> >>>
> >>>
> >>>util: add socket_set_fast_reuse function
> >>>gdbstub: call socket_set_fast_reuse instead of setting SO_REUSEADDR
> >>>net: call socket_set_fast_reuse instead of setting SO_REUSEADDR
> >>>slirp: call socket_set_fast_reuse instead of setting SO_REUSEADDR
> >>>util: call socket_set_fast_reuse instead of setting SO_REUSEADDR
> >>>
> >>Pinging this patch, as I think it is still an appropriate approach
> >>to the issue:
> >>
> >>I did some research and apparently there is a valid use case for
> >>SO_REUSEADDR
> >>on windows when multiple clients need to listen to the same port for
> >>the same
> >>multicast group. IMHO making qemu_setsockopt ignore SO_REUSEADDR on windows
> >>might be confusing for some use cases. Actually net_socket_mcast_create in
> >>net/socket.c should probably set SO_REUSEADDR on windows. This is
> >>also an issue
> >>with patch 3 I supplied that I will address in a new version of this
> >>patch set if there is
> >>an agreement on a general approach.
> >Sounds like a good idea.  The patch series overall looks good.
> >
> >Stefan
> Thanks for the feedback. I will resubmit the patch series including
> the change for net_socket_mcast_create and fixes for the style
> issues you pointed out soon.
> 
> When I submitted this new version of the patch set I think I was a
> little early as there was still some discussion in the thread of the
> original version. In general, what is a good period to wait before
> submitting a new version?

Sending an extra revision is not a problem.  In fact, I think that's
better than waiting too long and forgetting about the series.  If you
want to wait for discussion to end, two business days seems like a safe
period of time.

By being responsive you will also spur reviewers to be responsive :).

Stefan