From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1IdBKs-0000Or-Co for qemu-devel@nongnu.org; Wed, 03 Oct 2007 17:00:50 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1IdBKp-0000IO-R5 for qemu-devel@nongnu.org; Wed, 03 Oct 2007 17:00:50 -0400 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1IdBKp-0000HN-JF for qemu-devel@nongnu.org; Wed, 03 Oct 2007 17:00:47 -0400 Received: from wa-out-1112.google.com ([209.85.146.181]) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1IdBKo-0002uN-JV for qemu-devel@nongnu.org; Wed, 03 Oct 2007 17:00:46 -0400 Received: by wa-out-1112.google.com with SMTP id k22so7959181waf for ; Wed, 03 Oct 2007 14:00:45 -0700 (PDT) Message-ID: <83a4d4ca0710031400w6d8b7438yb63d3400384f085d@mail.gmail.com> Date: Wed, 3 Oct 2007 23:00:45 +0200 From: "Eduardo Felipe" Subject: Re: [Qemu-devel] [Patch] VNC: Fix crash with non-resizing clients In-Reply-To: <20071003195052.GF8342@redhat.com> MIME-Version: 1.0 Content-Type: multipart/alternative; boundary="----=_Part_23582_22651324.1191445245448" References: <83a4d4ca0710031229u68202e6aub32ba6257dfb0bc0@mail.gmail.com> <20071003195052.GF8342@redhat.com> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" , qemu-devel@nongnu.org ------=_Part_23582_22651324.1191445245448 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline 2007/10/3, Daniel P. Berrange : > > > The memset calls in that patch are bogus & not correctly fixing the buffer > update problem. You're merely setting the 'old data' to have pixel value > 42 - if the guest OS framebuffer happens to also have aras with pixel > value > of 42 too, the frame buffer will still not correctly update. The root > problem is overly-aggressive update minimization logic in > vnc_update_client. > This is in turn flawed beause the dirty_row aray is trying to encode two > separate concepts - areas which are dirty, and areas which need to be > sent to the client. The latter are a superset of the former, but the code > in vnc_update_client minimizes based on dirtiness, so updates will get > missed out. Setting the old data to 42 merely changes which areas will get > missed updates. Agreed. I just used the same hack that is scattered around several other places to make a patch quickly. The QEMU code in Xen has added a update_row field, separate from the > dirty_row > field. Thus after a resize it can update the entire framebuffer, > regardless > of whether QEMU's copy of the framebuffer is dirty wrt to the guest copy. I think that VNC server needs a deep rework, as several qemu related projects have done, but meanwhile correcting bugs with small non-intrusive patches may be the way to go. Edu. ------=_Part_23582_22651324.1191445245448 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline
2007/10/3, Daniel P. Berrange <berrange@redhat.com>:

The memset calls in that patch are bogus & not correctly fixing the buffer
update problem. You're merely setting the 'old data' to have pixel value
42 - if the guest OS framebuffer happens to also have aras with pixel value
of 42 too, the frame buffer will still not correctly update. The root
problem is overly-aggressive update minimization logic in vnc_update_client.
This is in turn flawed beause the dirty_row aray is trying to encode two
separate concepts - areas which are dirty, and areas which need to be
sent to the client. The latter are a superset of the former, but the code
in vnc_update_client minimizes based on dirtiness, so updates will get
missed out. Setting the old data to 42 merely changes which areas will get
missed updates.

Agreed. I just used the same hack that is scattered around several other places to make a patch quickly.


The QEMU code in Xen has added a update_row field, separate from the dirty_row
field. Thus after a resize it can update the entire framebuffer, regardless
of whether QEMU's copy of the framebuffer is dirty wrt to the guest copy.

I think that VNC server needs a deep rework, as several qemu related projects have done, but meanwhile correcting bugs with small non-intrusive patches may be the way to go.

Edu.

------=_Part_23582_22651324.1191445245448--