[PATCH v2 0/3] softmmu/vl: Deprecate old and crufty display ui options

[PATCH v2 0/3] softmmu/vl: Deprecate old and crufty display ui options

[Thomas Huth]

-display sdl uses a hand-crafted parser in vl.c, which is quite ugly
since the other parts of -display have been QAPIfied already. A straight
conversion to QAPI is not advisable since the "alt_grab" and "ctrl_grab"
parameters are not the best solution anyway. So this patch series
introduces a new "grab-mod" parameter as replacement instead and then
deprecates the old and crufty options.

While we're at it, the third patch also suggests to deprecated the
old -sdl and -curses top-level options.

v2:
 - Update version numbers to 6.2
 - Added Acked-bys from Peter Krempa

Thomas Huth (3):
  softmmu/vl: Add a "grab-mod" parameter to the -display sdl option
  softmmu/vl: Deprecate the old grab options
  softmmu/vl: Deprecate the -sdl and -curses option

 docs/about/deprecated.rst | 20 ++++++++++++++++++++
 qemu-options.hx           | 18 +++++++++++++-----
 softmmu/vl.c              | 24 +++++++++++++++++++++---
 3 files changed, 54 insertions(+), 8 deletions(-)

-- 
2.27.0

[PATCH v2 1/3] softmmu/vl: Add a "grab-mod" parameter to the -display sdl option

[Thomas Huth]

The -display sdl option is not using QAPI internally yet, and uses hand-
crafted parsing instead (see parse_display() in vl.c), which is quite
ugly, since most of the other code is using the QAPIfied DisplayOption
already. Unfortunately, the "alt_grab" and "ctrl_grab" use underscores in
their names which has recently been forbidden in new QAPI code, so
a straight conversion is not possible. While we could add some exceptions
to the QAPI schema parser for this, the way these parameters have been
designed was maybe a bad idea anyway: First, it's not possible to enable
both parameters at the same time, thus instead of two boolean parameters
it would be better to have only one multi-choice parameter instead.
Second, the naming is also somewhat unfortunate since the "alt_grab"
parameter is not about the ALT key, but rather about the left SHIFT key
that has to be used additionally when the parameter is enabled.

So instead of trying to QAPIfy "alt_grab" and "ctrl_grab", let's rather
introduce an alternative to these parameters instead, a new parameter
called "grab-mod" which can either be set to "lshift-lctrl-lalt" or to
"rctrl". In case we ever want to support additional modes later, we can
then also simply extend the list of supported strings here.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 qemu-options.hx |  6 +++++-
 softmmu/vl.c    | 15 ++++++++++++---
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 83aa59a920..0bff756ded 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1834,7 +1834,7 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
 #endif
 #if defined(CONFIG_SDL)
     "-display sdl[,alt_grab=on|off][,ctrl_grab=on|off][,gl=on|core|es|off]\n"
-    "            [,show-cursor=on|off][,window-close=on|off]\n"
+    "            [,grab-mod=<mod>][,show-cursor=on|off][,window-close=on|off]\n"
 #endif
 #if defined(CONFIG_GTK)
     "-display gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]\n"
@@ -1880,6 +1880,10 @@ SRST
         window; see the SDL documentation for other possibilities).
         Valid parameters are:
 
+        ``grab-mod=<mods>`` : Used to select the modifier keys for toggling
+        the mouse grabbing in conjunction with the "g" key. `<mods>` can be
+        either `lshift-lctrl-lalt` or `rctrl`.
+
         ``alt_grab=on|off`` : Use Control+Alt+Shift-g to toggle mouse grabbing
 
         ``ctrl_grab=on|off`` : Use Right-Control-g to toggle mouse grabbing
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 5ca11e7469..294990debf 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1017,15 +1017,24 @@ static void parse_display(const char *p)
          * parse_display_qapi() due to some options not in
          * DisplayOptions, specifically:
          *   - ctrl_grab + alt_grab
