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
next prev parent 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).