Re: [Qemu-devel] [6989] Fix display breakage when resizing the screen (v2) (Avi Kivity)

[Avi Kivity]

Stefano Stabellini wrote:
> Anthony Liguori wrote:
>
>   
> > Revision: 6989
> >           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6989
> > Author:   aliguori
> > Date:     2009-04-05 18:41:18 +0000 (Sun, 05 Apr 2009)
> > Log Message:
> > -----------
> > Fix display breakage when resizing the screen (v2) (Avi Kivity)
> >
> > When the vga resolution changes, a new display surface is not allocated
> > immediately; instead that is deferred until the next update.  However,
> > if we're running without a display client attached, that won't happen
> > and the next bitblt is likely to cause a segfault by overflowing the
> > display surface.
> >
> > Fix by reallocating the display immediately when the resolution changes.
> >
> > Tested with (Windows|Linux) x (cirrus|std) x (curses|sdl).
> >
> > Changes from v1:
> >  - fix segfault when switching virtual consoles with curses
> >
> > Signed-off-by: Avi Kivity <address@hidden>
> > Signed-off-by: Anthony Liguori <address@hidden>
> >
> >     
>
> While I understand the problem this patch tries to fix I think the way
> it fixes it is wrong.
>
> First of all this patch breaks the VC switching mechanism in qemu: just
> execute sleep 2; startx; in the first VC, then switch to the monitor and
> wait.
>   

I guess, this can be fixed by not reallocating the display surface if 
vga is not the active console?

> Secondly it doesn't follow the basic idea behind the DisplayState
> surface: it is supposed to be a pixel surface provided by the vga
> emulator to the frontends, cirrus shouldn't have to care what size it is.
> In fact cirrus emulates bitblit operations on the emulated framebuffer,
> not on the DisplayState surface; if it does so is a bug and should be fixed.
>   

This happens through cirrus_do_copy(), which implies that the display 
surface is involved.  Is that wrong?


> For these reasons I think this patch should be reverted

Can you suggest a better fix?  The failure (without the patch) is pretty 
disastrous from my point of view.

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