[Qemu-devel] [PATCH] Allow to specify a display ID whith the screendump command

[Qemu-devel] [PATCH] Allow to specify a display ID whith the screendump command

[Thomas Huth]

QEMU's screendump command can only take dumps from the primary display.
When using multiple VGA cards, there is no way to get a dump from a
secondary card yet. So let's add an 'id' parameter to the HMP and QMP
commands to be able to specify alternative devices with the screendump
command, too.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hmp-commands.hx |  6 +++---
 hmp.c           |  3 ++-
 qapi/ui.json    |  4 +++-
 ui/console.c    | 16 +++++++++++++++-
 4 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index d26eb41..045ffb0 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -253,9 +253,9 @@ ETEXI
 
     {
         .name       = "screendump",
-        .args_type  = "filename:F",
-        .params     = "filename",
-        .help       = "save screen into PPM image 'filename'",
+        .args_type  = "filename:F,id:s?",
+        .params     = "filename [id]",
+        .help       = "save screen from device 'id' into PPM image 'filename'",
         .cmd        = hmp_screendump,
     },
 
diff --git a/hmp.c b/hmp.c
index ae86bfb..e2f46f1 100644
--- a/hmp.c
+++ b/hmp.c
@@ -2132,9 +2132,10 @@ err_out:
 void hmp_screendump(Monitor *mon, const QDict *qdict)
 {
     const char *filename = qdict_get_str(qdict, "filename");
+    const char *id = qdict_get_try_str(qdict, "id");
     Error *err = NULL;
 
-    qmp_screendump(filename, &err);
+    qmp_screendump(filename, id != NULL, id, &err);
     hmp_handle_error(mon, &err);
 }
 
diff --git a/qapi/ui.json b/qapi/ui.json
index 3e82f25..977212a 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -77,6 +77,8 @@
 #
 # @filename: the path of a new PPM file to store the image
 #
+# @id: ID of the display device that should be used (since 2.12)
+#
 # Returns: Nothing on success
 #
 # Since: 0.14.0
@@ -88,7 +90,7 @@
 # <- { "return": {} }
 #
 ##
-{ 'command': 'screendump', 'data': {'filename': 'str'} }
+{ 'command': 'screendump', 'data': {'filename': 'str', '*id': 'str'} }
 
 ##
 # == Spice
diff --git a/ui/console.c b/ui/console.c
index e22931a..a0db0c5 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -344,10 +344,24 @@ write_err:
     goto out;
 }
 
-void qmp_screendump(const char *filename, Error **errp)
+void qmp_screendump(const char *filename, bool has_id, const char *id,
+                    Error **errp)
 {
     QemuConsole *con = qemu_console_lookup_by_index(0);
     DisplaySurface *surface;
+    DeviceState *dev;
+
+    if (has_id) {
+        dev = qdev_find_recursive(sysbus_get_default(), id);
+        if (!dev) {
+            error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+                      "Device '%s' not found", id);
+            return;
+        }
+        con = qemu_console_lookup_by_device(dev, 0);
+    } else {
+        con = qemu_console_lookup_by_index(0);
+    }
 
     if (con == NULL) {
         error_setg(errp, "There is no QemuConsole I can screendump from.");
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] Allow to specify a display ID whith the screendump command

[Gerd Hoffmann]

> -void qmp_screendump(const char *filename, Error **errp)
> +void qmp_screendump(const char *filename, bool has_id, const char *id,
> +                    Error **errp)
>  {
>      QemuConsole *con = qemu_console_lookup_by_index(0);
>      DisplaySurface *surface;
> +    DeviceState *dev;
> +
> +    if (has_id) {
> +        dev = qdev_find_recursive(sysbus_get_default(), id);
> +        if (!dev) {
> +            error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> +                      "Device '%s' not found", id);
> +            return;
> +        }
> +        con = qemu_console_lookup_by_device(dev, 0);

I'd suggest to name this 'device', because this is what it actually is.
While being at it you should also add a 'head' parameter (second arg of
qemu_console_lookup_by_device()), for devices like virtio-vga which can
support multiple heads.

>      if (con == NULL) {
>          error_setg(errp, "There is no QemuConsole I can screendump from.");

This might also be refined to say something like 'device $name is not a
display device' in case qemu_console_lookup_by_device() failed.

> -- 
> 1.8.3.1
>