Re: [Qemu-trivial] [PATCH v2] vnc: do not disconnect on EAGAIN

[Gerd Hoffmann]

On Fr, 2017-02-03 at 12:52 +0300, Michael Tokarev wrote:
> When qemu vnc server is trying to send large update to clients,
> there might be a situation when system responds with something
> like EAGAIN, indicating that there's no system memory to send
> that much data (depending on the network speed, client and server
> and what is happening).  In this case, something like this happens
> on qemu side (from strace):
> 
> sendmsg(16, {msg_name(0)=NULL,
>         msg_iov(1)=[{"\244\"..., 729186}],
>         msg_controllen=0, msg_flags=0}, 0) = 103950
> sendmsg(16, {msg_name(0)=NULL,
>         msg_iov(1)=[{"lz\346"..., 1559618}],
>         msg_controllen=0, msg_flags=0}, 0) = -1 EAGAIN
> sendmsg(-1, {msg_name(0)=NULL,
>         msg_iov(1)=[{"lz\346"..., 1559618}],
>         msg_controllen=0, msg_flags=0}, 0) = -1 EBADF
> 
> qemu closes the socket before the retry, and obviously it gets EBADF
> when trying to send to -1.
> 
> This is because there WAS a special handling for EAGAIN, but now it doesn't
> work anymore, after commit 04d2529da27db512dcbd5e99d0e26d333f16efcc, because
> now in all error-like cases we initiate vnc disconnect.
> 
> This change were introduced in qemu 2.6, and caused numerous grief for many
> people, resulting in their vnc clients reporting sporadic random disconnects
> from vnc server.
> 
> Fix that by doing the disconnect only when necessary, i.e. omitting this
> very case of EAGAIN.
> 
> Hopefully the existing condition (comparing with QIO_CHANNEL_ERR_BLOCK)
> is sufficient, as the original code (before the above commit) were
> checking for other errno values too.
> 
> Apparently there's another (semi?)bug exist somewhere here, since the
> code tries to write to fd# -1, it probably should check if the connection
> is open before. But this isn't important.
> 
> Signed-off-by: Michael Tokarev <address@hidden>
> Fixes: 04d2529da27db512dcbd5e99d0e26d333f16efcc
> Cc: Daniel P. Berrange <address@hidden>
> Cc: Gerd Hoffmann <address@hidden>
> Cc: address@hidden

Added to ui patch queue.

thanks,
  Gerd