[Qemu-devel] [PATCH] qxl-render: call ppm_save on bh

[Alon Levy]

Uses the newly introduced hw_screen_dump_async. Now that the deadlock
with virt-manager is fixed we need to call ppm_save in a bh, with the
new command we can notify virt-manager using the SCREEN_DUMP_COMPLETE
event.

Signed-off-by: Alon Levy <address@hidden>
---

This goes on top of the QEVENT_SCREEN_DUMP_ASYNC and vga_hw_screen_dump_async
patches.

 hw/qxl-render.c    |   63 ++++++++++++++++++++++++++++++++++++++++++++++++---
 hw/qxl.c           |   40 ++++++++++++++++++++++++++++-----
 hw/qxl.h           |    2 +-
 ui/spice-display.h |    3 ++
 4 files changed, 97 insertions(+), 11 deletions(-)

diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index f323a4d..d1aa9ff 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -19,6 +19,7 @@
  * along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "console.h"
 #include "qxl.h"
 
 static void qxl_flip(PCIQXLDevice *qxl, QXLRect *rect)
@@ -142,19 +143,59 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice 
*qxl)
 }
 
 /*
+ * struct used just for ppm save bh. We don't actually support multiple qxl
+ * screendump yet, but a) we will, and b) exporting qxl0 from qxl.c looks
+ * uglier imo.
+ */
+typedef struct QXLPPMSaveBHData {
+    PCIQXLDevice *qxl;
+    QXLCookie *cookie;
+} QXLPPMSaveBHData;
+
+static void qxl_render_ppm_save_bh(void *opaque)
+{
+    QXLPPMSaveBHData *data = opaque;
+    PCIQXLDevice *qxl = data->qxl;
+    QXLCookie *cookie = data->cookie;
+    QEMUBH *bh = cookie->u.render.ppm_save_bh;
+
+    qemu_mutex_lock(&qxl->ssd.lock);
+    dprint(qxl, 1, "%s: %p (primary %p)\n", __func__,
+           qxl->ssd.ds->surface->data, qxl->guest_primary.data);
+    qxl_render_update_area_unlocked(qxl);
+    ppm_save(cookie->u.render.filename, qxl->ssd.ds->surface);
+    if (cookie->u.render.async) {
+        monitor_protocol_screen_dump_complete_event(cookie->u.render.filename);
+    }
+    g_free(cookie->u.render.filename);
+    --qxl->render_update_cookie_num;
+    g_free(cookie);
+    g_free(data);
+    qemu_mutex_unlock(&qxl->ssd.lock);
+    qemu_bh_delete(bh);
+}
+
+/*
  * use ssd.lock to protect render_update_cookie_num.
  * qxl_render_update is called by io thread or vcpu thread, and the completion
  * callbacks are called by spice_server thread, defering to bh called from the
  * io thread.
  */
-void qxl_render_update(PCIQXLDevice *qxl)
+void qxl_render_update(PCIQXLDevice *qxl, const char *filename, bool async)
 {
     QXLCookie *cookie;
+    QEMUBH *ppm_save_bh;
+    QXLPPMSaveBHData *ppm_save_bh_data;
 
     qemu_mutex_lock(&qxl->ssd.lock);
 
     if (!runstate_is_running() || !qxl->guest_primary.commands) {
         qxl_render_update_area_unlocked(qxl);
+        if (filename) {
+            dprint(qxl, 1, "%s: screendump with no pending commands\n",
+                   __func__);
+            ppm_save(filename, qxl->ssd.ds->surface);
+        }
         qemu_mutex_unlock(&qxl->ssd.lock);
         return;
     }
@@ -165,6 +206,15 @@ void qxl_render_update(PCIQXLDevice *qxl)
     cookie = qxl_cookie_new(QXL_COOKIE_TYPE_RENDER_UPDATE_AREA,
                             0);
     qxl_set_rect_to_surface(qxl, &cookie->u.render.area);
+    if (filename) {
+        ppm_save_bh_data = g_malloc0(sizeof(*ppm_save_bh_data));
+        ppm_save_bh_data->qxl = qxl;
+        ppm_save_bh_data->cookie = cookie;
+        ppm_save_bh = qemu_bh_new(qxl_render_ppm_save_bh, ppm_save_bh_data);
+        cookie->u.render.filename = g_strdup(filename);
+        cookie->u.render.ppm_save_bh = ppm_save_bh;
+        cookie->u.render.async = async;
+    }
     qxl_spice_update_area(qxl, 0, &cookie->u.render.area, NULL,
                           0, 1 /* clear_dirty_region */, QXL_ASYNC, cookie);
 }
