On Mon, Mar 05, 2018 at 10:47:56AM +0100, 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.
>
> Signed-off-by: Thomas Huth <th...@redhat.com>
> ---
> v2:
> - Renamed 'id' to 'device'
> - Add suport for specifying the 'head', too
>
> hmp-commands.hx | 7 ++++---
> hmp.c | 4 +++-
> qapi/ui.json | 7 ++++++-
> ui/console.c | 22 ++++++++++++++++++----
> 4 files changed, 31 insertions(+), 9 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index d26eb41..ebb5fe4 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 ae86bfb..7dcf72f 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -2132,9 +2132,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..e60b323 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -77,6 +77,10 @@
> #
> # @filename: the path of a new PPM file to store the image
> #
> +# @device: ID of the display device that should be used (since 2.12)
> +#
> +# @head: head to use in case the device supports multiple heads (since 2.12)
> +#
> # Returns: Nothing on success
> #
> # Since: 0.14.0
> @@ -88,7 +92,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 e22931a..76b694e 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);
> 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;
> + }
Nitpick, we don't terminate error messages with a "." normally.
Since this is just pre-existing problem though, I figure whoever queues the
patch can tweak it if needed, so
Reviewed-by: Daniel P. Berrangé <berra...@redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
