diff mbox series

[v2,3/3] console: make QMP/HMP screendump run in coroutine

Message ID 20201027133602.3038018-4-marcandre.lureau@redhat.com
State Accepted
Commit 0d9b90ce5c73505648909a89bcd5272081b9c348
Headers show
Series console: make QMP screendump use coroutine | expand

Commit Message

Marc-André Lureau Oct. 27, 2020, 1:36 p.m. UTC 
From: Marc-André Lureau <marcandre.lureau@redhat.com>

Thanks to the monitors' coroutine support (merge commit b7092cda1b3),
the screendump handler can trigger a graphic_hw_update(), yield and let
the main loop run until update is done. Then the handler is resumed, and
ppm_save() will write the screen image to disk in the coroutine context.

The IO is still blocking though, as the file is set blocking so far,
this could be addressed by some future change (with other caveats).

Related to:
https://bugzilla.redhat.com/show_bug.cgi?id=1230527

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hmp-commands.hx    |  1 +
 monitor/hmp-cmds.c |  3 ++-
 qapi/ui.json       |  3 ++-
 ui/console.c       | 32 +++++++++++++++++++++++++++++---
 4 files changed, 34 insertions(+), 5 deletions(-)

Comments

Markus Armbruster Oct. 27, 2020, 4:01 p.m. UTC | #1 
marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>

>

> Thanks to the monitors' coroutine support (merge commit b7092cda1b3),

> the screendump handler can trigger a graphic_hw_update(), yield and let

> the main loop run until update is done. Then the handler is resumed, and

> ppm_save() will write the screen image to disk in the coroutine context.

>

> The IO is still blocking though, as the file is set blocking so far,

> this could be addressed by some future change (with other caveats).

>

> Related to:

> https://bugzilla.redhat.com/show_bug.cgi?id=1230527

>

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>


Thanks for persevering!

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Gerd Hoffmann Jan. 20, 2021, 2:18 p.m. UTC | #2 
On Tue, Oct 27, 2020 at 05:36:02PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>

> 

> Thanks to the monitors' coroutine support (merge commit b7092cda1b3),

> the screendump handler can trigger a graphic_hw_update(), yield and let

> the main loop run until update is done. Then the handler is resumed, and

> ppm_save() will write the screen image to disk in the coroutine context.

> 

> The IO is still blocking though, as the file is set blocking so far,

> this could be addressed by some future change (with other caveats).

> 

> Related to:

> https://bugzilla.redhat.com/show_bug.cgi?id=1230527

> 

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>


Hmm, just noticed that with this patch applied screendump hangs for vms
with "-device qxl" ("-device qxl-vga" works fine).

Can you have a look?

thanks,
  Gerd
Marc-André Lureau Jan. 20, 2021, 2:29 p.m. UTC | #3 
Hi Gerd

On Wed, Jan 20, 2021 at 6:18 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>

> On Tue, Oct 27, 2020 at 05:36:02PM +0400, marcandre.lureau@redhat.com wrote:

> > From: Marc-André Lureau <marcandre.lureau@redhat.com>

> >

> > Thanks to the monitors' coroutine support (merge commit b7092cda1b3),

> > the screendump handler can trigger a graphic_hw_update(), yield and let

> > the main loop run until update is done. Then the handler is resumed, and

> > ppm_save() will write the screen image to disk in the coroutine context.

> >

> > The IO is still blocking though, as the file is set blocking so far,

> > this could be addressed by some future change (with other caveats).

> >

> > Related to:

> > https://bugzilla.redhat.com/show_bug.cgi?id=1230527

> >

> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> > Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

>

> Hmm, just noticed that with this patch applied screendump hangs for vms

> with "-device qxl" ("-device qxl-vga" works fine).

>

> Can you have a look?


Weird, I cannot reproduce. I tried this way:

$ qemu-system-x86_64 -m 4096 -enable-kvm -device qxl -qmp
unix:/tmp/qmp.sock,server -snapshot rhel8
$ ./scripts/qmp/qmp-shell -v -p /tmp/qmp.sock
Welcome to the QMP low-level shell!
Connected to QEMU 5.2.0

