| Message ID | 1330118525-14522-4-git-send-email-alevy@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 02/24/2012 03:22 PM, Alon Levy wrote: > This is an across the board change since I wanted to keep the existing > (good imo) single graphic_console_init callback setter, instead of > introducing a new cb that isn't set by it but instead by a second > initialization function. > > Signed-off-by: Alon Levy<alevy@redhat.com> What's the rationale for this? I really don't want a plethora of pseudo-async commands. Regards, Anthony Liguori > --- > console.c | 25 +++++++++++++++++++++++-- > 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 ++ > monitor.c | 5 +++++ > qapi-schema.json | 20 ++++++++++++++++++++ > qmp-commands.hx | 26 ++++++++++++++++++++++++++ > 28 files changed, 115 insertions(+), 25 deletions(-) > > diff --git a/console.c b/console.c > index 6750538..ced2aa7 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; > > @@ -175,8 +176,9 @@ void vga_hw_invalidate(void) > active_console->hw_invalidate(active_console->hw); > } > > -void vga_hw_screen_dump(const char *filename) > +static void vga_hw_screen_dump_helper(const char *filename, bool async) > { > + bool event_sent = false; > TextConsole *previous_active_console; > bool cswitch; > > @@ -188,17 +190,33 @@ void vga_hw_screen_dump(const char *filename) > if (cswitch) { > console_select(0); > } > - if (consoles[0]&& consoles[0]->hw_screen_dump) { > + if (async&& consoles[0]&& consoles[0]->hw_screen_dump_async) { > + consoles[0]->hw_screen_dump_async(consoles[0]->hw, filename, cswitch); > + event_sent = true; > + } else if (consoles[0]&& consoles[0]->hw_screen_dump) { > consoles[0]->hw_screen_dump(consoles[0]->hw, filename, cswitch); > } else { > error_report("screen dump not implemented"); > } > + if (async&& !event_sent) { > + monitor_protocol_screen_dump_complete_event(filename); > + } > > if (cswitch) { > console_select(previous_active_console->index); > } > } > > +void vga_hw_screen_dump(const char *filename) > +{ > + vga_hw_screen_dump_helper(filename, false); > +} > + > +void vga_hw_screen_dump_async(const char *filename) > +{ > + vga_hw_screen_dump_helper(filename, true); > +} > + > void vga_hw_text_update(console_ch_t *chardata) > { > if (active_console&& active_console->hw_text_update) > @@ -1403,6 +1421,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 +1441,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); > > void vga_hw_update(void); > void vga_hw_invalidate(void); > void vga_hw_screen_dump(const char *filename); > +void vga_hw_screen_dump_async(const char *filename); > void vga_hw_text_update(console_ch_t *chardata); > void monitor_protocol_screen_dump_complete_event(const char *filename); > > diff --git a/hw/blizzard.c b/hw/blizzard.c > index c7d844d..cc51045 100644 > --- a/hw/blizzard.c > +++ b/hw/blizzard.c > @@ -963,7 +963,7 @@ void *s1d13745_init(qemu_irq gpio_int) > > s->state = graphic_console_init(blizzard_update_display, > blizzard_invalidate_display, > - blizzard_screen_dump, NULL, s); > + blizzard_screen_dump, NULL, NULL, s); > > switch (ds_get_bits_per_pixel(s->state)) { > case 0: > diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c > index 4edcb94..d2a9c27 100644 > --- a/hw/cirrus_vga.c > +++ b/hw/cirrus_vga.c > @@ -2900,7 +2900,7 @@ static int vga_initfn(ISADevice *dev) > isa_address_space(dev)); > s->ds = graphic_console_init(s->update, s->invalidate, > s->screen_dump, s->text_update, > - s); > + s->screen_dump_async, s); > rom_add_vga(VGABIOS_CIRRUS_FILENAME); > /* XXX ISA-LFB support */ > /* FIXME not qdev yet */ > @@ -2941,7 +2941,7 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev) > cirrus_init_common(s, device_id, 1, pci_address_space(dev)); > s->vga.ds = graphic_console_init(s->vga.update, s->vga.invalidate, > s->vga.screen_dump, s->vga.text_update, > -&s->vga); > + s->vga.screen_dump_async,&s->vga); > > /* setup PCI */ > > diff --git a/hw/exynos4210_fimd.c b/hw/exynos4210_fimd.c > index 3313f00..2053ed0 100644 > --- a/hw/exynos4210_fimd.c > +++ b/hw/exynos4210_fimd.c > @@ -1898,7 +1898,8 @@ static int exynos4210_fimd_init(SysBusDevice *dev) > "exynos4210.fimd", FIMD_REGS_SIZE); > sysbus_init_mmio(dev,&s->iomem); > s->console = graphic_console_init(exynos4210_fimd_update, > - exynos4210_fimd_invalidate, NULL, NULL, s); > + exynos4210_fimd_invalidate, > + NULL, NULL, NULL, s); > > return 0; > } > diff --git a/hw/g364fb.c b/hw/g364fb.c > index 3a0b68f..8ae40d8 100644 > --- a/hw/g364fb.c > +++ b/hw/g364fb.c > @@ -517,7 +517,7 @@ static void g364fb_init(DeviceState *dev, G364State *s) > > s->ds = graphic_console_init(g364fb_update_display, > g364fb_invalidate_display, > - g364fb_screen_dump, NULL, s); > + g364fb_screen_dump, NULL, NULL, s); > > memory_region_init_io(&s->mem_ctrl,&g364fb_ctrl_ops, s, "ctrl", 0x180000); > memory_region_init_ram_ptr(&s->mem_vram, "vram", > diff --git a/hw/jazz_led.c b/hw/jazz_led.c > index 6486523..bb5daf7 100644 > --- a/hw/jazz_led.c > +++ b/hw/jazz_led.c > @@ -251,7 +251,8 @@ static int jazz_led_init(SysBusDevice *dev) > s->ds = graphic_console_init(jazz_led_update_display, > jazz_led_invalidate_display, > NULL, > - jazz_led_text_update, s); > + jazz_led_text_update, > + NULL, s); > > return 0; > } > diff --git a/hw/milkymist-vgafb.c b/hw/milkymist-vgafb.c > index 69afd72..ae018d1 100644 > --- a/hw/milkymist-vgafb.c > +++ b/hw/milkymist-vgafb.c > @@ -276,7 +276,7 @@ static int milkymist_vgafb_init(SysBusDevice *dev) > > s->ds = graphic_console_init(vgafb_update_display, > vgafb_invalidate_display, > - NULL, NULL, s); > + NULL, NULL, NULL, s); > > return 0; > } > diff --git a/hw/musicpal.c b/hw/musicpal.c > index 187a1ae..2fc391b 100644 > --- a/hw/musicpal.c > +++ b/hw/musicpal.c > @@ -611,7 +611,7 @@ static int musicpal_lcd_init(SysBusDevice *dev) > sysbus_init_mmio(dev,&s->iomem); > > s->ds = graphic_console_init(lcd_refresh, lcd_invalidate, > - NULL, NULL, s); > + NULL, NULL, NULL, s); > qemu_console_resize(s->ds, 128*3, 64*3); > > qdev_init_gpio_in(&dev->qdev, musicpal_lcd_gpio_brigthness_in, 3); > diff --git a/hw/omap_dss.c b/hw/omap_dss.c > index 86ed6ea..0e8243e 100644 > --- a/hw/omap_dss.c > +++ b/hw/omap_dss.c > @@ -1072,7 +1072,9 @@ struct omap_dss_s *omap_dss_init(struct omap_target_agent_s *ta, > > #if 0 > s->state = graphic_console_init(omap_update_display, > - omap_invalidate_display, omap_screen_dump, s); > + omap_invalidate_display, > + omap_screen_dump, > + NULL, NULL, s); > #endif > > return s; > diff --git a/hw/omap_lcdc.c b/hw/omap_lcdc.c > index f172093..d91e88d 100644 > --- a/hw/omap_lcdc.c > +++ b/hw/omap_lcdc.c > @@ -452,7 +452,7 @@ struct omap_lcd_panel_s *omap_lcdc_init(MemoryRegion *sysmem, > > s->state = graphic_console_init(omap_update_display, > omap_invalidate_display, > - omap_screen_dump, NULL, s); > + omap_screen_dump, NULL, NULL, s); > > return s; > } > diff --git a/hw/pl110.c b/hw/pl110.c > index f94608c..32ee89c 100644 > --- a/hw/pl110.c > +++ b/hw/pl110.c > @@ -451,7 +451,7 @@ static int pl110_init(SysBusDevice *dev) > qdev_init_gpio_in(&s->busdev.qdev, pl110_mux_ctrl_set, 1); > s->ds = graphic_console_init(pl110_update_display, > pl110_invalidate_display, > - NULL, NULL, s); > + NULL, NULL, NULL, s); > return 0; > } > > diff --git a/hw/pxa2xx_lcd.c b/hw/pxa2xx_lcd.c > index fcbdfb3..5345fb4 100644 > --- a/hw/pxa2xx_lcd.c > +++ b/hw/pxa2xx_lcd.c > @@ -1004,7 +1004,7 @@ PXA2xxLCDState *pxa2xx_lcdc_init(MemoryRegion *sysmem, > > s->ds = graphic_console_init(pxa2xx_update_display, > pxa2xx_invalidate_display, > - NULL, NULL, s); > + NULL, NULL, NULL, s); > > switch (ds_get_bits_per_pixel(s->ds)) { > case 0: > diff --git a/hw/qxl.c b/hw/qxl.c > index d83d245..61a7edd 100644 > --- a/hw/qxl.c > +++ b/hw/qxl.c > @@ -1727,7 +1727,8 @@ static int qxl_init_primary(PCIDevice *dev) > portio_list_add(qxl_vga_port_list, pci_address_space_io(dev), 0x3b0); > > vga->ds = graphic_console_init(qxl_hw_update, qxl_hw_invalidate, > - qxl_hw_screen_dump, qxl_hw_text_update, qxl); > + qxl_hw_screen_dump, qxl_hw_text_update, > + NULL, qxl); > qemu_spice_display_init_common(&qxl->ssd, vga->ds); > > qxl0 = qxl; > diff --git a/hw/sm501.c b/hw/sm501.c > index 786e076..8b150be 100644 > --- a/hw/sm501.c > +++ b/hw/sm501.c > @@ -1442,6 +1442,6 @@ void sm501_init(MemoryRegion *address_space_mem, uint32_t base, > } > > /* create qemu graphic console */ > - s->ds = graphic_console_init(sm501_update_display, NULL, > - NULL, NULL, s); > + s->ds = graphic_console_init(sm501_update_display, > + NULL, NULL, NULL, NULL, s); > } > diff --git a/hw/ssd0303.c b/hw/ssd0303.c > index 4e1ee6e..05e6686 100644 > --- a/hw/ssd0303.c > +++ b/hw/ssd0303.c > @@ -289,7 +289,7 @@ static int ssd0303_init(I2CSlave *i2c) > > s->ds = graphic_console_init(ssd0303_update_display, > ssd0303_invalidate_display, > - NULL, NULL, s); > + NULL, NULL, NULL, s); > qemu_console_resize(s->ds, 96 * MAGNIFY, 16 * MAGNIFY); > return 0; > } > diff --git a/hw/ssd0323.c b/hw/ssd0323.c > index b0b2e94..5e3491f 100644 > --- a/hw/ssd0323.c > +++ b/hw/ssd0323.c > @@ -330,7 +330,7 @@ static int ssd0323_init(SSISlave *dev) > s->row_end = 79; > s->ds = graphic_console_init(ssd0323_update_display, > ssd0323_invalidate_display, > - NULL, NULL, s); > + NULL, NULL, NULL, s); > qemu_console_resize(s->ds, 128 * MAGNIFY, 64 * MAGNIFY); > > qdev_init_gpio_in(&dev->qdev, ssd0323_cd, 1); > diff --git a/hw/tc6393xb.c b/hw/tc6393xb.c > index 420925c..853b180 100644 > --- a/hw/tc6393xb.c > +++ b/hw/tc6393xb.c > @@ -581,6 +581,7 @@ TC6393xbState *tc6393xb_init(MemoryRegion *sysmem, uint32_t base, qemu_irq irq) > NULL, /* invalidate */ > NULL, /* screen_dump */ > NULL, /* text_update */ > + NULL, /* screen_dump_async */ > s); > > return s; > diff --git a/hw/tcx.c b/hw/tcx.c > index ac7dcb4..9214dec 100644 > --- a/hw/tcx.c > +++ b/hw/tcx.c > @@ -558,7 +558,7 @@ static int tcx_init1(SysBusDevice *dev) > > s->ds = graphic_console_init(tcx24_update_display, > tcx24_invalidate_display, > - tcx24_screen_dump, NULL, s); > + tcx24_screen_dump, NULL, NULL, s); > } else { > /* THC 8 bit (dummy) */ > memory_region_init_io(&s->thc8,&dummy_ops, s, "tcx.thc8", > @@ -567,7 +567,7 @@ static int tcx_init1(SysBusDevice *dev) > > s->ds = graphic_console_init(tcx_update_display, > tcx_invalidate_display, > - tcx_screen_dump, NULL, s); > + tcx_screen_dump, NULL, NULL, s); > } > > qemu_console_resize(s->ds, s->width, s->height); > diff --git a/hw/vga-isa-mm.c b/hw/vga-isa-mm.c > index f8984c6..87b8fc6 100644 > --- a/hw/vga-isa-mm.c > +++ b/hw/vga-isa-mm.c > @@ -132,7 +132,8 @@ int isa_vga_mm_init(target_phys_addr_t vram_base, > vga_mm_init(s, vram_base, ctrl_base, it_shift, address_space); > > s->vga.ds = graphic_console_init(s->vga.update, s->vga.invalidate, > - s->vga.screen_dump, s->vga.text_update, s); > + s->vga.screen_dump, s->vga.text_update, > + s->vga.screen_dump_async, s); > > vga_init_vbe(&s->vga, address_space); > return 0; > diff --git a/hw/vga-isa.c b/hw/vga-isa.c > index 4bcc4db..a362ed2 100644 > --- a/hw/vga-isa.c > +++ b/hw/vga-isa.c > @@ -61,7 +61,8 @@ static int vga_initfn(ISADevice *dev) > vga_io_memory, 1); > memory_region_set_coalescing(vga_io_memory); > s->ds = graphic_console_init(s->update, s->invalidate, > - s->screen_dump, s->text_update, s); > + s->screen_dump, s->text_update, > + s->screen_dump_async, s); > > vga_init_vbe(s, isa_address_space(dev)); > /* ROM BIOS */ > diff --git a/hw/vga-pci.c b/hw/vga-pci.c > index 465b643..8c8dd53 100644 > --- a/hw/vga-pci.c > +++ b/hw/vga-pci.c > @@ -57,7 +57,8 @@ static int pci_vga_initfn(PCIDevice *dev) > vga_init(s, pci_address_space(dev), pci_address_space_io(dev), true); > > s->ds = graphic_console_init(s->update, s->invalidate, > - s->screen_dump, s->text_update, s); > + s->screen_dump, s->text_update, > + s->screen_dump_async, s); > > /* XXX: VGA_RAM_SIZE must be a power of two */ > pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_MEM_PREFETCH,&s->vram); > diff --git a/hw/vga_int.h b/hw/vga_int.h > index 7685b2b..4486cfb 100644 > --- a/hw/vga_int.h > +++ b/hw/vga_int.h > @@ -161,6 +161,7 @@ typedef struct VGACommonState { > 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; > /* hardware mouse cursor support */ > uint32_t invalidated_y_table[VGA_MAX_HEIGHT / 32]; > void (*cursor_invalidate)(struct VGACommonState *s); > diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c > index 142d9f4..db96e61 100644 > --- a/hw/vmware_vga.c > +++ b/hw/vmware_vga.c > @@ -1087,7 +1087,8 @@ static void vmsvga_init(struct vmsvga_state_s *s, int vga_ram_size, > s->vga.ds = graphic_console_init(vmsvga_update_display, > vmsvga_invalidate_display, > vmsvga_screen_dump, > - vmsvga_text_update, s); > + vmsvga_text_update, > + NULL, s); > > > s->fifo_size = SVGA_FIFO_SIZE; > diff --git a/hw/xenfb.c b/hw/xenfb.c > index 1bcf171..ad9f0d7 100644 > --- a/hw/xenfb.c > +++ b/hw/xenfb.c > @@ -906,6 +906,7 @@ static int fb_initialise(struct XenDevice *xendev) > xenfb_invalidate, > NULL, > NULL, > + NULL, > fb); > fb->have_console = 1; > } > @@ -1015,6 +1016,7 @@ wait_more: > xenfb_invalidate, > NULL, > NULL, > + NULL, > fb); > fb->have_console = 1; > > diff --git a/monitor.c b/monitor.c > index 1a65c41..91a76a1 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -901,6 +901,11 @@ static int do_screen_dump(Monitor *mon, const QDict *qdict, QObject **ret_data) > return 0; > } > > +void qmp_screendump_async(const char *filename, Error **errp) > +{ > + vga_hw_screen_dump_async(filename); > +} > + > static void do_logfile(Monitor *mon, const QDict *qdict) > { > cpu_set_log_filename(qdict_get_str(qdict, "filename")); > diff --git a/qapi-schema.json b/qapi-schema.json > index 72b17f1..a5873cb 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -1595,3 +1595,23 @@ > { 'command': 'qom-list-types', > 'data': { '*implements': 'str', '*abstract': 'bool' }, > 'returns': [ 'ObjectTypeInfo' ] } > + > +## > +## @screendump-async: > +# > +# This command will perform a screen dump of the first console to the givem > +# filename. The additional parameters are unused at this time. > +# > +# @filename name of output file to write screen dump to > +# > +# Since: 1.1 > +# > +# Notes: This command is experimental and may change syntax in future releases. > +# > +# This command is the same as the qmp/hmp screendump command, except that on > +# successful completion of the scren dump the SCREEN_DUMP_COMPLETE event is > +# emitted. > +# > +## > +{ 'command': 'screendump-async', > + 'data': { 'filename': 'str' } } > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 705f704..ea0d051 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -170,6 +170,32 @@ Example: > EQMP > > { > + .name = "screendump-async", > + .args_type = "filename:F", > + .params = "filename", > + .help = "save screen into PPM image 'filename'", > + .user_print = monitor_user_noop, > + .mhandler.cmd_new = qmp_marshal_input_screendump_async, > + }, > + > +SQMP > +screendump-async > +---------------- > + > +Save screen into PPM image. Fires a SCREEN_DUMP_COMPLETE event on completion. > + > +Arguments: > + > +- "filename": file path (json-string) > + > +Example: > + > +-> { "execute": "screendump-async", "arguments": { "filename": "/tmp/image" } } > +<- { "return": {} } > + > +EQMP > + > + { > .name = "stop", > .args_type = "", > .mhandler.cmd_new = qmp_marshal_input_stop,
On Fri, Feb 24, 2012 at 04:40:15PM -0600, Anthony Liguori wrote: > On 02/24/2012 03:22 PM, Alon Levy wrote: > >This is an across the board change since I wanted to keep the existing > >(good imo) single graphic_console_init callback setter, instead of > >introducing a new cb that isn't set by it but instead by a second > >initialization function. > > > >Signed-off-by: Alon Levy<alevy@redhat.com> > > What's the rationale for this? There is a hang possible with the current screendump command, qxl, a spice client using libvirt and spice-gtk such as virt-viewer / remote-viewer, where you have: 1. libvirt waiting for screendump to complete 2. screendump waiting for spice server thread to render 3. spice server thread waiting for spice client to read messages 4. spice client == libvirt client, waiting for screendump completion The solution to this is to have 2 become asyncronous. See http://patchwork.ozlabs.org/patch/142985/ The result of 2 becoming async is that now screendump completes before the framebuffer has been updated, resulting in a screendump of an older state. To really fix it I need to have the screendump (ppm_save) done with the newer state, which is what http://patchwork.ozlabs.org/patch/142977/ does. But for the user (libvirt/autotest) to know when the file is ready there now needs to be some event. That's what this series does. > > I really don't want a plethora of pseudo-async commands. > > Regards, > > Anthony Liguori > > >--- > > console.c | 25 +++++++++++++++++++++++-- > > 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 ++ > > monitor.c | 5 +++++ > > qapi-schema.json | 20 ++++++++++++++++++++ > > qmp-commands.hx | 26 ++++++++++++++++++++++++++ > > 28 files changed, 115 insertions(+), 25 deletions(-) > > > >diff --git a/console.c b/console.c > >index 6750538..ced2aa7 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; > > > >@@ -175,8 +176,9 @@ void vga_hw_invalidate(void) > > active_console->hw_invalidate(active_console->hw); > > } > > > >-void vga_hw_screen_dump(const char *filename) > >+static void vga_hw_screen_dump_helper(const char *filename, bool async) > > { > >+ bool event_sent = false; > > TextConsole *previous_active_console; > > bool cswitch; > > > >@@ -188,17 +190,33 @@ void vga_hw_screen_dump(const char *filename) > > if (cswitch) { > > console_select(0); > > } > >- if (consoles[0]&& consoles[0]->hw_screen_dump) { > >+ if (async&& consoles[0]&& consoles[0]->hw_screen_dump_async) { > >+ consoles[0]->hw_screen_dump_async(consoles[0]->hw, filename, cswitch); > >+ event_sent = true; > >+ } else if (consoles[0]&& consoles[0]->hw_screen_dump) { > > consoles[0]->hw_screen_dump(consoles[0]->hw, filename, cswitch); > > } else { > > error_report("screen dump not implemented"); > > } > >+ if (async&& !event_sent) { > >+ monitor_protocol_screen_dump_complete_event(filename); > >+ } > > > > if (cswitch) { > > console_select(previous_active_console->index); > > } > > } > > > >+void vga_hw_screen_dump(const char *filename) > >+{ > >+ vga_hw_screen_dump_helper(filename, false); > >+} > >+ > >+void vga_hw_screen_dump_async(const char *filename) > >+{ > >+ vga_hw_screen_dump_helper(filename, true); > >+} > >+ > > void vga_hw_text_update(console_ch_t *chardata) > > { > > if (active_console&& active_console->hw_text_update) > >@@ -1403,6 +1421,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 +1441,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); > > > > void vga_hw_update(void); > > void vga_hw_invalidate(void); > > void vga_hw_screen_dump(const char *filename); > >+void vga_hw_screen_dump_async(const char *filename); > > void vga_hw_text_update(console_ch_t *chardata); > > void monitor_protocol_screen_dump_complete_event(const char *filename); > > > >diff --git a/hw/blizzard.c b/hw/blizzard.c > >index c7d844d..cc51045 100644 > >--- a/hw/blizzard.c > >+++ b/hw/blizzard.c > >@@ -963,7 +963,7 @@ void *s1d13745_init(qemu_irq gpio_int) > > > > s->state = graphic_console_init(blizzard_update_display, > > blizzard_invalidate_display, > >- blizzard_screen_dump, NULL, s); > >+ blizzard_screen_dump, NULL, NULL, s); > > > > switch (ds_get_bits_per_pixel(s->state)) { > > case 0: > >diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c > >index 4edcb94..d2a9c27 100644 > >--- a/hw/cirrus_vga.c > >+++ b/hw/cirrus_vga.c > >@@ -2900,7 +2900,7 @@ static int vga_initfn(ISADevice *dev) > > isa_address_space(dev)); > > s->ds = graphic_console_init(s->update, s->invalidate, > > s->screen_dump, s->text_update, > >- s); > >+ s->screen_dump_async, s); > > rom_add_vga(VGABIOS_CIRRUS_FILENAME); > > /* XXX ISA-LFB support */ > > /* FIXME not qdev yet */ > >@@ -2941,7 +2941,7 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev) > > cirrus_init_common(s, device_id, 1, pci_address_space(dev)); > > s->vga.ds = graphic_console_init(s->vga.update, s->vga.invalidate, > > s->vga.screen_dump, s->vga.text_update, > >-&s->vga); > >+ s->vga.screen_dump_async,&s->vga); > > > > /* setup PCI */ > > > >diff --git a/hw/exynos4210_fimd.c b/hw/exynos4210_fimd.c > >index 3313f00..2053ed0 100644 > >--- a/hw/exynos4210_fimd.c > >+++ b/hw/exynos4210_fimd.c > >@@ -1898,7 +1898,8 @@ static int exynos4210_fimd_init(SysBusDevice *dev) > > "exynos4210.fimd", FIMD_REGS_SIZE); > > sysbus_init_mmio(dev,&s->iomem); > > s->console = graphic_console_init(exynos4210_fimd_update, > >- exynos4210_fimd_invalidate, NULL, NULL, s); > >+ exynos4210_fimd_invalidate, > >+ NULL, NULL, NULL, s); > > > > return 0; > > } > >diff --git a/hw/g364fb.c b/hw/g364fb.c > >index 3a0b68f..8ae40d8 100644 > >--- a/hw/g364fb.c > >+++ b/hw/g364fb.c > >@@ -517,7 +517,7 @@ static void g364fb_init(DeviceState *dev, G364State *s) > > > > s->ds = graphic_console_init(g364fb_update_display, > > g364fb_invalidate_display, > >- g364fb_screen_dump, NULL, s); > >+ g364fb_screen_dump, NULL, NULL, s); > > > > memory_region_init_io(&s->mem_ctrl,&g364fb_ctrl_ops, s, "ctrl", 0x180000); > > memory_region_init_ram_ptr(&s->mem_vram, "vram", > >diff --git a/hw/jazz_led.c b/hw/jazz_led.c > >index 6486523..bb5daf7 100644 > >--- a/hw/jazz_led.c > >+++ b/hw/jazz_led.c > >@@ -251,7 +251,8 @@ static int jazz_led_init(SysBusDevice *dev) > > s->ds = graphic_console_init(jazz_led_update_display, > > jazz_led_invalidate_display, > > NULL, > >- jazz_led_text_update, s); > >+ jazz_led_text_update, > >+ NULL, s); > > > > return 0; > > } > >diff --git a/hw/milkymist-vgafb.c b/hw/milkymist-vgafb.c > >index 69afd72..ae018d1 100644 > >--- a/hw/milkymist-vgafb.c > >+++ b/hw/milkymist-vgafb.c > >@@ -276,7 +276,7 @@ static int milkymist_vgafb_init(SysBusDevice *dev) > > > > s->ds = graphic_console_init(vgafb_update_display, > > vgafb_invalidate_display, > >- NULL, NULL, s); > >+ NULL, NULL, NULL, s); > > > > return 0; > > } > >diff --git a/hw/musicpal.c b/hw/musicpal.c > >index 187a1ae..2fc391b 100644 > >--- a/hw/musicpal.c > >+++ b/hw/musicpal.c > >@@ -611,7 +611,7 @@ static int musicpal_lcd_init(SysBusDevice *dev) > > sysbus_init_mmio(dev,&s->iomem); > > > > s->ds = graphic_console_init(lcd_refresh, lcd_invalidate, > >- NULL, NULL, s); > >+ NULL, NULL, NULL, s); > > qemu_console_resize(s->ds, 128*3, 64*3); > > > > qdev_init_gpio_in(&dev->qdev, musicpal_lcd_gpio_brigthness_in, 3); > >diff --git a/hw/omap_dss.c b/hw/omap_dss.c > >index 86ed6ea..0e8243e 100644 > >--- a/hw/omap_dss.c > >+++ b/hw/omap_dss.c > >@@ -1072,7 +1072,9 @@ struct omap_dss_s *omap_dss_init(struct omap_target_agent_s *ta, > > > > #if 0 > > s->state = graphic_console_init(omap_update_display, > >- omap_invalidate_display, omap_screen_dump, s); > >+ omap_invalidate_display, > >+ omap_screen_dump, > >+ NULL, NULL, s); > > #endif > > > > return s; > >diff --git a/hw/omap_lcdc.c b/hw/omap_lcdc.c > >index f172093..d91e88d 100644 > >--- a/hw/omap_lcdc.c > >+++ b/hw/omap_lcdc.c > >@@ -452,7 +452,7 @@ struct omap_lcd_panel_s *omap_lcdc_init(MemoryRegion *sysmem, > > > > s->state = graphic_console_init(omap_update_display, > > omap_invalidate_display, > >- omap_screen_dump, NULL, s); > >+ omap_screen_dump, NULL, NULL, s); > > > > return s; > > } > >diff --git a/hw/pl110.c b/hw/pl110.c > >index f94608c..32ee89c 100644 > >--- a/hw/pl110.c > >+++ b/hw/pl110.c > >@@ -451,7 +451,7 @@ static int pl110_init(SysBusDevice *dev) > > qdev_init_gpio_in(&s->busdev.qdev, pl110_mux_ctrl_set, 1); > > s->ds = graphic_console_init(pl110_update_display, > > pl110_invalidate_display, > >- NULL, NULL, s); > >+ NULL, NULL, NULL, s); > > return 0; > > } > > > >diff --git a/hw/pxa2xx_lcd.c b/hw/pxa2xx_lcd.c > >index fcbdfb3..5345fb4 100644 > >--- a/hw/pxa2xx_lcd.c > >+++ b/hw/pxa2xx_lcd.c > >@@ -1004,7 +1004,7 @@ PXA2xxLCDState *pxa2xx_lcdc_init(MemoryRegion *sysmem, > > > > s->ds = graphic_console_init(pxa2xx_update_display, > > pxa2xx_invalidate_display, > >- NULL, NULL, s); > >+ NULL, NULL, NULL, s); > > > > switch (ds_get_bits_per_pixel(s->ds)) { > > case 0: > >diff --git a/hw/qxl.c b/hw/qxl.c > >index d83d245..61a7edd 100644 > >--- a/hw/qxl.c > >+++ b/hw/qxl.c > >@@ -1727,7 +1727,8 @@ static int qxl_init_primary(PCIDevice *dev) > > portio_list_add(qxl_vga_port_list, pci_address_space_io(dev), 0x3b0); > > > > vga->ds = graphic_console_init(qxl_hw_update, qxl_hw_invalidate, > >- qxl_hw_screen_dump, qxl_hw_text_update, qxl); > >+ qxl_hw_screen_dump, qxl_hw_text_update, > >+ NULL, qxl); > > qemu_spice_display_init_common(&qxl->ssd, vga->ds); > > > > qxl0 = qxl; > >diff --git a/hw/sm501.c b/hw/sm501.c > >index 786e076..8b150be 100644 > >--- a/hw/sm501.c > >+++ b/hw/sm501.c > >@@ -1442,6 +1442,6 @@ void sm501_init(MemoryRegion *address_space_mem, uint32_t base, > > } > > > > /* create qemu graphic console */ > >- s->ds = graphic_console_init(sm501_update_display, NULL, > >- NULL, NULL, s); > >+ s->ds = graphic_console_init(sm501_update_display, > >+ NULL, NULL, NULL, NULL, s); > > } > >diff --git a/hw/ssd0303.c b/hw/ssd0303.c > >index 4e1ee6e..05e6686 100644 > >--- a/hw/ssd0303.c > >+++ b/hw/ssd0303.c > >@@ -289,7 +289,7 @@ static int ssd0303_init(I2CSlave *i2c) > > > > s->ds = graphic_console_init(ssd0303_update_display, > > ssd0303_invalidate_display, > >- NULL, NULL, s); > >+ NULL, NULL, NULL, s); > > qemu_console_resize(s->ds, 96 * MAGNIFY, 16 * MAGNIFY); > > return 0; > > } > >diff --git a/hw/ssd0323.c b/hw/ssd0323.c > >index b0b2e94..5e3491f 100644 > >--- a/hw/ssd0323.c > >+++ b/hw/ssd0323.c > >@@ -330,7 +330,7 @@ static int ssd0323_init(SSISlave *dev) > > s->row_end = 79; > > s->ds = graphic_console_init(ssd0323_update_display, > > ssd0323_invalidate_display, > >- NULL, NULL, s); > >+ NULL, NULL, NULL, s); > > qemu_console_resize(s->ds, 128 * MAGNIFY, 64 * MAGNIFY); > > > > qdev_init_gpio_in(&dev->qdev, ssd0323_cd, 1); > >diff --git a/hw/tc6393xb.c b/hw/tc6393xb.c > >index 420925c..853b180 100644 > >--- a/hw/tc6393xb.c > >+++ b/hw/tc6393xb.c > >@@ -581,6 +581,7 @@ TC6393xbState *tc6393xb_init(MemoryRegion *sysmem, uint32_t base, qemu_irq irq) > > NULL, /* invalidate */ > > NULL, /* screen_dump */ > > NULL, /* text_update */ > >+ NULL, /* screen_dump_async */ > > s); > > > > return s; > >diff --git a/hw/tcx.c b/hw/tcx.c > >index ac7dcb4..9214dec 100644 > >--- a/hw/tcx.c > >+++ b/hw/tcx.c > >@@ -558,7 +558,7 @@ static int tcx_init1(SysBusDevice *dev) > > > > s->ds = graphic_console_init(tcx24_update_display, > > tcx24_invalidate_display, > >- tcx24_screen_dump, NULL, s); > >+ tcx24_screen_dump, NULL, NULL, s); > > } else { > > /* THC 8 bit (dummy) */ > > memory_region_init_io(&s->thc8,&dummy_ops, s, "tcx.thc8", > >@@ -567,7 +567,7 @@ static int tcx_init1(SysBusDevice *dev) > > > > s->ds = graphic_console_init(tcx_update_display, > > tcx_invalidate_display, > >- tcx_screen_dump, NULL, s); > >+ tcx_screen_dump, NULL, NULL, s); > > } > > > > qemu_console_resize(s->ds, s->width, s->height); > >diff --git a/hw/vga-isa-mm.c b/hw/vga-isa-mm.c > >index f8984c6..87b8fc6 100644 > >--- a/hw/vga-isa-mm.c > >+++ b/hw/vga-isa-mm.c > >@@ -132,7 +132,8 @@ int isa_vga_mm_init(target_phys_addr_t vram_base, > > vga_mm_init(s, vram_base, ctrl_base, it_shift, address_space); > > > > s->vga.ds = graphic_console_init(s->vga.update, s->vga.invalidate, > >- s->vga.screen_dump, s->vga.text_update, s); > >+ s->vga.screen_dump, s->vga.text_update, > >+ s->vga.screen_dump_async, s); > > > > vga_init_vbe(&s->vga, address_space); > > return 0; > >diff --git a/hw/vga-isa.c b/hw/vga-isa.c > >index 4bcc4db..a362ed2 100644 > >--- a/hw/vga-isa.c > >+++ b/hw/vga-isa.c > >@@ -61,7 +61,8 @@ static int vga_initfn(ISADevice *dev) > > vga_io_memory, 1); > > memory_region_set_coalescing(vga_io_memory); > > s->ds = graphic_console_init(s->update, s->invalidate, > >- s->screen_dump, s->text_update, s); > >+ s->screen_dump, s->text_update, > >+ s->screen_dump_async, s); > > > > vga_init_vbe(s, isa_address_space(dev)); > > /* ROM BIOS */ > >diff --git a/hw/vga-pci.c b/hw/vga-pci.c > >index 465b643..8c8dd53 100644 > >--- a/hw/vga-pci.c > >+++ b/hw/vga-pci.c > >@@ -57,7 +57,8 @@ static int pci_vga_initfn(PCIDevice *dev) > > vga_init(s, pci_address_space(dev), pci_address_space_io(dev), true); > > > > s->ds = graphic_console_init(s->update, s->invalidate, > >- s->screen_dump, s->text_update, s); > >+ s->screen_dump, s->text_update, > >+ s->screen_dump_async, s); > > > > /* XXX: VGA_RAM_SIZE must be a power of two */ > > pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_MEM_PREFETCH,&s->vram); > >diff --git a/hw/vga_int.h b/hw/vga_int.h > >index 7685b2b..4486cfb 100644 > >--- a/hw/vga_int.h > >+++ b/hw/vga_int.h > >@@ -161,6 +161,7 @@ typedef struct VGACommonState { > > 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; > > /* hardware mouse cursor support */ > > uint32_t invalidated_y_table[VGA_MAX_HEIGHT / 32]; > > void (*cursor_invalidate)(struct VGACommonState *s); > >diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c > >index 142d9f4..db96e61 100644 > >--- a/hw/vmware_vga.c > >+++ b/hw/vmware_vga.c > >@@ -1087,7 +1087,8 @@ static void vmsvga_init(struct vmsvga_state_s *s, int vga_ram_size, > > s->vga.ds = graphic_console_init(vmsvga_update_display, > > vmsvga_invalidate_display, > > vmsvga_screen_dump, > >- vmsvga_text_update, s); > >+ vmsvga_text_update, > >+ NULL, s); > > > > > > s->fifo_size = SVGA_FIFO_SIZE; > >diff --git a/hw/xenfb.c b/hw/xenfb.c > >index 1bcf171..ad9f0d7 100644 > >--- a/hw/xenfb.c > >+++ b/hw/xenfb.c > >@@ -906,6 +906,7 @@ static int fb_initialise(struct XenDevice *xendev) > > xenfb_invalidate, > > NULL, > > NULL, > >+ NULL, > > fb); > > fb->have_console = 1; > > } > >@@ -1015,6 +1016,7 @@ wait_more: > > xenfb_invalidate, > > NULL, > > NULL, > >+ NULL, > > fb); > > fb->have_console = 1; > > > >diff --git a/monitor.c b/monitor.c > >index 1a65c41..91a76a1 100644 > >--- a/monitor.c > >+++ b/monitor.c > >@@ -901,6 +901,11 @@ static int do_screen_dump(Monitor *mon, const QDict *qdict, QObject **ret_data) > > return 0; > > } > > > >+void qmp_screendump_async(const char *filename, Error **errp) > >+{ > >+ vga_hw_screen_dump_async(filename); > >+} > >+ > > static void do_logfile(Monitor *mon, const QDict *qdict) > > { > > cpu_set_log_filename(qdict_get_str(qdict, "filename")); > >diff --git a/qapi-schema.json b/qapi-schema.json > >index 72b17f1..a5873cb 100644 > >--- a/qapi-schema.json > >+++ b/qapi-schema.json > >@@ -1595,3 +1595,23 @@ > > { 'command': 'qom-list-types', > > 'data': { '*implements': 'str', '*abstract': 'bool' }, > > 'returns': [ 'ObjectTypeInfo' ] } > >+ > >+## > >+## @screendump-async: > >+# > >+# This command will perform a screen dump of the first console to the givem > >+# filename. The additional parameters are unused at this time. > >+# > >+# @filename name of output file to write screen dump to > >+# > >+# Since: 1.1 > >+# > >+# Notes: This command is experimental and may change syntax in future releases. > >+# > >+# This command is the same as the qmp/hmp screendump command, except that on > >+# successful completion of the scren dump the SCREEN_DUMP_COMPLETE event is > >+# emitted. > >+# > >+## > >+{ 'command': 'screendump-async', > >+ 'data': { 'filename': 'str' } } > >diff --git a/qmp-commands.hx b/qmp-commands.hx > >index 705f704..ea0d051 100644 > >--- a/qmp-commands.hx > >+++ b/qmp-commands.hx > >@@ -170,6 +170,32 @@ Example: > > EQMP > > > > { > >+ .name = "screendump-async", > >+ .args_type = "filename:F", > >+ .params = "filename", > >+ .help = "save screen into PPM image 'filename'", > >+ .user_print = monitor_user_noop, > >+ .mhandler.cmd_new = qmp_marshal_input_screendump_async, > >+ }, > >+ > >+SQMP > >+screendump-async > >+---------------- > >+ > >+Save screen into PPM image. Fires a SCREEN_DUMP_COMPLETE event on completion. > >+ > >+Arguments: > >+ > >+- "filename": file path (json-string) > >+ > >+Example: > >+ > >+-> { "execute": "screendump-async", "arguments": { "filename": "/tmp/image" } } > >+<- { "return": {} } > >+ > >+EQMP > >+ > >+ { > > .name = "stop", > > .args_type = "", > > .mhandler.cmd_new = qmp_marshal_input_stop, > >
On Fri, 24 Feb 2012 23:22:05 +0200 Alon Levy <alevy@redhat.com> wrote: > This is an across the board change since I wanted to keep the existing > (good imo) single graphic_console_init callback setter, instead of > introducing a new cb that isn't set by it but instead by a second > initialization function. > > Signed-off-by: Alon Levy <alevy@redhat.com> > --- > console.c | 25 +++++++++++++++++++++++-- > 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 ++ > monitor.c | 5 +++++ > qapi-schema.json | 20 ++++++++++++++++++++ > qmp-commands.hx | 26 ++++++++++++++++++++++++++ > 28 files changed, 115 insertions(+), 25 deletions(-) > > diff --git a/console.c b/console.c > index 6750538..ced2aa7 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; > > @@ -175,8 +176,9 @@ void vga_hw_invalidate(void) > active_console->hw_invalidate(active_console->hw); > } > > -void vga_hw_screen_dump(const char *filename) > +static void vga_hw_screen_dump_helper(const char *filename, bool async) > { > + bool event_sent = false; > TextConsole *previous_active_console; > bool cswitch; > > @@ -188,17 +190,33 @@ void vga_hw_screen_dump(const char *filename) > if (cswitch) { > console_select(0); > } > - if (consoles[0] && consoles[0]->hw_screen_dump) { > + if (async && consoles[0] && consoles[0]->hw_screen_dump_async) { > + consoles[0]->hw_screen_dump_async(consoles[0]->hw, filename, cswitch); > + event_sent = true; > + } else if (consoles[0] && consoles[0]->hw_screen_dump) { > consoles[0]->hw_screen_dump(consoles[0]->hw, filename, cswitch); > } else { > error_report("screen dump not implemented"); > } > + if (async && !event_sent) { > + monitor_protocol_screen_dump_complete_event(filename); > + } This is wrong. The event should be emitted by hw_screen_dump_async() when it completes. Also, this patch should be split into a least two patches. One patch does the console changes and the other one adds the qmp async command. It's also possible to make more patches by having individual devices' implementation of hw_screen_dump_async() in their own patches. > > if (cswitch) { > console_select(previous_active_console->index); > } > } > > +void vga_hw_screen_dump(const char *filename) > +{ > + vga_hw_screen_dump_helper(filename, false); > +} > + > +void vga_hw_screen_dump_async(const char *filename) > +{ > + vga_hw_screen_dump_helper(filename, true); > +} > + > void vga_hw_text_update(console_ch_t *chardata) > { > if (active_console && active_console->hw_text_update) > @@ -1403,6 +1421,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 +1441,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); > > void vga_hw_update(void); > void vga_hw_invalidate(void); > void vga_hw_screen_dump(const char *filename); > +void vga_hw_screen_dump_async(const char *filename); > void vga_hw_text_update(console_ch_t *chardata); > void monitor_protocol_screen_dump_complete_event(const char *filename); > > diff --git a/hw/blizzard.c b/hw/blizzard.c > index c7d844d..cc51045 100644 > --- a/hw/blizzard.c > +++ b/hw/blizzard.c > @@ -963,7 +963,7 @@ void *s1d13745_init(qemu_irq gpio_int) > > s->state = graphic_console_init(blizzard_update_display, > blizzard_invalidate_display, > - blizzard_screen_dump, NULL, s); > + blizzard_screen_dump, NULL, NULL, s); > > switch (ds_get_bits_per_pixel(s->state)) { > case 0: > diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c > index 4edcb94..d2a9c27 100644 > --- a/hw/cirrus_vga.c > +++ b/hw/cirrus_vga.c > @@ -2900,7 +2900,7 @@ static int vga_initfn(ISADevice *dev) > isa_address_space(dev)); > s->ds = graphic_console_init(s->update, s->invalidate, > s->screen_dump, s->text_update, > - s); > + s->screen_dump_async, s); > rom_add_vga(VGABIOS_CIRRUS_FILENAME); > /* XXX ISA-LFB support */ > /* FIXME not qdev yet */ > @@ -2941,7 +2941,7 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev) > cirrus_init_common(s, device_id, 1, pci_address_space(dev)); > s->vga.ds = graphic_console_init(s->vga.update, s->vga.invalidate, > s->vga.screen_dump, s->vga.text_update, > - &s->vga); > + s->vga.screen_dump_async, &s->vga); > > /* setup PCI */ > > diff --git a/hw/exynos4210_fimd.c b/hw/exynos4210_fimd.c > index 3313f00..2053ed0 100644 > --- a/hw/exynos4210_fimd.c > +++ b/hw/exynos4210_fimd.c > @@ -1898,7 +1898,8 @@ static int exynos4210_fimd_init(SysBusDevice *dev) > "exynos4210.fimd", FIMD_REGS_SIZE); > sysbus_init_mmio(dev, &s->iomem); > s->console = graphic_console_init(exynos4210_fimd_update, > - exynos4210_fimd_invalidate, NULL, NULL, s); > + exynos4210_fimd_invalidate, > + NULL, NULL, NULL, s); > > return 0; > } > diff --git a/hw/g364fb.c b/hw/g364fb.c > index 3a0b68f..8ae40d8 100644 > --- a/hw/g364fb.c > +++ b/hw/g364fb.c > @@ -517,7 +517,7 @@ static void g364fb_init(DeviceState *dev, G364State *s) > > s->ds = graphic_console_init(g364fb_update_display, > g364fb_invalidate_display, > - g364fb_screen_dump, NULL, s); > + g364fb_screen_dump, NULL, NULL, s); > > memory_region_init_io(&s->mem_ctrl, &g364fb_ctrl_ops, s, "ctrl", 0x180000); > memory_region_init_ram_ptr(&s->mem_vram, "vram", > diff --git a/hw/jazz_led.c b/hw/jazz_led.c > index 6486523..bb5daf7 100644 > --- a/hw/jazz_led.c > +++ b/hw/jazz_led.c > @@ -251,7 +251,8 @@ static int jazz_led_init(SysBusDevice *dev) > s->ds = graphic_console_init(jazz_led_update_display, > jazz_led_invalidate_display, > NULL, > - jazz_led_text_update, s); > + jazz_led_text_update, > + NULL, s); > > return 0; > } > diff --git a/hw/milkymist-vgafb.c b/hw/milkymist-vgafb.c > index 69afd72..ae018d1 100644 > --- a/hw/milkymist-vgafb.c > +++ b/hw/milkymist-vgafb.c > @@ -276,7 +276,7 @@ static int milkymist_vgafb_init(SysBusDevice *dev) > > s->ds = graphic_console_init(vgafb_update_display, > vgafb_invalidate_display, > - NULL, NULL, s); > + NULL, NULL, NULL, s); > > return 0; > } > diff --git a/hw/musicpal.c b/hw/musicpal.c > index 187a1ae..2fc391b 100644 > --- a/hw/musicpal.c > +++ b/hw/musicpal.c > @@ -611,7 +611,7 @@ static int musicpal_lcd_init(SysBusDevice *dev) > sysbus_init_mmio(dev, &s->iomem); > > s->ds = graphic_console_init(lcd_refresh, lcd_invalidate, > - NULL, NULL, s); > + NULL, NULL, NULL, s); > qemu_console_resize(s->ds, 128*3, 64*3); > > qdev_init_gpio_in(&dev->qdev, musicpal_lcd_gpio_brigthness_in, 3); > diff --git a/hw/omap_dss.c b/hw/omap_dss.c > index 86ed6ea..0e8243e 100644 > --- a/hw/omap_dss.c > +++ b/hw/omap_dss.c > @@ -1072,7 +1072,9 @@ struct omap_dss_s *omap_dss_init(struct omap_target_agent_s *ta, > > #if 0 > s->state = graphic_console_init(omap_update_display, > - omap_invalidate_display, omap_screen_dump, s); > + omap_invalidate_display, > + omap_screen_dump, > + NULL, NULL, s); > #endif > > return s; > diff --git a/hw/omap_lcdc.c b/hw/omap_lcdc.c > index f172093..d91e88d 100644 > --- a/hw/omap_lcdc.c > +++ b/hw/omap_lcdc.c > @@ -452,7 +452,7 @@ struct omap_lcd_panel_s *omap_lcdc_init(MemoryRegion *sysmem, > > s->state = graphic_console_init(omap_update_display, > omap_invalidate_display, > - omap_screen_dump, NULL, s); > + omap_screen_dump, NULL, NULL, s); > > return s; > } > diff --git a/hw/pl110.c b/hw/pl110.c > index f94608c..32ee89c 100644 > --- a/hw/pl110.c > +++ b/hw/pl110.c > @@ -451,7 +451,7 @@ static int pl110_init(SysBusDevice *dev) > qdev_init_gpio_in(&s->busdev.qdev, pl110_mux_ctrl_set, 1); > s->ds = graphic_console_init(pl110_update_display, > pl110_invalidate_display, > - NULL, NULL, s); > + NULL, NULL, NULL, s); > return 0; > } > > diff --git a/hw/pxa2xx_lcd.c b/hw/pxa2xx_lcd.c > index fcbdfb3..5345fb4 100644 > --- a/hw/pxa2xx_lcd.c > +++ b/hw/pxa2xx_lcd.c > @@ -1004,7 +1004,7 @@ PXA2xxLCDState *pxa2xx_lcdc_init(MemoryRegion *sysmem, > > s->ds = graphic_console_init(pxa2xx_update_display, > pxa2xx_invalidate_display, > - NULL, NULL, s); > + NULL, NULL, NULL, s); > > switch (ds_get_bits_per_pixel(s->ds)) { > case 0: > diff --git a/hw/qxl.c b/hw/qxl.c > index d83d245..61a7edd 100644 > --- a/hw/qxl.c > +++ b/hw/qxl.c > @@ -1727,7 +1727,8 @@ static int qxl_init_primary(PCIDevice *dev) > portio_list_add(qxl_vga_port_list, pci_address_space_io(dev), 0x3b0); > > vga->ds = graphic_console_init(qxl_hw_update, qxl_hw_invalidate, > - qxl_hw_screen_dump, qxl_hw_text_update, qxl); > + qxl_hw_screen_dump, qxl_hw_text_update, > + NULL, qxl); > qemu_spice_display_init_common(&qxl->ssd, vga->ds); > > qxl0 = qxl; > diff --git a/hw/sm501.c b/hw/sm501.c > index 786e076..8b150be 100644 > --- a/hw/sm501.c > +++ b/hw/sm501.c > @@ -1442,6 +1442,6 @@ void sm501_init(MemoryRegion *address_space_mem, uint32_t base, > } > > /* create qemu graphic console */ > - s->ds = graphic_console_init(sm501_update_display, NULL, > - NULL, NULL, s); > + s->ds = graphic_console_init(sm501_update_display, > + NULL, NULL, NULL, NULL, s); > } > diff --git a/hw/ssd0303.c b/hw/ssd0303.c > index 4e1ee6e..05e6686 100644 > --- a/hw/ssd0303.c > +++ b/hw/ssd0303.c > @@ -289,7 +289,7 @@ static int ssd0303_init(I2CSlave *i2c) > > s->ds = graphic_console_init(ssd0303_update_display, > ssd0303_invalidate_display, > - NULL, NULL, s); > + NULL, NULL, NULL, s); > qemu_console_resize(s->ds, 96 * MAGNIFY, 16 * MAGNIFY); > return 0; > } > diff --git a/hw/ssd0323.c b/hw/ssd0323.c > index b0b2e94..5e3491f 100644 > --- a/hw/ssd0323.c > +++ b/hw/ssd0323.c > @@ -330,7 +330,7 @@ static int ssd0323_init(SSISlave *dev) > s->row_end = 79; > s->ds = graphic_console_init(ssd0323_update_display, > ssd0323_invalidate_display, > - NULL, NULL, s); > + NULL, NULL, NULL, s); > qemu_console_resize(s->ds, 128 * MAGNIFY, 64 * MAGNIFY); > > qdev_init_gpio_in(&dev->qdev, ssd0323_cd, 1); > diff --git a/hw/tc6393xb.c b/hw/tc6393xb.c > index 420925c..853b180 100644 > --- a/hw/tc6393xb.c > +++ b/hw/tc6393xb.c > @@ -581,6 +581,7 @@ TC6393xbState *tc6393xb_init(MemoryRegion *sysmem, uint32_t base, qemu_irq irq) > NULL, /* invalidate */ > NULL, /* screen_dump */ > NULL, /* text_update */ > + NULL, /* screen_dump_async */ > s); > > return s; > diff --git a/hw/tcx.c b/hw/tcx.c > index ac7dcb4..9214dec 100644 > --- a/hw/tcx.c > +++ b/hw/tcx.c > @@ -558,7 +558,7 @@ static int tcx_init1(SysBusDevice *dev) > > s->ds = graphic_console_init(tcx24_update_display, > tcx24_invalidate_display, > - tcx24_screen_dump, NULL, s); > + tcx24_screen_dump, NULL, NULL, s); > } else { > /* THC 8 bit (dummy) */ > memory_region_init_io(&s->thc8, &dummy_ops, s, "tcx.thc8", > @@ -567,7 +567,7 @@ static int tcx_init1(SysBusDevice *dev) > > s->ds = graphic_console_init(tcx_update_display, > tcx_invalidate_display, > - tcx_screen_dump, NULL, s); > + tcx_screen_dump, NULL, NULL, s); > } > > qemu_console_resize(s->ds, s->width, s->height); > diff --git a/hw/vga-isa-mm.c b/hw/vga-isa-mm.c > index f8984c6..87b8fc6 100644 > --- a/hw/vga-isa-mm.c > +++ b/hw/vga-isa-mm.c > @@ -132,7 +132,8 @@ int isa_vga_mm_init(target_phys_addr_t vram_base, > vga_mm_init(s, vram_base, ctrl_base, it_shift, address_space); > > s->vga.ds = graphic_console_init(s->vga.update, s->vga.invalidate, > - s->vga.screen_dump, s->vga.text_update, s); > + s->vga.screen_dump, s->vga.text_update, > + s->vga.screen_dump_async, s); > > vga_init_vbe(&s->vga, address_space); > return 0; > diff --git a/hw/vga-isa.c b/hw/vga-isa.c > index 4bcc4db..a362ed2 100644 > --- a/hw/vga-isa.c > +++ b/hw/vga-isa.c > @@ -61,7 +61,8 @@ static int vga_initfn(ISADevice *dev) > vga_io_memory, 1); > memory_region_set_coalescing(vga_io_memory); > s->ds = graphic_console_init(s->update, s->invalidate, > - s->screen_dump, s->text_update, s); > + s->screen_dump, s->text_update, > + s->screen_dump_async, s); > > vga_init_vbe(s, isa_address_space(dev)); > /* ROM BIOS */ > diff --git a/hw/vga-pci.c b/hw/vga-pci.c > index 465b643..8c8dd53 100644 > --- a/hw/vga-pci.c > +++ b/hw/vga-pci.c > @@ -57,7 +57,8 @@ static int pci_vga_initfn(PCIDevice *dev) > vga_init(s, pci_address_space(dev), pci_address_space_io(dev), true); > > s->ds = graphic_console_init(s->update, s->invalidate, > - s->screen_dump, s->text_update, s); > + s->screen_dump, s->text_update, > + s->screen_dump_async, s); > > /* XXX: VGA_RAM_SIZE must be a power of two */ > pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_MEM_PREFETCH, &s->vram); > diff --git a/hw/vga_int.h b/hw/vga_int.h > index 7685b2b..4486cfb 100644 > --- a/hw/vga_int.h > +++ b/hw/vga_int.h > @@ -161,6 +161,7 @@ typedef struct VGACommonState { > 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; > /* hardware mouse cursor support */ > uint32_t invalidated_y_table[VGA_MAX_HEIGHT / 32]; > void (*cursor_invalidate)(struct VGACommonState *s); > diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c > index 142d9f4..db96e61 100644 > --- a/hw/vmware_vga.c > +++ b/hw/vmware_vga.c > @@ -1087,7 +1087,8 @@ static void vmsvga_init(struct vmsvga_state_s *s, int vga_ram_size, > s->vga.ds = graphic_console_init(vmsvga_update_display, > vmsvga_invalidate_display, > vmsvga_screen_dump, > - vmsvga_text_update, s); > + vmsvga_text_update, > + NULL, s); > > > s->fifo_size = SVGA_FIFO_SIZE; > diff --git a/hw/xenfb.c b/hw/xenfb.c > index 1bcf171..ad9f0d7 100644 > --- a/hw/xenfb.c > +++ b/hw/xenfb.c > @@ -906,6 +906,7 @@ static int fb_initialise(struct XenDevice *xendev) > xenfb_invalidate, > NULL, > NULL, > + NULL, > fb); > fb->have_console = 1; > } > @@ -1015,6 +1016,7 @@ wait_more: > xenfb_invalidate, > NULL, > NULL, > + NULL, > fb); > fb->have_console = 1; > > diff --git a/monitor.c b/monitor.c > index 1a65c41..91a76a1 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -901,6 +901,11 @@ static int do_screen_dump(Monitor *mon, const QDict *qdict, QObject **ret_data) > return 0; > } > > +void qmp_screendump_async(const char *filename, Error **errp) > +{ > + vga_hw_screen_dump_async(filename); > +} > + > static void do_logfile(Monitor *mon, const QDict *qdict) > { > cpu_set_log_filename(qdict_get_str(qdict, "filename")); > diff --git a/qapi-schema.json b/qapi-schema.json > index 72b17f1..a5873cb 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -1595,3 +1595,23 @@ > { 'command': 'qom-list-types', > 'data': { '*implements': 'str', '*abstract': 'bool' }, > 'returns': [ 'ObjectTypeInfo' ] } > + > +## > +## @screendump-async: > +# > +# This command will perform a screen dump of the first console to the givem > +# filename. The additional parameters are unused at this time. Isn't it interesting to allow the mngt app to specify the console? > +# > +# @filename name of output file to write screen dump to > +# > +# Since: 1.1 > +# > +# Notes: This command is experimental and may change syntax in future releases. Why is this needed? > +# > +# This command is the same as the qmp/hmp screendump command, except that on > +# successful completion of the scren dump the SCREEN_DUMP_COMPLETE event is > +# emitted. The real difference is that it's asynchronous, while the other is synchronous. > +# > +## > +{ 'command': 'screendump-async', > + 'data': { 'filename': 'str' } } > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 705f704..ea0d051 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -170,6 +170,32 @@ Example: > EQMP > > { > + .name = "screendump-async", > + .args_type = "filename:F", > + .params = "filename", > + .help = "save screen into PPM image 'filename'", > + .user_print = monitor_user_noop, > + .mhandler.cmd_new = qmp_marshal_input_screendump_async, > + }, > + > +SQMP > +screendump-async > +---------------- > + > +Save screen into PPM image. Fires a SCREEN_DUMP_COMPLETE event on completion. > + > +Arguments: > + > +- "filename": file path (json-string) > + > +Example: > + > +-> { "execute": "screendump-async", "arguments": { "filename": "/tmp/image" } } > +<- { "return": {} } > + > +EQMP > + > + { > .name = "stop", > .args_type = "", > .mhandler.cmd_new = qmp_marshal_input_stop,
On Sat, 25 Feb 2012 10:46:07 +0200 Alon Levy <alevy@redhat.com> wrote: > On Fri, Feb 24, 2012 at 04:40:15PM -0600, Anthony Liguori wrote: > > On 02/24/2012 03:22 PM, Alon Levy wrote: > > >This is an across the board change since I wanted to keep the existing > > >(good imo) single graphic_console_init callback setter, instead of > > >introducing a new cb that isn't set by it but instead by a second > > >initialization function. > > > > > >Signed-off-by: Alon Levy<alevy@redhat.com> > > > > What's the rationale for this? > > There is a hang possible with the current screendump command, qxl, a > spice client using libvirt and spice-gtk such as virt-viewer / > remote-viewer, where you have: > 1. libvirt waiting for screendump to complete > 2. screendump waiting for spice server thread to render > 3. spice server thread waiting for spice client to read messages Which messages? > 4. spice client == libvirt client, waiting for screendump completion The way I had understood this problem is that qxl takes a long time to perform a screen dump, which would cause the global mutex to be held for a long time. If this is really serious, then a async command for it makes sense IMO. Now, the problem you describe above is different and maybe it shouldn't be solved at QMP level. Can you describe it in more detail? > > I really don't want a plethora of pseudo-async commands. Agreed, but this is not supposed to be pseudo-async. If a command returns immediately and sends an event when it later completes, it's really async. Besides, we have agreed on adding new command + event for commands that really need to be async.
Hi, >> There is a hang possible with the current screendump command, qxl, a >> spice client using libvirt and spice-gtk such as virt-viewer / >> remote-viewer, where you have: >> 1. libvirt waiting for screendump to complete >> 2. screendump waiting for spice server thread to render >> 3. spice server thread waiting for spice client to read messages > > Which messages? spice protocol. >> 4. spice client == libvirt client, waiting for screendump completion > > The way I had understood this problem is that qxl takes a long time to > perform a screen dump, which would cause the global mutex to be held for > a long time. If this is really serious, then a async command for it > makes sense IMO. That describes it pretty good. Normal workflow is that spice-server sends the rendering commands received from the guest off to the spice client and the actual rendering happens there. spice-server also keeps track of the rendering commands so it can actually render on the host side too of needed (called 'local rendering'). But it doesn't do that unless it has to to avoid wasting cycles. Events which need local rendering are: (1) display on sdl/vnc, (2) screendump monitor command, (3) guest requesting screen content (happens when doing screen shots for example). So when issuing the screendump monitor command we have to ask spice-server to render the screen for us, when it is done we can write out the screen dump. We don't want block the iothread while the spice server is busy does the rendering. Additionally the local rendering can be delayed by a spice-client connected via slow network link. This is (3) in alon's list above. There are plans to fix that, but it isn't trivial to do due to the way spice-server keeps track of the rendering commands. It can delay the rendering several seconds. But even without that I've seen delays around 50ms, which is still way too much to have iothread blocked. cheers, Gerd
On Tue, Feb 28, 2012 at 05:10:39PM -0300, Luiz Capitulino wrote: > On Sat, 25 Feb 2012 10:46:07 +0200 > Alon Levy <alevy@redhat.com> wrote: > > > On Fri, Feb 24, 2012 at 04:40:15PM -0600, Anthony Liguori wrote: > > > On 02/24/2012 03:22 PM, Alon Levy wrote: > > > >This is an across the board change since I wanted to keep the existing > > > >(good imo) single graphic_console_init callback setter, instead of > > > >introducing a new cb that isn't set by it but instead by a second > > > >initialization function. > > > > > > > >Signed-off-by: Alon Levy<alevy@redhat.com> > > > > > > What's the rationale for this? > > > > There is a hang possible with the current screendump command, qxl, a > > spice client using libvirt and spice-gtk such as virt-viewer / > > remote-viewer, where you have: > > 1. libvirt waiting for screendump to complete > > 2. screendump waiting for spice server thread to render > > 3. spice server thread waiting for spice client to read messages > > Which messages? spice display channel messages. > > > 4. spice client == libvirt client, waiting for screendump completion > > The way I had understood this problem is that qxl takes a long time to > perform a screen dump, which would cause the global mutex to be held for > a long time. If this is really serious, then a async command for it > makes sense IMO. That is true, but it is not the immediate problem the bz is dealing with - if it was only this there would be no hang. > > Now, the problem you describe above is different and maybe it shouldn't > be solved at QMP level. Can you describe it in more detail? > I can just point to the diagram I put into the mail from october: """ This fixes a hang that is caused when the spice client is also a qemu monitor client and the client is single threaded: io thread worker thread client <---------------------------------------------------- screendump do_screen_dump-> read-------> flush, read client socket-------------> """ I have not managed to reproduce this myself, so I'm cc'ing Marc-Andre who has and reported the fix of using an asyncronous operation internally (unrelated to the asynchronous qevent in this patchset) between the monitor thread and the spice thread fixed the problem. > > > I really don't want a plethora of pseudo-async commands. > > Agreed, but this is not supposed to be pseudo-async. If a command returns > immediately and sends an event when it later completes, it's really async. Thanks for saying this. I didn't understand the pseudo-async angle.
On Tue, Feb 28, 2012 at 05:05:32PM -0300, Luiz Capitulino wrote: > On Fri, 24 Feb 2012 23:22:05 +0200 > Alon Levy <alevy@redhat.com> wrote: > > > This is an across the board change since I wanted to keep the existing > > (good imo) single graphic_console_init callback setter, instead of > > introducing a new cb that isn't set by it but instead by a second > > initialization function. > > > > Signed-off-by: Alon Levy <alevy@redhat.com> > > --- > > console.c | 25 +++++++++++++++++++++++-- > > 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 ++ > > monitor.c | 5 +++++ > > qapi-schema.json | 20 ++++++++++++++++++++ > > qmp-commands.hx | 26 ++++++++++++++++++++++++++ > > 28 files changed, 115 insertions(+), 25 deletions(-) > > > > diff --git a/console.c b/console.c > > index 6750538..ced2aa7 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; > > > > @@ -175,8 +176,9 @@ void vga_hw_invalidate(void) > > active_console->hw_invalidate(active_console->hw); > > } > > > > -void vga_hw_screen_dump(const char *filename) > > +static void vga_hw_screen_dump_helper(const char *filename, bool async) > > { > > + bool event_sent = false; > > TextConsole *previous_active_console; > > bool cswitch; > > > > @@ -188,17 +190,33 @@ void vga_hw_screen_dump(const char *filename) > > if (cswitch) { > > console_select(0); > > } > > - if (consoles[0] && consoles[0]->hw_screen_dump) { > > + if (async && consoles[0] && consoles[0]->hw_screen_dump_async) { > > + consoles[0]->hw_screen_dump_async(consoles[0]->hw, filename, cswitch); > > + event_sent = true; > > + } else if (consoles[0] && consoles[0]->hw_screen_dump) { > > consoles[0]->hw_screen_dump(consoles[0]->hw, filename, cswitch); > > } else { > > error_report("screen dump not implemented"); > > } > > + if (async && !event_sent) { > > + monitor_protocol_screen_dump_complete_event(filename); > > + } > > This is wrong. The event should be emitted by hw_screen_dump_async() when it > completes. So you think it's better to add a intermediate callback there? ok, I'll try to do that. > > Also, this patch should be split into a least two patches. One patch does the > console changes and the other one adds the qmp async command. It's also possible > to make more patches by having individual devices' implementation of > hw_screen_dump_async() in their own patches. will do all the splits. > > > > > if (cswitch) { > > console_select(previous_active_console->index); > > } > > } > > > > +void vga_hw_screen_dump(const char *filename) > > +{ > > + vga_hw_screen_dump_helper(filename, false); > > +} > > + > > +void vga_hw_screen_dump_async(const char *filename) > > +{ > > + vga_hw_screen_dump_helper(filename, true); > > +} > > + > > void vga_hw_text_update(console_ch_t *chardata) > > { > > if (active_console && active_console->hw_text_update) > > @@ -1403,6 +1421,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 +1441,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); > > > > void vga_hw_update(void); > > void vga_hw_invalidate(void); > > void vga_hw_screen_dump(const char *filename); > > +void vga_hw_screen_dump_async(const char *filename); > > void vga_hw_text_update(console_ch_t *chardata); > > void monitor_protocol_screen_dump_complete_event(const char *filename); > > > > diff --git a/hw/blizzard.c b/hw/blizzard.c > > index c7d844d..cc51045 100644 > > --- a/hw/blizzard.c > > +++ b/hw/blizzard.c > > @@ -963,7 +963,7 @@ void *s1d13745_init(qemu_irq gpio_int) > > > > s->state = graphic_console_init(blizzard_update_display, > > blizzard_invalidate_display, > > - blizzard_screen_dump, NULL, s); > > + blizzard_screen_dump, NULL, NULL, s); > > > > switch (ds_get_bits_per_pixel(s->state)) { > > case 0: > > diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c > > index 4edcb94..d2a9c27 100644 > > --- a/hw/cirrus_vga.c > > +++ b/hw/cirrus_vga.c > > @@ -2900,7 +2900,7 @@ static int vga_initfn(ISADevice *dev) > > isa_address_space(dev)); > > s->ds = graphic_console_init(s->update, s->invalidate, > > s->screen_dump, s->text_update, > > - s); > > + s->screen_dump_async, s); > > rom_add_vga(VGABIOS_CIRRUS_FILENAME); > > /* XXX ISA-LFB support */ > > /* FIXME not qdev yet */ > > @@ -2941,7 +2941,7 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev) > > cirrus_init_common(s, device_id, 1, pci_address_space(dev)); > > s->vga.ds = graphic_console_init(s->vga.update, s->vga.invalidate, > > s->vga.screen_dump, s->vga.text_update, > > - &s->vga); > > + s->vga.screen_dump_async, &s->vga); > > > > /* setup PCI */ > > > > diff --git a/hw/exynos4210_fimd.c b/hw/exynos4210_fimd.c > > index 3313f00..2053ed0 100644 > > --- a/hw/exynos4210_fimd.c > > +++ b/hw/exynos4210_fimd.c > > @@ -1898,7 +1898,8 @@ static int exynos4210_fimd_init(SysBusDevice *dev) > > "exynos4210.fimd", FIMD_REGS_SIZE); > > sysbus_init_mmio(dev, &s->iomem); > > s->console = graphic_console_init(exynos4210_fimd_update, > > - exynos4210_fimd_invalidate, NULL, NULL, s); > > + exynos4210_fimd_invalidate, > > + NULL, NULL, NULL, s); > > > > return 0; > > } > > diff --git a/hw/g364fb.c b/hw/g364fb.c > > index 3a0b68f..8ae40d8 100644 > > --- a/hw/g364fb.c > > +++ b/hw/g364fb.c > > @@ -517,7 +517,7 @@ static void g364fb_init(DeviceState *dev, G364State *s) > > > > s->ds = graphic_console_init(g364fb_update_display, > > g364fb_invalidate_display, > > - g364fb_screen_dump, NULL, s); > > + g364fb_screen_dump, NULL, NULL, s); > > > > memory_region_init_io(&s->mem_ctrl, &g364fb_ctrl_ops, s, "ctrl", 0x180000); > > memory_region_init_ram_ptr(&s->mem_vram, "vram", > > diff --git a/hw/jazz_led.c b/hw/jazz_led.c > > index 6486523..bb5daf7 100644 > > --- a/hw/jazz_led.c > > +++ b/hw/jazz_led.c > > @@ -251,7 +251,8 @@ static int jazz_led_init(SysBusDevice *dev) > > s->ds = graphic_console_init(jazz_led_update_display, > > jazz_led_invalidate_display, > > NULL, > > - jazz_led_text_update, s); > > + jazz_led_text_update, > > + NULL, s); > > > > return 0; > > } > > diff --git a/hw/milkymist-vgafb.c b/hw/milkymist-vgafb.c > > index 69afd72..ae018d1 100644 > > --- a/hw/milkymist-vgafb.c > > +++ b/hw/milkymist-vgafb.c > > @@ -276,7 +276,7 @@ static int milkymist_vgafb_init(SysBusDevice *dev) > > > > s->ds = graphic_console_init(vgafb_update_display, > > vgafb_invalidate_display, > > - NULL, NULL, s); > > + NULL, NULL, NULL, s); > > > > return 0; > > } > > diff --git a/hw/musicpal.c b/hw/musicpal.c > > index 187a1ae..2fc391b 100644 > > --- a/hw/musicpal.c > > +++ b/hw/musicpal.c > > @@ -611,7 +611,7 @@ static int musicpal_lcd_init(SysBusDevice *dev) > > sysbus_init_mmio(dev, &s->iomem); > > > > s->ds = graphic_console_init(lcd_refresh, lcd_invalidate, > > - NULL, NULL, s); > > + NULL, NULL, NULL, s); > > qemu_console_resize(s->ds, 128*3, 64*3); > > > > qdev_init_gpio_in(&dev->qdev, musicpal_lcd_gpio_brigthness_in, 3); > > diff --git a/hw/omap_dss.c b/hw/omap_dss.c > > index 86ed6ea..0e8243e 100644 > > --- a/hw/omap_dss.c > > +++ b/hw/omap_dss.c > > @@ -1072,7 +1072,9 @@ struct omap_dss_s *omap_dss_init(struct omap_target_agent_s *ta, > > > > #if 0 > > s->state = graphic_console_init(omap_update_display, > > - omap_invalidate_display, omap_screen_dump, s); > > + omap_invalidate_display, > > + omap_screen_dump, > > + NULL, NULL, s); > > #endif > > > > return s; > > diff --git a/hw/omap_lcdc.c b/hw/omap_lcdc.c > > index f172093..d91e88d 100644 > > --- a/hw/omap_lcdc.c > > +++ b/hw/omap_lcdc.c > > @@ -452,7 +452,7 @@ struct omap_lcd_panel_s *omap_lcdc_init(MemoryRegion *sysmem, > > > > s->state = graphic_console_init(omap_update_display, > > omap_invalidate_display, > > - omap_screen_dump, NULL, s); > > + omap_screen_dump, NULL, NULL, s); > > > > return s; > > } > > diff --git a/hw/pl110.c b/hw/pl110.c > > index f94608c..32ee89c 100644 > > --- a/hw/pl110.c > > +++ b/hw/pl110.c > > @@ -451,7 +451,7 @@ static int pl110_init(SysBusDevice *dev) > > qdev_init_gpio_in(&s->busdev.qdev, pl110_mux_ctrl_set, 1); > > s->ds = graphic_console_init(pl110_update_display, > > pl110_invalidate_display, > > - NULL, NULL, s); > > + NULL, NULL, NULL, s); > > return 0; > > } > > > > diff --git a/hw/pxa2xx_lcd.c b/hw/pxa2xx_lcd.c > > index fcbdfb3..5345fb4 100644 > > --- a/hw/pxa2xx_lcd.c > > +++ b/hw/pxa2xx_lcd.c > > @@ -1004,7 +1004,7 @@ PXA2xxLCDState *pxa2xx_lcdc_init(MemoryRegion *sysmem, > > > > s->ds = graphic_console_init(pxa2xx_update_display, > > pxa2xx_invalidate_display, > > - NULL, NULL, s); > > + NULL, NULL, NULL, s); > > > > switch (ds_get_bits_per_pixel(s->ds)) { > > case 0: > > diff --git a/hw/qxl.c b/hw/qxl.c > > index d83d245..61a7edd 100644 > > --- a/hw/qxl.c > > +++ b/hw/qxl.c > > @@ -1727,7 +1727,8 @@ static int qxl_init_primary(PCIDevice *dev) > > portio_list_add(qxl_vga_port_list, pci_address_space_io(dev), 0x3b0); > > > > vga->ds = graphic_console_init(qxl_hw_update, qxl_hw_invalidate, > > - qxl_hw_screen_dump, qxl_hw_text_update, qxl); > > + qxl_hw_screen_dump, qxl_hw_text_update, > > + NULL, qxl); > > qemu_spice_display_init_common(&qxl->ssd, vga->ds); > > > > qxl0 = qxl; > > diff --git a/hw/sm501.c b/hw/sm501.c > > index 786e076..8b150be 100644 > > --- a/hw/sm501.c > > +++ b/hw/sm501.c > > @@ -1442,6 +1442,6 @@ void sm501_init(MemoryRegion *address_space_mem, uint32_t base, > > } > > > > /* create qemu graphic console */ > > - s->ds = graphic_console_init(sm501_update_display, NULL, > > - NULL, NULL, s); > > + s->ds = graphic_console_init(sm501_update_display, > > + NULL, NULL, NULL, NULL, s); > > } > > diff --git a/hw/ssd0303.c b/hw/ssd0303.c > > index 4e1ee6e..05e6686 100644 > > --- a/hw/ssd0303.c > > +++ b/hw/ssd0303.c > > @@ -289,7 +289,7 @@ static int ssd0303_init(I2CSlave *i2c) > > > > s->ds = graphic_console_init(ssd0303_update_display, > > ssd0303_invalidate_display, > > - NULL, NULL, s); > > + NULL, NULL, NULL, s); > > qemu_console_resize(s->ds, 96 * MAGNIFY, 16 * MAGNIFY); > > return 0; > > } > > diff --git a/hw/ssd0323.c b/hw/ssd0323.c > > index b0b2e94..5e3491f 100644 > > --- a/hw/ssd0323.c > > +++ b/hw/ssd0323.c > > @@ -330,7 +330,7 @@ static int ssd0323_init(SSISlave *dev) > > s->row_end = 79; > > s->ds = graphic_console_init(ssd0323_update_display, > > ssd0323_invalidate_display, > > - NULL, NULL, s); > > + NULL, NULL, NULL, s); > > qemu_console_resize(s->ds, 128 * MAGNIFY, 64 * MAGNIFY); > > > > qdev_init_gpio_in(&dev->qdev, ssd0323_cd, 1); > > diff --git a/hw/tc6393xb.c b/hw/tc6393xb.c > > index 420925c..853b180 100644 > > --- a/hw/tc6393xb.c > > +++ b/hw/tc6393xb.c > > @@ -581,6 +581,7 @@ TC6393xbState *tc6393xb_init(MemoryRegion *sysmem, uint32_t base, qemu_irq irq) > > NULL, /* invalidate */ > > NULL, /* screen_dump */ > > NULL, /* text_update */ > > + NULL, /* screen_dump_async */ > > s); > > > > return s; > > diff --git a/hw/tcx.c b/hw/tcx.c > > index ac7dcb4..9214dec 100644 > > --- a/hw/tcx.c > > +++ b/hw/tcx.c > > @@ -558,7 +558,7 @@ static int tcx_init1(SysBusDevice *dev) > > > > s->ds = graphic_console_init(tcx24_update_display, > > tcx24_invalidate_display, > > - tcx24_screen_dump, NULL, s); > > + tcx24_screen_dump, NULL, NULL, s); > > } else { > > /* THC 8 bit (dummy) */ > > memory_region_init_io(&s->thc8, &dummy_ops, s, "tcx.thc8", > > @@ -567,7 +567,7 @@ static int tcx_init1(SysBusDevice *dev) > > > > s->ds = graphic_console_init(tcx_update_display, > > tcx_invalidate_display, > > - tcx_screen_dump, NULL, s); > > + tcx_screen_dump, NULL, NULL, s); > > } > > > > qemu_console_resize(s->ds, s->width, s->height); > > diff --git a/hw/vga-isa-mm.c b/hw/vga-isa-mm.c > > index f8984c6..87b8fc6 100644 > > --- a/hw/vga-isa-mm.c > > +++ b/hw/vga-isa-mm.c > > @@ -132,7 +132,8 @@ int isa_vga_mm_init(target_phys_addr_t vram_base, > > vga_mm_init(s, vram_base, ctrl_base, it_shift, address_space); > > > > s->vga.ds = graphic_console_init(s->vga.update, s->vga.invalidate, > > - s->vga.screen_dump, s->vga.text_update, s); > > + s->vga.screen_dump, s->vga.text_update, > > + s->vga.screen_dump_async, s); > > > > vga_init_vbe(&s->vga, address_space); > > return 0; > > diff --git a/hw/vga-isa.c b/hw/vga-isa.c > > index 4bcc4db..a362ed2 100644 > > --- a/hw/vga-isa.c > > +++ b/hw/vga-isa.c > > @@ -61,7 +61,8 @@ static int vga_initfn(ISADevice *dev) > > vga_io_memory, 1); > > memory_region_set_coalescing(vga_io_memory); > > s->ds = graphic_console_init(s->update, s->invalidate, > > - s->screen_dump, s->text_update, s); > > + s->screen_dump, s->text_update, > > + s->screen_dump_async, s); > > > > vga_init_vbe(s, isa_address_space(dev)); > > /* ROM BIOS */ > > diff --git a/hw/vga-pci.c b/hw/vga-pci.c > > index 465b643..8c8dd53 100644 > > --- a/hw/vga-pci.c > > +++ b/hw/vga-pci.c > > @@ -57,7 +57,8 @@ static int pci_vga_initfn(PCIDevice *dev) > > vga_init(s, pci_address_space(dev), pci_address_space_io(dev), true); > > > > s->ds = graphic_console_init(s->update, s->invalidate, > > - s->screen_dump, s->text_update, s); > > + s->screen_dump, s->text_update, > > + s->screen_dump_async, s); > > > > /* XXX: VGA_RAM_SIZE must be a power of two */ > > pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_MEM_PREFETCH, &s->vram); > > diff --git a/hw/vga_int.h b/hw/vga_int.h > > index 7685b2b..4486cfb 100644 > > --- a/hw/vga_int.h > > +++ b/hw/vga_int.h > > @@ -161,6 +161,7 @@ typedef struct VGACommonState { > > 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; > > /* hardware mouse cursor support */ > > uint32_t invalidated_y_table[VGA_MAX_HEIGHT / 32]; > > void (*cursor_invalidate)(struct VGACommonState *s); > > diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c > > index 142d9f4..db96e61 100644 > > --- a/hw/vmware_vga.c > > +++ b/hw/vmware_vga.c > > @@ -1087,7 +1087,8 @@ static void vmsvga_init(struct vmsvga_state_s *s, int vga_ram_size, > > s->vga.ds = graphic_console_init(vmsvga_update_display, > > vmsvga_invalidate_display, > > vmsvga_screen_dump, > > - vmsvga_text_update, s); > > + vmsvga_text_update, > > + NULL, s); > > > > > > s->fifo_size = SVGA_FIFO_SIZE; > > diff --git a/hw/xenfb.c b/hw/xenfb.c > > index 1bcf171..ad9f0d7 100644 > > --- a/hw/xenfb.c > > +++ b/hw/xenfb.c > > @@ -906,6 +906,7 @@ static int fb_initialise(struct XenDevice *xendev) > > xenfb_invalidate, > > NULL, > > NULL, > > + NULL, > > fb); > > fb->have_console = 1; > > } > > @@ -1015,6 +1016,7 @@ wait_more: > > xenfb_invalidate, > > NULL, > > NULL, > > + NULL, > > fb); > > fb->have_console = 1; > > > > diff --git a/monitor.c b/monitor.c > > index 1a65c41..91a76a1 100644 > > --- a/monitor.c > > +++ b/monitor.c > > @@ -901,6 +901,11 @@ static int do_screen_dump(Monitor *mon, const QDict *qdict, QObject **ret_data) > > return 0; > > } > > > > +void qmp_screendump_async(const char *filename, Error **errp) > > +{ > > + vga_hw_screen_dump_async(filename); > > +} > > + > > static void do_logfile(Monitor *mon, const QDict *qdict) > > { > > cpu_set_log_filename(qdict_get_str(qdict, "filename")); > > diff --git a/qapi-schema.json b/qapi-schema.json > > index 72b17f1..a5873cb 100644 > > --- a/qapi-schema.json > > +++ b/qapi-schema.json > > @@ -1595,3 +1595,23 @@ > > { 'command': 'qom-list-types', > > 'data': { '*implements': 'str', '*abstract': 'bool' }, > > 'returns': [ 'ObjectTypeInfo' ] } > > + > > +## > > +## @screendump-async: > > +# > > +# This command will perform a screen dump of the first console to the givem > > +# filename. The additional parameters are unused at this time. > > Isn't it interesting to allow the mngt app to specify the console? Will add. > > > +# > > +# @filename name of output file to write screen dump to > > +# > > +# Since: 1.1 > > +# > > +# Notes: This command is experimental and may change syntax in future releases. > > Why is this needed? Because I wanted an option to specify the device and the monitor in the device (or a general index for a device, interpreted as a monitor for video output devices like vga), but I wasn't sure if we have a string representation of a device yet. > > > +# > > +# This command is the same as the qmp/hmp screendump command, except that on > > +# successful completion of the scren dump the SCREEN_DUMP_COMPLETE event is > > +# emitted. > > The real difference is that it's asynchronous, while the other is synchronous. Yes, I was trying to be more specific. I'll add that line. > > > +# > > +## > > +{ 'command': 'screendump-async', > > + 'data': { 'filename': 'str' } } > > diff --git a/qmp-commands.hx b/qmp-commands.hx > > index 705f704..ea0d051 100644 > > --- a/qmp-commands.hx > > +++ b/qmp-commands.hx > > @@ -170,6 +170,32 @@ Example: > > EQMP > > > > { > > + .name = "screendump-async", > > + .args_type = "filename:F", > > + .params = "filename", > > + .help = "save screen into PPM image 'filename'", > > + .user_print = monitor_user_noop, > > + .mhandler.cmd_new = qmp_marshal_input_screendump_async, > > + }, > > + > > +SQMP > > +screendump-async > > +---------------- > > + > > +Save screen into PPM image. Fires a SCREEN_DUMP_COMPLETE event on completion. > > + > > +Arguments: > > + > > +- "filename": file path (json-string) > > + > > +Example: > > + > > +-> { "execute": "screendump-async", "arguments": { "filename": "/tmp/image" } } > > +<- { "return": {} } > > + > > +EQMP > > + > > + { > > .name = "stop", > > .args_type = "", > > .mhandler.cmd_new = qmp_marshal_input_stop, > >
On Wed, 29 Feb 2012 08:49:10 +0100 Gerd Hoffmann <kraxel@redhat.com> wrote: > Hi, > > >> There is a hang possible with the current screendump command, qxl, a > >> spice client using libvirt and spice-gtk such as virt-viewer / > >> remote-viewer, where you have: > >> 1. libvirt waiting for screendump to complete > >> 2. screendump waiting for spice server thread to render > >> 3. spice server thread waiting for spice client to read messages > > > > Which messages? > > spice protocol. > > >> 4. spice client == libvirt client, waiting for screendump completion > > > > The way I had understood this problem is that qxl takes a long time to > > perform a screen dump, which would cause the global mutex to be held for > > a long time. If this is really serious, then a async command for it > > makes sense IMO. > > That describes it pretty good. Normal workflow is that spice-server > sends the rendering commands received from the guest off to the spice > client and the actual rendering happens there. spice-server also keeps > track of the rendering commands so it can actually render on the host > side too of needed (called 'local rendering'). But it doesn't do that > unless it has to to avoid wasting cycles. Events which need local > rendering are: (1) display on sdl/vnc, (2) screendump monitor command, > (3) guest requesting screen content (happens when doing screen shots > for example). > > So when issuing the screendump monitor command we have to ask > spice-server to render the screen for us, when it is done we can write > out the screen dump. We don't want block the iothread while the spice > server is busy does the rendering. Anthony, the only way I can think of solving this is making the screendump asynchronous. Do you have a different suggestion? > > Additionally the local rendering can be delayed by a spice-client > connected via slow network link. This is (3) in alon's list above. > There are plans to fix that, but it isn't trivial to do due to the way > spice-server keeps track of the rendering commands. It can delay the > rendering several seconds. But even without that I've seen delays > around 50ms, which is still way too much to have iothread blocked. > > cheers, > Gerd >
On Wed, 29 Feb 2012 10:15:53 +0200 Alon Levy <alevy@redhat.com> wrote: > On Tue, Feb 28, 2012 at 05:10:39PM -0300, Luiz Capitulino wrote: > > On Sat, 25 Feb 2012 10:46:07 +0200 > > Alon Levy <alevy@redhat.com> wrote: > > > > > On Fri, Feb 24, 2012 at 04:40:15PM -0600, Anthony Liguori wrote: > > > > On 02/24/2012 03:22 PM, Alon Levy wrote: > > > > >This is an across the board change since I wanted to keep the existing > > > > >(good imo) single graphic_console_init callback setter, instead of > > > > >introducing a new cb that isn't set by it but instead by a second > > > > >initialization function. > > > > > > > > > >Signed-off-by: Alon Levy<alevy@redhat.com> > > > > > > > > What's the rationale for this? > > > > > > There is a hang possible with the current screendump command, qxl, a > > > spice client using libvirt and spice-gtk such as virt-viewer / > > > remote-viewer, where you have: > > > 1. libvirt waiting for screendump to complete > > > 2. screendump waiting for spice server thread to render > > > 3. spice server thread waiting for spice client to read messages > > > > Which messages? > > spice display channel messages. > > > > > > 4. spice client == libvirt client, waiting for screendump completion > > > > The way I had understood this problem is that qxl takes a long time to > > perform a screen dump, which would cause the global mutex to be held for > > a long time. If this is really serious, then a async command for it > > makes sense IMO. > > That is true, but it is not the immediate problem the bz is dealing with > - if it was only this there would be no hang. Well, this kind of hang always smells like a spice threading synchronization problem to me. I thought that I'd be capable of showing that if I really understood what was going on, but I can't, even with your diagram. An asynchronous command solves the global mutex contention problem, but I think this hang should be further investigated, otherwise the async command risks just hiding the real problem.
On Wed, 29 Feb 2012 10:39:29 +0200 Alon Levy <alevy@redhat.com> wrote: > On Tue, Feb 28, 2012 at 05:05:32PM -0300, Luiz Capitulino wrote: > > On Fri, 24 Feb 2012 23:22:05 +0200 > > Alon Levy <alevy@redhat.com> wrote: > > > > > This is an across the board change since I wanted to keep the existing > > > (good imo) single graphic_console_init callback setter, instead of > > > introducing a new cb that isn't set by it but instead by a second > > > initialization function. > > > > > > Signed-off-by: Alon Levy <alevy@redhat.com> > > > --- > > > console.c | 25 +++++++++++++++++++++++-- > > > 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 ++ > > > monitor.c | 5 +++++ > > > qapi-schema.json | 20 ++++++++++++++++++++ > > > qmp-commands.hx | 26 ++++++++++++++++++++++++++ > > > 28 files changed, 115 insertions(+), 25 deletions(-) > > > > > > diff --git a/console.c b/console.c > > > index 6750538..ced2aa7 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; > > > > > > @@ -175,8 +176,9 @@ void vga_hw_invalidate(void) > > > active_console->hw_invalidate(active_console->hw); > > > } > > > > > > -void vga_hw_screen_dump(const char *filename) > > > +static void vga_hw_screen_dump_helper(const char *filename, bool async) > > > { > > > + bool event_sent = false; > > > TextConsole *previous_active_console; > > > bool cswitch; > > > > > > @@ -188,17 +190,33 @@ void vga_hw_screen_dump(const char *filename) > > > if (cswitch) { > > > console_select(0); > > > } > > > - if (consoles[0] && consoles[0]->hw_screen_dump) { > > > + if (async && consoles[0] && consoles[0]->hw_screen_dump_async) { > > > + consoles[0]->hw_screen_dump_async(consoles[0]->hw, filename, cswitch); > > > + event_sent = true; > > > + } else if (consoles[0] && consoles[0]->hw_screen_dump) { > > > consoles[0]->hw_screen_dump(consoles[0]->hw, filename, cswitch); > > > } else { > > > error_report("screen dump not implemented"); > > > } > > > + if (async && !event_sent) { > > > + monitor_protocol_screen_dump_complete_event(filename); > > > + } > > > > This is wrong. The event should be emitted by hw_screen_dump_async() when it > > completes. > > So you think it's better to add a intermediate callback there? ok, I'll try to > do that. A completion callback, yes. But I'd wait for a conclusion in the other thread regarding if we should add the asynchronous command or not.
On Wed, Feb 29, 2012 at 11:58:37AM -0300, Luiz Capitulino wrote: > On Wed, 29 Feb 2012 10:15:53 +0200 > Alon Levy <alevy@redhat.com> wrote: > > > On Tue, Feb 28, 2012 at 05:10:39PM -0300, Luiz Capitulino wrote: > > > On Sat, 25 Feb 2012 10:46:07 +0200 > > > Alon Levy <alevy@redhat.com> wrote: > > > > > > > On Fri, Feb 24, 2012 at 04:40:15PM -0600, Anthony Liguori wrote: > > > > > On 02/24/2012 03:22 PM, Alon Levy wrote: > > > > > >This is an across the board change since I wanted to keep the existing > > > > > >(good imo) single graphic_console_init callback setter, instead of > > > > > >introducing a new cb that isn't set by it but instead by a second > > > > > >initialization function. > > > > > > > > > > > >Signed-off-by: Alon Levy<alevy@redhat.com> > > > > > > > > > > What's the rationale for this? > > > > > > > > There is a hang possible with the current screendump command, qxl, a > > > > spice client using libvirt and spice-gtk such as virt-viewer / > > > > remote-viewer, where you have: > > > > 1. libvirt waiting for screendump to complete > > > > 2. screendump waiting for spice server thread to render > > > > 3. spice server thread waiting for spice client to read messages > > > > > > Which messages? > > > > spice display channel messages. > > > > > > > > > 4. spice client == libvirt client, waiting for screendump completion > > > > > > The way I had understood this problem is that qxl takes a long time to > > > perform a screen dump, which would cause the global mutex to be held for > > > a long time. If this is really serious, then a async command for it > > > makes sense IMO. > > > > That is true, but it is not the immediate problem the bz is dealing with > > - if it was only this there would be no hang. > > Well, this kind of hang always smells like a spice threading synchronization > problem to me. I thought that I'd be capable of showing that if I really > understood what was going on, but I can't, even with your diagram. It isn't a spice threading synchronization problem, it's much simpler then that: spice server flushes messages to the spice client, so the client not handling them causes it to sleep for a very long time (the timeout is 150 seconds) server: handle_dev_update/flush_display_commands The client in this case is not reading from the socket. I couldn't reproduce, but I imagined it was simply waiting for completion of the screendump command before it came around to reading the spice sockets. > > An asynchronous command solves the global mutex contention problem, but I > think this hang should be further investigated, otherwise the async command > risks just hiding the real problem.
On Tue, Feb 28, 2012 at 05:05:32PM -0300, Luiz Capitulino wrote: > On Fri, 24 Feb 2012 23:22:05 +0200 > Alon Levy <alevy@redhat.com> wrote: > > > This is an across the board change since I wanted to keep the existing > > (good imo) single graphic_console_init callback setter, instead of > > introducing a new cb that isn't set by it but instead by a second > > initialization function. > > > > Signed-off-by: Alon Levy <alevy@redhat.com> > > --- > > console.c | 25 +++++++++++++++++++++++-- > > 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 ++ > > monitor.c | 5 +++++ > > qapi-schema.json | 20 ++++++++++++++++++++ > > qmp-commands.hx | 26 ++++++++++++++++++++++++++ > > 28 files changed, 115 insertions(+), 25 deletions(-) > > > > diff --git a/console.c b/console.c > > index 6750538..ced2aa7 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; > > > > @@ -175,8 +176,9 @@ void vga_hw_invalidate(void) > > active_console->hw_invalidate(active_console->hw); > > } > > > > -void vga_hw_screen_dump(const char *filename) > > +static void vga_hw_screen_dump_helper(const char *filename, bool async) > > { > > + bool event_sent = false; > > TextConsole *previous_active_console; > > bool cswitch; > > > > @@ -188,17 +190,33 @@ void vga_hw_screen_dump(const char *filename) > > if (cswitch) { > > console_select(0); > > } > > - if (consoles[0] && consoles[0]->hw_screen_dump) { > > + if (async && consoles[0] && consoles[0]->hw_screen_dump_async) { > > + consoles[0]->hw_screen_dump_async(consoles[0]->hw, filename, cswitch); > > + event_sent = true; > > + } else if (consoles[0] && consoles[0]->hw_screen_dump) { > > consoles[0]->hw_screen_dump(consoles[0]->hw, filename, cswitch); > > } else { > > error_report("screen dump not implemented"); > > } > > + if (async && !event_sent) { > > + monitor_protocol_screen_dump_complete_event(filename); > > + } > > This is wrong. The event should be emitted by hw_screen_dump_async() when it > completes. I misunderstood before. The variable names may be misleading, but this is the logic: if async requested: if console has hw_screen_dump_async callback use it else use hw_screen_dump, and send the event ourselves. So actually it's exactly what you wanted, i.e. that hw_screen_dump_async will be responsible for calling the monitor callback, but with the added benefit that any implementation of hw_screen_dump is automatically an implmementation of the new async interface. The point is that the only backends that should change implementation are those that actually need it, i.e. at this time only qxl. > > Also, this patch should be split into a least two patches. One patch does the > console changes and the other one adds the qmp async command. It's also possible > to make more patches by having individual devices' implementation of > hw_screen_dump_async() in their own patches. > > > > > if (cswitch) { > > console_select(previous_active_console->index); > > } > > } > > > > +void vga_hw_screen_dump(const char *filename) > > +{ > > + vga_hw_screen_dump_helper(filename, false); > > +} > > + > > +void vga_hw_screen_dump_async(const char *filename) > > +{ > > + vga_hw_screen_dump_helper(filename, true); > > +} > > + > > void vga_hw_text_update(console_ch_t *chardata) > > { > > if (active_console && active_console->hw_text_update) > > @@ -1403,6 +1421,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 +1441,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); > > > > void vga_hw_update(void); > > void vga_hw_invalidate(void); > > void vga_hw_screen_dump(const char *filename); > > +void vga_hw_screen_dump_async(const char *filename); > > void vga_hw_text_update(console_ch_t *chardata); > > void monitor_protocol_screen_dump_complete_event(const char *filename); > > > > diff --git a/hw/blizzard.c b/hw/blizzard.c > > index c7d844d..cc51045 100644 > > --- a/hw/blizzard.c > > +++ b/hw/blizzard.c > > @@ -963,7 +963,7 @@ void *s1d13745_init(qemu_irq gpio_int) > > > > s->state = graphic_console_init(blizzard_update_display, > > blizzard_invalidate_display, > > - blizzard_screen_dump, NULL, s); > > + blizzard_screen_dump, NULL, NULL, s); > > > > switch (ds_get_bits_per_pixel(s->state)) { > > case 0: > > diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c > > index 4edcb94..d2a9c27 100644 > > --- a/hw/cirrus_vga.c > > +++ b/hw/cirrus_vga.c > > @@ -2900,7 +2900,7 @@ static int vga_initfn(ISADevice *dev) > > isa_address_space(dev)); > > s->ds = graphic_console_init(s->update, s->invalidate, > > s->screen_dump, s->text_update, > > - s); > > + s->screen_dump_async, s); > > rom_add_vga(VGABIOS_CIRRUS_FILENAME); > > /* XXX ISA-LFB support */ > > /* FIXME not qdev yet */ > > @@ -2941,7 +2941,7 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev) > > cirrus_init_common(s, device_id, 1, pci_address_space(dev)); > > s->vga.ds = graphic_console_init(s->vga.update, s->vga.invalidate, > > s->vga.screen_dump, s->vga.text_update, > > - &s->vga); > > + s->vga.screen_dump_async, &s->vga); > > > > /* setup PCI */ > > > > diff --git a/hw/exynos4210_fimd.c b/hw/exynos4210_fimd.c > > index 3313f00..2053ed0 100644 > > --- a/hw/exynos4210_fimd.c > > +++ b/hw/exynos4210_fimd.c > > @@ -1898,7 +1898,8 @@ static int exynos4210_fimd_init(SysBusDevice *dev) > > "exynos4210.fimd", FIMD_REGS_SIZE); > > sysbus_init_mmio(dev, &s->iomem); > > s->console = graphic_console_init(exynos4210_fimd_update, > > - exynos4210_fimd_invalidate, NULL, NULL, s); > > + exynos4210_fimd_invalidate, > > + NULL, NULL, NULL, s); > > > > return 0; > > } > > diff --git a/hw/g364fb.c b/hw/g364fb.c > > index 3a0b68f..8ae40d8 100644 > > --- a/hw/g364fb.c > > +++ b/hw/g364fb.c > > @@ -517,7 +517,7 @@ static void g364fb_init(DeviceState *dev, G364State *s) > > > > s->ds = graphic_console_init(g364fb_update_display, > > g364fb_invalidate_display, > > - g364fb_screen_dump, NULL, s); > > + g364fb_screen_dump, NULL, NULL, s); > > > > memory_region_init_io(&s->mem_ctrl, &g364fb_ctrl_ops, s, "ctrl", 0x180000); > > memory_region_init_ram_ptr(&s->mem_vram, "vram", > > diff --git a/hw/jazz_led.c b/hw/jazz_led.c > > index 6486523..bb5daf7 100644 > > --- a/hw/jazz_led.c > > +++ b/hw/jazz_led.c > > @@ -251,7 +251,8 @@ static int jazz_led_init(SysBusDevice *dev) > > s->ds = graphic_console_init(jazz_led_update_display, > > jazz_led_invalidate_display, > > NULL, > > - jazz_led_text_update, s); > > + jazz_led_text_update, > > + NULL, s); > > > > return 0; > > } > > diff --git a/hw/milkymist-vgafb.c b/hw/milkymist-vgafb.c > > index 69afd72..ae018d1 100644 > > --- a/hw/milkymist-vgafb.c > > +++ b/hw/milkymist-vgafb.c > > @@ -276,7 +276,7 @@ static int milkymist_vgafb_init(SysBusDevice *dev) > > > > s->ds = graphic_console_init(vgafb_update_display, > > vgafb_invalidate_display, > > - NULL, NULL, s); > > + NULL, NULL, NULL, s); > > > > return 0; > > } > > diff --git a/hw/musicpal.c b/hw/musicpal.c > > index 187a1ae..2fc391b 100644 > > --- a/hw/musicpal.c > > +++ b/hw/musicpal.c > > @@ -611,7 +611,7 @@ static int musicpal_lcd_init(SysBusDevice *dev) > > sysbus_init_mmio(dev, &s->iomem); > > > > s->ds = graphic_console_init(lcd_refresh, lcd_invalidate, > > - NULL, NULL, s); > > + NULL, NULL, NULL, s); > > qemu_console_resize(s->ds, 128*3, 64*3); > > > > qdev_init_gpio_in(&dev->qdev, musicpal_lcd_gpio_brigthness_in, 3); > > diff --git a/hw/omap_dss.c b/hw/omap_dss.c > > index 86ed6ea..0e8243e 100644 > > --- a/hw/omap_dss.c > > +++ b/hw/omap_dss.c > > @@ -1072,7 +1072,9 @@ struct omap_dss_s *omap_dss_init(struct omap_target_agent_s *ta, > > > > #if 0 > > s->state = graphic_console_init(omap_update_display, > > - omap_invalidate_display, omap_screen_dump, s); > > + omap_invalidate_display, > > + omap_screen_dump, > > + NULL, NULL, s); > > #endif > > > > return s; > > diff --git a/hw/omap_lcdc.c b/hw/omap_lcdc.c > > index f172093..d91e88d 100644 > > --- a/hw/omap_lcdc.c > > +++ b/hw/omap_lcdc.c > > @@ -452,7 +452,7 @@ struct omap_lcd_panel_s *omap_lcdc_init(MemoryRegion *sysmem, > > > > s->state = graphic_console_init(omap_update_display, > > omap_invalidate_display, > > - omap_screen_dump, NULL, s); > > + omap_screen_dump, NULL, NULL, s); > > > > return s; > > } > > diff --git a/hw/pl110.c b/hw/pl110.c > > index f94608c..32ee89c 100644 > > --- a/hw/pl110.c > > +++ b/hw/pl110.c > > @@ -451,7 +451,7 @@ static int pl110_init(SysBusDevice *dev) > > qdev_init_gpio_in(&s->busdev.qdev, pl110_mux_ctrl_set, 1); > > s->ds = graphic_console_init(pl110_update_display, > > pl110_invalidate_display, > > - NULL, NULL, s); > > + NULL, NULL, NULL, s); > > return 0; > > } > > > > diff --git a/hw/pxa2xx_lcd.c b/hw/pxa2xx_lcd.c > > index fcbdfb3..5345fb4 100644 > > --- a/hw/pxa2xx_lcd.c > > +++ b/hw/pxa2xx_lcd.c > > @@ -1004,7 +1004,7 @@ PXA2xxLCDState *pxa2xx_lcdc_init(MemoryRegion *sysmem, > > > > s->ds = graphic_console_init(pxa2xx_update_display, > > pxa2xx_invalidate_display, > > - NULL, NULL, s); > > + NULL, NULL, NULL, s); > > > > switch (ds_get_bits_per_pixel(s->ds)) { > > case 0: > > diff --git a/hw/qxl.c b/hw/qxl.c > > index d83d245..61a7edd 100644 > > --- a/hw/qxl.c > > +++ b/hw/qxl.c > > @@ -1727,7 +1727,8 @@ static int qxl_init_primary(PCIDevice *dev) > > portio_list_add(qxl_vga_port_list, pci_address_space_io(dev), 0x3b0); > > > > vga->ds = graphic_console_init(qxl_hw_update, qxl_hw_invalidate, > > - qxl_hw_screen_dump, qxl_hw_text_update, qxl); > > + qxl_hw_screen_dump, qxl_hw_text_update, > > + NULL, qxl); > > qemu_spice_display_init_common(&qxl->ssd, vga->ds); > > > > qxl0 = qxl; > > diff --git a/hw/sm501.c b/hw/sm501.c > > index 786e076..8b150be 100644 > > --- a/hw/sm501.c > > +++ b/hw/sm501.c > > @@ -1442,6 +1442,6 @@ void sm501_init(MemoryRegion *address_space_mem, uint32_t base, > > } > > > > /* create qemu graphic console */ > > - s->ds = graphic_console_init(sm501_update_display, NULL, > > - NULL, NULL, s); > > + s->ds = graphic_console_init(sm501_update_display, > > + NULL, NULL, NULL, NULL, s); > > } > > diff --git a/hw/ssd0303.c b/hw/ssd0303.c > > index 4e1ee6e..05e6686 100644 > > --- a/hw/ssd0303.c > > +++ b/hw/ssd0303.c > > @@ -289,7 +289,7 @@ static int ssd0303_init(I2CSlave *i2c) > > > > s->ds = graphic_console_init(ssd0303_update_display, > > ssd0303_invalidate_display, > > - NULL, NULL, s); > > + NULL, NULL, NULL, s); > > qemu_console_resize(s->ds, 96 * MAGNIFY, 16 * MAGNIFY); > > return 0; > > } > > diff --git a/hw/ssd0323.c b/hw/ssd0323.c > > index b0b2e94..5e3491f 100644 > > --- a/hw/ssd0323.c > > +++ b/hw/ssd0323.c > > @@ -330,7 +330,7 @@ static int ssd0323_init(SSISlave *dev) > > s->row_end = 79; > > s->ds = graphic_console_init(ssd0323_update_display, > > ssd0323_invalidate_display, > > - NULL, NULL, s); > > + NULL, NULL, NULL, s); > > qemu_console_resize(s->ds, 128 * MAGNIFY, 64 * MAGNIFY); > > > > qdev_init_gpio_in(&dev->qdev, ssd0323_cd, 1); > > diff --git a/hw/tc6393xb.c b/hw/tc6393xb.c > > index 420925c..853b180 100644 > > --- a/hw/tc6393xb.c > > +++ b/hw/tc6393xb.c > > @@ -581,6 +581,7 @@ TC6393xbState *tc6393xb_init(MemoryRegion *sysmem, uint32_t base, qemu_irq irq) > > NULL, /* invalidate */ > > NULL, /* screen_dump */ > > NULL, /* text_update */ > > + NULL, /* screen_dump_async */ > > s); > > > > return s; > > diff --git a/hw/tcx.c b/hw/tcx.c > > index ac7dcb4..9214dec 100644 > > --- a/hw/tcx.c > > +++ b/hw/tcx.c > > @@ -558,7 +558,7 @@ static int tcx_init1(SysBusDevice *dev) > > > > s->ds = graphic_console_init(tcx24_update_display, > > tcx24_invalidate_display, > > - tcx24_screen_dump, NULL, s); > > + tcx24_screen_dump, NULL, NULL, s); > > } else { > > /* THC 8 bit (dummy) */ > > memory_region_init_io(&s->thc8, &dummy_ops, s, "tcx.thc8", > > @@ -567,7 +567,7 @@ static int tcx_init1(SysBusDevice *dev) > > > > s->ds = graphic_console_init(tcx_update_display, > > tcx_invalidate_display, > > - tcx_screen_dump, NULL, s); > > + tcx_screen_dump, NULL, NULL, s); > > } > > > > qemu_console_resize(s->ds, s->width, s->height); > > diff --git a/hw/vga-isa-mm.c b/hw/vga-isa-mm.c > > index f8984c6..87b8fc6 100644 > > --- a/hw/vga-isa-mm.c > > +++ b/hw/vga-isa-mm.c > > @@ -132,7 +132,8 @@ int isa_vga_mm_init(target_phys_addr_t vram_base, > > vga_mm_init(s, vram_base, ctrl_base, it_shift, address_space); > > > > s->vga.ds = graphic_console_init(s->vga.update, s->vga.invalidate, > > - s->vga.screen_dump, s->vga.text_update, s); > > + s->vga.screen_dump, s->vga.text_update, > > + s->vga.screen_dump_async, s); > > > > vga_init_vbe(&s->vga, address_space); > > return 0; > > diff --git a/hw/vga-isa.c b/hw/vga-isa.c > > index 4bcc4db..a362ed2 100644 > > --- a/hw/vga-isa.c > > +++ b/hw/vga-isa.c > > @@ -61,7 +61,8 @@ static int vga_initfn(ISADevice *dev) > > vga_io_memory, 1); > > memory_region_set_coalescing(vga_io_memory); > > s->ds = graphic_console_init(s->update, s->invalidate, > > - s->screen_dump, s->text_update, s); > > + s->screen_dump, s->text_update, > > + s->screen_dump_async, s); > > > > vga_init_vbe(s, isa_address_space(dev)); > > /* ROM BIOS */ > > diff --git a/hw/vga-pci.c b/hw/vga-pci.c > > index 465b643..8c8dd53 100644 > > --- a/hw/vga-pci.c > > +++ b/hw/vga-pci.c > > @@ -57,7 +57,8 @@ static int pci_vga_initfn(PCIDevice *dev) > > vga_init(s, pci_address_space(dev), pci_address_space_io(dev), true); > > > > s->ds = graphic_console_init(s->update, s->invalidate, > > - s->screen_dump, s->text_update, s); > > + s->screen_dump, s->text_update, > > + s->screen_dump_async, s); > > > > /* XXX: VGA_RAM_SIZE must be a power of two */ > > pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_MEM_PREFETCH, &s->vram); > > diff --git a/hw/vga_int.h b/hw/vga_int.h > > index 7685b2b..4486cfb 100644 > > --- a/hw/vga_int.h > > +++ b/hw/vga_int.h > > @@ -161,6 +161,7 @@ typedef struct VGACommonState { > > 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; > > /* hardware mouse cursor support */ > > uint32_t invalidated_y_table[VGA_MAX_HEIGHT / 32]; > > void (*cursor_invalidate)(struct VGACommonState *s); > > diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c > > index 142d9f4..db96e61 100644 > > --- a/hw/vmware_vga.c > > +++ b/hw/vmware_vga.c > > @@ -1087,7 +1087,8 @@ static void vmsvga_init(struct vmsvga_state_s *s, int vga_ram_size, > > s->vga.ds = graphic_console_init(vmsvga_update_display, > > vmsvga_invalidate_display, > > vmsvga_screen_dump, > > - vmsvga_text_update, s); > > + vmsvga_text_update, > > + NULL, s); > > > > > > s->fifo_size = SVGA_FIFO_SIZE; > > diff --git a/hw/xenfb.c b/hw/xenfb.c > > index 1bcf171..ad9f0d7 100644 > > --- a/hw/xenfb.c > > +++ b/hw/xenfb.c > > @@ -906,6 +906,7 @@ static int fb_initialise(struct XenDevice *xendev) > > xenfb_invalidate, > > NULL, > > NULL, > > + NULL, > > fb); > > fb->have_console = 1; > > } > > @@ -1015,6 +1016,7 @@ wait_more: > > xenfb_invalidate, > > NULL, > > NULL, > > + NULL, > > fb); > > fb->have_console = 1; > > > > diff --git a/monitor.c b/monitor.c > > index 1a65c41..91a76a1 100644 > > --- a/monitor.c > > +++ b/monitor.c > > @@ -901,6 +901,11 @@ static int do_screen_dump(Monitor *mon, const QDict *qdict, QObject **ret_data) > > return 0; > > } > > > > +void qmp_screendump_async(const char *filename, Error **errp) > > +{ > > + vga_hw_screen_dump_async(filename); > > +} > > + > > static void do_logfile(Monitor *mon, const QDict *qdict) > > { > > cpu_set_log_filename(qdict_get_str(qdict, "filename")); > > diff --git a/qapi-schema.json b/qapi-schema.json > > index 72b17f1..a5873cb 100644 > > --- a/qapi-schema.json > > +++ b/qapi-schema.json > > @@ -1595,3 +1595,23 @@ > > { 'command': 'qom-list-types', > > 'data': { '*implements': 'str', '*abstract': 'bool' }, > > 'returns': [ 'ObjectTypeInfo' ] } > > + > > +## > > +## @screendump-async: > > +# > > +# This command will perform a screen dump of the first console to the givem > > +# filename. The additional parameters are unused at this time. > > Isn't it interesting to allow the mngt app to specify the console? > > > +# > > +# @filename name of output file to write screen dump to > > +# > > +# Since: 1.1 > > +# > > +# Notes: This command is experimental and may change syntax in future releases. > > Why is this needed? > > > +# > > +# This command is the same as the qmp/hmp screendump command, except that on > > +# successful completion of the scren dump the SCREEN_DUMP_COMPLETE event is > > +# emitted. > > The real difference is that it's asynchronous, while the other is synchronous. > > > +# > > +## > > +{ 'command': 'screendump-async', > > + 'data': { 'filename': 'str' } } > > diff --git a/qmp-commands.hx b/qmp-commands.hx > > index 705f704..ea0d051 100644 > > --- a/qmp-commands.hx > > +++ b/qmp-commands.hx > > @@ -170,6 +170,32 @@ Example: > > EQMP > > > > { > > + .name = "screendump-async", > > + .args_type = "filename:F", > > + .params = "filename", > > + .help = "save screen into PPM image 'filename'", > > + .user_print = monitor_user_noop, > > + .mhandler.cmd_new = qmp_marshal_input_screendump_async, > > + }, > > + > > +SQMP > > +screendump-async > > +---------------- > > + > > +Save screen into PPM image. Fires a SCREEN_DUMP_COMPLETE event on completion. > > + > > +Arguments: > > + > > +- "filename": file path (json-string) > > + > > +Example: > > + > > +-> { "execute": "screendump-async", "arguments": { "filename": "/tmp/image" } } > > +<- { "return": {} } > > + > > +EQMP > > + > > + { > > .name = "stop", > > .args_type = "", > > .mhandler.cmd_new = qmp_marshal_input_stop, > >
On Tue, Feb 28, 2012 at 05:05:32PM -0300, Luiz Capitulino wrote: > On Fri, 24 Feb 2012 23:22:05 +0200 > Alon Levy <alevy@redhat.com> wrote: > > > This is an across the board change since I wanted to keep the existing > > (good imo) single graphic_console_init callback setter, instead of > > introducing a new cb that isn't set by it but instead by a second > > initialization function. > > > > Signed-off-by: Alon Levy <alevy@redhat.com> > > --- > > console.c | 25 +++++++++++++++++++++++-- > > 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 ++ > > monitor.c | 5 +++++ > > qapi-schema.json | 20 ++++++++++++++++++++ > > qmp-commands.hx | 26 ++++++++++++++++++++++++++ > > 28 files changed, 115 insertions(+), 25 deletions(-) > > > > diff --git a/console.c b/console.c > > index 6750538..ced2aa7 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; > > > > @@ -175,8 +176,9 @@ void vga_hw_invalidate(void) > > active_console->hw_invalidate(active_console->hw); > > } > > > > -void vga_hw_screen_dump(const char *filename) > > +static void vga_hw_screen_dump_helper(const char *filename, bool async) > > { > > + bool event_sent = false; > > TextConsole *previous_active_console; > > bool cswitch; > > > > @@ -188,17 +190,33 @@ void vga_hw_screen_dump(const char *filename) > > if (cswitch) { > > console_select(0); > > } > > - if (consoles[0] && consoles[0]->hw_screen_dump) { > > + if (async && consoles[0] && consoles[0]->hw_screen_dump_async) { > > + consoles[0]->hw_screen_dump_async(consoles[0]->hw, filename, cswitch); > > + event_sent = true; > > + } else if (consoles[0] && consoles[0]->hw_screen_dump) { > > consoles[0]->hw_screen_dump(consoles[0]->hw, filename, cswitch); > > } else { > > error_report("screen dump not implemented"); > > } > > + if (async && !event_sent) { > > + monitor_protocol_screen_dump_complete_event(filename); > > + } > > This is wrong. The event should be emitted by hw_screen_dump_async() when it > completes. > > Also, this patch should be split into a least two patches. One patch does the > console changes and the other one adds the qmp async command. It's also possible > to make more patches by having individual devices' implementation of > hw_screen_dump_async() in their own patches. There are no implementations of hw_screen_dump_async in this patch, but I'll do the first split you suggested. > > > > > if (cswitch) { > > console_select(previous_active_console->index); > > } > > } > > > > +void vga_hw_screen_dump(const char *filename) > > +{ > > + vga_hw_screen_dump_helper(filename, false); > > +} > > + > > +void vga_hw_screen_dump_async(const char *filename) > > +{ > > + vga_hw_screen_dump_helper(filename, true); > > +} > > + > > void vga_hw_text_update(console_ch_t *chardata) > > { > > if (active_console && active_console->hw_text_update) > > @@ -1403,6 +1421,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 +1441,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); > > > > void vga_hw_update(void); > > void vga_hw_invalidate(void); > > void vga_hw_screen_dump(const char *filename); > > +void vga_hw_screen_dump_async(const char *filename); > > void vga_hw_text_update(console_ch_t *chardata); > > void monitor_protocol_screen_dump_complete_event(const char *filename); > > > > diff --git a/hw/blizzard.c b/hw/blizzard.c > > index c7d844d..cc51045 100644 > > --- a/hw/blizzard.c > > +++ b/hw/blizzard.c > > @@ -963,7 +963,7 @@ void *s1d13745_init(qemu_irq gpio_int) > > > > s->state = graphic_console_init(blizzard_update_display, > > blizzard_invalidate_display, > > - blizzard_screen_dump, NULL, s); > > + blizzard_screen_dump, NULL, NULL, s); > > > > switch (ds_get_bits_per_pixel(s->state)) { > > case 0: > > diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c > > index 4edcb94..d2a9c27 100644 > > --- a/hw/cirrus_vga.c > > +++ b/hw/cirrus_vga.c > > @@ -2900,7 +2900,7 @@ static int vga_initfn(ISADevice *dev) > > isa_address_space(dev)); > > s->ds = graphic_console_init(s->update, s->invalidate, > > s->screen_dump, s->text_update, > > - s); > > + s->screen_dump_async, s); > > rom_add_vga(VGABIOS_CIRRUS_FILENAME); > > /* XXX ISA-LFB support */ > > /* FIXME not qdev yet */ > > @@ -2941,7 +2941,7 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev) > > cirrus_init_common(s, device_id, 1, pci_address_space(dev)); > > s->vga.ds = graphic_console_init(s->vga.update, s->vga.invalidate, > > s->vga.screen_dump, s->vga.text_update, > > - &s->vga); > > + s->vga.screen_dump_async, &s->vga); > > > > /* setup PCI */ > > > > diff --git a/hw/exynos4210_fimd.c b/hw/exynos4210_fimd.c > > index 3313f00..2053ed0 100644 > > --- a/hw/exynos4210_fimd.c > > +++ b/hw/exynos4210_fimd.c > > @@ -1898,7 +1898,8 @@ static int exynos4210_fimd_init(SysBusDevice *dev) > > "exynos4210.fimd", FIMD_REGS_SIZE); > > sysbus_init_mmio(dev, &s->iomem); > > s->console = graphic_console_init(exynos4210_fimd_update, > > - exynos4210_fimd_invalidate, NULL, NULL, s); > > + exynos4210_fimd_invalidate, > > + NULL, NULL, NULL, s); > > > > return 0; > > } > > diff --git a/hw/g364fb.c b/hw/g364fb.c > > index 3a0b68f..8ae40d8 100644 > > --- a/hw/g364fb.c > > +++ b/hw/g364fb.c > > @@ -517,7 +517,7 @@ static void g364fb_init(DeviceState *dev, G364State *s) > > > > s->ds = graphic_console_init(g364fb_update_display, > > g364fb_invalidate_display, > > - g364fb_screen_dump, NULL, s); > > + g364fb_screen_dump, NULL, NULL, s); > > > > memory_region_init_io(&s->mem_ctrl, &g364fb_ctrl_ops, s, "ctrl", 0x180000); > > memory_region_init_ram_ptr(&s->mem_vram, "vram", > > diff --git a/hw/jazz_led.c b/hw/jazz_led.c > > index 6486523..bb5daf7 100644 > > --- a/hw/jazz_led.c > > +++ b/hw/jazz_led.c > > @@ -251,7 +251,8 @@ static int jazz_led_init(SysBusDevice *dev) > > s->ds = graphic_console_init(jazz_led_update_display, > > jazz_led_invalidate_display, > > NULL, > > - jazz_led_text_update, s); > > + jazz_led_text_update, > > + NULL, s); > > > > return 0; > > } > > diff --git a/hw/milkymist-vgafb.c b/hw/milkymist-vgafb.c > > index 69afd72..ae018d1 100644 > > --- a/hw/milkymist-vgafb.c > > +++ b/hw/milkymist-vgafb.c > > @@ -276,7 +276,7 @@ static int milkymist_vgafb_init(SysBusDevice *dev) > > > > s->ds = graphic_console_init(vgafb_update_display, > > vgafb_invalidate_display, > > - NULL, NULL, s); > > + NULL, NULL, NULL, s); > > > > return 0; > > } > > diff --git a/hw/musicpal.c b/hw/musicpal.c > > index 187a1ae..2fc391b 100644 > > --- a/hw/musicpal.c > > +++ b/hw/musicpal.c > > @@ -611,7 +611,7 @@ static int musicpal_lcd_init(SysBusDevice *dev) > > sysbus_init_mmio(dev, &s->iomem); > > > > s->ds = graphic_console_init(lcd_refresh, lcd_invalidate, > > - NULL, NULL, s); > > + NULL, NULL, NULL, s); > > qemu_console_resize(s->ds, 128*3, 64*3); > > > > qdev_init_gpio_in(&dev->qdev, musicpal_lcd_gpio_brigthness_in, 3); > > diff --git a/hw/omap_dss.c b/hw/omap_dss.c > > index 86ed6ea..0e8243e 100644 > > --- a/hw/omap_dss.c > > +++ b/hw/omap_dss.c > > @@ -1072,7 +1072,9 @@ struct omap_dss_s *omap_dss_init(struct omap_target_agent_s *ta, > > > > #if 0 > > s->state = graphic_console_init(omap_update_display, > > - omap_invalidate_display, omap_screen_dump, s); > > + omap_invalidate_display, > > + omap_screen_dump, > > + NULL, NULL, s); > > #endif > > > > return s; > > diff --git a/hw/omap_lcdc.c b/hw/omap_lcdc.c > > index f172093..d91e88d 100644 > > --- a/hw/omap_lcdc.c > > +++ b/hw/omap_lcdc.c > > @@ -452,7 +452,7 @@ struct omap_lcd_panel_s *omap_lcdc_init(MemoryRegion *sysmem, > > > > s->state = graphic_console_init(omap_update_display, > > omap_invalidate_display, > > - omap_screen_dump, NULL, s); > > + omap_screen_dump, NULL, NULL, s); > > > > return s; > > } > > diff --git a/hw/pl110.c b/hw/pl110.c > > index f94608c..32ee89c 100644 > > --- a/hw/pl110.c > > +++ b/hw/pl110.c > > @@ -451,7 +451,7 @@ static int pl110_init(SysBusDevice *dev) > > qdev_init_gpio_in(&s->busdev.qdev, pl110_mux_ctrl_set, 1); > > s->ds = graphic_console_init(pl110_update_display, > > pl110_invalidate_display, > > - NULL, NULL, s); > > + NULL, NULL, NULL, s); > > return 0; > > } > > > > diff --git a/hw/pxa2xx_lcd.c b/hw/pxa2xx_lcd.c > > index fcbdfb3..5345fb4 100644 > > --- a/hw/pxa2xx_lcd.c > > +++ b/hw/pxa2xx_lcd.c > > @@ -1004,7 +1004,7 @@ PXA2xxLCDState *pxa2xx_lcdc_init(MemoryRegion *sysmem, > > > > s->ds = graphic_console_init(pxa2xx_update_display, > > pxa2xx_invalidate_display, > > - NULL, NULL, s); > > + NULL, NULL, NULL, s); > > > > switch (ds_get_bits_per_pixel(s->ds)) { > > case 0: > > diff --git a/hw/qxl.c b/hw/qxl.c > > index d83d245..61a7edd 100644 > > --- a/hw/qxl.c > > +++ b/hw/qxl.c > > @@ -1727,7 +1727,8 @@ static int qxl_init_primary(PCIDevice *dev) > > portio_list_add(qxl_vga_port_list, pci_address_space_io(dev), 0x3b0); > > > > vga->ds = graphic_console_init(qxl_hw_update, qxl_hw_invalidate, > > - qxl_hw_screen_dump, qxl_hw_text_update, qxl); > > + qxl_hw_screen_dump, qxl_hw_text_update, > > + NULL, qxl); > > qemu_spice_display_init_common(&qxl->ssd, vga->ds); > > > > qxl0 = qxl; > > diff --git a/hw/sm501.c b/hw/sm501.c > > index 786e076..8b150be 100644 > > --- a/hw/sm501.c > > +++ b/hw/sm501.c > > @@ -1442,6 +1442,6 @@ void sm501_init(MemoryRegion *address_space_mem, uint32_t base, > > } > > > > /* create qemu graphic console */ > > - s->ds = graphic_console_init(sm501_update_display, NULL, > > - NULL, NULL, s); > > + s->ds = graphic_console_init(sm501_update_display, > > + NULL, NULL, NULL, NULL, s); > > } > > diff --git a/hw/ssd0303.c b/hw/ssd0303.c > > index 4e1ee6e..05e6686 100644 > > --- a/hw/ssd0303.c > > +++ b/hw/ssd0303.c > > @@ -289,7 +289,7 @@ static int ssd0303_init(I2CSlave *i2c) > > > > s->ds = graphic_console_init(ssd0303_update_display, > > ssd0303_invalidate_display, > > - NULL, NULL, s); > > + NULL, NULL, NULL, s); > > qemu_console_resize(s->ds, 96 * MAGNIFY, 16 * MAGNIFY); > > return 0; > > } > > diff --git a/hw/ssd0323.c b/hw/ssd0323.c > > index b0b2e94..5e3491f 100644 > > --- a/hw/ssd0323.c > > +++ b/hw/ssd0323.c > > @@ -330,7 +330,7 @@ static int ssd0323_init(SSISlave *dev) > > s->row_end = 79; > > s->ds = graphic_console_init(ssd0323_update_display, > > ssd0323_invalidate_display, > > - NULL, NULL, s); > > + NULL, NULL, NULL, s); > > qemu_console_resize(s->ds, 128 * MAGNIFY, 64 * MAGNIFY); > > > > qdev_init_gpio_in(&dev->qdev, ssd0323_cd, 1); > > diff --git a/hw/tc6393xb.c b/hw/tc6393xb.c > > index 420925c..853b180 100644 > > --- a/hw/tc6393xb.c > > +++ b/hw/tc6393xb.c > > @@ -581,6 +581,7 @@ TC6393xbState *tc6393xb_init(MemoryRegion *sysmem, uint32_t base, qemu_irq irq) > > NULL, /* invalidate */ > > NULL, /* screen_dump */ > > NULL, /* text_update */ > > + NULL, /* screen_dump_async */ > > s); > > > > return s; > > diff --git a/hw/tcx.c b/hw/tcx.c > > index ac7dcb4..9214dec 100644 > > --- a/hw/tcx.c > > +++ b/hw/tcx.c > > @@ -558,7 +558,7 @@ static int tcx_init1(SysBusDevice *dev) > > > > s->ds = graphic_console_init(tcx24_update_display, > > tcx24_invalidate_display, > > - tcx24_screen_dump, NULL, s); > > + tcx24_screen_dump, NULL, NULL, s); > > } else { > > /* THC 8 bit (dummy) */ > > memory_region_init_io(&s->thc8, &dummy_ops, s, "tcx.thc8", > > @@ -567,7 +567,7 @@ static int tcx_init1(SysBusDevice *dev) > > > > s->ds = graphic_console_init(tcx_update_display, > > tcx_invalidate_display, > > - tcx_screen_dump, NULL, s); > > + tcx_screen_dump, NULL, NULL, s); > > } > > > > qemu_console_resize(s->ds, s->width, s->height); > > diff --git a/hw/vga-isa-mm.c b/hw/vga-isa-mm.c > > index f8984c6..87b8fc6 100644 > > --- a/hw/vga-isa-mm.c > > +++ b/hw/vga-isa-mm.c > > @@ -132,7 +132,8 @@ int isa_vga_mm_init(target_phys_addr_t vram_base, > > vga_mm_init(s, vram_base, ctrl_base, it_shift, address_space); > > > > s->vga.ds = graphic_console_init(s->vga.update, s->vga.invalidate, > > - s->vga.screen_dump, s->vga.text_update, s); > > + s->vga.screen_dump, s->vga.text_update, > > + s->vga.screen_dump_async, s); > > > > vga_init_vbe(&s->vga, address_space); > > return 0; > > diff --git a/hw/vga-isa.c b/hw/vga-isa.c > > index 4bcc4db..a362ed2 100644 > > --- a/hw/vga-isa.c > > +++ b/hw/vga-isa.c > > @@ -61,7 +61,8 @@ static int vga_initfn(ISADevice *dev) > > vga_io_memory, 1); > > memory_region_set_coalescing(vga_io_memory); > > s->ds = graphic_console_init(s->update, s->invalidate, > > - s->screen_dump, s->text_update, s); > > + s->screen_dump, s->text_update, > > + s->screen_dump_async, s); > > > > vga_init_vbe(s, isa_address_space(dev)); > > /* ROM BIOS */ > > diff --git a/hw/vga-pci.c b/hw/vga-pci.c > > index 465b643..8c8dd53 100644 > > --- a/hw/vga-pci.c > > +++ b/hw/vga-pci.c > > @@ -57,7 +57,8 @@ static int pci_vga_initfn(PCIDevice *dev) > > vga_init(s, pci_address_space(dev), pci_address_space_io(dev), true); > > > > s->ds = graphic_console_init(s->update, s->invalidate, > > - s->screen_dump, s->text_update, s); > > + s->screen_dump, s->text_update, > > + s->screen_dump_async, s); > > > > /* XXX: VGA_RAM_SIZE must be a power of two */ > > pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_MEM_PREFETCH, &s->vram); > > diff --git a/hw/vga_int.h b/hw/vga_int.h > > index 7685b2b..4486cfb 100644 > > --- a/hw/vga_int.h > > +++ b/hw/vga_int.h > > @@ -161,6 +161,7 @@ typedef struct VGACommonState { > > 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; > > /* hardware mouse cursor support */ > > uint32_t invalidated_y_table[VGA_MAX_HEIGHT / 32]; > > void (*cursor_invalidate)(struct VGACommonState *s); > > diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c > > index 142d9f4..db96e61 100644 > > --- a/hw/vmware_vga.c > > +++ b/hw/vmware_vga.c > > @@ -1087,7 +1087,8 @@ static void vmsvga_init(struct vmsvga_state_s *s, int vga_ram_size, > > s->vga.ds = graphic_console_init(vmsvga_update_display, > > vmsvga_invalidate_display, > > vmsvga_screen_dump, > > - vmsvga_text_update, s); > > + vmsvga_text_update, > > + NULL, s); > > > > > > s->fifo_size = SVGA_FIFO_SIZE; > > diff --git a/hw/xenfb.c b/hw/xenfb.c > > index 1bcf171..ad9f0d7 100644 > > --- a/hw/xenfb.c > > +++ b/hw/xenfb.c > > @@ -906,6 +906,7 @@ static int fb_initialise(struct XenDevice *xendev) > > xenfb_invalidate, > > NULL, > > NULL, > > + NULL, > > fb); > > fb->have_console = 1; > > } > > @@ -1015,6 +1016,7 @@ wait_more: > > xenfb_invalidate, > > NULL, > > NULL, > > + NULL, > > fb); > > fb->have_console = 1; > > > > diff --git a/monitor.c b/monitor.c > > index 1a65c41..91a76a1 100644 > > --- a/monitor.c > > +++ b/monitor.c > > @@ -901,6 +901,11 @@ static int do_screen_dump(Monitor *mon, const QDict *qdict, QObject **ret_data) > > return 0; > > } > > > > +void qmp_screendump_async(const char *filename, Error **errp) > > +{ > > + vga_hw_screen_dump_async(filename); > > +} > > + > > static void do_logfile(Monitor *mon, const QDict *qdict) > > { > > cpu_set_log_filename(qdict_get_str(qdict, "filename")); > > diff --git a/qapi-schema.json b/qapi-schema.json > > index 72b17f1..a5873cb 100644 > > --- a/qapi-schema.json > > +++ b/qapi-schema.json > > @@ -1595,3 +1595,23 @@ > > { 'command': 'qom-list-types', > > 'data': { '*implements': 'str', '*abstract': 'bool' }, > > 'returns': [ 'ObjectTypeInfo' ] } > > + > > +## > > +## @screendump-async: > > +# > > +# This command will perform a screen dump of the first console to the givem > > +# filename. The additional parameters are unused at this time. > > Isn't it interesting to allow the mngt app to specify the console? > > > +# > > +# @filename name of output file to write screen dump to > > +# > > +# Since: 1.1 > > +# > > +# Notes: This command is experimental and may change syntax in future releases. > > Why is this needed? > > > +# > > +# This command is the same as the qmp/hmp screendump command, except that on > > +# successful completion of the scren dump the SCREEN_DUMP_COMPLETE event is > > +# emitted. > > The real difference is that it's asynchronous, while the other is synchronous. > > > +# > > +## > > +{ 'command': 'screendump-async', > > + 'data': { 'filename': 'str' } } > > diff --git a/qmp-commands.hx b/qmp-commands.hx > > index 705f704..ea0d051 100644 > > --- a/qmp-commands.hx > > +++ b/qmp-commands.hx > > @@ -170,6 +170,32 @@ Example: > > EQMP > > > > { > > + .name = "screendump-async", > > + .args_type = "filename:F", > > + .params = "filename", > > + .help = "save screen into PPM image 'filename'", > > + .user_print = monitor_user_noop, > > + .mhandler.cmd_new = qmp_marshal_input_screendump_async, > > + }, > > + > > +SQMP > > +screendump-async > > +---------------- > > + > > +Save screen into PPM image. Fires a SCREEN_DUMP_COMPLETE event on completion. > > + > > +Arguments: > > + > > +- "filename": file path (json-string) > > + > > +Example: > > + > > +-> { "execute": "screendump-async", "arguments": { "filename": "/tmp/image" } } > > +<- { "return": {} } > > + > > +EQMP > > + > > + { > > .name = "stop", > > .args_type = "", > > .mhandler.cmd_new = qmp_marshal_input_stop, > >
diff --git a/console.c b/console.c index 6750538..ced2aa7 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; @@ -175,8 +176,9 @@ void vga_hw_invalidate(void) active_console->hw_invalidate(active_console->hw); } -void vga_hw_screen_dump(const char *filename) +static void vga_hw_screen_dump_helper(const char *filename, bool async) { + bool event_sent = false; TextConsole *previous_active_console; bool cswitch; @@ -188,17 +190,33 @@ void vga_hw_screen_dump(const char *filename) if (cswitch) { console_select(0); } - if (consoles[0] && consoles[0]->hw_screen_dump) { + if (async && consoles[0] && consoles[0]->hw_screen_dump_async) { + consoles[0]->hw_screen_dump_async(consoles[0]->hw, filename, cswitch); + event_sent = true; + } else if (consoles[0] && consoles[0]->hw_screen_dump) { consoles[0]->hw_screen_dump(consoles[0]->hw, filename, cswitch); } else { error_report("screen dump not implemented"); } + if (async && !event_sent) { + monitor_protocol_screen_dump_complete_event(filename); + } if (cswitch) { console_select(previous_active_console->index); } } +void vga_hw_screen_dump(const char *filename) +{ + vga_hw_screen_dump_helper(filename, false); +} + +void vga_hw_screen_dump_async(const char *filename) +{ + vga_hw_screen_dump_helper(filename, true); +} + void vga_hw_text_update(console_ch_t *chardata) { if (active_console && active_console->hw_text_update) @@ -1403,6 +1421,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 +1441,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); void vga_hw_update(void); void vga_hw_invalidate(void); void vga_hw_screen_dump(const char *filename); +void vga_hw_screen_dump_async(const char *filename); void vga_hw_text_update(console_ch_t *chardata); void monitor_protocol_screen_dump_complete_event(const char *filename); diff --git a/hw/blizzard.c b/hw/blizzard.c index c7d844d..cc51045 100644 --- a/hw/blizzard.c +++ b/hw/blizzard.c @@ -963,7 +963,7 @@ void *s1d13745_init(qemu_irq gpio_int) s->state = graphic_console_init(blizzard_update_display, blizzard_invalidate_display, - blizzard_screen_dump, NULL, s); + blizzard_screen_dump, NULL, NULL, s); switch (ds_get_bits_per_pixel(s->state)) { case 0: diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c index 4edcb94..d2a9c27 100644 --- a/hw/cirrus_vga.c +++ b/hw/cirrus_vga.c @@ -2900,7 +2900,7 @@ static int vga_initfn(ISADevice *dev) isa_address_space(dev)); s->ds = graphic_console_init(s->update, s->invalidate, s->screen_dump, s->text_update, - s); + s->screen_dump_async, s); rom_add_vga(VGABIOS_CIRRUS_FILENAME); /* XXX ISA-LFB support */ /* FIXME not qdev yet */ @@ -2941,7 +2941,7 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev) cirrus_init_common(s, device_id, 1, pci_address_space(dev)); s->vga.ds = graphic_console_init(s->vga.update, s->vga.invalidate, s->vga.screen_dump, s->vga.text_update, - &s->vga); + s->vga.screen_dump_async, &s->vga); /* setup PCI */ diff --git a/hw/exynos4210_fimd.c b/hw/exynos4210_fimd.c index 3313f00..2053ed0 100644 --- a/hw/exynos4210_fimd.c +++ b/hw/exynos4210_fimd.c @@ -1898,7 +1898,8 @@ static int exynos4210_fimd_init(SysBusDevice *dev) "exynos4210.fimd", FIMD_REGS_SIZE); sysbus_init_mmio(dev, &s->iomem); s->console = graphic_console_init(exynos4210_fimd_update, - exynos4210_fimd_invalidate, NULL, NULL, s); + exynos4210_fimd_invalidate, + NULL, NULL, NULL, s); return 0; } diff --git a/hw/g364fb.c b/hw/g364fb.c index 3a0b68f..8ae40d8 100644 --- a/hw/g364fb.c +++ b/hw/g364fb.c @@ -517,7 +517,7 @@ static void g364fb_init(DeviceState *dev, G364State *s) s->ds = graphic_console_init(g364fb_update_display, g364fb_invalidate_display, - g364fb_screen_dump, NULL, s); + g364fb_screen_dump, NULL, NULL, s); memory_region_init_io(&s->mem_ctrl, &g364fb_ctrl_ops, s, "ctrl", 0x180000); memory_region_init_ram_ptr(&s->mem_vram, "vram", diff --git a/hw/jazz_led.c b/hw/jazz_led.c index 6486523..bb5daf7 100644 --- a/hw/jazz_led.c +++ b/hw/jazz_led.c @@ -251,7 +251,8 @@ static int jazz_led_init(SysBusDevice *dev) s->ds = graphic_console_init(jazz_led_update_display, jazz_led_invalidate_display, NULL, - jazz_led_text_update, s); + jazz_led_text_update, + NULL, s); return 0; } diff --git a/hw/milkymist-vgafb.c b/hw/milkymist-vgafb.c index 69afd72..ae018d1 100644 --- a/hw/milkymist-vgafb.c +++ b/hw/milkymist-vgafb.c @@ -276,7 +276,7 @@ static int milkymist_vgafb_init(SysBusDevice *dev) s->ds = graphic_console_init(vgafb_update_display, vgafb_invalidate_display, - NULL, NULL, s); + NULL, NULL, NULL, s); return 0; } diff --git a/hw/musicpal.c b/hw/musicpal.c index 187a1ae..2fc391b 100644 --- a/hw/musicpal.c +++ b/hw/musicpal.c @@ -611,7 +611,7 @@ static int musicpal_lcd_init(SysBusDevice *dev) sysbus_init_mmio(dev, &s->iomem); s->ds = graphic_console_init(lcd_refresh, lcd_invalidate, - NULL, NULL, s); + NULL, NULL, NULL, s); qemu_console_resize(s->ds, 128*3, 64*3); qdev_init_gpio_in(&dev->qdev, musicpal_lcd_gpio_brigthness_in, 3); diff --git a/hw/omap_dss.c b/hw/omap_dss.c index 86ed6ea..0e8243e 100644 --- a/hw/omap_dss.c +++ b/hw/omap_dss.c @@ -1072,7 +1072,9 @@ struct omap_dss_s *omap_dss_init(struct omap_target_agent_s *ta, #if 0 s->state = graphic_console_init(omap_update_display, - omap_invalidate_display, omap_screen_dump, s); + omap_invalidate_display, + omap_screen_dump, + NULL, NULL, s); #endif return s; diff --git a/hw/omap_lcdc.c b/hw/omap_lcdc.c index f172093..d91e88d 100644 --- a/hw/omap_lcdc.c +++ b/hw/omap_lcdc.c @@ -452,7 +452,7 @@ struct omap_lcd_panel_s *omap_lcdc_init(MemoryRegion *sysmem, s->state = graphic_console_init(omap_update_display, omap_invalidate_display, - omap_screen_dump, NULL, s); + omap_screen_dump, NULL, NULL, s); return s; } diff --git a/hw/pl110.c b/hw/pl110.c index f94608c..32ee89c 100644 --- a/hw/pl110.c +++ b/hw/pl110.c @@ -451,7 +451,7 @@ static int pl110_init(SysBusDevice *dev) qdev_init_gpio_in(&s->busdev.qdev, pl110_mux_ctrl_set, 1); s->ds = graphic_console_init(pl110_update_display, pl110_invalidate_display, - NULL, NULL, s); + NULL, NULL, NULL, s); return 0; } diff --git a/hw/pxa2xx_lcd.c b/hw/pxa2xx_lcd.c index fcbdfb3..5345fb4 100644 --- a/hw/pxa2xx_lcd.c +++ b/hw/pxa2xx_lcd.c @@ -1004,7 +1004,7 @@ PXA2xxLCDState *pxa2xx_lcdc_init(MemoryRegion *sysmem, s->ds = graphic_console_init(pxa2xx_update_display, pxa2xx_invalidate_display, - NULL, NULL, s); + NULL, NULL, NULL, s); switch (ds_get_bits_per_pixel(s->ds)) { case 0: diff --git a/hw/qxl.c b/hw/qxl.c index d83d245..61a7edd 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -1727,7 +1727,8 @@ static int qxl_init_primary(PCIDevice *dev) portio_list_add(qxl_vga_port_list, pci_address_space_io(dev), 0x3b0); vga->ds = graphic_console_init(qxl_hw_update, qxl_hw_invalidate, - qxl_hw_screen_dump, qxl_hw_text_update, qxl); + qxl_hw_screen_dump, qxl_hw_text_update, + NULL, qxl); qemu_spice_display_init_common(&qxl->ssd, vga->ds); qxl0 = qxl; diff --git a/hw/sm501.c b/hw/sm501.c index 786e076..8b150be 100644 --- a/hw/sm501.c +++ b/hw/sm501.c @@ -1442,6 +1442,6 @@ void sm501_init(MemoryRegion *address_space_mem, uint32_t base, } /* create qemu graphic console */ - s->ds = graphic_console_init(sm501_update_display, NULL, - NULL, NULL, s); + s->ds = graphic_console_init(sm501_update_display, + NULL, NULL, NULL, NULL, s); } diff --git a/hw/ssd0303.c b/hw/ssd0303.c index 4e1ee6e..05e6686 100644 --- a/hw/ssd0303.c +++ b/hw/ssd0303.c @@ -289,7 +289,7 @@ static int ssd0303_init(I2CSlave *i2c) s->ds = graphic_console_init(ssd0303_update_display, ssd0303_invalidate_display, - NULL, NULL, s); + NULL, NULL, NULL, s); qemu_console_resize(s->ds, 96 * MAGNIFY, 16 * MAGNIFY); return 0; } diff --git a/hw/ssd0323.c b/hw/ssd0323.c index b0b2e94..5e3491f 100644 --- a/hw/ssd0323.c +++ b/hw/ssd0323.c @@ -330,7 +330,7 @@ static int ssd0323_init(SSISlave *dev) s->row_end = 79; s->ds = graphic_console_init(ssd0323_update_display, ssd0323_invalidate_display, - NULL, NULL, s); + NULL, NULL, NULL, s); qemu_console_resize(s->ds, 128 * MAGNIFY, 64 * MAGNIFY); qdev_init_gpio_in(&dev->qdev, ssd0323_cd, 1); diff --git a/hw/tc6393xb.c b/hw/tc6393xb.c index 420925c..853b180 100644 --- a/hw/tc6393xb.c +++ b/hw/tc6393xb.c @@ -581,6 +581,7 @@ TC6393xbState *tc6393xb_init(MemoryRegion *sysmem, uint32_t base, qemu_irq irq) NULL, /* invalidate */ NULL, /* screen_dump */ NULL, /* text_update */ + NULL, /* screen_dump_async */ s); return s; diff --git a/hw/tcx.c b/hw/tcx.c index ac7dcb4..9214dec 100644 --- a/hw/tcx.c +++ b/hw/tcx.c @@ -558,7 +558,7 @@ static int tcx_init1(SysBusDevice *dev) s->ds = graphic_console_init(tcx24_update_display, tcx24_invalidate_display, - tcx24_screen_dump, NULL, s); + tcx24_screen_dump, NULL, NULL, s); } else { /* THC 8 bit (dummy) */ memory_region_init_io(&s->thc8, &dummy_ops, s, "tcx.thc8", @@ -567,7 +567,7 @@ static int tcx_init1(SysBusDevice *dev) s->ds = graphic_console_init(tcx_update_display, tcx_invalidate_display, - tcx_screen_dump, NULL, s); + tcx_screen_dump, NULL, NULL, s); } qemu_console_resize(s->ds, s->width, s->height); diff --git a/hw/vga-isa-mm.c b/hw/vga-isa-mm.c index f8984c6..87b8fc6 100644 --- a/hw/vga-isa-mm.c +++ b/hw/vga-isa-mm.c @@ -132,7 +132,8 @@ int isa_vga_mm_init(target_phys_addr_t vram_base, vga_mm_init(s, vram_base, ctrl_base, it_shift, address_space); s->vga.ds = graphic_console_init(s->vga.update, s->vga.invalidate, - s->vga.screen_dump, s->vga.text_update, s); + s->vga.screen_dump, s->vga.text_update, + s->vga.screen_dump_async, s); vga_init_vbe(&s->vga, address_space); return 0; diff --git a/hw/vga-isa.c b/hw/vga-isa.c index 4bcc4db..a362ed2 100644 --- a/hw/vga-isa.c +++ b/hw/vga-isa.c @@ -61,7 +61,8 @@ static int vga_initfn(ISADevice *dev) vga_io_memory, 1); memory_region_set_coalescing(vga_io_memory); s->ds = graphic_console_init(s->update, s->invalidate, - s->screen_dump, s->text_update, s); + s->screen_dump, s->text_update, + s->screen_dump_async, s); vga_init_vbe(s, isa_address_space(dev)); /* ROM BIOS */ diff --git a/hw/vga-pci.c b/hw/vga-pci.c index 465b643..8c8dd53 100644 --- a/hw/vga-pci.c +++ b/hw/vga-pci.c @@ -57,7 +57,8 @@ static int pci_vga_initfn(PCIDevice *dev) vga_init(s, pci_address_space(dev), pci_address_space_io(dev), true); s->ds = graphic_console_init(s->update, s->invalidate, - s->screen_dump, s->text_update, s); + s->screen_dump, s->text_update, + s->screen_dump_async, s); /* XXX: VGA_RAM_SIZE must be a power of two */ pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_MEM_PREFETCH, &s->vram); diff --git a/hw/vga_int.h b/hw/vga_int.h index 7685b2b..4486cfb 100644 --- a/hw/vga_int.h +++ b/hw/vga_int.h @@ -161,6 +161,7 @@ typedef struct VGACommonState { 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; /* hardware mouse cursor support */ uint32_t invalidated_y_table[VGA_MAX_HEIGHT / 32]; void (*cursor_invalidate)(struct VGACommonState *s); diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c index 142d9f4..db96e61 100644 --- a/hw/vmware_vga.c +++ b/hw/vmware_vga.c @@ -1087,7 +1087,8 @@ static void vmsvga_init(struct vmsvga_state_s *s, int vga_ram_size, s->vga.ds = graphic_console_init(vmsvga_update_display, vmsvga_invalidate_display, vmsvga_screen_dump, - vmsvga_text_update, s); + vmsvga_text_update, + NULL, s); s->fifo_size = SVGA_FIFO_SIZE; diff --git a/hw/xenfb.c b/hw/xenfb.c index 1bcf171..ad9f0d7 100644 --- a/hw/xenfb.c +++ b/hw/xenfb.c @@ -906,6 +906,7 @@ static int fb_initialise(struct XenDevice *xendev) xenfb_invalidate, NULL, NULL, + NULL, fb); fb->have_console = 1; } @@ -1015,6 +1016,7 @@ wait_more: xenfb_invalidate, NULL, NULL, + NULL, fb); fb->have_console = 1; diff --git a/monitor.c b/monitor.c index 1a65c41..91a76a1 100644 --- a/monitor.c +++ b/monitor.c @@ -901,6 +901,11 @@ static int do_screen_dump(Monitor *mon, const QDict *qdict, QObject **ret_data) return 0; } +void qmp_screendump_async(const char *filename, Error **errp) +{ + vga_hw_screen_dump_async(filename); +} + static void do_logfile(Monitor *mon, const QDict *qdict) { cpu_set_log_filename(qdict_get_str(qdict, "filename")); diff --git a/qapi-schema.json b/qapi-schema.json index 72b17f1..a5873cb 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1595,3 +1595,23 @@ { 'command': 'qom-list-types', 'data': { '*implements': 'str', '*abstract': 'bool' }, 'returns': [ 'ObjectTypeInfo' ] } + +## +## @screendump-async: +# +# This command will perform a screen dump of the first console to the givem +# filename. The additional parameters are unused at this time. +# +# @filename name of output file to write screen dump to +# +# Since: 1.1 +# +# Notes: This command is experimental and may change syntax in future releases. +# +# This command is the same as the qmp/hmp screendump command, except that on +# successful completion of the scren dump the SCREEN_DUMP_COMPLETE event is +# emitted. +# +## +{ 'command': 'screendump-async', + 'data': { 'filename': 'str' } } diff --git a/qmp-commands.hx b/qmp-commands.hx index 705f704..ea0d051 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -170,6 +170,32 @@ Example: EQMP { + .name = "screendump-async", + .args_type = "filename:F", + .params = "filename", + .help = "save screen into PPM image 'filename'", + .user_print = monitor_user_noop, + .mhandler.cmd_new = qmp_marshal_input_screendump_async, + }, + +SQMP +screendump-async +---------------- + +Save screen into PPM image. Fires a SCREEN_DUMP_COMPLETE event on completion. + +Arguments: + +- "filename": file path (json-string) + +Example: + +-> { "execute": "screendump-async", "arguments": { "filename": "/tmp/image" } } +<- { "return": {} } + +EQMP + + { .name = "stop", .args_type = "", .mhandler.cmd_new = qmp_marshal_input_stop,
This is an across the board change since I wanted to keep the existing (good imo) single graphic_console_init callback setter, instead of introducing a new cb that isn't set by it but instead by a second initialization function. Signed-off-by: Alon Levy <alevy@redhat.com> --- console.c | 25 +++++++++++++++++++++++-- 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 ++ monitor.c | 5 +++++ qapi-schema.json | 20 ++++++++++++++++++++ qmp-commands.hx | 26 ++++++++++++++++++++++++++ 28 files changed, 115 insertions(+), 25 deletions(-)