@@ -181,10 +231,15 @@ void qxl_render_update_area_bh(void *opaque)
 void qxl_render_update_area_done(PCIQXLDevice *qxl, QXLCookie *cookie)
 {
     qemu_mutex_lock(&qxl->ssd.lock);
-    qemu_bh_schedule(qxl->update_area_bh);
-    qxl->render_update_cookie_num--;
+    if (cookie->u.render.filename) {
+        dprint(qxl, 1, "%s: scheduling ppm_save_bh\n", __func__);
+        qemu_bh_schedule(cookie->u.render.ppm_save_bh);
+    } else {
+        qemu_bh_schedule(qxl->update_area_bh);
+        --qxl->render_update_cookie_num;
+        g_free(cookie);
+    }
     qemu_mutex_unlock(&qxl->ssd.lock);
-    g_free(cookie);
 }
 
 static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor)
diff --git a/hw/qxl.c b/hw/qxl.c
index 61a7edd..17bf7da 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1475,7 +1475,7 @@ static void qxl_hw_update(void *opaque)
         break;
     case QXL_MODE_COMPAT:
     case QXL_MODE_NATIVE:
-        qxl_render_update(qxl);
+        qxl_render_update(qxl, NULL, false);
         break;
     default:
         break;
@@ -1490,7 +1490,8 @@ static void qxl_hw_invalidate(void *opaque)
     vga->invalidate(vga);
 }
 
-static void qxl_hw_screen_dump(void *opaque, const char *filename, bool 
cswitch)
+static void qxl_hw_screen_dump_helper(void *opaque, const char *filename,
+                                      bool cswitch, bool async)
 {
     PCIQXLDevice *qxl = opaque;
     VGACommonState *vga = &qxl->vga;
@@ -1498,17 +1499,44 @@ static void qxl_hw_screen_dump(void *opaque, const char 
*filename, bool cswitch)
     switch (qxl->mode) {
     case QXL_MODE_COMPAT:
     case QXL_MODE_NATIVE:
-        qxl_render_update(qxl);
-        ppm_save(filename, qxl->ssd.ds->surface);
+        /*
+         * TODO: if we use an async update_area, to avoid deadlock with
+         * virt-manager, we postpone the saving of the image until the
+         * rendering is done. This means the image isn't guranteed to be
+         * written when we return to the monitor. Fixing this needs an async
+         * monitor command, whatever the implementation of the concept is
+         * called.
+         */
+        qxl_render_update(qxl, filename, async);
         break;
     case QXL_MODE_VGA:
-        vga->screen_dump(vga, filename, cswitch);
+        if (async) {
+            if (vga->screen_dump_async) {
+                vga->screen_dump_async(vga, filename, cswitch);
+            } else {
+                vga->screen_dump(vga, filename, cswitch);
+                monitor_protocol_screen_dump_complete_event(filename);
+            }
+        } else {
+            vga->screen_dump(vga, filename, cswitch);
+        }
         break;
     default:
         break;
     }
 }
 
+static void qxl_hw_screen_dump(void *opaque, const char *filename, bool 
cswitch)
+{
+    qxl_hw_screen_dump_helper(opaque, filename, cswitch, false);
+}
+
+static void qxl_hw_screen_dump_async(void *opaque, const char *filename,
+                                     bool cswitch)
+{
+    qxl_hw_screen_dump_helper(opaque, filename, cswitch, true);
+}
+
 static void qxl_hw_text_update(void *opaque, console_ch_t *chardata)
 {
     PCIQXLDevice *qxl = opaque;
@@ -1728,7 +1756,7 @@ static int qxl_init_primary(PCIDevice *dev)
 
     vga->ds = graphic_console_init(qxl_hw_update, qxl_hw_invalidate,
                                    qxl_hw_screen_dump, qxl_hw_text_update,
-                                   NULL, qxl);
+                                   qxl_hw_screen_dump_async, qxl);
     qemu_spice_display_init_common(&qxl->ssd, vga->ds);
 
     qxl0 = qxl;
diff --git a/hw/qxl.h b/hw/qxl.h
index 86e415b..e727358 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -140,7 +140,7 @@ void qxl_log_command(PCIQXLDevice *qxl, const char *ring, 
QXLCommandExt *ext);
 
 /* qxl-render.c */
 void qxl_render_resize(PCIQXLDevice *qxl);
-void qxl_render_update(PCIQXLDevice *qxl);
+void qxl_render_update(PCIQXLDevice *qxl, const char *filename, bool async);
 void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext);
 void qxl_render_update_area_done(PCIQXLDevice *qxl, QXLCookie *cookie);
 void qxl_render_update_area_bh(void *opaque);
diff --git a/ui/spice-display.h b/ui/spice-display.h
index 12e50b6..ea36de2 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -62,6 +62,9 @@ typedef struct QXLCookie {
         struct {
             QXLRect area;
             int redraw;
+            char *filename;
+            QEMUBH *ppm_save_bh;
+            bool async;
         } render;
     } u;
 } QXLCookie;
-- 
1.7.9.1