(QEMU) screendump filename=/tmp/foo
{
    "arguments": {
        "filename": "/tmp/foo"
    },
    "execute": "screendump"
}
{
    "return": {}
}

etc.. multiple times at different stages.

Can you also provide a backtrace?




-- 
Marc-André Lureau
Gerd Hoffmann Jan. 20, 2021, 4:09 p.m. UTC | #4 
On Wed, Jan 20, 2021 at 06:29:41PM +0400, Marc-André Lureau wrote:
> Hi Gerd

> 

> On Wed, Jan 20, 2021 at 6:18 PM Gerd Hoffmann <kraxel@redhat.com> wrote:

> >

> > On Tue, Oct 27, 2020 at 05:36:02PM +0400, marcandre.lureau@redhat.com wrote:

> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>

> > >

> > > Thanks to the monitors' coroutine support (merge commit b7092cda1b3),

> > > the screendump handler can trigger a graphic_hw_update(), yield and let

> > > the main loop run until update is done. Then the handler is resumed, and

> > > ppm_save() will write the screen image to disk in the coroutine context.

> > >

> > > The IO is still blocking though, as the file is set blocking so far,

> > > this could be addressed by some future change (with other caveats).

> > >

> > > Related to:

> > > https://bugzilla.redhat.com/show_bug.cgi?id=1230527

> > >

> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> > > Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

> >

> > Hmm, just noticed that with this patch applied screendump hangs for vms

> > with "-device qxl" ("-device qxl-vga" works fine).

> >

> > Can you have a look?

> 

> Weird, I cannot reproduce. I tried this way:

> 

> $ qemu-system-x86_64 -m 4096 -enable-kvm -device qxl -qmp

> unix:/tmp/qmp.sock,server -snapshot rhel8


-vga none or -nodefaults is needed, otherwise you'll get both VGA and
qxl devices.

Using gtk ui, just saying "screendump /tmp/x" in the monitor tab.

> Can you also provide a backtrace?


Thread 9 (Thread 0x7fab97638640 (LWP 2822854) "pool-qemu"):
#0  0x00007fab9c8af30d in syscall () at /lib64/libc.so.6
#1  0x00007fab9db7f2ec in g_cond_wait_until () at /lib64/libglib-2.0.so.0
#2  0x00007fab9db033c1 in g_async_queue_pop_intern_unlocked () at /lib64/libglib-2.0.so.0
#3  0x00007fab9db03546 in g_async_queue_timeout_pop () at /lib64/libglib-2.0.so.0
#4  0x00007fab9db62ef9 in g_thread_pool_thread_proxy.lto_priv () at /lib64/libglib-2.0.so.0
#5  0x00007fab9db602b2 in g_thread_proxy () at /lib64/libglib-2.0.so.0
#6  0x00007fab9c9873f9 in start_thread () at /lib64/libpthread.so.0
#7  0x00007fab9c8b4903 in clone () at /lib64/libc.so.6

Thread 8 (Thread 0x7fab37dff640 (LWP 2822804) "SPICE Worker"):
#0  0x00007fab9c8a980f in poll () at /lib64/libc.so.6
#1  0x00007fab9db846f6 in g_main_context_iterate.constprop () at /lib64/libglib-2.0.so.0
#2  0x00007fab9db32033 in g_main_loop_run () at /lib64/libglib-2.0.so.0
#3  0x00007fab9de87c66 in red_worker_main.lto_priv () at /lib64/libspice-server.so.1
#4  0x00007fab9c9873f9 in start_thread () at /lib64/libpthread.so.0
#5  0x00007fab9c8b4903 in clone () at /lib64/libc.so.6

Thread 7 (Thread 0x7fab95a8b640 (LWP 2822803) "qemu-system-x86"):
#0  0x00007fab9c990ea0 in __lll_lock_wait () at /lib64/libpthread.so.0
#1  0x00007fab9c989763 in pthread_mutex_lock () at /lib64/libpthread.so.0
#2  0x000055f3f9c1c7e9 in qemu_mutex_lock_impl ()
#3  0x000055f3f9a1357f in qemu_mutex_lock_iothread_impl ()
#4  0x000055f3f9a52c86 in mttcg_cpu_thread_fn ()
#5  0x000055f3f9c1c689 in qemu_thread_start ()
#6  0x00007fab9c9873f9 in start_thread () at /lib64/libpthread.so.0
#7  0x00007fab9c8b4903 in clone () at /lib64/libc.so.6

