Message ID | 20100305165209.GL29711@arachsys.com |
---|---|
State | New |
Headers | show |
On 05.03.2010, at 17:52, Chris Webb wrote: > Anthony Liguori <anthony@codemonkey.ws> writes: > >> On 03/01/2010 12:14 PM, Chris Webb wrote: >>> We've just seen another VNC related qemu-kvm crash, this time an arithmetic >>> exception at vnc.c:1424 in the newly release qemu-kvm 0.12.3. >>> >>> [...] >>> 1423 if (vs->absolute) { >>> 1424 kbd_mouse_event(x * 0x7FFF / (ds_get_width(vs->ds) - 1), >>> 1425 y * 0x7FFF / (ds_get_height(vs->ds) - 1), >>> 1426 dz, buttons); >>> 1427 } else if (vnc_has_feature(vs, VNC_FEATURE_POINTER_TYPE_CHANGE)) { >>> 1428 x -= 0x7FFF; >>> [...] >>> >>> and sure enough: >>> >>> (gdb) p vs->ds->surface->width >>> $1 = 9 >>> (gdb) p vs->ds->surface->height >>> $2 = 1 >>> >>> What a 9x1 display surface is doing on this guest is a mystery to me, but you >>> definitely can't divide by one less than its height! >> >> Can you reproduce this reliably? If so, what's the procedure? > > No, I'm afraid not, although I have had a thorough play myself with a variety > of VNC clients in an attempt to reproduce. > > The background here is that we're running a public hosting service where > customers can install and run their own OSes on their own qemu-kvm virtual > machines. I don't even know what VNC client (if any) was connected at the > time. I only see the core dump if the qemu-kvm crashes. > > Of course, if the screen width or height is 1, it doesn't really matter what > the value of the mouse position for the click is, so something as simple as > > diff --git a/vnc.c b/vnc.c > --- a/vnc.c > +++ b/vnc.c > @@ -1421,8 +1421,10 @@ > dz = 1; > > if (vs->absolute) { > - kbd_mouse_event(x * 0x7FFF / (ds_get_width(vs->ds) - 1), > - y * 0x7FFF / (ds_get_height(vs->ds) - 1), > + kbd_mouse_event(ds_get_width(vs->ds) > 1 ? > + x * 0x7FFF / (ds_get_width(vs->ds) - 1) : 0x4000, > + ds_get_height(vs->ds) > 1 ? > + y * 0x7FFF / (ds_get_height(vs->ds) - 1) : 0x4000, > dz, buttons); > } else if (vnc_has_feature(vs, VNC_FEATURE_POINTER_TYPE_CHANGE)) { > x -= 0x7FFF; > > will fix the symptom: the division by zero. The underlying cause of a 9x1 > display surface is a bit mysterious though. Is it? When booting the screen gets resized to something like 9x1 for a few ms. Try putting debug code in the resize callback - you'll see it. Alex
Alexander Graf <agraf@suse.de> writes: > On 05.03.2010, at 17:52, Chris Webb wrote: > > > Of course, if the screen width or height is 1, it doesn't really matter what > > the value of the mouse position for the click is, so something as simple as > > > > diff --git a/vnc.c b/vnc.c > > --- a/vnc.c > > +++ b/vnc.c > > @@ -1421,8 +1421,10 @@ > > dz = 1; > > > > if (vs->absolute) { > > - kbd_mouse_event(x * 0x7FFF / (ds_get_width(vs->ds) - 1), > > - y * 0x7FFF / (ds_get_height(vs->ds) - 1), > > + kbd_mouse_event(ds_get_width(vs->ds) > 1 ? > > + x * 0x7FFF / (ds_get_width(vs->ds) - 1) : 0x4000, > > + ds_get_height(vs->ds) > 1 ? > > + y * 0x7FFF / (ds_get_height(vs->ds) - 1) : 0x4000, > > dz, buttons); > > } else if (vnc_has_feature(vs, VNC_FEATURE_POINTER_TYPE_CHANGE)) { > > x -= 0x7FFF; > > > > will fix the symptom: the division by zero. The underlying cause of a 9x1 > > display surface is a bit mysterious though. > > Is it? When booting the screen gets resized to something like 9x1 for a > few ms. Try putting debug code in the resize callback - you'll see it. Ah, okay. In that case, this patch could well be the correct fix rather than just a work-around. I'll have a look for any other places in vnc.c that might do a similar division-by-zero for small screen sizes at the same point. Best wishes, Chris.
diff --git a/vnc.c b/vnc.c --- a/vnc.c +++ b/vnc.c @@ -1421,8 +1421,10 @@ dz = 1; if (vs->absolute) { - kbd_mouse_event(x * 0x7FFF / (ds_get_width(vs->ds) - 1), - y * 0x7FFF / (ds_get_height(vs->ds) - 1), + kbd_mouse_event(ds_get_width(vs->ds) > 1 ? + x * 0x7FFF / (ds_get_width(vs->ds) - 1) : 0x4000, + ds_get_height(vs->ds) > 1 ? + y * 0x7FFF / (ds_get_height(vs->ds) - 1) : 0x4000, dz, buttons); } else if (vnc_has_feature(vs, VNC_FEATURE_POINTER_TYPE_CHANGE)) { x -= 0x7FFF;