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

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

[Gerd Hoffmann]

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.

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

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

[Erik Skultety]

On Thu, Nov 22, 2018 at 08:16:13AM +0100, Gerd Hoffmann wrote:
> 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.
>
> 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>
> ---

FYI I have the first libvirt prototype patches [1] (need some polishing though)
ready and everything worked even with this v3 patch.

[1] https://github.com/eskultety/libvirt/commits/egl-headless

Thanks,
Erik

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

[Gerd Hoffmann]

On Thu, Nov 22, 2018 at 03:58:02PM +0100, Erik Skultety wrote:
> On Thu, Nov 22, 2018 at 08:16:13AM +0100, Gerd Hoffmann wrote:
> > 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.
> >
> > 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>
> > ---
> 
> FYI I have the first libvirt prototype patches [1] (need some polishing though)
> ready and everything worked even with this v3 patch.
> 
> [1] https://github.com/eskultety/libvirt/commits/egl-headless

Good.  Queued up for 3.1

cheers,
  Gerd

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

[Markus Armbruster]

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.

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

[Gerd Hoffmann]

On Mon, Nov 26, 2018 at 03:01:42PM +0100, 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.
> >
> > 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.

Sure.  I assumed the "intentional side effect" message is clear enough
though.

The command itself isn't that helpful, you should know how you have
started qemu ...

cheers,
  Gerd

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

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

> On Mon, Nov 26, 2018 at 03:01:42PM +0100, 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.
>> >
>> > 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.
>
> Sure.  I assumed the "intentional side effect" message is clear enough
> though.
>
> The command itself isn't that helpful, you should know how you have
> started qemu ...

If it's not too much trouble, please tweak the commit message to be a
bit more explicit.  Perhaps:

    Add query-display-options command, which allows querying the qemu
    display configuration.  This isn't particularly useful, except it
    exposes QAPI type DisplayOptions in query-qmp-schema, so that
    libvirt can discover recently added -display parameter rendernode
    (commit d4dc4ab133b).  Works around lack of sufficiently powerful
    command line introspection.

This should give me a fighting chance to remember deprecating the
command once we got sufficiently powerful command line introspection.

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

[Gerd Hoffmann]

  Hi,

> If it's not too much trouble, please tweak the commit message to be a
> bit more explicit.  Perhaps:
> 
>     Add query-display-options command, which allows querying the qemu
>     display configuration.  This isn't particularly useful, except it
>     exposes QAPI type DisplayOptions in query-qmp-schema, so that
>     libvirt can discover recently added -display parameter rendernode
>     (commit d4dc4ab133b).  Works around lack of sufficiently powerful
>     command line introspection.

Done, pull req with this and other 3.1 fixes sent.

> This should give me a fighting chance to remember deprecating the
> command once we got sufficiently powerful command line introspection.

I'm wondering how difficuilt it would be to add that when limiting that
to the command line switches which already use qapi parsers (-blockdev
and -display as far I know).  Might increase the motivation of others to
help moving parsers from whatever they do today (QemuOpts, ...) to qapi
to get introspection support ;)

cheers,
  Gerd

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

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

>   Hi,
>
>> If it's not too much trouble, please tweak the commit message to be a
>> bit more explicit.  Perhaps:
>> 
>>     Add query-display-options command, which allows querying the qemu
>>     display configuration.  This isn't particularly useful, except it
>>     exposes QAPI type DisplayOptions in query-qmp-schema, so that
>>     libvirt can discover recently added -display parameter rendernode
>>     (commit d4dc4ab133b).  Works around lack of sufficiently powerful
>>     command line introspection.
>
> Done, pull req with this and other 3.1 fixes sent.
>
>> This should give me a fighting chance to remember deprecating the
>> command once we got sufficiently powerful command line introspection.
>
> I'm wondering how difficuilt it would be to add that when limiting that
> to the command line switches which already use qapi parsers (-blockdev
> and -display as far I know).  Might increase the motivation of others to
> help moving parsers from whatever they do today (QemuOpts, ...) to qapi
> to get introspection support ;)

I like the idea.  The clean way to do it would be a partial QAPIfication
of the command line.  I'm wary of partial "we'll finish this eventually"
conversions.  That said, the complete job may well be too large to
tackle in one go, giving us no choice.