Thread 6 (Thread 0x7fab96596640 (LWP 2822802) "dconf worker"):
#0  0x00007fab9c8a980f in poll () at /lib64/libc.so.6
#1  0x00007fab9db846f6 in g_main_context_iterate.constprop () at /lib64/libglib-2.0.so.0
#2  0x00007fab9db2fd43 in g_main_context_iteration () at /lib64/libglib-2.0.so.0
#3  0x00007fab9e0ac64d in dconf_gdbus_worker_thread () at /usr/lib64/gio/modules/libdconfsettings.so
#4  0x00007fab9db602b2 in g_thread_proxy () at /lib64/libglib-2.0.so.0
#5  0x00007fab9c9873f9 in start_thread () at /lib64/libpthread.so.0
#6  0x00007fab9c8b4903 in clone () at /lib64/libc.so.6

Thread 5 (Thread 0x7fab96d97640 (LWP 2822801) "gdbus"):
#0  0x00007fab9c8a980f in poll () at /lib64/libc.so.6
#1  0x00007fab9db846f6 in g_main_context_iterate.constprop () at /lib64/libglib-2.0.so.0
#2  0x00007fab9db32033 in g_main_loop_run () at /lib64/libglib-2.0.so.0
#3  0x00007fab9da21d1a in gdbus_shared_thread_func.lto_priv () at /lib64/libgio-2.0.so.0
#4  0x00007fab9db602b2 in g_thread_proxy () at /lib64/libglib-2.0.so.0
#5  0x00007fab9c9873f9 in start_thread () at /lib64/libpthread.so.0
#6  0x00007fab9c8b4903 in clone () at /lib64/libc.so.6

Thread 3 (Thread 0x7fab97e39640 (LWP 2822799) "gmain"):
#0  0x00007fab9c8a980f in poll () at /lib64/libc.so.6
#1  0x00007fab9db846f6 in g_main_context_iterate.constprop () at /lib64/libglib-2.0.so.0
#2  0x00007fab9db2fd43 in g_main_context_iteration () at /lib64/libglib-2.0.so.0
#3  0x00007fab9db31961 in glib_worker_main () at /lib64/libglib-2.0.so.0
#4  0x00007fab9db602b2 in g_thread_proxy () at /lib64/libglib-2.0.so.0
#5  0x00007fab9c9873f9 in start_thread () at /lib64/libpthread.so.0
#6  0x00007fab9c8b4903 in clone () at /lib64/libc.so.6

Thread 2 (Thread 0x7fab9873b640 (LWP 2822798) "qemu-system-x86"):
#0  0x00007fab9c8af30d in syscall () at /lib64/libc.so.6
#1  0x000055f3f9c1d53a in qemu_event_wait ()
#2  0x000055f3f9c17c9a in call_rcu_thread ()
#3  0x000055f3f9c1c689 in qemu_thread_start ()
#4  0x00007fab9c9873f9 in start_thread () at /lib64/libpthread.so.0
#5  0x00007fab9c8b4903 in clone () at /lib64/libc.so.6