-         *     Not clear yet what happens to them long-term.  Should
-         *     replaced by something better or deprecated and dropped.
+         *     They can't be moved into the QAPI since they use underscores,
+         *     thus they will get replaced by "grab-mod" in the long term
          */
 #if defined(CONFIG_SDL)
         dpy.type = DISPLAY_TYPE_SDL;
         while (*opts) {
             const char *nextopt;
 
-            if (strstart(opts, ",alt_grab=", &nextopt)) {
+            if (strstart(opts, ",grab-mod=", &nextopt)) {
+                opts = nextopt;
+                if (strstart(opts, "lshift-lctrl-lalt", &nextopt)) {
+                    alt_grab = 1;
+                } else if (strstart(opts, "rctrl", &nextopt)) {
+                    ctrl_grab = 1;
+                } else {
+                    goto invalid_sdl_args;
+                }
+            } else if (strstart(opts, ",alt_grab=", &nextopt)) {
                 opts = nextopt;
                 if (strstart(opts, "on", &nextopt)) {
                     alt_grab = 1;
-- 
2.27.0

[PATCH v2 2/3] softmmu/vl: Deprecate the old grab options

[Thomas Huth]

The alt_grab and ctrl_grab parameter of the -display sdl option prevent
the QAPIfication of the "sdl" part of the -display option, so we should
eventually remove them. And since this feature is also rather niche anyway,
we should not clutter the top-level option list with these, so let's
also deprecate the "-alt-grab" and the "-ctrl-grab" options while we're
at it.

Once the deprecation period of "alt_grab" and "ctrl_grab" is over, we
then can finally switch the -display sdl option to use QAPI internally,
too.

Acked-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 docs/about/deprecated.rst | 10 ++++++++++
 qemu-options.hx           | 12 ++++++++----
 softmmu/vl.c              |  6 ++++++
 3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 6d438f1c8d..868eca0dd4 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -138,6 +138,16 @@ an underscore between "window" and "close").
 The ``-no-quit`` is a synonym for ``-display ...,window-close=off`` which
 should be used instead.
 
+``-alt-grab`` and ``-display sdl,alt_grab=on`` (since 6.2)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Use ``-display sdl,grab-mod=lshift-lctrl-lalt`` instead.
+
+``-ctrl-grab`` and ``-display sdl,ctrl_grab=on`` (since 6.2)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Use ``-display sdl,grab-mod=rctrl`` instead.
+
 
 QEMU Machine Protocol (QMP) commands
 ------------------------------------
diff --git a/qemu-options.hx b/qemu-options.hx
index 0bff756ded..4f46233527 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1884,9 +1884,11 @@ SRST
         the mouse grabbing in conjunction with the "g" key. `<mods>` can be
         either `lshift-lctrl-lalt` or `rctrl`.
 
-        ``alt_grab=on|off`` : Use Control+Alt+Shift-g to toggle mouse grabbing
+        ``alt_grab=on|off`` : Use Control+Alt+Shift-g to toggle mouse grabbing.
+        This parameter is deprecated - use ``grab-mod`` instead.
 
-        ``ctrl_grab=on|off`` : Use Right-Control-g to toggle mouse grabbing
+        ``ctrl_grab=on|off`` : Use Right-Control-g to toggle mouse grabbing.
+        This parameter is deprecated - use ``grab-mod`` instead.
 
         ``gl=on|off|core|es`` : Use OpenGL for displaying
 
@@ -1971,7 +1973,8 @@ SRST
 ``-alt-grab``
     Use Ctrl-Alt-Shift to grab mouse (instead of Ctrl-Alt). Note that
     this also affects the special keys (for fullscreen, monitor-mode
-    switching, etc).
+    switching, etc). This option is deprecated - please use
+    ``-display sdl,grab-mod=lshift-lctrl-lalt`` instead.
 ERST
 
 DEF("ctrl-grab", 0, QEMU_OPTION_ctrl_grab,
@@ -1981,7 +1984,8 @@ SRST
 ``-ctrl-grab``
     Use Right-Ctrl to grab mouse (instead of Ctrl-Alt). Note that this
     also affects the special keys (for fullscreen, monitor-mode
-    switching, etc).
+    switching, etc). This option is deprecated - please use
+    ``-display sdl,grab-mod=rctrl`` instead.
 ERST
 
 DEF("no-quit", 0, QEMU_OPTION_no_quit,
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 294990debf..613948ab46 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1043,6 +1043,7 @@ static void parse_display(const char *p)
                 } else {
                     goto invalid_sdl_args;
                 }
+                warn_report("alt_grab is deprecated, use grab-mod instead.");
             } else if (strstart(opts, ",ctrl_grab=", &nextopt)) {
                 opts = nextopt;
                 if (strstart(opts, "on", &nextopt)) {
@@ -1052,6 +1053,7 @@ static void parse_display(const char *p)
                 } else {
                     goto invalid_sdl_args;
                 }
+                warn_report("ctrl_grab is deprecated, use grab-mod instead.");
             } else if (strstart(opts, ",window_close=", &nextopt) ||
                        strstart(opts, ",window-close=", &nextopt)) {
                 if (strstart(opts, ",window_close=", NULL)) {
@@ -3253,9 +3255,13 @@ void qemu_init(int argc, char **argv, char **envp)
                 break;
             case QEMU_OPTION_alt_grab:
                 alt_grab = 1;
+                warn_report("-alt-grab is deprecated, please use "
+                            "-display sdl,grab-mod=lshift-lctrl-lalt instead.");
                 break;
             case QEMU_OPTION_ctrl_grab:
                 ctrl_grab = 1;
+                warn_report("-ctrl-grab is deprecated, please use "
+                            "-display sdl,grab-mod=rctrl instead.");
                 break;
             case QEMU_OPTION_no_quit:
                 dpy.has_window_close = true;
-- 
2.27.0

Re: [PATCH v2 2/3] softmmu/vl: Deprecate the old grab options

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 5042 bytes --]

No objections at all here, though. :)

Paolo

Il mer 25 ago 2021, 11:20 Thomas Huth <thuth@redhat.com> ha scritto:

> The alt_grab and ctrl_grab parameter of the -display sdl option prevent
> the QAPIfication of the "sdl" part of the -display option, so we should
> eventually remove them. And since this feature is also rather niche anyway,
> we should not clutter the top-level option list with these, so let's
> also deprecate the "-alt-grab" and the "-ctrl-grab" options while we're
> at it.
>
> Once the deprecation period of "alt_grab" and "ctrl_grab" is over, we
> then can finally switch the -display sdl option to use QAPI internally,
> too.
>
> Acked-by: Peter Krempa <pkrempa@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  docs/about/deprecated.rst | 10 ++++++++++
>  qemu-options.hx           | 12 ++++++++----
>  softmmu/vl.c              |  6 ++++++
>  3 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 6d438f1c8d..868eca0dd4 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -138,6 +138,16 @@ an underscore between "window" and "close").
>  The ``-no-quit`` is a synonym for ``-display ...,window-close=off`` which
>  should be used instead.
>
> +``-alt-grab`` and ``-display sdl,alt_grab=on`` (since 6.2)
> +''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> +
> +Use ``-display sdl,grab-mod=lshift-lctrl-lalt`` instead.
> +
> +``-ctrl-grab`` and ``-display sdl,ctrl_grab=on`` (since 6.2)
> +''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> +
> +Use ``-display sdl,grab-mod=rctrl`` instead.
> +
>
>  QEMU Machine Protocol (QMP) commands
>  ------------------------------------
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 0bff756ded..4f46233527 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1884,9 +1884,11 @@ SRST
>          the mouse grabbing in conjunction with the "g" key. `<mods>` can
> be
>          either `lshift-lctrl-lalt` or `rctrl`.
>
> -        ``alt_grab=on|off`` : Use Control+Alt+Shift-g to toggle mouse
> grabbing
> +        ``alt_grab=on|off`` : Use Control+Alt+Shift-g to toggle mouse
> grabbing.
> +        This parameter is deprecated - use ``grab-mod`` instead.
>
> -        ``ctrl_grab=on|off`` : Use Right-Control-g to toggle mouse
> grabbing
> +        ``ctrl_grab=on|off`` : Use Right-Control-g to toggle mouse
> grabbing.
> +        This parameter is deprecated - use ``grab-mod`` instead.
>
>          ``gl=on|off|core|es`` : Use OpenGL for displaying
>
> @@ -1971,7 +1973,8 @@ SRST
>  ``-alt-grab``
>      Use Ctrl-Alt-Shift to grab mouse (instead of Ctrl-Alt). Note that
>      this also affects the special keys (for fullscreen, monitor-mode
> -    switching, etc).
> +    switching, etc). This option is deprecated - please use
> +    ``-display sdl,grab-mod=lshift-lctrl-lalt`` instead.
>  ERST
>
>  DEF("ctrl-grab", 0, QEMU_OPTION_ctrl_grab,
> @@ -1981,7 +1984,8 @@ SRST
>  ``-ctrl-grab``
>      Use Right-Ctrl to grab mouse (instead of Ctrl-Alt). Note that this
>      also affects the special keys (for fullscreen, monitor-mode
> -    switching, etc).
> +    switching, etc). This option is deprecated - please use
> +    ``-display sdl,grab-mod=rctrl`` instead.
>  ERST
>
>  DEF("no-quit", 0, QEMU_OPTION_no_quit,
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 294990debf..613948ab46 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -1043,6 +1043,7 @@ static void parse_display(const char *p)
>                  } else {
>                      goto invalid_sdl_args;
>                  }
> +                warn_report("alt_grab is deprecated, use grab-mod
> instead.");
>              } else if (strstart(opts, ",ctrl_grab=", &nextopt)) {
>                  opts = nextopt;
>                  if (strstart(opts, "on", &nextopt)) {
> @@ -1052,6 +1053,7 @@ static void parse_display(const char *p)
>                  } else {
>                      goto invalid_sdl_args;
>                  }
> +                warn_report("ctrl_grab is deprecated, use grab-mod
> instead.");
>              } else if (strstart(opts, ",window_close=", &nextopt) ||
>                         strstart(opts, ",window-close=", &nextopt)) {
>                  if (strstart(opts, ",window_close=", NULL)) {
> @@ -3253,9 +3255,13 @@ void qemu_init(int argc, char **argv, char **envp)
>                  break;
>              case QEMU_OPTION_alt_grab:
>                  alt_grab = 1;
> +                warn_report("-alt-grab is deprecated, please use "
> +                            "-display sdl,grab-mod=lshift-lctrl-lalt
> instead.");
>                  break;
>              case QEMU_OPTION_ctrl_grab:
>                  ctrl_grab = 1;
> +                warn_report("-ctrl-grab is deprecated, please use "
> +                            "-display sdl,grab-mod=rctrl instead.");
>                  break;
>              case QEMU_OPTION_no_quit:
>                  dpy.has_window_close = true;
> --
> 2.27.0
>
>

[-- Attachment #2: Type: text/html, Size: 6891 bytes --]

[PATCH v2 3/3] softmmu/vl: Deprecate the -sdl and -curses option

[Thomas Huth]

It's not that much complicated to type "-display sdl" or "-display curses",
so we should not clutter our main option name space with such simple
wrapper options and rather present the users with a concise interface
instead. Thus let's deprecate the "-sdl" and "-curses" wrapper options now.

Acked-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 docs/about/deprecated.rst | 10 ++++++++++
 softmmu/vl.c              |  3 +++
 2 files changed, 13 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 868eca0dd4..d5bec67a78 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -148,6 +148,16 @@ Use ``-display sdl,grab-mod=lshift-lctrl-lalt`` instead.
 
 Use ``-display sdl,grab-mod=rctrl`` instead.
 
+``-sdl`` (since 6.2)
+''''''''''''''''''''
+
+Use ``-display sdl`` instead.
+
+``-curses`` (since 6.2)
+'''''''''''''''''''''''
+
+Use ``-display curses`` instead.
+
 
 QEMU Machine Protocol (QMP) commands
 ------------------------------------
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 613948ab46..bb59dbf0de 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2897,6 +2897,8 @@ void qemu_init(int argc, char **argv, char **envp)
                 dpy.type = DISPLAY_TYPE_NONE;
                 break;
             case QEMU_OPTION_curses:
+                warn_report("-curses is deprecated, "
+                            "use -display curses instead.");
 #ifdef CONFIG_CURSES
                 dpy.type = DISPLAY_TYPE_CURSES;
 #else
@@ -3270,6 +3272,7 @@ void qemu_init(int argc, char **argv, char **envp)
                             "-display ...,window-close=off instead.");
                 break;
             case QEMU_OPTION_sdl:
+                warn_report("-sdl is deprecated, use -display sdl instead.");
 #ifdef CONFIG_SDL
                 dpy.type = DISPLAY_TYPE_SDL;
                 break;
-- 
2.27.0

Re: [PATCH v2 3/3] softmmu/vl: Deprecate the -sdl and -curses option

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 2860 bytes --]

As an alternative, you may want to turn it into "-display sdl" rather than
poke at dpy. This isn't much more code, but it keeps the shortcut isolated
within a single "case". This follows a lot of recently cleaned up command
line parsing code such as -no-hpet, -enable-kvm, -smp etc.

In the end (spoiler alert for my upcoming KVM Forum presentation—slides are
already on sched.com :)) what really produces complexity is the lack of
isolation/modularity. As long as UI code doesn't care about command line
parsing, and command line parsing doesn't care about global variables from
all over the place, the cost of shortcuts is so small that it may tilt in
favor of keeping them.

Paolo

Il mer 25 ago 2021, 11:20 Thomas Huth <thuth@redhat.com> ha scritto:

> It's not that much complicated to type "-display sdl" or "-display curses",
> so we should not clutter our main option name space with such simple
> wrapper options and rather present the users with a concise interface
> instead. Thus let's deprecate the "-sdl" and "-curses" wrapper options now.
>
> Acked-by: Peter Krempa <pkrempa@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  docs/about/deprecated.rst | 10 ++++++++++
>  softmmu/vl.c              |  3 +++
>  2 files changed, 13 insertions(+)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 868eca0dd4..d5bec67a78 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -148,6 +148,16 @@ Use ``-display sdl,grab-mod=lshift-lctrl-lalt``
> instead.
>
>  Use ``-display sdl,grab-mod=rctrl`` instead.
>
> +``-sdl`` (since 6.2)
> +''''''''''''''''''''
> +
> +Use ``-display sdl`` instead.
> +
> +``-curses`` (since 6.2)
> +'''''''''''''''''''''''
> +
> +Use ``-display curses`` instead.
> +
>
>  QEMU Machine Protocol (QMP) commands
>  ------------------------------------
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 613948ab46..bb59dbf0de 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2897,6 +2897,8 @@ void qemu_init(int argc, char **argv, char **envp)
>                  dpy.type = DISPLAY_TYPE_NONE;
>                  break;
>              case QEMU_OPTION_curses:
> +                warn_report("-curses is deprecated, "
> +                            "use -display curses instead.");
>  #ifdef CONFIG_CURSES
>                  dpy.type = DISPLAY_TYPE_CURSES;
>  #else
> @@ -3270,6 +3272,7 @@ void qemu_init(int argc, char **argv, char **envp)
>                              "-display ...,window-close=off instead.");
>                  break;
>              case QEMU_OPTION_sdl:
> +                warn_report("-sdl is deprecated, use -display sdl
> instead.");
>  #ifdef CONFIG_SDL
>                  dpy.type = DISPLAY_TYPE_SDL;
>                  break;
> --
> 2.27.0
>
>

[-- Attachment #2: Type: text/html, Size: 3915 bytes --]

Re: [PATCH v2 3/3] softmmu/vl: Deprecate the -sdl and -curses option

[Thomas Huth]

On 31/08/2021 15.53, Paolo Bonzini wrote:
> As an alternative, you may want to turn it into "-display sdl" rather than 
> poke at dpy. This isn't much more code, but it keeps the shortcut isolated 
> within a single "case". This follows a lot of recently cleaned up command 
> line parsing code such as -no-hpet, -enable-kvm, -smp etc.
> 
> In the end (spoiler alert for my upcoming KVM Forum presentation—slides are 
> already on sched.com <http://sched.com> :)) what really produces complexity 
> is the lack of isolation/modularity. As long as UI code doesn't care about 
> command line parsing, and command line parsing doesn't care about global 
> variables from all over the place, the cost of shortcuts is so small that it 
> may tilt in favor of keeping them.

Honestly, I'd rather like to see them removed in the end. Our user interface 
is so terribly inconsistent here that I think that these options are rather 
confusing for the users than helpful. For example, why do we have -sdl and 
-curses, but no -gtk ? And as a normal user, I'd also always wonder what's 
the difference between "-display sdl" and "-sdl", since the difference in 
the amount of characters that you have to type here is not that much that it 
justifies the shortcut option. So IMHO let's rather clean this up completely 
than dragging the shortcut options along forever.

  Thomas

Re: [PATCH v2 3/3] softmmu/vl: Deprecate the -sdl and -curses option

[Daniel P. Berrangé]

On Thu, Sep 02, 2021 at 12:51:02PM +0200, Thomas Huth wrote:
> On 31/08/2021 15.53, Paolo Bonzini wrote:
> > As an alternative, you may want to turn it into "-display sdl" rather
> > than poke at dpy. This isn't much more code, but it keeps the shortcut
> > isolated within a single "case". This follows a lot of recently cleaned
> > up command line parsing code such as -no-hpet, -enable-kvm, -smp etc.
> > 
> > In the end (spoiler alert for my upcoming KVM Forum presentation—slides
> > are already on sched.com <http://sched.com> :)) what really produces
> > complexity is the lack of isolation/modularity. As long as UI code
> > doesn't care about command line parsing, and command line parsing
> > doesn't care about global variables from all over the place, the cost of
> > shortcuts is so small that it may tilt in favor of keeping them.
> 
> Honestly, I'd rather like to see them removed in the end. Our user interface
> is so terribly inconsistent here that I think that these options are rather
> confusing for the users than helpful. For example, why do we have -sdl and
> -curses, but no -gtk ? And as a normal user, I'd also always wonder what's
> the difference between "-display sdl" and "-sdl", since the difference in
> the amount of characters that you have to type here is not that much that it
> justifies the shortcut option. So IMHO let's rather clean this up completely
> than dragging the shortcut options along forever.

There's also the elephant in the room "-vnc" which has never been mapped
into -display, but which is also one of the most widely used options for
display backends :-(

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 3/3] softmmu/vl: Deprecate the -sdl and -curses option

[Thomas Huth]

On 02/09/2021 12.58, Daniel P. Berrangé wrote:
> On Thu, Sep 02, 2021 at 12:51:02PM +0200, Thomas Huth wrote:
>> On 31/08/2021 15.53, Paolo Bonzini wrote:
>>> As an alternative, you may want to turn it into "-display sdl" rather
>>> than poke at dpy. This isn't much more code, but it keeps the shortcut
>>> isolated within a single "case". This follows a lot of recently cleaned
>>> up command line parsing code such as -no-hpet, -enable-kvm, -smp etc.
>>>
>>> In the end (spoiler alert for my upcoming KVM Forum presentation—slides
>>> are already on sched.com <http://sched.com> :)) what really produces
>>> complexity is the lack of isolation/modularity. As long as UI code
>>> doesn't care about command line parsing, and command line parsing
>>> doesn't care about global variables from all over the place, the cost of
>>> shortcuts is so small that it may tilt in favor of keeping them.
>>
>> Honestly, I'd rather like to see them removed in the end. Our user interface
>> is so terribly inconsistent here that I think that these options are rather
>> confusing for the users than helpful. For example, why do we have -sdl and
>> -curses, but no -gtk ? And as a normal user, I'd also always wonder what's
>> the difference between "-display sdl" and "-sdl", since the difference in
>> the amount of characters that you have to type here is not that much that it
>> justifies the shortcut option. So IMHO let's rather clean this up completely
>> than dragging the shortcut options along forever.
> 
> There's also the elephant in the room "-vnc" which has never been mapped
> into -display, but which is also one of the most widely used options for
> display backends :-(

Yeah, for -vnc, it likely makes sense to keep the shortcut since it's in 
wide use and also takes additional parameters ... but -sdl and -curses? They 
are IMHO rather niche, and don't take additional parameters, so it should be 
ok to mark them as deprecated. We can still reconsider in case anybody 
complains about the deprecation later.

  Thomas

Re: [PATCH v2 3/3] softmmu/vl: Deprecate the -sdl and -curses option

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Thu, Sep 02, 2021 at 12:51:02PM +0200, Thomas Huth wrote:
>> On 31/08/2021 15.53, Paolo Bonzini wrote:
>> > As an alternative, you may want to turn it into "-display sdl" rather
>> > than poke at dpy. This isn't much more code, but it keeps the shortcut
>> > isolated within a single "case". This follows a lot of recently cleaned
>> > up command line parsing code such as -no-hpet, -enable-kvm, -smp etc.
>> > 
>> > In the end (spoiler alert for my upcoming KVM Forum presentation—slides
>> > are already on sched.com <http://sched.com> :)) what really produces
>> > complexity is the lack of isolation/modularity. As long as UI code
>> > doesn't care about command line parsing, and command line parsing
>> > doesn't care about global variables from all over the place, the cost of
>> > shortcuts is so small that it may tilt in favor of keeping them.
>> 
>> Honestly, I'd rather like to see them removed in the end. Our user interface
>> is so terribly inconsistent here that I think that these options are rather
>> confusing for the users than helpful. For example, why do we have -sdl and
>> -curses, but no -gtk ? And as a normal user, I'd also always wonder what's
>> the difference between "-display sdl" and "-sdl", since the difference in
>> the amount of characters that you have to type here is not that much that it
>> justifies the shortcut option. So IMHO let's rather clean this up completely
>> than dragging the shortcut options along forever.
>
> There's also the elephant in the room "-vnc" which has never been mapped
> into -display, but which is also one of the most widely used options for
> display backends :-(

There's -display vnc=...  Option -help shows it, -display help doesn't,
but that's just a bug, I guess.  More serious: -display '{"type": "vnc",
...} isn't implemented.

Re: [PATCH v2 3/3] softmmu/vl: Deprecate the -sdl and -curses option

[Gerd Hoffmann]

  Hi,

> There's also the elephant in the room "-vnc" which has never been mapped
> into -display, but which is also one of the most widely used options for
> display backends :-(

Other way around, -display vnc should be dropped.  -display is about
local displays, and there can be only one instance.  -vnc / -spice
enable remote access, and this can be done in addition to a local
display.

not possible:
 -display gtk + -display sdl

possible:
 -display gtk + -vnc
 -display gtk + -vnc + -spice
 -display none + -vnc + -spice

take care,
  Gerd

Re: [PATCH v2 0/3] softmmu/vl: Deprecate old and crufty display ui options

[Gerd Hoffmann]

On Wed, Aug 25, 2021 at 11:20:20AM +0200, Thomas Huth wrote:
> -display sdl uses a hand-crafted parser in vl.c, which is quite ugly
> since the other parts of -display have been QAPIfied already. A straight
> conversion to QAPI is not advisable since the "alt_grab" and "ctrl_grab"
> parameters are not the best solution anyway. So this patch series
> introduces a new "grab-mod" parameter as replacement instead and then
> deprecates the old and crufty options.
> 
> While we're at it, the third patch also suggests to deprecated the
> old -sdl and -curses top-level options.

Looks all sane to me.

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

take care,
  Gerd