From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MTAPW-0000hG-6A for qemu-devel@nongnu.org; Tue, 21 Jul 2009 04:09:18 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MTAPR-0000eY-Kp for qemu-devel@nongnu.org; Tue, 21 Jul 2009 04:09:17 -0400 Received: from [199.232.76.173] (port=45976 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MTAPR-0000eM-By for qemu-devel@nongnu.org; Tue, 21 Jul 2009 04:09:13 -0400 Received: from mx20.gnu.org ([199.232.41.8]:62739) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1MTAPQ-0004Uf-TQ for qemu-devel@nongnu.org; Tue, 21 Jul 2009 04:09:13 -0400 Received: from mx2.redhat.com ([66.187.237.31]) by mx20.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MTAPQ-0003Vn-4k for qemu-devel@nongnu.org; Tue, 21 Jul 2009 04:09:12 -0400 Message-ID: <4A6577A1.9010509@redhat.com> Date: Tue, 21 Jul 2009 10:09:05 +0200 From: Gerd Hoffmann MIME-Version: 1.0 References: <1247892434-21209-1-git-send-email-glommer@redhat.com> In-Reply-To: <1247892434-21209-1-git-send-email-glommer@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH] update server bits on vnc_update List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Glauber Costa Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org On 07/18/09 06:47, Glauber Costa wrote: > Since the server/guest split in vnc architecture, we > no longer update the server bits on large updates. Result > is screen gets garbled, specially on scroll actions. > > I must admit I don't fully understand our vnc code, but after > a careful reading, it seemed to me the proposed patch would fix > it, and it indeed, works. NAK. VNC screen update workflow is this: (1) vnc_update() is called to mark a region as dirty, the guest bitmap is updated accordingly. May happen multiple times. (2) vnc_update_client() is called by timer. It walks the guest bitmap. For dirty regions it checks whenever guest and server bitmap are *really* different, if so it does a guest->server copy and updates the server dirty bitmap. (3) vnc_update_client() walks the server dirty map and sends over the changes to the client. Your patch kills the optimization in (2) and papers over the real bug. Some extra care is needed in vnc_dpy_copy() which can send a bitblit command to the guest (VNC_ENCODING_COPYRECT). And while looking at the code I think I've spotted the actual bug: When sending a bitblit to the client we do *not* update the local server surface. Result is the vnc clients and the vnc servers idea of the screen content are not in sync any more. Which in turn will break the optimization in (2) and can easily result in a corrupted screen ... cheers, Gerd