Hi all, reading qemu code I realized that the qemu vnc server sometimes sends framebuffer updates even if the client didn''t request any. This is not consistent with the RFB protocol spec and can break some clients. The patch I am attaching strictly enforces compliance with the RFB protocol making sure framebuffer updates are sent only if the client requested one. Doing so is more difficult than it seems because some framebuffer pseudo-encoding updates cannot be discarded but must be sent anyway: for example desktop resize and pixel format change messages. To solve the problem I wrote a queue that stores those messages and sends them as soon as the client asks for an update. Since 90% of the times the queue is used to store only few elements, the queue allocates 10 elements at the beginning and every time it runs out of elements allocates other 10 elements. This is should drastically limit the number of malloc and free needed to maintain the queue. I did some stress tests in the last couple of days and seems to work well. Best Regards, Stefano Stabellini Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini wrote:> Hi all, > reading qemu code I realized that the qemu vnc server sometimes sends > framebuffer updates even if the client didn''t request any. > This is not consistent with the RFB protocol spec and can break some > clients.It''s actually consistent with the RFB spec. Have you seen any clients break? The RFB spec states pretty clearly that a single FramebufferUpdateRequest may generate 0 or more FramebufferUpdate events. Once a client has sent a single FramebufferUpdate request, it should expect to continue to receive more FramebufferUpdates for an indefinite period of time according to the specification. Regards, Anthony Liguori> The patch I am attaching strictly enforces compliance with the RFB > protocol making sure framebuffer updates are sent only if the client > requested one. > Doing so is more difficult than it seems because some framebuffer > pseudo-encoding updates cannot be discarded but must be sent anyway: for > example desktop resize and pixel format change messages. To solve the > problem I wrote a queue that stores those messages and sends them as > soon as the client asks for an update. > Since 90% of the times the queue is used to store only few elements, the > queue allocates 10 elements at the beginning and every time it runs out > of elements allocates other 10 elements. This is should drastically > limit the number of malloc and free needed to maintain the queue. > I did some stress tests in the last couple of days and seems to work well. > Best Regards, > > Stefano Stabellini > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > ------------------------------------------------------------------------ > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Tue, Feb 26, 2008 at 10:15:51AM -0600, Anthony Liguori wrote:> Stefano Stabellini wrote: > >Hi all, > >reading qemu code I realized that the qemu vnc server sometimes sends > >framebuffer updates even if the client didn''t request any. > >This is not consistent with the RFB protocol spec and can break some > >clients. > > > It''s actually consistent with the RFB spec. Have you seen any clients > break? > > The RFB spec states pretty clearly that a single > FramebufferUpdateRequest may generate 0 or more FramebufferUpdate > events. Once a client has sent a single FramebufferUpdate request, it > should expect to continue to receive more FramebufferUpdates for an > indefinite period of time according to the specification.The reverse is true too - the server may coallese multiple FramebufferUpdateRequest into a single FramebufferUpdate reply. There is no 1-to-1 mapping between request & reply as this patch attempts to enforce. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=| _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Daniel P. Berrange wrote:> On Tue, Feb 26, 2008 at 10:15:51AM -0600, Anthony Liguori wrote: >> Stefano Stabellini wrote: >>> Hi all, >>> reading qemu code I realized that the qemu vnc server sometimes sends >>> framebuffer updates even if the client didn''t request any. >>> This is not consistent with the RFB protocol spec and can break some >>> clients. >> >> It''s actually consistent with the RFB spec. Have you seen any clients >> break? >> >> The RFB spec states pretty clearly that a single >> FramebufferUpdateRequest may generate 0 or more FramebufferUpdate >> events. Once a client has sent a single FramebufferUpdate request, it >> should expect to continue to receive more FramebufferUpdates for an >> indefinite period of time according to the specification. > > The reverse is true too - the server may coallese multiple FramebufferUpdateRequest > into a single FramebufferUpdate reply. There is no 1-to-1 mapping between > request & reply as this patch attempts to enforce. >I have just re-read the rfb protocol spec and it specifies quite clearly that framebuffer updates are sent in response to framebuffer update requests. However it is true that the server can collapse multiple requests in a single reply. It is also clear that the reply can come at any time. I have seen the linux vncviewer client breaking because of race conditions due to this problem. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini wrote:> > I have just re-read the rfb protocol spec and it specifies quite > clearly that framebuffer updates are sent in response to framebuffer > update requests. > However it is true that the server can collapse multiple requests in a > single reply. It is also clear that the reply can come at any time. > > I have seen the linux vncviewer client breaking because of race > conditions due to this problem.You mean realvnc? The race condition is due to it''s use of SetPixelFormat. By slowing the updates, what you''re really doing is just working around that race condition. That''s not a proper solution though. This goes away if you set the realvnc options so that it doesn''t change the pixel format from what the server specifies. Regards, Anthony Liguori _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony Liguori wrote:> You mean realvnc? The race condition is due to it''s use of > SetPixelFormat. By slowing the updates, what you''re really doing is > just working around that race condition. That''s not a proper solution > though. > > This goes away if you set the realvnc options so that it doesn''t change > the pixel format from what the server specifies. >I think that the realvnc "bug" is due to the fact that they assume that if they don''t send any update requests, they shouldn''t expect any. This is not a wrong assumption to make unless you are right about the 1 request -> N reply argument (I still think that this is not what the rfb spec says). Besides my patch doesn''t slow down the connection so much, I cannot tell the difference on a quick connections. I''ll let you know what is the behaviour on a slow connection. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini wrote:> Anthony Liguori wrote: >> You mean realvnc? The race condition is due to it''s use of >> SetPixelFormat. By slowing the updates, what you''re really doing is >> just working around that race condition. That''s not a proper >> solution though. >> >> This goes away if you set the realvnc options so that it doesn''t >> change the pixel format from what the server specifies. >> > > I think that the realvnc "bug" is due to the fact that they assume > that if they don''t send any update requests, they shouldn''t expect any. > This is not a wrong assumption to make unless you are right about the > 1 request -> N reply argument (I still think that this is not what the > rfb spec says).Regardless of whether you interpret the spec to allow 1 req -> N replies, the spec is clear that each multiple requests can result in a single reply. The requests/replies aren''t sequenced though so you have no way of knowing what reply the request is in response too. For instance, consider the following: Request => Reply Request => Reply Request => Request => Reply Request => Reply This is entirely indistinguishable from: Request => Reply Request => Reply Request => Request => Request => Reply => Reply Because the requests and replies don''t carry any sort of sequence number. Regards, Anthony Liguori> Besides my patch doesn''t slow down the connection so much, I cannot > tell the difference on a quick connections. I''ll let you know what is > the behaviour on a slow connection._______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel