Re: [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async

[Anthony Liguori <anthony@codemonkey.ws>]

On 03/05/2012 08:16 AM, Alon Levy wrote:
> adds a handler for the following qmp screendump-async command.
>
> graphics_console_init signature change required touching every user, but
> no implementation of the new vga_hw_screen_dump_async_ptr is provided
> in this patch.
>
> Signed-off-by: Alon Levy<alevy@redhat.com>
> ---
>   console.c            |    4 ++++
>   console.h            |    5 +++++
>   hw/blizzard.c        |    2 +-
>   hw/cirrus_vga.c      |    4 ++--
>   hw/exynos4210_fimd.c |    3 ++-
>   hw/g364fb.c          |    2 +-
>   hw/jazz_led.c        |    3 ++-
>   hw/milkymist-vgafb.c |    2 +-
>   hw/musicpal.c        |    2 +-
>   hw/omap_dss.c        |    4 +++-
>   hw/omap_lcdc.c       |    2 +-
>   hw/pl110.c           |    2 +-
>   hw/pxa2xx_lcd.c      |    2 +-
>   hw/qxl.c             |    3 ++-
>   hw/sm501.c           |    4 ++--
>   hw/ssd0303.c         |    2 +-
>   hw/ssd0323.c         |    2 +-
>   hw/tc6393xb.c        |    1 +
>   hw/tcx.c             |    4 ++--
>   hw/vga-isa-mm.c      |    3 ++-
>   hw/vga-isa.c         |    3 ++-
>   hw/vga-pci.c         |    3 ++-
>   hw/vga_int.h         |    1 +
>   hw/vmware_vga.c      |    3 ++-
>   hw/xenfb.c           |    2 ++
>   25 files changed, 45 insertions(+), 23 deletions(-)
>
> diff --git a/console.c b/console.c
> index 25d8acb..9a49e93 100644
> --- a/console.c
> +++ b/console.c
> @@ -124,6 +124,7 @@ struct TextConsole {
>       vga_hw_update_ptr hw_update;
>       vga_hw_invalidate_ptr hw_invalidate;
>       vga_hw_screen_dump_ptr hw_screen_dump;
> +    vga_hw_screen_dump_async_ptr hw_screen_dump_async;
>       vga_hw_text_update_ptr hw_text_update;
>       void *hw;
>
> @@ -1403,6 +1404,8 @@ DisplayState *graphic_console_init(vga_hw_update_ptr update,
>                                      vga_hw_invalidate_ptr invalidate,
>                                      vga_hw_screen_dump_ptr screen_dump,
>                                      vga_hw_text_update_ptr text_update,
> +                                   vga_hw_screen_dump_async_ptr
> +                                                    screen_dump_async,
>                                      void *opaque)
>   {
>       TextConsole *s;
> @@ -1421,6 +1424,7 @@ DisplayState *graphic_console_init(vga_hw_update_ptr update,
>       s->hw_update = update;
>       s->hw_invalidate = invalidate;
>       s->hw_screen_dump = screen_dump;
> +    s->hw_screen_dump_async = screen_dump_async;
>       s->hw_text_update = text_update;
>       s->hw = opaque;
>
> diff --git a/console.h b/console.h
> index c22803c..3cf28c0 100644
> --- a/console.h
> +++ b/console.h
> @@ -341,17 +341,22 @@ static inline void console_write_ch(console_ch_t *dest, uint32_t ch)
>   typedef void (*vga_hw_update_ptr)(void *);
>   typedef void (*vga_hw_invalidate_ptr)(void *);
>   typedef void (*vga_hw_screen_dump_ptr)(void *, const char *, bool cswitch);
> +typedef void (*vga_hw_screen_dump_async_ptr)(void *, const char *filename,
> +                                             bool cswitch);
>   typedef void (*vga_hw_text_update_ptr)(void *, console_ch_t *);
>
>   DisplayState *graphic_console_init(vga_hw_update_ptr update,
>                                      vga_hw_invalidate_ptr invalidate,
>                                      vga_hw_screen_dump_ptr screen_dump,
>                                      vga_hw_text_update_ptr text_update,
> +                                   vga_hw_screen_dump_async_ptr
> +                                                    screen_dump_async,
>                                      void *opaque);


async in QEMU doesn't mean "generate a QMP event when you're done".  It should 
mean execute this closure when you finish (function pointer + opaque).

The QMP event should be dispatched from the closure such that the screendump 
code doesn't have to have a direct dependency on QMP.

Regards,

Anthony Liguori