Re: [Qemu-devel] [PATCH 0/4] fix qxl screendump using monitor_suspend

[Alon Levy]

On Sun, Mar 11, 2012 at 06:39:33PM +0200, Alon Levy wrote:
> This patchset starts and ends with trace event additions that make it easier 
> to see the change.

Self NACK to v1, see v2 on the same thread.

> 
> It applies on top of 
> http://lists.gnu.org/archive/html/qemu-devel/2012-03/msg01784.html
> due to trace-events.
> 
> The problem addressed by this patchset is that after recent fixes (81fb6f)
> screendump with a qxl device in native mode saves a stale screen dump.
> 
> The solution is to use monitor_suspend and monitor_release in qxl's 
> implementation.
> 
> This is done by:
>  1. introducing an extra parameter to vga_hw_screen_dump & hw_vga_dump
>   console: pass Monitor to vga_hw_screen_dump/hw_vga_dump
>  2. using it in qxl via a bh.
>   qxl-render: call ppm_save on bh
> 
> Additional patches add trace events to qxl and qxl_render, making it easy to 
> see the difference:
> 
> events setup: (using stderr backend)
> 
> (qemu) trace-event ppm_save on
> (qemu) trace-event qxl* on
> (qemu) trace-event qxl_interface_get_command_enter off
> (qemu) trace-event qxl_interface_release_resource off
> (qemu) trace-event qxl_interface_get_command_ret off
> 
> before: ppm_save done before update:
> 
>     (qemu) screendump /tmp/a.ppm
>     ppm_save /tmp/a.ppm surface=0x7fc0267b3ad0
>     qxl_interface_update_area_complete surface=0 [152,160,464,480] #=1
>     qxl_interface_update_area_complete_schedule_bh #dirty=1
>     qxl_render_update_area_done 0x7fc02b603db0
>     (qemu) qxl_blit stride=-2560 [152, 160, 464, 480]
> 
> after:
> 
>     (qemu) screendump /tmp/a.ppm
>     qxl_interface_update_area_complete surface=0 [152,160,464,480] #=1
>     qxl_interface_update_area_complete_schedule_bh #dirty=1
>     qxl_render_update_area_done 0x7f407af72210
>     qxl_render_ppm_save_bh 0x7f407f845b60 (primary 0x7f401bc00000)
>     qxl_blit stride=-2560 [152, 160, 464, 480]
>     ppm_save /tmp/a.ppm surface=0x7f4077204ad0
>     (qemu)
> 
> Note: This doesn't address a possible libvirt problem that was mentioned in
> length before, but since it has not been reproduced it will be fixed when it
> is. Meanwhile other users like autotest will be fixed by this patch (by fix I
> mean screendump will produce the correct output).
> 
> Alon Levy (4):
>   qxl: switch qxl.c to trace-events
>   qxl/qxl_render.c: add trace events
>   console: pass Monitor to vga_hw_screen_dump/hw_vga_dump
>   qxl-render: call ppm_save on bh
> 
>  console.c          |    4 +-
>  console.h          |    5 +-
>  hw/blizzard.c      |    2 +-
>  hw/g364fb.c        |    3 +-
>  hw/omap_dss.c      |    4 +-
>  hw/omap_lcdc.c     |    3 +-
>  hw/qxl-render.c    |   95 +++++++++++++++++++++++++++------
>  hw/qxl.c           |  150 ++++++++++++++++++++++++---------------------------
>  hw/qxl.h           |    2 +-
>  hw/sm501.c         |    4 +-
>  hw/tcx.c           |   12 +++--
>  hw/vga.c           |    6 ++-
>  hw/vmware_vga.c    |    5 +-
>  monitor.c          |    2 +-
>  trace-events       |   55 +++++++++++++++++++
>  ui/spice-display.h |    3 +
>  16 files changed, 240 insertions(+), 115 deletions(-)
> 
> -- 
> 1.7.9.1
> 
>