From: Gianni Tedesco <gianni.tedesco@citrix.com>
To: Anthony Liguori <anthony@codemonkey.ws>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [PATCH]: VNC: set listener socket to non-blocking mode
Date: Fri, 22 Oct 2010 12:34:30 +0100 [thread overview]
Message-ID: <1287747270.12843.4388.camel@qabil.uk.xensource.com> (raw)
In-Reply-To: <4CC06DDC.9010302@codemonkey.ws>
On Thu, 2010-10-21 at 17:44 +0100, Anthony Liguori wrote:
> On 10/21/2010 11:18 AM, Gianni Tedesco wrote:
> > This prevents qemu from hanging waiting for a client to connect. I have
> > reproduced this when doing a loadvm but it may be a more general problem
> > in that poll/accept may race if a client aborts the connection with a
> > RST before the accept has completed. In either case the fix seems
> > harmless.
> >
> > Signed-off-by: Gianni Tedesco<gianni.tedesco@citrix.com>
> >
>
> I'd feel a little better with explicit error handling in the accept()
> path and a comment explaining why this was necessary.
>
> But otherwise, nice catch!
Thanks!
Not sure what you mean about the explicit error handling in the accept()
path though? Accept can fail for a few reasons, the most worrying is
EMFILE which easily leads to livelock. Poll reports listener as ready to
accept but accept fails because we maxed out the fd table so go back to
poll and the listener is still ready to accept etc... It's not always
clear how to handle specific accept() errors.
I think only calling vnc_connect() when we have an fd is the best we can
hope for right now.
Unless I misunderstood?
-----8<-----8<-----8<-----
diff --git a/ui/vnc.c b/ui/vnc.c
index 864342e..65dc55c 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2736,5 +2736,9 @@ int vnc_display_open(DisplayState *ds, const char *display)
vs->display = dpy;
}
}
+ /* necessary to prevent accept() hanging indefinitely if a clients
+ * connection-reset wins the race between poll() and accept()
+ */
+ socket_set_nonblock(vs->lsock);
return qemu_set_fd_handler2(vs->lsock, NULL, vnc_listen_read, NULL, vs);
}
prev parent reply other threads:[~2010-10-22 11:34 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-10-21 16:18 [Qemu-devel] [PATCH]: VNC: set listener socket to non-blocking mode Gianni Tedesco
2010-10-21 16:44 ` Anthony Liguori
2010-10-22 11:34 ` Gianni Tedesco [this message]
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=1287747270.12843.4388.camel@qabil.uk.xensource.com \
--to=gianni.tedesco@citrix.com \
--cc=anthony@codemonkey.ws \
--cc=qemu-devel@nongnu.org \
/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).