* [Qemu-devel] [PATCH 1/4] ui: add qapi parser for -display
2018-04-19 13:20 [Qemu-devel] [PATCH 0/4] ui: use qapi-based parser for most -display options Gerd Hoffmann
@ 2018-04-19 13:20 ` Gerd Hoffmann
2018-04-19 15:57 ` Eric Blake
2018-07-26 12:45 ` Markus Armbruster
2018-04-19 13:20 ` [Qemu-devel] [PATCH 2/4] ui: switch trivial displays to qapi parser Gerd Hoffmann
` (2 subsequent siblings)
3 siblings, 2 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2018-04-19 13:20 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Paolo Bonzini, Michael Tokarev, qemu-trivial,
Markus Armbruster, Gerd Hoffmann
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(-)
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);
}
}
--
2.9.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] ui: add qapi parser for -display
2018-04-19 13:20 ` [Qemu-devel] [PATCH 1/4] ui: add qapi parser for -display Gerd Hoffmann
@ 2018-04-19 15:57 ` Eric Blake
2018-04-19 19:57 ` Gerd Hoffmann
2018-07-26 12:45 ` Markus Armbruster
1 sibling, 1 reply; 11+ messages in thread
From: Eric Blake @ 2018-04-19 15:57 UTC (permalink / raw)
To: Gerd Hoffmann, qemu-devel
Cc: qemu-trivial, Michael Tokarev, Laurent Vivier, Markus Armbruster,
Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 993 bytes --]
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?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] ui: add qapi parser for -display
2018-04-19 15:57 ` Eric Blake
@ 2018-04-19 19:57 ` Gerd Hoffmann
0 siblings, 0 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2018-04-19 19:57 UTC (permalink / raw)
To: Eric Blake
Cc: qemu-devel, qemu-trivial, Michael Tokarev, Laurent Vivier,
Markus Armbruster, Paolo Bonzini
> > + /*
> > + * 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] ui: add qapi parser for -display
2018-04-19 13:20 ` [Qemu-devel] [PATCH 1/4] ui: add qapi parser for -display Gerd Hoffmann
2018-04-19 15:57 ` Eric Blake
@ 2018-07-26 12:45 ` Markus Armbruster
2018-08-24 6:30 ` Markus Armbruster
1 sibling, 1 reply; 11+ messages in thread
From: Markus Armbruster @ 2018-07-26 12:45 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: qemu-devel, qemu-trivial, Michael Tokarev, Laurent Vivier, Paolo Bonzini
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(-)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] ui: add qapi parser for -display
2018-07-26 12:45 ` Markus Armbruster
@ 2018-08-24 6:30 ` Markus Armbruster
2018-08-24 6:51 ` Gerd Hoffmann
2018-08-27 10:02 ` Gerd Hoffmann
0 siblings, 2 replies; 11+ messages in thread
From: Markus Armbruster @ 2018-08-24 6:30 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: qemu-devel, qemu-trivial, Michael Tokarev, Laurent Vivier, Paolo Bonzini
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] ui: add qapi parser for -display
2018-08-24 6:30 ` Markus Armbruster
@ 2018-08-24 6:51 ` Gerd Hoffmann
2018-08-27 10:02 ` Gerd Hoffmann
1 sibling, 0 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2018-08-24 6:51 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, qemu-trivial, Michael Tokarev, Laurent Vivier, Paolo Bonzini
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] ui: add qapi parser for -display
2018-08-24 6:30 ` Markus Armbruster
2018-08-24 6:51 ` Gerd Hoffmann
@ 2018-08-27 10:02 ` Gerd Hoffmann
1 sibling, 0 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2018-08-27 10:02 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, qemu-trivial, Michael Tokarev, Laurent Vivier, Paolo Bonzini
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH 2/4] ui: switch trivial displays to qapi parser
2018-04-19 13:20 [Qemu-devel] [PATCH 0/4] ui: use qapi-based parser for most -display options Gerd Hoffmann
2018-04-19 13:20 ` [Qemu-devel] [PATCH 1/4] ui: add qapi parser for -display Gerd Hoffmann
@ 2018-04-19 13:20 ` Gerd Hoffmann
2018-04-19 13:20 ` [Qemu-devel] [PATCH 3/4] ui: switch gtk display " Gerd Hoffmann
2018-04-19 13:20 ` [Qemu-devel] [PATCH 4/4] ui: document non-qapi parser cases Gerd Hoffmann
3 siblings, 0 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2018-04-19 13:20 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Paolo Bonzini, Michael Tokarev, qemu-trivial,
Markus Armbruster, Gerd Hoffmann
Drop the option-less display types (egl-headless, curses, none) from
parse_display(), so they'll be handled by parse_display_qapi().
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
vl.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/vl.c b/vl.c
index 784c3fb795..39303d6b55 100644
--- a/vl.c
+++ b/vl.c
@@ -2188,10 +2188,6 @@ static void parse_display(const char *p)
error_report("VNC requires a display argument vnc=<display>");
exit(1);
}
- } else if (strstart(p, "egl-headless", &opts)) {
- dpy.type = DISPLAY_TYPE_EGL_HEADLESS;
- } else if (strstart(p, "curses", &opts)) {
- dpy.type = DISPLAY_TYPE_CURSES;
} else if (strstart(p, "gtk", &opts)) {
dpy.type = DISPLAY_TYPE_GTK;
while (*opts) {
@@ -2224,8 +2220,6 @@ static void parse_display(const char *p)
}
opts = nextopt;
}
- } else if (strstart(p, "none", &opts)) {
- dpy.type = DISPLAY_TYPE_NONE;
} else {
parse_display_qapi(p);
}
--
2.9.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH 3/4] ui: switch gtk display to qapi parser
2018-04-19 13:20 [Qemu-devel] [PATCH 0/4] ui: use qapi-based parser for most -display options Gerd Hoffmann
2018-04-19 13:20 ` [Qemu-devel] [PATCH 1/4] ui: add qapi parser for -display Gerd Hoffmann
2018-04-19 13:20 ` [Qemu-devel] [PATCH 2/4] ui: switch trivial displays to qapi parser Gerd Hoffmann
@ 2018-04-19 13:20 ` Gerd Hoffmann
2018-04-19 13:20 ` [Qemu-devel] [PATCH 4/4] ui: document non-qapi parser cases Gerd Hoffmann
3 siblings, 0 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2018-04-19 13:20 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Paolo Bonzini, Michael Tokarev, qemu-trivial,
Markus Armbruster, Gerd Hoffmann
Drop the gtk option parser from parse_display(), so parse_display_qapi()
will handle it instead.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
vl.c | 32 --------------------------------
1 file changed, 32 deletions(-)
diff --git a/vl.c b/vl.c
index 39303d6b55..1123cd792e 100644
--- a/vl.c
+++ b/vl.c
@@ -2188,38 +2188,6 @@ static void parse_display(const char *p)
error_report("VNC requires a display argument vnc=<display>");
exit(1);
}
- } else if (strstart(p, "gtk", &opts)) {
- dpy.type = DISPLAY_TYPE_GTK;
- while (*opts) {
- const char *nextopt;
-
- if (strstart(opts, ",grab_on_hover=", &nextopt)) {
- opts = nextopt;
- dpy.u.gtk.has_grab_on_hover = true;
- if (strstart(opts, "on", &nextopt)) {
- dpy.u.gtk.grab_on_hover = true;
- } else if (strstart(opts, "off", &nextopt)) {
- dpy.u.gtk.grab_on_hover = false;
- } else {
- goto invalid_gtk_args;
- }
- } else if (strstart(opts, ",gl=", &nextopt)) {
- opts = nextopt;
- dpy.has_gl = true;
- if (strstart(opts, "on", &nextopt)) {
- dpy.gl = true;
- } else if (strstart(opts, "off", &nextopt)) {
- dpy.gl = false;
- } else {
- goto invalid_gtk_args;
- }
- } else {
- invalid_gtk_args:
- error_report("invalid GTK option string");
- exit(1);
- }
- opts = nextopt;
- }
} else {
parse_display_qapi(p);
}
--
2.9.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH 4/4] ui: document non-qapi parser cases.
2018-04-19 13:20 [Qemu-devel] [PATCH 0/4] ui: use qapi-based parser for most -display options Gerd Hoffmann
` (2 preceding siblings ...)
2018-04-19 13:20 ` [Qemu-devel] [PATCH 3/4] ui: switch gtk display " Gerd Hoffmann
@ 2018-04-19 13:20 ` Gerd Hoffmann
3 siblings, 0 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2018-04-19 13:20 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Paolo Bonzini, Michael Tokarev, qemu-trivial,
Markus Armbruster, Gerd Hoffmann
Add comments to the cases not (yet) switched
over to parse_display_qapi().
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
vl.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/vl.c b/vl.c
index 1123cd792e..d35c443b8c 100644
--- a/vl.c
+++ b/vl.c
@@ -2121,6 +2121,16 @@ static void parse_display(const char *p)
const char *opts;
if (strstart(p, "sdl", &opts)) {
+ /*
+ * sdl DisplayType needs hand-crafted parser instead of
+ * parse_display_qapi() due to some options not in
+ * DisplayOptions, specifically:
+ * - frame
+ * Already deprecated.
+ * - ctrl_grab + alt_grab
+ * Not clear yet what happens to them long-term. Should
+ * replaced by something better or deprecated and dropped.
+ */
dpy.type = DISPLAY_TYPE_SDL;
while (*opts) {
const char *nextopt;
@@ -2182,6 +2192,10 @@ static void parse_display(const char *p)
opts = nextopt;
}
} else if (strstart(p, "vnc", &opts)) {
+ /*
+ * vnc isn't a (local) DisplayType but a protocol for remote
+ * display access.
+ */
if (*opts == '=') {
vnc_parse(opts + 1, &error_fatal);
} else {
--
2.9.3
^ permalink raw reply related [flat|nested] 11+ messages in thread