Re: [Qemu-devel] [PATCH] make vga screen_dump use DisplayState properly

[Anthony Liguori <anthony@codemonkey.ws>]

Stefano Stabellini wrote:
> Hi all,
> currently the vga screen_dump code doesn't use the DisplayState
> interface properly and tries to replace it temporarily while taking the
> screenshot.
> A better approach is to register a DisplayChangeListener, call
> vga_hw_update, and finally write the ppm in the next call from dpy_update.
>
> Testing is appreciated.
>   

Does this fix kvm-autotest Avi?

> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>
> ---
>
> diff --git a/hw/vga.c b/hw/vga.c
> index 4a0f197..3882f20 100644
> --- a/hw/vga.c
> +++ b/hw/vga.c
> @@ -150,6 +150,8 @@ static uint16_t expand2[256];
>  static uint8_t expand4to8[16];
>  
>  static void vga_screen_dump(void *opaque, const char *filename);
> +static char *screen_dump_filename;
> +static DisplayChangeListener *screen_dump_dcl;
>  
>  static void vga_dumb_update_retrace_info(VGAState *s)
>  {
> @@ -2548,9 +2550,13 @@ device_init(vga_register);
>  /********************************************************/
>  /* vga screen dump */
>  
> -static void vga_save_dpy_update(DisplayState *s,
> +static void vga_save_dpy_update(DisplayState *ds,
>                                  int x, int y, int w, int h)
>  {
> +    if (screen_dump_filename) {
> +        ppm_save(screen_dump_filename, ds->surface);
> +        screen_dump_filename = NULL;
> +    }
>  }
>  
>  static void vga_save_dpy_resize(DisplayState *s)
> @@ -2599,70 +2605,16 @@ int ppm_save(const char *filename, struct DisplaySurface *ds)
>      return 0;
>  }
>  
> -static void vga_screen_dump_blank(VGAState *s, const char *filename)
> -{
> -    FILE *f;
> -    unsigned int y, x, w, h;
> -    unsigned char blank_sample[3] = { 0, 0, 0 };
> -
> -    w = s->last_scr_width;
> -    h = s->last_scr_height;
> -
> -    f = fopen(filename, "wb");
> -    if (!f)
> -        return;
> -    fprintf(f, "P6\n%d %d\n%d\n", w, h, 255);
> -    for (y = 0; y < h; y++) {
> -        for (x = 0; x < w; x++) {
> -            fwrite(blank_sample, 3, 1, f);
> -        }
> -    }
> -    fclose(f);
> -}
> -
> -static void vga_screen_dump_common(VGAState *s, const char *filename,
> -                                   int w, int h)
> -{
> -    DisplayState *saved_ds, ds1, *ds = &ds1;
> -    DisplayChangeListener dcl;
> -
> -    /* XXX: this is a little hackish */
> -    vga_invalidate_display(s);
> -    saved_ds = s->ds;
> -
> -    memset(ds, 0, sizeof(DisplayState));
> -    memset(&dcl, 0, sizeof(DisplayChangeListener));
> -    dcl.dpy_update = vga_save_dpy_update;
> -    dcl.dpy_resize = vga_save_dpy_resize;
> -    dcl.dpy_refresh = vga_save_dpy_refresh;
> -    register_displaychangelistener(ds, &dcl);
> -    ds->allocator = &default_allocator;
> -    ds->surface = qemu_create_displaysurface(ds, w, h);
> -
> -    s->ds = ds;
> -    s->graphic_mode = -1;
> -    vga_update_display(s);
> -
> -    ppm_save(filename, ds->surface);
> -
> -    qemu_free_displaysurface(ds);
> -    s->ds = saved_ds;
> -}
> -
> -static void vga_screen_dump_graphic(VGAState *s, const char *filename)
> +static DisplayChangeListener* vga_screen_dump_init(DisplayState *ds)
>  {
> -    int w, h;
> +    DisplayChangeListener *dcl;
>  
> -    s->get_resolution(s, &w, &h);
> -    vga_screen_dump_common(s, filename, w, h);
> -}
> -
> -static void vga_screen_dump_text(VGAState *s, const char *filename)
> -{
> -    int w, h, cwidth, cheight;
> -
> -    vga_get_text_resolution(s, &w, &h, &cwidth, &cheight);
> -    vga_screen_dump_common(s, filename, w * cwidth, h * cheight);
> +    dcl = qemu_mallocz(sizeof(DisplayChangeListener));
> +    dcl->dpy_update = vga_save_dpy_update;
> +    dcl->dpy_resize = vga_save_dpy_resize;
> +    dcl->dpy_refresh = vga_save_dpy_refresh;
> +    register_displaychangelistener(ds, dcl);
> +    return dcl;
>  }
>  
>  /* save the vga display in a PPM image even if no display is
> @@ -2671,11 +2623,11 @@ static void vga_screen_dump(void *opaque, const char *filename)
>  {
>      VGAState *s = (VGAState *)opaque;
>  
> -    if (!(s->ar_index & 0x20))
> -        vga_screen_dump_blank(s, filename);
> -    else if (s->gr[6] & 1)
> -        vga_screen_dump_graphic(s, filename);
> -    else
> -        vga_screen_dump_text(s, filename);
> +    if (!screen_dump_dcl)
> +        screen_dump_dcl = vga_screen_dump_init(s->ds);
> +
> +    screen_dump_filename = (char *)filename;
>      vga_invalidate_display(s);
> +    vga_hw_update();
>  }
> +
>
>
>