qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Sebastian Ottlik <ottlik@fzi.de>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Anthony Liguori <aliguori@us.ibm.com>,
	Jan Kiszka <jan.kiszka@siemens.com>,
	Michael Tokarev <mjt@tls.msk.ru>,
	qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>,
	Stefan Weil <sw@weilnetz.de>, Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2 0/5] Do not set SO_REUSEADDR on Windows
Date: Mon, 9 Sep 2013 14:15:20 +0200	[thread overview]
Message-ID: <522DBBD8.1030802@fzi.de> (raw)
In-Reply-To: <20130909120543.GD20215@stefanha-thinkpad.redhat.com>

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

  reply	other threads:[~2013-09-09 12:15 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-04 17:08 [Qemu-devel] [PATCH v2 0/5] Do not set SO_REUSEADDR on Windows Sebastian Ottlik
2013-09-04 17:08 ` [Qemu-devel] [PATCH v2 1/5] util: add socket_set_fast_reuse function which will replace setting SO_REUSEADDR Sebastian Ottlik
2013-09-09 12:02   ` Stefan Hajnoczi
2013-09-04 17:08 ` [Qemu-devel] [PATCH v2 2/5] gdbstub: call socket_set_fast_reuse instead of " Sebastian Ottlik
2013-09-04 17:08 ` [Qemu-devel] [PATCH v2 3/5] net: " Sebastian Ottlik
2013-09-04 17:08 ` [Qemu-devel] [PATCH v2 4/5] slirp: " Sebastian Ottlik
2013-09-04 17:08 ` [Qemu-devel] [PATCH v2 5/5] util: " Sebastian Ottlik
2013-09-05 13:48 ` [Qemu-devel] [PATCH v2 0/5] Do not set SO_REUSEADDR on Windows Sebastian Ottlik
2013-09-09 12:05   ` Stefan Hajnoczi
2013-09-09 12:15     ` Sebastian Ottlik [this message]
2013-09-10  7:53       ` Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=522DBBD8.1030802@fzi.de \
    --to=ottlik@fzi.de \
    --cc=aliguori@us.ibm.com \
    --cc=jan.kiszka@siemens.com \
    --cc=mjt@tls.msk.ru \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    --cc=stefanha@redhat.com \
    --cc=sw@weilnetz.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).