[Qemu-devel] [PATCH v4] qapi: Add device ID and head parameters to screendump

Thomas Huth posted 1 patch 7 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1520267868-31778-1-git-send-email-thuth@redhat.com
Test checkpatch passed
Test docker-build@min-glib passed
Test docker-mingw@fedora passed
Test docker-quick@centos6 passed
Test ppcbe passed
Test ppcle passed
Test s390x passed
hmp-commands.hx |  7 ++++---
hmp.c           |  4 +++-
qapi/ui.json    | 10 +++++++++-
ui/console.c    | 24 +++++++++++++++++++-----
4 files changed, 35 insertions(+), 10 deletions(-)
[Qemu-devel] [PATCH v4] qapi: Add device ID and head parameters to screendump
Posted by Thomas Huth 7 years, 7 months ago
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 or other display heads yet. So let's add an 'device' and
a 'head' parameter to the HMP and QMP commands to be able to specify
alternative devices and heads with the screendump command, too.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 v4:
 - Improved the documentation of the parameters in ui.json a little bit
   (according to the suggestions from Eric)

 hmp-commands.hx |  7 ++++---
 hmp.c           |  4 +++-
 qapi/ui.json    | 10 +++++++++-
 ui/console.c    | 24 +++++++++++++++++++-----
 4 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 964eb51..1723cbe 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -253,9 +253,10 @@ ETEXI
 
     {
         .name       = "screendump",
-        .args_type  = "filename:F",
-        .params     = "filename",
-        .help       = "save screen into PPM image 'filename'",
+        .args_type  = "filename:F,device:s?,head:i?",
+        .params     = "filename [device [head]]",
+        .help       = "save screen from head 'head' of display device 'device' "
+                      "into PPM image 'filename'",
         .cmd        = hmp_screendump,
     },
 
diff --git a/hmp.c b/hmp.c
index 016cb5c..ba9e299 100644
--- a/hmp.c
+++ b/hmp.c
@@ -2140,9 +2140,11 @@ 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, "device");
+    int64_t head = qdict_get_try_int(qdict, "head", 0);
     Error *err = NULL;
 
-    qmp_screendump(filename, &err);
+    qmp_screendump(filename, id != NULL, id, id != NULL, head, &err);
     hmp_handle_error(mon, &err);
 }
 
diff --git a/qapi/ui.json b/qapi/ui.json
index 3e82f25..5d01ad4 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -77,6 +77,13 @@
 #
 # @filename: the path of a new PPM file to store the image
 #
+# @device: ID of the display device that should be dumped. If this parameter
+#          is missing, the primary display will be used. (Since 2.12)
+#
+# @head: head to use in case the device supports multiple heads. If this
+#        parameter is missing, head #0 will be used. Also note that the head
+#        can only be specified in conjunction with the device ID. (Since 2.12)
+#
 # Returns: Nothing on success
 #
 # Since: 0.14.0
@@ -88,7 +95,8 @@
 # <- { "return": {} }
 #
 ##
-{ 'command': 'screendump', 'data': {'filename': 'str'} }
+{ 'command': 'screendump',
+  'data': {'filename': 'str', '*device': 'str', '*head': 'int'} }
 
 ##
 # == Spice
diff --git a/ui/console.c b/ui/console.c
index 6a1f499..f2794c7 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -344,14 +344,28 @@ write_err:
     goto out;
 }
 
-void qmp_screendump(const char *filename, Error **errp)
+void qmp_screendump(const char *filename, bool has_device, const char *device,
+                    bool has_head, int64_t head, Error **errp)
 {
-    QemuConsole *con = qemu_console_lookup_by_index(0);
+    QemuConsole *con;
     DisplaySurface *surface;
 
-    if (con == NULL) {
-        error_setg(errp, "There is no QemuConsole I can screendump from.");
-        return;
+    if (has_device) {
+        con = qemu_console_lookup_by_device_name(device, has_head ? head : 0,
+                                                 errp);
+        if (!con) {
+            return;
+        }
+    } else {
+        if (has_head) {
+            error_setg(errp, "'head' must be specified together with 'device'");
+            return;
+        }
+        con = qemu_console_lookup_by_index(0);
+        if (!con) {
+            error_setg(errp, "There is no console to take a screendump from");
+            return;
+        }
     }
 
     graphic_hw_update(con);
-- 
1.8.3.1


Re: [Qemu-devel] [PATCH v4] qapi: Add device ID and head parameters to screendump
Posted by Eric Blake 7 years, 7 months ago
On 03/05/2018 10:37 AM, Thomas Huth wrote:
> 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 or other display heads yet. So let's add an 'device' and

s/an/a/

> a 'head' parameter to the HMP and QMP commands to be able to specify
> alternative devices and heads with the screendump command, too.
> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v4] qapi: Add device ID and head parameters to screendump
Posted by Thomas Huth 7 years, 7 months ago
On 05.03.2018 17:50, Eric Blake wrote:
> On 03/05/2018 10:37 AM, Thomas Huth wrote:
>> 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 or other display heads yet. So let's add an 'device' and
> 
> s/an/a/

Gerd, could you please fix that up when picking up the patch? Thanks!

>> a 'head' parameter to the HMP and QMP commands to be able to specify
>> alternative devices and heads with the screendump command, too.
>>
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks for the reviews!

 Thomas

Re: [Qemu-devel] [PATCH v4] qapi: Add device ID and head parameters to screendump
Posted by Eric Blake 7 years, 7 months ago
On 03/05/2018 10:37 AM, Thomas Huth wrote:
> 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 or other display heads yet. So let's add an 'device' and
> a 'head' parameter to the HMP and QMP commands to be able to specify
> alternative devices and heads with the screendump command, too.
> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   v4:
>   - Improved the documentation of the parameters in ui.json a little bit
>     (according to the suggestions from Eric)

We're getting close to soft freeze; I'll include this in my QAPI tree, 
since I have an upcoming pull request, rather than figuring out if Gerd 
is also doing one more pull (but of course, if he does, it doesn't hurt; 
git handles patches on multiple merge branches just fine, or I can 
rebase it out of mine if Gerd's tree goes in first).

Queued:

git://repo.or.cz/qemu/ericb.git qapi
http://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/qapi

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v4] qapi: Add device ID and head parameters to screendump
Posted by Eric Blake 7 years, 7 months ago
On 03/10/2018 09:05 PM, Eric Blake wrote:
> On 03/05/2018 10:37 AM, Thomas Huth wrote:
>> 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 or other display heads yet. So let's add an 'device' and
>> a 'head' parameter to the HMP and QMP commands to be able to specify
>> alternative devices and heads with the screendump command, too.
>>
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   v4:
>>   - Improved the documentation of the parameters in ui.json a little bit
>>     (according to the suggestions from Eric)
> 
> We're getting close to soft freeze; I'll include this in my QAPI tree, 
> since I have an upcoming pull request, rather than figuring out if Gerd 
> is also doing one more pull (but of course, if he does, it doesn't hurt; 
> git handles patches on multiple merge branches just fine, or I can 
> rebase it out of mine if Gerd's tree goes in first).
> 
> Queued:
> 
> git://repo.or.cz/qemu/ericb.git qapi
> http://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/qapi

Removed from my QAPI queue; Gerd picked it up in his PR:
https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg03266.html

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org