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

[Markus Armbruster <armbru@redhat.com>]

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.
>
> Use case: commit d4dc4ab1 added rendernode parameter for egl-headless.

I understand why exposing DisplayOptions in query-qmp-schema is useful.
But can you think of a use for the new command?

If not, then this is a workaround for lack of CLI introspection.
That's okay, ball's in my court on that.  But I'd like to have the
"workaroundness" spelled out in the commit message then.

> 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         |  6 ++++++
>  qapi/ui.json | 13 +++++++++++++
>  2 files changed, 19 insertions(+)
>
> diff --git a/vl.c b/vl.c
> index fa25d1ae2d..d6fd95c227 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,11 @@ static void parse_display_qapi(const char *optarg)
>      visit_free(v);
>  }
>  
> +DisplayOptions *qmp_query_display_options(Error **errp)
> +{
> +    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' }

Patch looks good to me.