[v2] qapi: add query-display-options command

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(+)

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' }

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.