[Qemu-devel] [PATCH 0/5] vnc: desktop size patches.

[Qemu-devel] [PATCH 0/5] vnc: desktop size patches.

[Gerd Hoffmann]

  Hi,

This series brings a bunch of vnc desktop size patches, fixing the
issues discussed in the "Possible race condition in VNC display
resizing" thread.  Check list archive here:

http://lists.gnu.org/archive/html/qemu-devel/2010-04/msg01778.html

cheers,
  Gerd

Gerd Hoffmann (5):
  vnc: factor out vnc_desktop_resize()
  vnc: send desktopresize event as reply to set encodings
  vnc: keep track of client desktop size
  vnc: don't send invalid screen updates.
  vnc: move size-changed check into the vnc_desktop_resize function.

 vnc.c |   50 +++++++++++++++++++++++++++++++++-----------------
 vnc.h |    2 ++
 2 files changed, 35 insertions(+), 17 deletions(-)

[Qemu-devel] [PATCH 1/5] vnc: factor out vnc_desktop_resize()

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 vnc.c |   24 ++++++++++++++++--------
 1 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/vnc.c b/vnc.c
index 11ae3e5..aaebe24 100644
--- a/vnc.c
+++ b/vnc.c
@@ -514,6 +514,21 @@ void buffer_append(Buffer *buffer, const void *data, size_t len)
     buffer->offset += len;
 }
 