Thread 1 (Thread 0x7fab988e4440 (LWP 2822797) "qemu-system-x86"):
#0  0x00007fab9c8a990e in ppoll () at /lib64/libc.so.6
#1  0x000055f3f9c33dd5 in fdmon_poll_wait ()
#2  0x000055f3f9c26c6a in aio_poll ()
#3  0x000055f3f99552a5 in handle_hmp_command ()
#4  0x000055f3f99553cd in monitor_command_cb ()
#5  0x000055f3f9c29a37 in readline_handle_byte ()
#6  0x000055f3f995541b in monitor_read ()
#7  0x000055f3f992373c in gd_vc_in ()
#8  0x00007fab9d84e22d in _vte_marshal_VOID__STRING_UINTv () at /lib64/libvte-2.91.so.0
#9  0x00007fab9d8e9080 in g_signal_emit_valist () at /lib64/libgobject-2.0.so.0
#10 0x00007fab9d8e91a3 in g_signal_emit () at /lib64/libgobject-2.0.so.0
#11 0x00007fab9d858a7e in vte::terminal::Terminal::emit_commit(std::basic_string_view<char, std::char_traits<char> > const&) () at /lib64/libvte-2.91.so.0
#12 0x00007fab9d85cf1d in vte::terminal::Terminal::send_child(std::basic_string_view<char, std::char_traits<char> > const&) () at /lib64/libvte-2.91.so.0
#13 0x00007fab9d8715bc in vte_terminal_key_press(_GtkWidget*, _GdkEventKey*) () at /lib64/libvte-2.91.so.0
#14 0x00007fab9d47eccc in _gtk_marshal_BOOLEAN__BOXEDv () at /lib64/libgtk-3.so.0
#15 0x00007fab9d8e869a in g_signal_emit_valist () at /lib64/libgobject-2.0.so.0
#16 0x00007fab9d8e91a3 in g_signal_emit () at /lib64/libgobject-2.0.so.0
#17 0x00007fab9d441bb4 in gtk_widget_event_internal.part.0.lto_priv () at /lib64/libgtk-3.so.0
#18 0x00007fab9d45029b in gtk_window_propagate_key_event () at /lib64/libgtk-3.so.0
#19 0x00007fab9d453283 in gtk_window_key_press_event.lto_priv () at /lib64/libgtk-3.so.0
#20 0x00007fab9d47eccc in _gtk_marshal_BOOLEAN__BOXEDv () at /lib64/libgtk-3.so.0
#21 0x00007fab9d8e9080 in g_signal_emit_valist () at /lib64/libgobject-2.0.so.0
#22 0x00007fab9d8e91a3 in g_signal_emit () at /lib64/libgobject-2.0.so.0
#23 0x00007fab9d441bb4 in gtk_widget_event_internal.part.0.lto_priv () at /lib64/libgtk-3.so.0
#24 0x00007fab9d2e000f in propagate_event.lto_priv () at /lib64/libgtk-3.so.0
#25 0x00007fab9d2e1223 in gtk_main_do_event () at /lib64/libgtk-3.so.0
#26 0x00007fab9cfbb633 in _gdk_event_emit () at /lib64/libgdk-3.so.0
#27 0x00007fab9d022ba6 in gdk_event_source_dispatch.lto_priv () at /lib64/libgdk-3.so.0
#28 0x00007fab9db3296f in g_main_context_dispatch () at /lib64/libglib-2.0.so.0
#29 0x000055f3f9c24f58 in main_loop_wait ()
#30 0x000055f3f9a564b1 in qemu_main_loop ()
#31 0x000055f3f97609ee in main ()

take care,
  Gerd
Dr. David Alan Gilbert Jan. 20, 2021, 4:25 p.m. UTC | #5 
* Gerd Hoffmann (kraxel@redhat.com) wrote:
> On Wed, Jan 20, 2021 at 06:29:41PM +0400, Marc-André Lureau wrote:

> > Hi Gerd

> > 

> > On Wed, Jan 20, 2021 at 6:18 PM Gerd Hoffmann <kraxel@redhat.com> wrote:

> > >

> > > On Tue, Oct 27, 2020 at 05:36:02PM +0400, marcandre.lureau@redhat.com wrote:

> > > > From: Marc-André Lureau <marcandre.lureau@redhat.com>

> > > >

> > > > Thanks to the monitors' coroutine support (merge commit b7092cda1b3),

> > > > the screendump handler can trigger a graphic_hw_update(), yield and let

> > > > the main loop run until update is done. Then the handler is resumed, and

> > > > ppm_save() will write the screen image to disk in the coroutine context.

> > > >

> > > > The IO is still blocking though, as the file is set blocking so far,

> > > > this could be addressed by some future change (with other caveats).

> > > >

> > > > Related to:

> > > > https://bugzilla.redhat.com/show_bug.cgi?id=1230527

> > > >

> > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> > > > Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

> > >

> > > Hmm, just noticed that with this patch applied screendump hangs for vms

> > > with "-device qxl" ("-device qxl-vga" works fine).

> > >

> > > Can you have a look?

> > 

> > Weird, I cannot reproduce. I tried this way:

