Message ID | 20181122071613.2889-1-kraxel@redhat.com |
---|---|
State | New |
Headers | show |
Series | [v3] qapi: add query-display-options command | expand |
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
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
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.
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
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.
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
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.
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' }