[Qemu-devel] [PATCH] console: extend screendump monitor cmd

[Qemu-devel] [PATCH] console: extend screendump monitor cmd

[Gerd Hoffmann]

Add an optional device parameter to the screendump command.

https://bugzilla.redhat.com/show_bug.cgi?id=903910

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hmp-commands.hx  |    6 +++---
 hmp.c            |    3 ++-
 qapi-schema.json |    4 +++-
 qmp-commands.hx  |    3 ++-
 ui/console.c     |   17 ++++++++++++++++-
 5 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 6991a97..5a44bda 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -237,9 +237,9 @@ ETEXI
 
     {
         .name       = "screendump",
-        .args_type  = "filename:F",
-        .params     = "filename",
-        .help       = "save screen into PPM image 'filename'",
+        .args_type  = "filename:F,device:B?",
+        .params     = "filename [device]",
+        .help       = "save screen from device into PPM image 'filename'",
         .mhandler.cmd = hmp_screen_dump,
     },
 
diff --git a/hmp.c b/hmp.c
index 55f195f..21fb37e 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1344,9 +1344,10 @@ err_out:
 void hmp_screen_dump(Monitor *mon, const QDict *qdict)
 {
     const char *filename = qdict_get_str(qdict, "filename");
+    const char *device = qdict_get_try_str(qdict, "device");
     Error *err = NULL;
 
-    qmp_screendump(filename, &err);
+    qmp_screendump(filename, device != NULL, device, &err);
     hmp_handle_error(mon, &err);
 }
 
diff --git a/qapi-schema.json b/qapi-schema.json
index adcf801..6cfccf4 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3117,12 +3117,14 @@
 # Write a PPM of the VGA screen to a file.
 #
 # @filename: the path of a new PPM file to store the image
+# @device: #optional device to take the screenshot from
 #
 # Returns: Nothing on success
 #
 # Since: 0.14.0
 ##
-{ 'command': 'screendump', 'data': {'filename': 'str'} }
+{ 'command': 'screendump', 'data': {'filename': 'str',
+                                    '*device' : 'str'} }
 
 ##
 # @nbd-server-start:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 8e69fba..27b138d 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -145,7 +145,7 @@ EQMP
 
     {
         .name       = "screendump",
-        .args_type  = "filename:F",
+        .args_type  = "filename:F,device:B?",
         .mhandler.cmd_new = qmp_marshal_input_screendump,
     },
 
@@ -158,6 +158,7 @@ Save screen into PPM image.
 Arguments:
 
 - "filename": file path (json-string)
+- "device": video device (json-string, optional)
 
 Example:
 
diff --git a/ui/console.c b/ui/console.c
index 07d4d63..a1401f2 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -313,10 +313,25 @@ write_err:
     goto out;
 }
 
