[Qemu-devel] Re: [Qemu-commits] [COMMIT 0bd8246] vga: do not resize the screen on hw_invalidate

[Qemu-devel] Re: [Qemu-commits] [COMMIT 0bd8246] vga: do not resize the screen on hw_invalidate

[Avi Kivity]

On 08/11/2009 12:48 AM, Anthony Liguori wrote:
> From: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
>
> Hi all,
> currently vga always resizes the screen when vga_hw_invalidate is called
> while this is not required and all the other graphic emulators don't.
> This patch fixes it, making vga invalidate behaviour consistent with the
> other emulated devices.
>
>    

This commit causes qemu to segfault in kvm-autotest:

#0  0x000000000043b3f9 in vga_draw_line24_32 (s1=<value optimized out>, 
d=0x7f57c3096000 <Address 0x7f57c3096000 out of bounds>, 
s=0x7f57c31f6077 "",
     width=<value optimized out>) at 
/home/avi/qemu-kvm/hw/vga_template.h:484
#1  0x000000000043ec7f in vga_draw_graphic (full_update=1) at 
/home/avi/qemu-kvm/hw/vga.c:1767
#2  vga_update_display (opaque=<value optimized out>) at 
/home/avi/qemu-kvm/hw/vga.c:1864
#3  0x00000000004cc202 in vnc_refresh (opaque=0x7f57c31f6077) at vnc.c:2120
#4  0x000000000040a552 in qemu_run_timers () at /home/avi/qemu-kvm/vl.c:1030
#5  main_loop_wait (timeout=<value optimized out>) at 
/home/avi/qemu-kvm/vl.c:4121
#6  0x0000000000421faa in kvm_main_loop () at 
/home/avi/qemu-kvm/qemu-kvm.c:2001
#7  0x000000000040d2c6 in main_loop () at /home/avi/qemu-kvm/vl.c:4291
#8  main (argc=21, argv=0x7ffff181f088, envp=<value optimized out>) at 
/home/avi/qemu-kvm/vl.c:6225

kvm-autotest runs the screendump command repeatedly, perhaps this will 
provide a clue as to the cause.  I've reverted it in qemu-kvm.git.

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [Qemu-commits] [COMMIT 0bd8246] vga: do not resize the screen on hw_invalidate

[Avi Kivity]

On 08/11/2009 02:48 PM, Avi Kivity wrote:
> On 08/11/2009 12:48 AM, Anthony Liguori wrote:
>> From: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
>>
>> Hi all,
>> currently vga always resizes the screen when vga_hw_invalidate is called
>> while this is not required and all the other graphic emulators don't.
>> This patch fixes it, making vga invalidate behaviour consistent with the
>> other emulated devices.
>>
>
> This commit causes qemu to segfault in kvm-autotest:
>
> #0  0x000000000043b3f9 in vga_draw_line24_32 (s1=<value optimized 
> out>, d=0x7f57c3096000 <Address 0x7f57c3096000 out of bounds>, 
> s=0x7f57c31f6077 "",
>     width=<value optimized out>) at 
> /home/avi/qemu-kvm/hw/vga_template.h:484
> #1  0x000000000043ec7f in vga_draw_graphic (full_update=1) at 
> /home/avi/qemu-kvm/hw/vga.c:1767
> #2  vga_update_display (opaque=<value optimized out>) at 
> /home/avi/qemu-kvm/hw/vga.c:1864
> #3  0x00000000004cc202 in vnc_refresh (opaque=0x7f57c31f6077) at 
> vnc.c:2120
> #4  0x000000000040a552 in qemu_run_timers () at 
> /home/avi/qemu-kvm/vl.c:1030
> #5  main_loop_wait (timeout=<value optimized out>) at 
> /home/avi/qemu-kvm/vl.c:4121
> #6  0x0000000000421faa in kvm_main_loop () at 
> /home/avi/qemu-kvm/qemu-kvm.c:2001
> #7  0x000000000040d2c6 in main_loop () at /home/avi/qemu-kvm/vl.c:4291
> #8  main (argc=21, argv=0x7ffff181f088, envp=<value optimized out>) at 
> /home/avi/qemu-kvm/vl.c:6225
>
> kvm-autotest runs the screendump command repeatedly, perhaps this will 
> provide a clue as to the cause.  I've reverted it in qemu-kvm.git.
>

btw, the crash occurs immediately after the fedora installer switches to 
graphics mode.

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [Qemu-commits] [COMMIT 0bd8246] vga: do not resize the screen on hw_invalidate

[Stefano Stabellini]

