Re: [Qemu-devel] qemu vnc.c

[Johannes Schindelin]

Hi,

On Mon, 14 Jan 2008, Anders Melchiorsen wrote:

> diff --git a/qemu/vl.c b/qemu/vl.c
> index 30c9537..8d67314 100644
> --- a/qemu/vl.c
> +++ b/qemu/vl.c
> @@ -4902,13 +4902,6 @@ static void dumb_resize(DisplayState *ds, int w, int h)
>  {
>  }
>  
> -static void dumb_refresh(DisplayState *ds)
> -{
> -#if defined(CONFIG_SDL)
> -    vga_hw_update();
> -#endif
> -}
> -
>  static void dumb_display_init(DisplayState *ds)
>  {
>      ds->data = NULL;
> @@ -4916,7 +4909,7 @@ static void dumb_display_init(DisplayState *ds)
>      ds->depth = 0;
>      ds->dpy_update = dumb_update;
>      ds->dpy_resize = dumb_resize;
> -    ds->dpy_refresh = dumb_refresh;
> +    ds->dpy_refresh = NULL;
>  }
>  
>  /***********************************************************/
> 
> 
> > By your change to dumb_refresh() you are affecting definitely more 
> > than just VNC.  I have doubts that this is intended, or good.
> 
> This patch is not for VNC, but for -nographic. I resent it because it
> is part of my timer optimization crusade as well.
> 
> About the "good" part: I tried stepping through the dumb_refresh() call 
> before removing it, and it ended up doing nothing. Is it needed anyway? 
> (in that case, the VNC patch is also wrong, as it stops the periodic 
> timer when no client is connected).

With stepping, you mean that you took one specific setup, and did not 
inspect what vga_hw_update actually does?

> Also, the CONFIG_SDL thing that can make the function into a noop made 
> me quite confident that nothing globally important was going on with the 
> refresh.

I haven't looked that deeply myself, but your explanations do not give me 
a cozy feeling about your patch.

Ciao,
Dscho