-void qmp_screendump(const char *filename, Error **errp)
+void qmp_screendump(const char *filename,
+                    bool has_device, const char *device, Error **errp)
 {
     QemuConsole *con = qemu_console_lookup_by_index(0);
     DisplaySurface *surface;
+    DeviceState *dev;
+
+    if (has_device) {
+        dev = qdev_find_recursive(sysbus_get_default(), device);
+        if (NULL == dev) {
+            error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+            return;
+        }
+        con = qemu_console_lookup_by_device(dev);
+        if (NULL == con) {
+            error_setg(errp, "There is no QemuConsole linked to %s", device);
+            return;
+        }
+    }
 
     if (con == NULL) {
         error_setg(errp, "There is no QemuConsole I can screendump from.");
-- 
1.7.9.7

Re: [Qemu-devel] [PATCH] console: extend screendump monitor cmd

[Luiz Capitulino]

On Wed, 12 Jun 2013 13:51:17 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> Add an optional device parameter to the screendump command.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=903910
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hmp-commands.hx  |    6 +++---
>  hmp.c            |    3 ++-
>  qapi-schema.json |    4 +++-
>  qmp-commands.hx  |    3 ++-
>  ui/console.c     |   17 ++++++++++++++++-
>  5 files changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 6991a97..5a44bda 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -237,9 +237,9 @@ ETEXI
>  
>      {
>          .name       = "screendump",
> -        .args_type  = "filename:F",
> -        .params     = "filename",
> -        .help       = "save screen into PPM image 'filename'",
> +        .args_type  = "filename:F,device:B?",
> +        .params     = "filename [device]",
> +        .help       = "save screen from device into PPM image 'filename'",
>          .mhandler.cmd = hmp_screen_dump,
>      },
>  
> diff --git a/hmp.c b/hmp.c
> index 55f195f..21fb37e 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1344,9 +1344,10 @@ err_out:
>  void hmp_screen_dump(Monitor *mon, const QDict *qdict)
>  {
>      const char *filename = qdict_get_str(qdict, "filename");
> +    const char *device = qdict_get_try_str(qdict, "device");
>      Error *err = NULL;
>  
> -    qmp_screendump(filename, &err);
> +    qmp_screendump(filename, device != NULL, device, &err);
>      hmp_handle_error(mon, &err);
>  }
>  
> diff --git a/qapi-schema.json b/qapi-schema.json
> index adcf801..6cfccf4 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3117,12 +3117,14 @@
>  # Write a PPM of the VGA screen to a file.
>  #
>  # @filename: the path of a new PPM file to store the image
> +# @device: #optional device to take the screenshot from
>  #
>  # Returns: Nothing on success
>  #
>  # Since: 0.14.0
>  ##
> -{ 'command': 'screendump', 'data': {'filename': 'str'} }
> +{ 'command': 'screendump', 'data': {'filename': 'str',
> +                                    '*device' : 'str'} }

We can't add new optional parameters to QMP commands because it's
currently impossible for mngt apps to discover them.

We have two options: 1. add this as a new command or 2. wait for
full schema introspection (which we might get for 1.6).

>  
>  ##
>  # @nbd-server-start:
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 8e69fba..27b138d 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -145,7 +145,7 @@ EQMP
>  
>      {
>          .name       = "screendump",
> -        .args_type  = "filename:F",
> +        .args_type  = "filename:F,device:B?",
>          .mhandler.cmd_new = qmp_marshal_input_screendump,
>      },
>  
> @@ -158,6 +158,7 @@ Save screen into PPM image.
>  Arguments:
>  
>  - "filename": file path (json-string)
> +- "device": video device (json-string, optional)
>  
>  Example:
>  
> diff --git a/ui/console.c b/ui/console.c
> index 07d4d63..a1401f2 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -313,10 +313,25 @@ write_err:
>      goto out;
>  }
>  
> -void qmp_screendump(const char *filename, Error **errp)
> +void qmp_screendump(const char *filename,
> +                    bool has_device, const char *device, Error **errp)
>  {
>      QemuConsole *con = qemu_console_lookup_by_index(0);
>      DisplaySurface *surface;
> +    DeviceState *dev;
> +
> +    if (has_device) {
> +        dev = qdev_find_recursive(sysbus_get_default(), device);
> +        if (NULL == dev) {
> +            error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> +            return;
> +        }
> +        con = qemu_console_lookup_by_device(dev);
> +        if (NULL == con) {
> +            error_setg(errp, "There is no QemuConsole linked to %s", device);
> +            return;
> +        }
> +    }
>  
>      if (con == NULL) {
>          error_setg(errp, "There is no QemuConsole I can screendump from.");

Re: [Qemu-devel] [PATCH] console: extend screendump monitor cmd

[Gerd Hoffmann]

  Hi,

>> -{ 'command': 'screendump', 'data': {'filename': 'str'} }
>> +{ 'command': 'screendump', 'data': {'filename': 'str',
>> +                                    '*device' : 'str'} }
> 
> We can't add new optional parameters to QMP commands because it's
> currently impossible for mngt apps to discover them.
> 
> We have two options: 1. add this as a new command or 2. wait for
> full schema introspection (which we might get for 1.6).

"might get" sounds like it isn't a save bet, so I guess I better should
go for option (1) ...

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] console: extend screendump monitor cmd

[Luiz Capitulino]

On Wed, 12 Jun 2013 15:21:20 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

>   Hi,
> 
> >> -{ 'command': 'screendump', 'data': {'filename': 'str'} }
> >> +{ 'command': 'screendump', 'data': {'filename': 'str',
> >> +                                    '*device' : 'str'} }
> > 
> > We can't add new optional parameters to QMP commands because it's
> > currently impossible for mngt apps to discover them.
> > 
> > We have two options: 1. add this as a new command or 2. wait for
> > full schema introspection (which we might get for 1.6).
> 
> "might get" sounds like it isn't a save bet, so I guess I better should
> go for option (1) ...

Amos, do you think we'll get it for 1.6?

Re: [Qemu-devel] [PATCH] console: extend screendump monitor cmd

[Amos Kong]

On Wed, Jun 12, 2013 at 09:23:37AM -0400, Luiz Capitulino wrote:
> On Wed, 12 Jun 2013 15:21:20 +0200
> Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
> >   Hi,
> > 
> > >> -{ 'command': 'screendump', 'data': {'filename': 'str'} }
> > >> +{ 'command': 'screendump', 'data': {'filename': 'str',
> > >> +                                    '*device' : 'str'} }
> > > 
> > > We can't add new optional parameters to QMP commands because it's
> > > currently impossible for mngt apps to discover them.
> > > 
> > > We have two options: 1. add this as a new command or 2. wait for
> > > full schema introspection (which we might get for 1.6).
> > 
> > "might get" sounds like it isn't a save bet, so I guess I better should
> > go for option (1) ...
> 
> Amos, do you think we'll get it for 1.6?

Yes, we can. 

2013-07-29      Hard feature freeze
2013-08-01      Tag v1.6.0-rc1

-- 
			Amos.