qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Gerd Hoffmann <kraxel@redhat.com>
To: Glauber Costa <glommer@redhat.com>
Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org
Subject: [Qemu-devel] Re: [PATCH] update server bits on vnc_update
Date: Tue, 21 Jul 2009 10:09:05 +0200	[thread overview]
Message-ID: <4A6577A1.9010509@redhat.com> (raw)
In-Reply-To: <1247892434-21209-1-git-send-email-glommer@redhat.com>

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

  parent reply	other threads:[~2009-07-21  8:09 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-18  4:47 [Qemu-devel] [PATCH] update server bits on vnc_update Glauber Costa
2009-07-18 16:20 ` [Qemu-devel] " Anthony Liguori
2009-07-19  7:57 ` [Qemu-devel] " Lucas Meneghel Rodrigues
2009-07-21  8:09 ` Gerd Hoffmann [this message]
2009-07-21 11:19   ` [Qemu-devel] " Gerd Hoffmann
2009-07-21 12:56   ` Stefano Stabellini
2009-07-21 13:16     ` Stefano Stabellini
2009-07-21 13:19     ` Gerd Hoffmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4A6577A1.9010509@redhat.com \
    --to=kraxel@redhat.com \
    --cc=aliguori@us.ibm.com \
    --cc=glommer@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).