> > 

> > $ qemu-system-x86_64 -m 4096 -enable-kvm -device qxl -qmp

> > unix:/tmp/qmp.sock,server -snapshot rhel8

> 

> -vga none or -nodefaults is needed, otherwise you'll get both VGA and

> qxl devices.

> 

> Using gtk ui, just saying "screendump /tmp/x" in the monitor tab.

> 

> > Can you also provide a backtrace?

> 


> Thread 1 (Thread 0x7fab988e4440 (LWP 2822797) "qemu-system-x86"):

> #0  0x00007fab9c8a990e in ppoll () at /lib64/libc.so.6

> #1  0x000055f3f9c33dd5 in fdmon_poll_wait ()

> #2  0x000055f3f9c26c6a in aio_poll ()


> #3  0x000055f3f99552a5 in handle_hmp_command ()


That's the :

1117	        AIO_WAIT_WHILE(qemu_get_aio_context(), !data.done);

Dave

> #4  0x000055f3f99553cd in monitor_command_cb ()

> #5  0x000055f3f9c29a37 in readline_handle_byte ()

> #6  0x000055f3f995541b in monitor_read ()

> #7  0x000055f3f992373c in gd_vc_in ()

> #8  0x00007fab9d84e22d in _vte_marshal_VOID__STRING_UINTv () at /lib64/libvte-2.91.so.0

> #9  0x00007fab9d8e9080 in g_signal_emit_valist () at /lib64/libgobject-2.0.so.0

> #10 0x00007fab9d8e91a3 in g_signal_emit () at /lib64/libgobject-2.0.so.0

> #11 0x00007fab9d858a7e in vte::terminal::Terminal::emit_commit(std::basic_string_view<char, std::char_traits<char> > const&) () at /lib64/libvte-2.91.so.0

> #12 0x00007fab9d85cf1d in vte::terminal::Terminal::send_child(std::basic_string_view<char, std::char_traits<char> > const&) () at /lib64/libvte-2.91.so.0

> #13 0x00007fab9d8715bc in vte_terminal_key_press(_GtkWidget*, _GdkEventKey*) () at /lib64/libvte-2.91.so.0

> #14 0x00007fab9d47eccc in _gtk_marshal_BOOLEAN__BOXEDv () at /lib64/libgtk-3.so.0

> #15 0x00007fab9d8e869a in g_signal_emit_valist () at /lib64/libgobject-2.0.so.0

> #16 0x00007fab9d8e91a3 in g_signal_emit () at /lib64/libgobject-2.0.so.0

> #17 0x00007fab9d441bb4 in gtk_widget_event_internal.part.0.lto_priv () at /lib64/libgtk-3.so.0

> #18 0x00007fab9d45029b in gtk_window_propagate_key_event () at /lib64/libgtk-3.so.0

> #19 0x00007fab9d453283 in gtk_window_key_press_event.lto_priv () at /lib64/libgtk-3.so.0

> #20 0x00007fab9d47eccc in _gtk_marshal_BOOLEAN__BOXEDv () at /lib64/libgtk-3.so.0

> #21 0x00007fab9d8e9080 in g_signal_emit_valist () at /lib64/libgobject-2.0.so.0

> #22 0x00007fab9d8e91a3 in g_signal_emit () at /lib64/libgobject-2.0.so.0

> #23 0x00007fab9d441bb4 in gtk_widget_event_internal.part.0.lto_priv () at /lib64/libgtk-3.so.0

> #24 0x00007fab9d2e000f in propagate_event.lto_priv () at /lib64/libgtk-3.so.0

> #25 0x00007fab9d2e1223 in gtk_main_do_event () at /lib64/libgtk-3.so.0

> #26 0x00007fab9cfbb633 in _gdk_event_emit () at /lib64/libgdk-3.so.0

> #27 0x00007fab9d022ba6 in gdk_event_source_dispatch.lto_priv () at /lib64/libgdk-3.so.0

> #28 0x00007fab9db3296f in g_main_context_dispatch () at /lib64/libglib-2.0.so.0

> #29 0x000055f3f9c24f58 in main_loop_wait ()

> #30 0x000055f3f9a564b1 in qemu_main_loop ()

