Re: [Qemu-devel] [PATCH v2 0/5] Do not set SO_REUSEADDR on Windows
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
Re: [Qemu-devel] [PATCH v2 0/5] Do not set SO_REUSEADDR on Windows
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
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
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
[Qemu-devel] [PATCH v2 0/5] Do not set SO_REUSEADDR on Windows
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
