On May 17, 2008, at 2:55 PM, Anthony Liguori wrote:
Marcelo Tosatti wrote:
Hi Anthony,
We're experiencing qemu segfaults when using VNC over high latency
links.
(gdb) bt
#0 0x0000003a8ec838d3 in memcpy () from /lib64/libc.so.6
#1 0x00000000004b9aff in vnc_update_client (opaque=0x3514140) at
vnc.c:223
#2 0x000000000040822d in qemu_run_timers (ptimer_head=0x8e9500,
current_time=5942450)
at /root/marcelo/kvm-userspace/qemu/vl.c:1112
#3 0x0000000000413208 in main_loop_wait (timeout=1000)
at /root/marcelo/kvm-userspace/qemu/vl.c:7482
#4 0x000000000060de86 in kvm_main_loop ()
at /root/marcelo/kvm-userspace/qemu/qemu-kvm.c:524
#5 0x0000000000413259 in main_loop () at /root/marcelo/kvm-
userspace/qemu/vl.c:7506
#6 0x0000000000415d3a in main (argc=21, argv=0x7fff00907dd8)
at /root/marcelo/kvm-userspace/qemu/vl.c:9369
Problem is that sometimes vs->width and vs->weight are not updated to
reflect the size allocated for the display memory. If they are larger
than whats allocated it segfaults:
(gdb) p vs->old_data_h
$22 = 400
(gdb) p vs->old_data_w
$23 = 720
(gdb) p vs->old_data_depth
$24 = 4
(gdb) p vs->height
$20 = 480
(gdb) p vs->width
$21 = 640
(gdb) p vs->depth
$25 = 4
old_data_h, old_data_w and old_data_depth have been saved from the
last
vnc_dpy_resize run. The code relies on the client's "set_encondings"
processing to happen _before_ the vnc_update_client() timer triggers,
which might not always be the case.
I have no clue about correctness of the following though. What do you
say?
The patch isn't right because it will unconditionally send
DesktopResize
updates even with clients that don't support it. It happens to
work for
you because your client ignores it and there's no data associated with
DesktopResize. A client really should just stop though.
vs->{width,height} are supposed to correspond to the client's buffer.
In the absence of DesktopResize, the client's buffer stays fixed.
I think what's probably needed is some checks vnc_dpy_update() to not
exceed the dimensions of vs->{width,height}. Something like the
following:
diff --git a/vnc.c b/vnc.c
index 842124b..4b57838 100644
--- a/vnc.c
+++ b/vnc.c
@@ -265,6 +265,11 @@ static void vnc_dpy_update(DisplayState *ds,
int x,
int y,
w += (x % 16);
x -= (x % 16);
+ x = MIN(x, vs->width);
+ y = MIN(y, vs->height);
+ w = MIN(x + w, vs->width) - x;
+ h = MIN(y + h, vs->height) - y;
+
for (; y < h; y++)
for (i = 0; i < w; i += 16)
vnc_set_bit(vs->dirty_row[y], (x + i) / 16);
Regards,
Anthony Liguori
This doesn't work right for me.
Based on my observations the setting of w and h are causing the
display updates to not occur in the lower portion of the VNC screen.
I discovered this because my mouse pointer stopped before hitting the
bottom of the screen which made installation of RedHat EL 5 nearly
impossible to complete. The following patch just limits w and h to
the max width and it works *much* better but still seems like a hack
instead of a real fix. The incorrect x and y values could still be
used by this function to modify w and h.
--- qemu/vnc.c.orig 2008-05-12 04:30:43.000000000 -0700
+++ qemu/vnc.c 2008-05-19 15:55:47.000000000 -0700
@@ -265,6 +265,11 @@ static void vnc_dpy_update(DisplayState
w += (x % 16);
x -= (x % 16);
+ x = MIN(x, vs->width);
+ y = MIN(y, vs->height);
+ w = MIN(w, vs->width);
+ h = MIN(h, vs->height);
+
for (; y < h; y++)
for (i = 0; i < w; i += 16)
vnc_set_bit(vs->dirty_row[y], (x + i) / 16);
Lynn Kerby
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