> #31 0x000055f3f97609ee in main ()

> 

> take care,

>   Gerd

> 

-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff mbox series

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index cd068389de..ff2d7aa8f3 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -254,6 +254,7 @@  ERST
         .help       = "save screen from head 'head' of display device 'device' "
                       "into PPM image 'filename'",
         .cmd        = hmp_screendump,
+        .coroutine  = true,
     },
 
 SRST
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 9789f4277f..91608bac6d 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1756,7 +1756,8 @@  err_out:
     goto out;
 }
 
-void hmp_screendump(Monitor *mon, const QDict *qdict)
+void coroutine_fn
+hmp_screendump(Monitor *mon, const QDict *qdict)
 {
     const char *filename = qdict_get_str(qdict, "filename");
     const char *id = qdict_get_try_str(qdict, "device");
diff --git a/qapi/ui.json b/qapi/ui.json
index 9d6721037f..6c7b33cb72 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -98,7 +98,8 @@ 
 #
 ##
 { 'command': 'screendump',
-  'data': {'filename': 'str', '*device': 'str', '*head': 'int'} }
+  'data': {'filename': 'str', '*device': 'str', '*head': 'int'},
+  'coroutine': true }
 
 ##
 # == Spice
diff --git a/ui/console.c b/ui/console.c
index 96dd212a5d..e8e59707d3 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -168,6 +168,7 @@  struct QemuConsole {
     QEMUFIFO out_fifo;
     uint8_t out_fifo_buf[16];
     QEMUTimer *kbd_timer;
+    CoQueue dump_queue;
 
     QTAILQ_ENTRY(QemuConsole) next;
 };
@@ -263,6 +264,7 @@  static void gui_setup_refresh(DisplayState *ds)
 
 void graphic_hw_update_done(QemuConsole *con)
 {
+    qemu_co_queue_restart_all(&con->dump_queue);
 }
 
 void graphic_hw_update(QemuConsole *con)
@@ -340,8 +342,15 @@  static bool ppm_save(int fd, pixman_image_t *image, Error **errp)
     return true;
 }
 
-void qmp_screendump(const char *filename, bool has_device, const char *device,
-                    bool has_head, int64_t head, Error **errp)
+static void graphic_hw_update_bh(void *con)
+{
+    graphic_hw_update(con);
+}
+
+/* Safety: coroutine-only, concurrent-coroutine safe, main thread only */
+void coroutine_fn
+qmp_screendump(const char *filename, bool has_device, const char *device,
+               bool has_head, int64_t head, Error **errp)
 {
     g_autoptr(pixman_image_t) image = NULL;
     QemuConsole *con;
@@ -366,7 +375,18 @@  void qmp_screendump(const char *filename, bool has_device, const char *device,
         }
     }
 
-    graphic_hw_update(con);
+    if (qemu_co_queue_empty(&con->dump_queue)) {
+        /* Defer the update, it will restart the pending coroutines */
+        aio_bh_schedule_oneshot(qemu_get_aio_context(),
+                                graphic_hw_update_bh, con);
+    }
+    qemu_co_queue_wait(&con->dump_queue, NULL);
+
+    /*
+     * All pending coroutines are woken up, while the BQL is held.  No
+     * further graphic update are possible until it is released.  Take
+     * an image ref before that.
+     */
     surface = qemu_console_surface(con);
     if (!surface) {
         error_setg(errp, "no surface");
@@ -381,6 +401,11 @@  void qmp_screendump(const char *filename, bool has_device, const char *device,
         return;
     }
 
+    /*
+     * The image content could potentially be updated as the coroutine
+     * yields and releases the BQL. It could produce corrupted dump, but
+     * it should be otherwise safe.
+     */
     if (!ppm_save(fd, image, errp)) {
         qemu_unlink(filename);
     }
@@ -1297,6 +1322,7 @@  static QemuConsole *new_console(DisplayState *ds, console_type_t console_type,
 
     obj = object_new(TYPE_QEMU_CONSOLE);
     s = QEMU_CONSOLE(obj);
+    qemu_co_queue_init(&s->dump_queue);
     s->head = head;
     object_property_add_link(obj, "device", TYPE_DEVICE,
                              (Object **)&s->device,