All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] console: make QMP screendump use coroutine
@ 2020-10-27 13:35 marcandre.lureau
  2020-10-27 13:36 ` [PATCH v2 1/3] coroutine: let CoQueue wake up outside a coroutine marcandre.lureau
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: marcandre.lureau @ 2020-10-27 13:35 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Markus Armbruster, Dr. David Alan Gilbert, kraxel,
	Stefan Hajnoczi, Marc-André Lureau

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

Hi,

Thanks to recent work by Kevin, it becomes possible to run HMP/QMP commands i=
n a
coroutine. The screendump command is a good target, as it requires to re-enter
the main-loop in ordre to flush the display, and write to file. Ideally, IO
would be done with a non-blocking fd, however it's not currently enabled. This
is left for a future work.

v2:
 - change summary to not falsly claim non-blocking write support
 - code styles fixes
 - rebased, add reviewed-by tags

Marc-Andr=C3=A9 Lureau (3):
  coroutine: let CoQueue wake up outside a coroutine
  console: modify ppm_save to take a pixman image ref
  console: make QMP/HMP screendump run in coroutine

 hmp-commands.hx            |  1 +
 monitor/hmp-cmds.c         |  3 ++-
 qapi/ui.json               |  3 ++-
 ui/console.c               | 47 ++++++++++++++++++++++++++++++--------
 ui/trace-events            |  2 +-
 util/qemu-coroutine-lock.c |  6 ++---
 6 files changed, 45 insertions(+), 17 deletions(-)

--=20
2.29.0




^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH v2 1/3] coroutine: let CoQueue wake up outside a coroutine
  2020-10-27 13:35 [PATCH v2 0/3] console: make QMP screendump use coroutine marcandre.lureau
@ 2020-10-27 13:36 ` marcandre.lureau
  2020-10-27 13:36 ` [PATCH v2 2/3] console: modify ppm_save to take a pixman image ref marcandre.lureau
  2020-10-27 13:36 ` [PATCH v2 3/3] console: make QMP/HMP screendump run in coroutine marcandre.lureau
  2 siblings, 0 replies; 10+ messages in thread
From: marcandre.lureau @ 2020-10-27 13:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Markus Armbruster, Dr. David Alan Gilbert, kraxel,
	Stefan Hajnoczi, Marc-André Lureau

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

The assert() was added in commit b681a1c73e15 ("block: Repair the
throttling code."), when the qemu_co_queue_do_restart() function
required to be running in a coroutine. It was later made unnecessary in
commit a9d9235567e7 ("coroutine-lock: reschedule coroutine on the
AioContext it was running on").

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
---
 util/qemu-coroutine-lock.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/util/qemu-coroutine-lock.c b/util/qemu-coroutine-lock.c
index 36927b5f88..5816bf8900 100644
--- a/util/qemu-coroutine-lock.c
+++ b/util/qemu-coroutine-lock.c
@@ -85,15 +85,13 @@ static bool qemu_co_queue_do_restart(CoQueue *queue, bool single)
     return true;
 }
 
-bool coroutine_fn qemu_co_queue_next(CoQueue *queue)
+bool qemu_co_queue_next(CoQueue *queue)
 {
-    assert(qemu_in_coroutine());
     return qemu_co_queue_do_restart(queue, true);
 }
 
-void coroutine_fn qemu_co_queue_restart_all(CoQueue *queue)
+void qemu_co_queue_restart_all(CoQueue *queue)
 {
-    assert(qemu_in_coroutine());
     qemu_co_queue_do_restart(queue, false);
 }
 
-- 
2.29.0



^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH v2 2/3] console: modify ppm_save to take a pixman image ref
  2020-10-27 13:35 [PATCH v2 0/3] console: make QMP screendump use coroutine marcandre.lureau
  2020-10-27 13:36 ` [PATCH v2 1/3] coroutine: let CoQueue wake up outside a coroutine marcandre.lureau
@ 2020-10-27 13:36 ` marcandre.lureau
  2020-11-03 13:24   ` Gerd Hoffmann
  2020-10-27 13:36 ` [PATCH v2 3/3] console: make QMP/HMP screendump run in coroutine marcandre.lureau
  2 siblings, 1 reply; 10+ messages in thread
From: marcandre.lureau @ 2020-10-27 13:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Markus Armbruster, Dr. David Alan Gilbert, kraxel,
	Stefan Hajnoczi, Marc-André Lureau

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

