Patchwork : VNC: set listener socket to non-blocking mode

login
register
mail settings
Submitter Gianni Tedesco
Date Oct. 22, 2010, 11:34 a.m.
Message ID <1287747270.12843.4388.camel@qabil.uk.xensource.com>
Download mbox | patch
Permalink /patch/68839/
State New
Headers show

Comments

Gianni Tedesco - Oct. 22, 2010, 11:34 a.m.
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<-----

Patch

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);
 }