[Qemu-devel] [PATCH v2] qapi: add query-display-options command

Gerd Hoffmann posted 1 patch 6 years, 11 months ago
Test asan passed
Test checkpatch passed
Test docker-quick@centos7 passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20181121131638.26369-1-kraxel@redhat.com
There is a newer version of this series
vl.c         | 10 ++++++++++
qapi/ui.json | 13 +++++++++++++
2 files changed, 23 insertions(+)
[Qemu-devel] [PATCH v2] qapi: add query-display-options command
Posted by Gerd Hoffmann 6 years, 11 months ago
Add query-display-options command, which allows querying the qemu
display configuration, and -- as an intentional side effect -- makes
DisplayOptions discoverable via query-qmp-schema so libvirt can go
figure which display options are supported.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Tested-by: Eric Blake <eblake@redhat.com>
Tested-by: Erik Skultety <eskultet@redhat.com>
---
 vl.c         | 10 ++++++++++
 qapi/ui.json | 13 +++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/vl.c b/vl.c
index fa25d1ae2d..c6e662677a 100644
--- a/vl.c
+++ b/vl.c
@@ -128,6 +128,7 @@ int main(int argc, char **argv)
 #include "qapi/qapi-commands-block-core.h"
 #include "qapi/qapi-commands-misc.h"
 #include "qapi/qapi-commands-run-state.h"
+#include "qapi/qapi-commands-ui.h"
 #include "qapi/qmp/qerror.h"
 #include "sysemu/iothread.h"
 
@@ -2055,6 +2056,15 @@ static void parse_display_qapi(const char *optarg)
     visit_free(v);
 }
 
+DisplayOptions *qmp_query_display_options(Error **errp)
+{
+    DisplayOptions *opts;
+
+    opts = g_new(DisplayOptions, 1);
+    QAPI_CLONE_MEMBERS(DisplayOptions, opts, &dpy);
+    return opts;
+}
+
 static void parse_display(const char *p)
 {
     const char *opts;
diff --git a/qapi/ui.json b/qapi/ui.json
index e0000248d3..fd39acb5c3 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1102,3 +1102,16 @@
   'discriminator' : 'type',
   'data'    : { 'gtk'            : 'DisplayGTK',
                 'egl-headless'   : 'DisplayEGLHeadless'} }
+
+##
+# @query-display-options:
+#
+# Returns information about display configuration
+#
+# Returns: @DisplayOptions
+#
+# Since: 3.1
+#
+##
+{ 'command': 'query-display-options',
+  'returns': 'DisplayOptions' }
-- 
2.9.3


Re: [Qemu-devel] [PATCH v2] qapi: add query-display-options command
Posted by Markus Armbruster 6 years, 11 months ago
Gerd Hoffmann <kraxel@redhat.com> writes:

> Add query-display-options command, which allows querying the qemu
> display configuration, and -- as an intentional side effect -- makes
> DisplayOptions discoverable via query-qmp-schema so libvirt can go
> figure which display options are supported.

I understand the why the side effect is useful.  But is it the only
reason for the new command?

> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Tested-by: Eric Blake <eblake@redhat.com>
> Tested-by: Erik Skultety <eskultet@redhat.com>
> ---
>  vl.c         | 10 ++++++++++
>  qapi/ui.json | 13 +++++++++++++
>  2 files changed, 23 insertions(+)
>
> diff --git a/vl.c b/vl.c
> index fa25d1ae2d..c6e662677a 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -128,6 +128,7 @@ int main(int argc, char **argv)
>  #include "qapi/qapi-commands-block-core.h"
>  #include "qapi/qapi-commands-misc.h"
>  #include "qapi/qapi-commands-run-state.h"
> +#include "qapi/qapi-commands-ui.h"
>  #include "qapi/qmp/qerror.h"
>  #include "sysemu/iothread.h"
>  
> @@ -2055,6 +2056,15 @@ static void parse_display_qapi(const char *optarg)
>      visit_free(v);
>  }
>  
> +DisplayOptions *qmp_query_display_options(Error **errp)
> +{
> +    DisplayOptions *opts;
> +
> +    opts = g_new(DisplayOptions, 1);
> +    QAPI_CLONE_MEMBERS(DisplayOptions, opts, &dpy);
> +    return opts;

What's wrong with

       return QAPI_CLONE(DisplayOptions, &dpy)

?

> +}
> +
>  static void parse_display(const char *p)
>  {
>      const char *opts;
> diff --git a/qapi/ui.json b/qapi/ui.json
> index e0000248d3..fd39acb5c3 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1102,3 +1102,16 @@
>    'discriminator' : 'type',
>    'data'    : { 'gtk'            : 'DisplayGTK',
>                  'egl-headless'   : 'DisplayEGLHeadless'} }
> +
> +##
> +# @query-display-options:
> +#
> +# Returns information about display configuration
> +#
> +# Returns: @DisplayOptions
> +#
> +# Since: 3.1
> +#
> +##
> +{ 'command': 'query-display-options',
> +  'returns': 'DisplayOptions' }

Re: [Qemu-devel] [PATCH v2] qapi: add query-display-options command
Posted by Eric Blake 6 years, 11 months ago
On 11/21/18 11:09 AM, Markus Armbruster wrote:
> Gerd Hoffmann <kraxel@redhat.com> writes:
> 
>> Add query-display-options command, which allows querying the qemu
>> display configuration, and -- as an intentional side effect -- makes
>> DisplayOptions discoverable via query-qmp-schema so libvirt can go
>> figure which display options are supported.
> 
> I understand the why the side effect is useful.  But is it the only
> reason for the new command?

The reason for needing the side effect in 3.1 is because commit d4dc4ab1 
also landed in 3.1; the commit message should really mention that 
relationship.

You are right that in general, a management app should remember what it 
asked for on the command line, and is therefore unlikely to learn 
anything by invoking this command directly.  On the other hand, if the 
display options populate any defaults omitted from the command line, 
this query command might still be useful to show what defaults got 
populated.


>> +DisplayOptions *qmp_query_display_options(Error **errp)
>> +{
>> +    DisplayOptions *opts;
>> +
>> +    opts = g_new(DisplayOptions, 1);
>> +    QAPI_CLONE_MEMBERS(DisplayOptions, opts, &dpy);
>> +    return opts;
> 
> What's wrong with
> 
>         return QAPI_CLONE(DisplayOptions, &dpy)
> 
> ?

Looks like it should work.

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