The function is going to be called from a coroutine, and may yield.
Let's ensure our image reference doesn't change over time (due to resize
etc) by keeping a ref.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/console.c    | 15 ++++++++-------
 ui/trace-events |  2 +-
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/ui/console.c b/ui/console.c
index 820e408170..96dd212a5d 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -195,7 +195,6 @@ static void dpy_refresh(DisplayState *s);
 static DisplayState *get_alloc_displaystate(void);
 static void text_console_update_cursor_timer(void);
 static void text_console_update_cursor(void *opaque);
-static bool ppm_save(int fd, DisplaySurface *ds, Error **errp);
 
 static void gui_update(void *opaque)
 {
@@ -311,16 +310,16 @@ void graphic_hw_invalidate(QemuConsole *con)
     }
 }
 
-static bool ppm_save(int fd, DisplaySurface *ds, Error **errp)
+static bool ppm_save(int fd, pixman_image_t *image, Error **errp)
 {
-    int width = pixman_image_get_width(ds->image);
-    int height = pixman_image_get_height(ds->image);
+    int width = pixman_image_get_width(image);
+    int height = pixman_image_get_height(image);
     g_autoptr(Object) ioc = OBJECT(qio_channel_file_new_fd(fd));
     g_autofree char *header = NULL;
     g_autoptr(pixman_image_t) linebuf = NULL;
     int y;
 
-    trace_ppm_save(fd, ds);
+    trace_ppm_save(fd, image);
 
     header = g_strdup_printf("P6\n%d %d\n%d\n", width, height, 255);
     if (qio_channel_write_all(QIO_CHANNEL(ioc),
@@ -330,7 +329,7 @@ static bool ppm_save(int fd, DisplaySurface *ds, Error **errp)
 
     linebuf = qemu_pixman_linebuf_create(PIXMAN_BE_r8g8b8, width);
     for (y = 0; y < height; y++) {
-        qemu_pixman_linebuf_fill(linebuf, ds->image, width, 0, y);
+        qemu_pixman_linebuf_fill(linebuf, image, width, 0, y);
         if (qio_channel_write_all(QIO_CHANNEL(ioc),
                                   (char *)pixman_image_get_data(linebuf),
                                   pixman_image_get_stride(linebuf), errp) < 0) {
@@ -344,6 +343,7 @@ static bool ppm_save(int fd, DisplaySurface *ds, Error **errp)
 void 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;
     DisplaySurface *surface;
     int fd;
@@ -372,6 +372,7 @@ void qmp_screendump(const char *filename, bool has_device, const char *device,
         error_setg(errp, "no surface");
         return;
     }
+    image = pixman_image_ref(surface->image);
 
     fd = qemu_open_old(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0666);
     if (fd == -1) {
@@ -380,7 +381,7 @@ void qmp_screendump(const char *filename, bool has_device, const char *device,
         return;
     }
 
-    if (!ppm_save(fd, surface, errp)) {
+    if (!ppm_save(fd, image, errp)) {
         qemu_unlink(filename);
     }
 }
diff --git a/ui/trace-events b/ui/trace-events
index b7d7270c02..0ffcdb4408 100644
--- a/ui/trace-events
+++ b/ui/trace-events
@@ -15,7 +15,7 @@ displaysurface_create_pixman(void *display_surface) "surface=%p"
 displaysurface_free(void *display_surface) "surface=%p"
 displaychangelistener_register(void *dcl, const char *name) "%p [ %s ]"
 displaychangelistener_unregister(void *dcl, const char *name) "%p [ %s ]"
-ppm_save(int fd, void *display_surface) "fd=%d surface=%p"
+ppm_save(int fd, void *image) "fd=%d image=%p"
 
 # gtk-egl.c
 # gtk-gl-area.c
-- 
2.29.0



^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH v2 3/3] console: make QMP/HMP screendump run in coroutine
  2020-10-27 13:35 [PATCH v2 0/3] console: make QMP screendump use coroutine marcandre.lureau
  2020-10-27 13:36 ` [PATCH v2 1/3] coroutine: let CoQueue wake up outside a coroutine marcandre.lureau
  2020-10-27 13:36 ` [PATCH v2 2/3] console: modify ppm_save to take a pixman image ref marcandre.lureau
@ 2020-10-27 13:36 ` marcandre.lureau
  2020-10-27 16:01   ` Markus Armbruster
  2021-01-20 14:18   ` Gerd Hoffmann
  2 siblings, 2 replies; 10+ messages in thread
From: marcandre.lureau @ 2020-10-27 13:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Markus Armbruster, Dr. David Alan Gilbert, kraxel,
	Stefan Hajnoczi, Marc-André Lureau

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(-)

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,
-- 
2.29.0



^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] console: make QMP/HMP screendump run in coroutine
  2020-10-27 13:36 ` [PATCH v2 3/3] console: make QMP/HMP screendump run in coroutine marcandre.lureau
@ 2020-10-27 16:01   ` Markus Armbruster
  2021-01-20 14:18   ` Gerd Hoffmann
  1 sibling, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2020-10-27 16:01 UTC (permalink / raw)
  To: marcandre.lureau
  Cc: Kevin Wolf, kraxel, qemu-devel, Stefan Hajnoczi, Dr. David Alan Gilbert

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>



^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/3] console: modify ppm_save to take a pixman image ref
  2020-10-27 13:36 ` [PATCH v2 2/3] console: modify ppm_save to take a pixman image ref marcandre.lureau
@ 2020-11-03 13:24   ` Gerd Hoffmann
  0 siblings, 0 replies; 10+ messages in thread
From: Gerd Hoffmann @ 2020-11-03 13:24 UTC (permalink / raw)
  To: marcandre.lureau
  Cc: Kevin Wolf, Markus Armbruster, qemu-devel,
	Dr. David Alan Gilbert, Stefan Hajnoczi

On Tue, Oct 27, 2020 at 05:36:01PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> The function is going to be called from a coroutine, and may yield.
> Let's ensure our image reference doesn't change over time (due to resize
> etc) by keeping a ref.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

Added all to UI queue.

thanks,
  Gerd



^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] console: make QMP/HMP screendump run in coroutine
  2020-10-27 13:36 ` [PATCH v2 3/3] console: make QMP/HMP screendump run in coroutine marcandre.lureau
  2020-10-27 16:01   ` Markus Armbruster
@ 2021-01-20 14:18   ` Gerd Hoffmann
  2021-01-20 14:29     ` Marc-André Lureau
  1 sibling, 1 reply; 10+ messages in thread
From: Gerd Hoffmann @ 2021-01-20 14:18 UTC (permalink / raw)
  To: marcandre.lureau
  Cc: Kevin Wolf, Stefan Hajnoczi, qemu-devel, Dr. David Alan Gilbert,
	Markus Armbruster

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



^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] console: make QMP/HMP screendump run in coroutine
  2021-01-20 14:18   ` Gerd Hoffmann
@ 2021-01-20 14:29     ` Marc-André Lureau
  2021-01-20 16:09       ` Gerd Hoffmann
  0 siblings, 1 reply; 10+ messages in thread
From: Marc-André Lureau @ 2021-01-20 14:29 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: Kevin Wolf, Markus Armbruster, QEMU, Stefan Hajnoczi,
	Dr. David Alan Gilbert

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


^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] console: make QMP/HMP screendump run in coroutine
  2021-01-20 14:29     ` Marc-André Lureau
@ 2021-01-20 16:09       ` Gerd Hoffmann
  2021-01-20 16:25         ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 10+ messages in thread
From: Gerd Hoffmann @ 2021-01-20 16:09 UTC (permalink / raw)
  To: Marc-André Lureau
  Cc: Kevin Wolf, Markus Armbruster, QEMU, Stefan Hajnoczi,
	Dr. David Alan Gilbert

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



^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] console: make QMP/HMP screendump run in coroutine
  2021-01-20 16:09       ` Gerd Hoffmann
@ 2021-01-20 16:25         ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 10+ messages in thread
From: Dr. David Alan Gilbert @ 2021-01-20 16:25 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: Kevin Wolf, Marc-André Lureau, QEMU, Stefan Hajnoczi,
	Markus Armbruster

* 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



^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-01-20 16:29 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-27 13:35 [PATCH v2 0/3] console: make QMP screendump use coroutine marcandre.lureau
2020-10-27 13:36 ` [PATCH v2 1/3] coroutine: let CoQueue wake up outside a coroutine marcandre.lureau
2020-10-27 13:36 ` [PATCH v2 2/3] console: modify ppm_save to take a pixman image ref marcandre.lureau
2020-11-03 13:24   ` Gerd Hoffmann
2020-10-27 13:36 ` [PATCH v2 3/3] console: make QMP/HMP screendump run in coroutine marcandre.lureau
2020-10-27 16:01   ` Markus Armbruster
2021-01-20 14:18   ` Gerd Hoffmann
2021-01-20 14:29     ` Marc-André Lureau
2021-01-20 16:09       ` Gerd Hoffmann
2021-01-20 16:25         ` Dr. David Alan Gilbert

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.