[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 8/9] vnc: add support for extended desktop resize
From: |
Gerd Hoffmann |
Subject: |
Re: [PATCH v2 8/9] vnc: add support for extended desktop resize |
Date: |
Tue, 15 Dec 2020 11:15:03 +0100 |
On Tue, Dec 08, 2020 at 06:13:28PM +0000, Daniel P. Berrangé wrote:
> On Tue, Dec 08, 2020 at 12:57:36PM +0100, Gerd Hoffmann wrote:
> > The extended desktop resize encoding adds support for (a) clients
> > sending resize requests to the server, and (b) multihead support.
> >
> > This patch implements (a). All resize requests are rejected by qemu.
> > Qemu can't resize the framebuffer on its own, this is in the hands of
> > the guest, so all qemu can do is forward the request to the guest.
> > Should the guest actually resize the framebuffer we can notify the vnc
> > client later with a separate message.
> >
> > This requires support in the display device. Works with virtio-gpu.
> >
> > https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#extendeddesktopsize-pseudo-encoding
>
> The spec says
>
> "The server must send an ExtendedDesktopSize rectangle in response
> to a FramebufferUpdateRequest with incremental set to zero, assuming
> the client has requested the ExtendedDesktopSize pseudo-encoding
> using the SetEncodings message. This requirement is needed so that
> the client has a reliable way of fetching the initial screen
> configuration, and to determine if the server supports the
> SetDesktopSize message.
>
> A consequence of this is that a client must not respond to an
> ExtendedDesktopSize rectangle by sending a FramebufferUpdateRequest
> with incremental set to zero. Doing so would make the system go into
> an infinite loop."
>
> Historically gtk-vnc always sets incremental=0 after a resize message,
> so it should trigger an infinite loop. This doesn't happen with QEMU's
> impl, so I think QEMU isn't correctly following the sec here. IIUC,
> tt needs to force send an ExtendedDesktopSize message every time
> incremental=0, not merely when a resize happens.
There are three cases where qemu sends an ExtendedDesktopSize message:
(1) during the initial handshake (to cover the feature detection).
(2) as response to a SetDesktopSize request, and
(3) when an resize happens.
So, yes, I skimmed the spec a bit to fast and didn't implement it
correctly. Which also explains why you can't trigger the infinite
loop issue.
> Having said that, I find this part of the spec rather crazy. I dont
> see why clients need more than the first ExtendedDesktopSize message
> in order to detect the feature, rather than after every single
> incremental=0 update request.
Yes, it doesn't make much sense.
> None the less the spec says we should get an infinite loop and with
> my intentional attempt to cause this, QEMU doesn't play ball.
Not fully sure I should consider this a bug or a feature.
This patch doesn't conform to the spec.
But it clearly is more robust ...
take care,
Gerd
- [PATCH v2 1/9] console: drop qemu_console_get_ui_info, (continued)
- [PATCH v2 1/9] console: drop qemu_console_get_ui_info, Gerd Hoffmann, 2020/12/08
- [PATCH v2 4/9] vnc: drop unused copyrect feature, Gerd Hoffmann, 2020/12/08
- [PATCH v2 9/9] vnc: move check into vnc_cursor_define, Gerd Hoffmann, 2020/12/08
- [PATCH v2 7/9] vnc: force initial resize message, Gerd Hoffmann, 2020/12/08
- [PATCH v2 8/9] vnc: add support for extended desktop resize, Gerd Hoffmann, 2020/12/08
[PATCH v2 3/9] vnc: use enum for features, Gerd Hoffmann, 2020/12/08
[PATCH v2 6/9] vnc: add alpha cursor support, Gerd Hoffmann, 2020/12/08
[PATCH v2 5/9] vnc: add pseudo encodings, Gerd Hoffmann, 2020/12/08
Re: [PATCH v2 0/9] vnc: support some new extensions., Daniel P . Berrangé, 2020/12/08