[Qemu-devel] [PATCH] make screendump an async command

[Alon Levy <alevy@redhat.com>]

This fixes the broken screendump behavior for qxl devices in native mode
since 81fb6f1504fb9ef71f2382f44af34756668296e8.

Note: due to QAPI not generating async commands yet I had to remove the
schema screendump definition.

Related RHBZ: 973374
This patch is not enough to fix said bz, with the linux qxl driver you get garbage still, just not out of date garbage.

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hmp.c                     |  2 +-
 hw/display/qxl-render.c   |  1 +
 hw/display/vga.c          |  1 +
 include/qapi/qmp/qerror.h |  6 +++++
 include/ui/console.h      | 10 ++++++++
 qapi-schema.json          | 13 -----------
 qmp-commands.hx           |  3 ++-
 ui/console.c              | 58 ++++++++++++++++++++++++++++++++++++++++++++++-
 8 files changed, 78 insertions(+), 16 deletions(-)

diff --git a/hmp.c b/hmp.c
index 494a9aa..2a37975 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1346,7 +1346,7 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
     const char *filename = qdict_get_str(qdict, "filename");
     Error *err = NULL;
 
-    qmp_screendump(filename, &err);
+    hmp_screen_dump_helper(filename, &err);
     hmp_handle_error(mon, &err);
 }
 
diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index f511a62..d719448 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -139,6 +139,7 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
                        qxl->dirty[i].bottom - qxl->dirty[i].top);
     }
     qxl->num_dirty_rects = 0;
+    console_screendump_complete(vga->con);
 }
 
 /*
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 21a108d..1fc52eb 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1922,6 +1922,7 @@ static void vga_update_display(void *opaque)
             break;
         }
     }
+    console_screendump_complete(s->con);
 }
 
 /* force a full display refresh */
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 6c0a18d..1bd7efa 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -237,6 +237,12 @@ void assert_no_error(Error *err);
 #define QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION \
     ERROR_CLASS_GENERIC_ERROR, "Migration is disabled when VirtFS export path '%s' is mounted in the guest using mount_tag '%s'"
 
+#define QERR_SCREENDUMP_NOT_AVAILABLE \
+    ERROR_CLASS_GENERIC_ERROR, "There is no QemuConsole I can screendump from"
+
+#define QERR_SCREENDUMP_IN_PROGRESS \
+    ERROR_CLASS_GENERIC_ERROR, "Existing screendump in progress"
+
 #define QERR_SOCKET_CONNECT_FAILED \
     ERROR_CLASS_GENERIC_ERROR, "Failed to connect to socket"
 
diff --git a/include/ui/console.h b/include/ui/console.h
index 4307b5f..643da45 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -341,4 +341,14 @@ int index_from_keycode(int code);
 void early_gtk_display_init(void);
 void gtk_display_init(DisplayState *ds);
 
+/* hw/display/vga.c hw/display/qxl.c */
+void console_screendump_complete(QemuConsole *con);
+
+/* monitor.c */
+int qmp_screendump(Monitor *mon, const QDict *qdict,
+                   MonitorCompletion cb, void *opaque);
+
+/* hmp.c */
+void hmp_screen_dump_helper(const char *filename, Error **errp);
+
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 5ad6894..f5fdc2f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3112,19 +3112,6 @@
   'data': { 'keys': ['KeyValue'], '*hold-time': 'int' } }
 
 ##
-# @screendump:
-#
-# Write a PPM of the VGA screen to a file.
-#
-# @filename: the path of a new PPM file to store the image
-#
-# Returns: Nothing on success
-#
-# Since: 0.14.0
-##
-{ 'command': 'screendump', 'data': {'filename': 'str'} }
-
-##
 # @nbd-server-start:
 #
 # Start an NBD server listening on the given host and port.  Block
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 8cea5e5..bde8f43 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -146,7 +146,8 @@ EQMP
     {
         .name       = "screendump",
         .args_type  = "filename:F",
-        .mhandler.cmd_new = qmp_marshal_input_screendump,
+        .mhandler.cmd_async = qmp_screendump,
+        .flags      = MONITOR_CMD_ASYNC,
     },
 
 SQMP
diff --git a/ui/console.c b/ui/console.c
index 28bba6d..0efd588 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -116,6 +116,12 @@ typedef enum {
 struct QemuConsole {
     Object parent;
 
+    struct {
+        char *filename;
+        MonitorCompletion *completion_cb;
+        void *completion_opaque;
+    } screendump;
+
     int index;
     console_type_t console_type;
     DisplayState *ds;
@@ -313,11 +319,61 @@ write_err:
     goto out;
 }
 
-void qmp_screendump(const char *filename, Error **errp)
+int qmp_screendump(Monitor *mon, const QDict *qdict,
+                   MonitorCompletion cb, void *opaque)
 {
     QemuConsole *con = qemu_console_lookup_by_index(0);
+    const char *filename = qdict_get_str(qdict, "filename");
+
+    if (con == NULL) {
+        qerror_report(QERR_SCREENDUMP_NOT_AVAILABLE);
+        return -1;
+    }
+    if (filename == NULL) {
+        qerror_report(QERR_MISSING_PARAMETER, "filename");
+        return -1;
+    }
+    if (con->screendump.filename != NULL) {
+        qerror_report(QERR_SCREENDUMP_IN_PROGRESS);
+        return -1;
+    }
+    con->screendump.filename = g_strdup(filename);
+    con->screendump.completion_cb = cb;
+    con->screendump.completion_opaque = opaque;
+
+    graphic_hw_update(con);
+    return 0;
+}
+
+void console_screendump_complete(QemuConsole *con)
+{
+    Error *errp = NULL;
     DisplaySurface *surface;
 
+    if (!con->screendump.filename) {
+        return;
+    }
+    assert(con->screendump.completion_cb);
+    surface = qemu_console_surface(con);
+    ppm_save(con->screendump.filename, surface, &errp);
+    if (errp) {
+        /*
+         * TODO: return data? raise error somehow? read qmp-spec for async
+         * error reporting.
+         */
+    }
+    con->screendump.completion_cb(con->screendump.completion_opaque, NULL);
+    g_free(con->screendump.filename);
+    con->screendump.filename = NULL;
+    con->screendump.completion_cb = NULL;
+    con->screendump.completion_opaque = NULL;
+}
+
+void hmp_screen_dump_helper(const char *filename, Error **errp)
+{
+    DisplaySurface *surface;
+    QemuConsole *con = qemu_console_lookup_by_index(0);
+
     if (con == NULL) {
         error_setg(errp, "There is no QemuConsole I can screendump from.");
         return;
-- 
1.8.2.1