Re: [Qemu-devel] [PATCH v3] qapi: add query-display-options command
Gerd Hoffmann 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.
Re: [Qemu-devel] [PATCH v3] qapi: add query-display-options command
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
Gerd Hoffmann writes: > On Mon, Nov 26, 2018 at 03:01:42PM +0100, Markus Armbruster wrote: >> Gerd Hoffmann 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
On Mon, Nov 26, 2018 at 03:01:42PM +0100, Markus Armbruster wrote: > Gerd Hoffmann 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
Gerd Hoffmann 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
> Reviewed-by: Eric Blake
> Tested-by: Eric Blake
> Tested-by: Erik Skultety
> ---
> 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 e248d3..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
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 > > Reviewed-by: Eric Blake > > Tested-by: Eric Blake > > Tested-by: Erik Skultety > > --- > > 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
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 > Reviewed-by: Eric Blake > Tested-by: Eric Blake > Tested-by: Erik Skultety > --- 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
[Qemu-devel] [PATCH v3] 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.
Use case: commit d4dc4ab1 added rendernode parameter for egl-headless.
Signed-off-by: Gerd Hoffmann
Reviewed-by: Eric Blake
Tested-by: Eric Blake
Tested-by: Erik Skultety
---
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 e248d3..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