On Tue, 11 Aug 2009, Avi Kivity wrote:
> On 08/11/2009 12:48 AM, Anthony Liguori wrote:
> > From: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
> >
> > Hi all,
> > currently vga always resizes the screen when vga_hw_invalidate is called
> > while this is not required and all the other graphic emulators don't.
> > This patch fixes it, making vga invalidate behaviour consistent with the
> > other emulated devices.
> >
> >    
> 
> This commit causes qemu to segfault in kvm-autotest:
> 
> #0  0x000000000043b3f9 in vga_draw_line24_32 (s1=<value optimized out>, 
> d=0x7f57c3096000 <Address 0x7f57c3096000 out of bounds>, 
> s=0x7f57c31f6077 "",
>      width=<value optimized out>) at 
> /home/avi/qemu-kvm/hw/vga_template.h:484
> #1  0x000000000043ec7f in vga_draw_graphic (full_update=1) at 
> /home/avi/qemu-kvm/hw/vga.c:1767
> #2  vga_update_display (opaque=<value optimized out>) at 
> /home/avi/qemu-kvm/hw/vga.c:1864
> #3  0x00000000004cc202 in vnc_refresh (opaque=0x7f57c31f6077) at vnc.c:2120
> #4  0x000000000040a552 in qemu_run_timers () at /home/avi/qemu-kvm/vl.c:1030
> #5  main_loop_wait (timeout=<value optimized out>) at 
> /home/avi/qemu-kvm/vl.c:4121
> #6  0x0000000000421faa in kvm_main_loop () at 
> /home/avi/qemu-kvm/qemu-kvm.c:2001
> #7  0x000000000040d2c6 in main_loop () at /home/avi/qemu-kvm/vl.c:4291
> #8  main (argc=21, argv=0x7ffff181f088, envp=<value optimized out>) at 
> /home/avi/qemu-kvm/vl.c:6225
> 
> kvm-autotest runs the screendump command repeatedly, perhaps this will 
> provide a clue as to the cause.  I've reverted it in qemu-kvm.git.
> 

is the screendump taken using a vnc screendump utility or is it done by
issuing the screendump command from the monitor?

[Qemu-devel] Re: [Qemu-commits] [COMMIT 0bd8246] vga: do not resize the screen on hw_invalidate

[Avi Kivity]

On 08/11/2009 04:20 PM, Stefano Stabellini wrote:
> is the screendump taken using a vnc screendump utility or is it done by
> issuing the screendump command from the monitor?
>    

The screendump command from the monitor.

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [Qemu-commits] [COMMIT 0bd8246] vga: do not resize the screen on hw_invalidate

[Anthony Liguori]

Avi Kivity wrote:
> On 08/11/2009 04:20 PM, Stefano Stabellini wrote:
>> is the screendump taken using a vnc screendump utility or is it done by
>> issuing the screendump command from the monitor?
>>    
>
> The screendump command from the monitor.

As an aside, could we make screendump just work with the current 
DisplayState instead of trying to install a new one?  That would 
eliminate this entire class of bugs...

-- 
Regards,

Anthony Liguori

[Qemu-devel] Re: [Qemu-commits] [COMMIT 0bd8246] vga: do not resize the screen on hw_invalidate

[Stefano Stabellini]

On Tue, 11 Aug 2009, Avi Kivity wrote:
> On 08/11/2009 04:20 PM, Stefano Stabellini wrote:
> > is the screendump taken using a vnc screendump utility or is it done by
> > issuing the screendump command from the monitor?
> >    
> 
> The screendump command from the monitor.
> 

Then you must also have at least a vnc client always connected to vnc,
right?
Because in the trace you posted before there is a call from vnc_refresh,
and that should happen only when there are vnc clients connected.

[Qemu-devel] Re: [Qemu-commits] [COMMIT 0bd8246] vga: do not resize the screen on hw_invalidate

[Avi Kivity]

On 08/11/2009 04:39 PM, Stefano Stabellini wrote:
> On Tue, 11 Aug 2009, Avi Kivity wrote:
>    
>> On 08/11/2009 04:20 PM, Stefano Stabellini wrote:
>>      
>>> is the screendump taken using a vnc screendump utility or is it done by
>>> issuing the screendump command from the monitor?
>>>
>>>        
>> The screendump command from the monitor.
>>
>>      
>
> Then you must also have at least a vnc client always connected to vnc,
> right?
> Because in the trace you posted before there is a call from vnc_refresh,
> and that should happen only when there are vnc clients connected.
>    

I observed this even without a vnc client connected (kvm-autotest + lunch).

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [Qemu-commits] [COMMIT 0bd8246] vga: do not resize the screen on hw_invalidate

[Stefano Stabellini]

On Tue, 11 Aug 2009, Avi Kivity wrote:
> On 08/11/2009 04:39 PM, Stefano Stabellini wrote:
> > On Tue, 11 Aug 2009, Avi Kivity wrote:
> >    
> >> On 08/11/2009 04:20 PM, Stefano Stabellini wrote:
> >>      
> >>> is the screendump taken using a vnc screendump utility or is it done by
> >>> issuing the screendump command from the monitor?
> >>>
> >>>        
> >> The screendump command from the monitor.
> >>
> >>      
> >
> > Then you must also have at least a vnc client always connected to vnc,
> > right?
> > Because in the trace you posted before there is a call from vnc_refresh,
> > and that should happen only when there are vnc clients connected.
> >    
> 
> I observed this even without a vnc client connected (kvm-autotest + lunch).
> 

If a vnc client is connected and a resize event happens while we are
taking a screendump, we loose the resize event in vnc and we end up with
a DisplaySurface of the wrong size.

The same thing could happen without vnc client connected because we are
not handling resize events in the screen_dump code at all.

I am going to take Anthony suggestion and refactor the screen_dump code
to use the DisplayState interface properly.