From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47424) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VJIli-0003lm-Ci for qemu-devel@nongnu.org; Tue, 10 Sep 2013 03:53:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VJIlZ-0007Y4-V3 for qemu-devel@nongnu.org; Tue, 10 Sep 2013 03:53:50 -0400 Received: from mail-ee0-x234.google.com ([2a00:1450:4013:c00::234]:59604) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VJIlZ-0007Y0-KC for qemu-devel@nongnu.org; Tue, 10 Sep 2013 03:53:41 -0400 Received: by mail-ee0-f52.google.com with SMTP id c41so3697041eek.11 for ; Tue, 10 Sep 2013 00:53:40 -0700 (PDT) Date: Tue, 10 Sep 2013 09:53:38 +0200 From: Stefan Hajnoczi Message-ID: <20130910075338.GC10477@stefanha-thinkpad.redhat.com> References: <1378314535-27587-1-git-send-email-ottlik@fzi.de> <52288BA0.6060508@fzi.de> <20130909120543.GD20215@stefanha-thinkpad.redhat.com> <522DBBD8.1030802@fzi.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <522DBBD8.1030802@fzi.de> Subject: Re: [Qemu-devel] [PATCH v2 0/5] Do not set SO_REUSEADDR on Windows List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sebastian Ottlik Cc: Peter Maydell , Anthony Liguori , Jan Kiszka , Michael Tokarev , qemu-devel@nongnu.org, Stefan Hajnoczi , Stefan Weil , Paolo Bonzini 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