| Message ID | 20180419132026.5494-2-kraxel@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | ui: use qapi-based parser for most -display options. | expand |
On 04/19/2018 08:20 AM, Gerd Hoffmann wrote: > Add parse_display_qapi() function which parses the -display command line > using a qapi visitor for DisplayOptions. Wire up as default catch in > parse_display(). > > Improves the error message for unknown display types. > > Also enables json as -display argument, i.e. -display "{ 'type': 'gtk' }" > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > + > + /* > + * We don't have any dynamically allocated stuff inside > + * DisplayOptions, so we can simply copy the struct content and > + * free opts without ending up with pointers pointing into > + * nowhere. > + */ > + dpy = *opts; > + qapi_free_DisplayOptions(opts); That's risky; would it be better to use QAPI_CLONE_MEMBERS() to not have to worry about if we add a pointer in the future?
> > + /* > > + * We don't have any dynamically allocated stuff inside > > + * DisplayOptions, so we can simply copy the struct content and > > + * free opts without ending up with pointers pointing into > > + * nowhere. > > + */ > > + dpy = *opts; > > + qapi_free_DisplayOptions(opts); > > That's risky; would it be better to use QAPI_CLONE_MEMBERS() to not have > to worry about if we add a pointer in the future? Didn't know QAPI_CLONE_MEMBERS() exists. Yes, probably more future-proof to just use that instead of adding that comment. Will look into that for v2. cheers, Gerd
Gerd Hoffmann <kraxel@redhat.com> writes: > Add parse_display_qapi() function which parses the -display command line > using a qapi visitor for DisplayOptions. Wire up as default catch in > parse_display(). > > Improves the error message for unknown display types. > > Also enables json as -display argument, i.e. -display "{ 'type': 'gtk' }" This new option argument syntax is undocumented. Intentional? > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > vl.c | 29 +++++++++++++++++++++++++++-- > 1 file changed, 27 insertions(+), 2 deletions(-)
Markus Armbruster <armbru@redhat.com> writes: > Gerd Hoffmann <kraxel@redhat.com> writes: > >> Add parse_display_qapi() function which parses the -display command line >> using a qapi visitor for DisplayOptions. Wire up as default catch in >> parse_display(). >> >> Improves the error message for unknown display types. >> >> Also enables json as -display argument, i.e. -display "{ 'type': 'gtk' }" > > This new option argument syntax is undocumented. Intentional? Please advise.
On Fri, Aug 24, 2018 at 08:30:23AM +0200, Markus Armbruster wrote: > Markus Armbruster <armbru@redhat.com> writes: > > > Gerd Hoffmann <kraxel@redhat.com> writes: > > > >> Add parse_display_qapi() function which parses the -display command line > >> using a qapi visitor for DisplayOptions. Wire up as default catch in > >> parse_display(). > >> > >> Improves the error message for unknown display types. > >> > >> Also enables json as -display argument, i.e. -display "{ 'type': 'gtk' }" > > > > This new option argument syntax is undocumented. Intentional? No, just -ENOTIME. > Please advise. Well, just need to find some time to do it. In case you want tackle it: Works simliar to -blockdev, with a few exceptions. Some legacy sdl options I plan to deprecate are not supported via json. cheers, Gerd
On Fri, Aug 24, 2018 at 08:30:23AM +0200, Markus Armbruster wrote: > Markus Armbruster <armbru@redhat.com> writes: > > > Gerd Hoffmann <kraxel@redhat.com> writes: > > > >> Add parse_display_qapi() function which parses the -display command line > >> using a qapi visitor for DisplayOptions. Wire up as default catch in > >> parse_display(). > >> > >> Improves the error message for unknown display types. > >> > >> Also enables json as -display argument, i.e. -display "{ 'type': 'gtk' }" > > > > This new option argument syntax is undocumented. Intentional? > > Please advise. Hmm, checking -blockdev doc in qemu-options.hx, nothing about json. Is this documented elsewhere? cheers, Gerd
diff --git a/vl.c b/vl.c index fce1fd12d8..784c3fb795 100644 --- a/vl.c +++ b/vl.c @@ -126,6 +126,7 @@ int main(int argc, char **argv) #include "sysemu/replay.h" #include "qapi/qapi-events-run-state.h" #include "qapi/qapi-visit-block-core.h" +#include "qapi/qapi-visit-ui.h" #include "qapi/qapi-commands-block-core.h" #include "qapi/qapi-commands-misc.h" #include "qapi/qapi-commands-run-state.h" @@ -2090,6 +2091,31 @@ static void select_vgahw(const char *p) } } +static void parse_display_qapi(const char *optarg) +{ + Error *err = NULL; + DisplayOptions *opts; + Visitor *v; + + v = qobject_input_visitor_new_str(optarg, "type", &err); + if (!v) { + error_report_err(err); + exit(1); + } + + visit_type_DisplayOptions(v, NULL, &opts, &error_fatal); + visit_free(v); + + /* + * We don't have any dynamically allocated stuff inside + * DisplayOptions, so we can simply copy the struct content and + * free opts without ending up with pointers pointing into + * nowhere. + */ + dpy = *opts; + qapi_free_DisplayOptions(opts); +} + static void parse_display(const char *p) { const char *opts; @@ -2201,8 +2227,7 @@ static void parse_display(const char *p) } else if (strstart(p, "none", &opts)) { dpy.type = DISPLAY_TYPE_NONE; } else { - error_report("unknown display type"); - exit(1); + parse_display_qapi(p); } }
Add parse_display_qapi() function which parses the -display command line using a qapi visitor for DisplayOptions. Wire up as default catch in parse_display(). Improves the error message for unknown display types. Also enables json as -display argument, i.e. -display "{ 'type': 'gtk' }" Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- vl.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-)