[Qemu-devel] [PATCH 0/3] Fix incoming migration regression of QXL in VGA mode

[Qemu-devel] [PATCH 0/3] Fix incoming migration regression of QXL in VGA mode

[Marc-André Lureau]

Hi,

Commit cd958edb1fae85d introduced a migration regression in QXL VGA
mode. The last patch fixes it. The first two patches helped me reason
about the related console code.

Marc-André Lureau (3):
  console: add same surface replace pre-condition
  console: add same displaychangelistener registration pre-condition
  qxl: switch display on entering VGA

 hw/display/qxl.c | 1 +
 ui/console.c     | 4 ++++
 2 files changed, 5 insertions(+)

-- 
2.12.0.191.gc5d8de91d

[Qemu-devel] [PATCH 1/3] console: add same surface replace pre-condition

[Marc-André Lureau]

Catch an invalid state early, before a potential use-after-free. This is
mainly useful for documentation purposes.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/console.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ui/console.c b/ui/console.c
index 419b098c11..0cbe5033dd 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1538,6 +1538,8 @@ void dpy_gfx_replace_surface(QemuConsole *con,
     DisplaySurface *old_surface = con->surface;
     DisplayChangeListener *dcl;
 
+    assert(old_surface != surface);
+
     con->surface = surface;
     QLIST_FOREACH(dcl, &s->listeners, next) {
         if (con != (dcl->con ? dcl->con : active_console)) {
-- 
2.12.0.191.gc5d8de91d

[Qemu-devel] [PATCH 2/3] console: add same displaychangelistener registration pre-condition

[Marc-André Lureau]

Catch an invalid state. Mainly useful for documentation purposes.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/console.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ui/console.c b/ui/console.c
index 0cbe5033dd..189eecfd29 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1410,6 +1410,8 @@ void register_displaychangelistener(DisplayChangeListener *dcl)
     static DisplaySurface *dummy;
     QemuConsole *con;
 
+    assert(!dcl->ds);
+
     if (dcl->ops->dpy_gl_ctx_create) {
         /* display has opengl support */
         assert(dcl->con);
-- 
2.12.0.191.gc5d8de91d

[Qemu-devel] [PATCH 3/3] qxl: switch display on entering VGA

[Marc-André Lureau]

Since commit cd958edb1fae85d, same size console resize is skipped. This
change broke QXL incoming migration in VGA mode,
qemu_spice_display_switch() is no longer called during qxl_post_load(),
because default message surface is of the same size, and during
displaychangelistener registration, PCIQXLDevice.mode is
QXL_MODE_UNDEFINED. This triggers a later crash on refresh:

==2634== Invalid read of size 4
==3516== at 0x65F3050: pixman_image_get_data (in /usr/lib64/libpixman-1.so.0.34.0)
==3516== by 0x6F0CEB: qemu_spice_create_update (spice-display.c:215)
==3516== by 0x6F1CC7: qemu_spice_display_refresh (spice-display.c:502)
==3516== by 0x58CF77: display_refresh (qxl.c:1948)
==3516== by 0x6E8084: do_safe_dpy_refresh (console.c:1591)
==3516== by 0x6E80D5: dpy_refresh (console.c:1604)
==3516== by 0x6E4508: gui_update (console.c:201)
==3516== by 0x81898E: timerlist_run_timers (qemu-timer.c:536)
==3516== by 0x8189D6: qemu_clock_run_timers (qemu-timer.c:547)
==3516== by 0x818D98: qemu_clock_run_all_timers (qemu-timer.c:662)
==3516== by 0x81952A: main_loop_wait (main-loop.c:514)
==3516== by 0x4ADD29: main_loop (vl.c:1898)

One way to solve this is to explicitely call qemu_spice_display_switch()
on entering VGA mode, which is called during qxl_post_load().

Fixes:
"null pointer access on migration resume of systemrescuecd boot menu with qxl-vga"
https://bugs.launchpad.net/qemu/+bug/1679126
https://bugzilla.redhat.com/show_bug.cgi?id=1438566

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/display/qxl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 0d02f0efe6..c31b293bb7 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1146,6 +1146,7 @@ static void qxl_enter_vga_mode(PCIQXLDevice *d)
     update_displaychangelistener(&d->ssd.dcl, GUI_REFRESH_INTERVAL_DEFAULT);
     qemu_spice_create_host_primary(&d->ssd);
     d->mode = QXL_MODE_VGA;
+    qemu_spice_display_switch(&d->ssd, d->ssd.ds);
     vga_dirty_log_start(&d->vga);
     graphic_hw_update(d->vga.con);
 }
-- 
2.12.0.191.gc5d8de91d

Re: [Qemu-devel] [PATCH 0/3] Fix incoming migration regression of QXL in VGA mode

[Gerd Hoffmann]

On Do, 2017-04-06 at 14:05 +0200, Marc-André Lureau wrote:
> Hi,
> 
> Commit cd958edb1fae85d introduced a migration regression in QXL VGA
> mode. The last patch fixes it. The first two patches helped me reason
> about the related console code.
> 
> Marc-André Lureau (3):
>   console: add same surface replace pre-condition
>   console: add same displaychangelistener registration pre-condition
>   qxl: switch display on entering VGA
> 
>  hw/display/qxl.c | 1 +
>  ui/console.c     | 4 ++++
>  2 files changed, 5 insertions(+)
> 

Added to vga queue.  Will cherry-pick 3/3 for 2.9

cheers,
  Gerd