+static void vnc_desktop_resize(VncState *vs)
+{
+    DisplayState *ds = vs->ds;
+
+    if (vs->csock == -1 || !vnc_has_feature(vs, VNC_FEATURE_RESIZE)) {
+        return;
+    }
+    vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
+    vnc_write_u8(vs, 0);
+    vnc_write_u16(vs, 1); /* number of rects */
+    vnc_framebuffer_update(vs, 0, 0, ds_get_width(ds), ds_get_height(ds),
+                           VNC_ENCODING_DESKTOPRESIZE);
+    vnc_flush(vs);
+}
+
 static void vnc_dpy_resize(DisplayState *ds)
 {
     int size_changed;
@@ -542,14 +557,7 @@ static void vnc_dpy_resize(DisplayState *ds)
     QTAILQ_FOREACH(vs, &vd->clients, next) {
         vnc_colordepth(vs);
         if (size_changed) {
-            if (vs->csock != -1 && vnc_has_feature(vs, VNC_FEATURE_RESIZE)) {
-                vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
-                vnc_write_u8(vs, 0);
-                vnc_write_u16(vs, 1); /* number of rects */
-                vnc_framebuffer_update(vs, 0, 0, ds_get_width(ds), ds_get_height(ds),
-                        VNC_ENCODING_DESKTOPRESIZE);
-                vnc_flush(vs);
-            }
+            vnc_desktop_resize(vs);
         }
         if (vs->vd->cursor) {
             vnc_cursor_define(vs);
-- 
1.6.6.1

Re: [Qemu-devel] [PATCH 1/5] vnc: factor out vnc_desktop_resize()

[Anthony Liguori]

On 05/25/2010 11:25 AM, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
>    

Applied all.  Thanks.

Regards,

Anthony Liguori

> ---
>   vnc.c |   24 ++++++++++++++++--------
>   1 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/vnc.c b/vnc.c
> index 11ae3e5..aaebe24 100644
> --- a/vnc.c
> +++ b/vnc.c
> @@ -514,6 +514,21 @@ void buffer_append(Buffer *buffer, const void *data, size_t len)
>       buffer->offset += len;
>   }
>
> +static void vnc_desktop_resize(VncState *vs)
> +{
> +    DisplayState *ds = vs->ds;
> +
> +    if (vs->csock == -1 || !vnc_has_feature(vs, VNC_FEATURE_RESIZE)) {
> +        return;
> +    }
> +    vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
> +    vnc_write_u8(vs, 0);
> +    vnc_write_u16(vs, 1); /* number of rects */
> +    vnc_framebuffer_update(vs, 0, 0, ds_get_width(ds), ds_get_height(ds),
> +                           VNC_ENCODING_DESKTOPRESIZE);
> +    vnc_flush(vs);
> +}
> +
>   static void vnc_dpy_resize(DisplayState *ds)
>   {
>       int size_changed;
> @@ -542,14 +557,7 @@ static void vnc_dpy_resize(DisplayState *ds)
>       QTAILQ_FOREACH(vs,&vd->clients, next) {
>           vnc_colordepth(vs);
>           if (size_changed) {
> -            if (vs->csock != -1&&  vnc_has_feature(vs, VNC_FEATURE_RESIZE)) {
> -                vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
> -                vnc_write_u8(vs, 0);
> -                vnc_write_u16(vs, 1); /* number of rects */
> -                vnc_framebuffer_update(vs, 0, 0, ds_get_width(ds), ds_get_height(ds),
> -                        VNC_ENCODING_DESKTOPRESIZE);
> -                vnc_flush(vs);
> -            }
> +            vnc_desktop_resize(vs);
>           }
>           if (vs->vd->cursor) {
>               vnc_cursor_define(vs);
>    

[Qemu-devel] [PATCH 2/5] vnc: send desktopresize event as reply to set encodings

[Gerd Hoffmann]

In case the desktop did resize while the vnc connection setup was still
in progress the client isn't informed about it.  Send a desktop resize
event as soon as the client told us it can handle deskop resize via set
encodings message to make sure the client us up to date.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 vnc.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/vnc.c b/vnc.c
index aaebe24..0e0e566 100644
--- a/vnc.c
+++ b/vnc.c
@@ -1709,6 +1709,7 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
             break;
         }
     }
+    vnc_desktop_resize(vs);
     check_pointer_type_change(&vs->mouse_mode_notifier);
 }
 
-- 
1.6.6.1

Re: [Qemu-devel] [PATCH 2/5] vnc: send desktopresize event as reply to set encodings

[Corentin Chary]

On Tue, May 25, 2010 at 6:25 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> In case the desktop did resize while the vnc connection setup was still
> in progress the client isn't informed about it.  Send a desktop resize
> event as soon as the client told us it can handle deskop resize via set
> encodings message to make sure the client us up to date.

I had a similar patch on my queue but yours is probably cleaner :).

-- 
Corentin Chary
http://xf.iksaif.net

[Qemu-devel] [PATCH 3/5] vnc: keep track of client desktop size

[Gerd Hoffmann]

Add two new variables to keep track of the vnc clients desktop size.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 vnc.c |   10 +++++++---
 vnc.h |    2 ++
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/vnc.c b/vnc.c
index 0e0e566..30e0bed 100644
--- a/vnc.c
+++ b/vnc.c
@@ -521,10 +521,12 @@ static void vnc_desktop_resize(VncState *vs)
     if (vs->csock == -1 || !vnc_has_feature(vs, VNC_FEATURE_RESIZE)) {
         return;
     }
+    vs->client_width = ds_get_width(ds);
+    vs->client_height = ds_get_height(ds);
     vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
     vnc_write_u8(vs, 0);
     vnc_write_u16(vs, 1); /* number of rects */
-    vnc_framebuffer_update(vs, 0, 0, ds_get_width(ds), ds_get_height(ds),
+    vnc_framebuffer_update(vs, 0, 0, vs->client_width, vs->client_height,
                            VNC_ENCODING_DESKTOPRESIZE);
     vnc_flush(vs);
 }
@@ -1958,8 +1960,10 @@ static int protocol_client_init(VncState *vs, uint8_t *data, size_t len)
     char buf[1024];
     int size;
 
-    vnc_write_u16(vs, ds_get_width(vs->ds));
-    vnc_write_u16(vs, ds_get_height(vs->ds));
+    vs->client_width = ds_get_width(vs->ds);
+    vs->client_height = ds_get_height(vs->ds);
+    vnc_write_u16(vs, vs->client_width);
+    vnc_write_u16(vs, vs->client_height);
 
     pixel_format_message(vs);
 
diff --git a/vnc.h b/vnc.h
index 0d39897..d648832 100644
--- a/vnc.h
+++ b/vnc.h
@@ -134,6 +134,8 @@ struct VncState
     int absolute;
     int last_x;
     int last_y;
+    int client_width;
+    int client_height;
 
     uint32_t vnc_encoding;
 
-- 
1.6.6.1

[Qemu-devel] [PATCH 4/5] vnc: don't send invalid screen updates.

[Gerd Hoffmann]

Don't send updates for screen areas which are outside the clients
desktop.  May happed with vnc clients which don't support the desktop
resize message.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 vnc.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/vnc.c b/vnc.c
index 30e0bed..119ffe8 100644
--- a/vnc.c
+++ b/vnc.c
@@ -836,6 +836,7 @@ static int vnc_update_client(VncState *vs, int has_dirty)
         int y;
         int n_rectangles;
         int saved_offset;
+        int width, height;
 
         if (vs->output.offset && !vs->audio_cap && !vs->force_update)
             /* kernel send buffers are full -> drop frames to throttle */
@@ -856,10 +857,13 @@ static int vnc_update_client(VncState *vs, int has_dirty)
         saved_offset = vs->output.offset;
         vnc_write_u16(vs, 0);
 
-        for (y = 0; y < vd->server->height; y++) {
+        width = MIN(vd->server->width, vs->client_width);
+        height = MIN(vd->server->height, vs->client_height);
+
+        for (y = 0; y < height; y++) {
             int x;
             int last_x = -1;
-            for (x = 0; x < vd->server->width / 16; x++) {
+            for (x = 0; x < width / 16; x++) {
                 if (vnc_get_bit(vs->dirty[y], x)) {
                     if (last_x == -1) {
                         last_x = x;
-- 
1.6.6.1

[Qemu-devel] [PATCH 5/5] vnc: move size-changed check into the vnc_desktop_resize function.

[Gerd Hoffmann]

This make sure we send a desktop resize message only in case we actually
have to, using the new variables which track the clients desktop size.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 vnc.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/vnc.c b/vnc.c
index 119ffe8..5715006 100644
--- a/vnc.c
+++ b/vnc.c
@@ -521,6 +521,10 @@ static void vnc_desktop_resize(VncState *vs)
     if (vs->csock == -1 || !vnc_has_feature(vs, VNC_FEATURE_RESIZE)) {
         return;
     }
+    if (vs->client_width == ds_get_width(ds) &&
+        vs->client_height == ds_get_height(ds)) {
+        return;
+    }
     vs->client_width = ds_get_width(ds);
     vs->client_height = ds_get_height(ds);
     vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
@@ -533,7 +537,6 @@ static void vnc_desktop_resize(VncState *vs)
 
 static void vnc_dpy_resize(DisplayState *ds)
 {
-    int size_changed;
     VncDisplay *vd = ds->opaque;
     VncState *vs;
 
@@ -551,16 +554,12 @@ static void vnc_dpy_resize(DisplayState *ds)
         vd->guest.ds = qemu_mallocz(sizeof(*vd->guest.ds));
     if (ds_get_bytes_per_pixel(ds) != vd->guest.ds->pf.bytes_per_pixel)
         console_color_init(ds);
-    size_changed = ds_get_width(ds) != vd->guest.ds->width ||
-                   ds_get_height(ds) != vd->guest.ds->height;
     *(vd->guest.ds) = *(ds->surface);
     memset(vd->guest.dirty, 0xFF, sizeof(vd->guest.dirty));
 
     QTAILQ_FOREACH(vs, &vd->clients, next) {
         vnc_colordepth(vs);
-        if (size_changed) {
-            vnc_desktop_resize(vs);
-        }
+        vnc_desktop_resize(vs);
         if (vs->vd->cursor) {
             vnc_cursor_define(vs);
         }
-- 
1.6.6.1

Re: [Qemu-devel] [PATCH 0/5] vnc: desktop size patches.

[Daniel P. Berrange]

On Tue, May 25, 2010 at 06:25:15PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> This series brings a bunch of vnc desktop size patches, fixing the
> issues discussed in the "Possible race condition in VNC display
> resizing" thread.  Check list archive here:
> 
> http://lists.gnu.org/archive/html/qemu-devel/2010-04/msg01778.html
> 
> cheers,
>   Gerd
> 
> Gerd Hoffmann (5):
>   vnc: factor out vnc_desktop_resize()
>   vnc: send desktopresize event as reply to set encodings
>   vnc: keep track of client desktop size
>   vnc: don't send invalid screen updates.
>   vnc: move size-changed check into the vnc_desktop_resize function.
> 
>  vnc.c |   50 +++++++++++++++++++++++++++++++++-----------------
>  vnc.h |    2 ++
>  2 files changed, 35 insertions(+), 17 deletions(-)

ACK, these patches look good for solving both of the problems I encountered
(the resize race + the out-of-bounds updates) in that quoted thread.

Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|