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

[PATCH 0/3] softmmu/vl: Deprecate old and crufty display 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.

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/system/deprecated.rst | 20 ++++++++++++++++++++
 qemu-options.hx            | 18 +++++++++++++-----
 softmmu/vl.c               | 24 +++++++++++++++++++++---
 3 files changed, 54 insertions(+), 8 deletions(-)

-- 
2.27.0

[PATCH 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 8965dabc83..144afad8d1 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1784,7 +1784,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"
@@ -1830,6 +1830,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 a50c8575a1..506b88b1a2 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1013,15 +1013,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 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.

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

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 70e08baff6..89069856f1 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/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.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Use ``-display sdl,grab-mod=lshift-lctrl-lalt`` instead.
+
+``-ctrl-grab`` and ``-display sdl,ctrl_grab=on`` (since 6.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Use ``-display sdl,grab-mod=rctrl`` instead.
+
 
 QEMU Machine Protocol (QMP) commands
 ------------------------------------
diff --git a/qemu-options.hx b/qemu-options.hx
index 144afad8d1..5a76544e8b 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1834,9 +1834,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
 
@@ -1921,7 +1923,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,
@@ -1931,7 +1934,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 506b88b1a2..47cd47c840 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1039,6 +1039,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)) {
@@ -1048,6 +1049,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)) {
@@ -3237,9 +3239,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

[PATCH 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.

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

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 89069856f1..52ce343963 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/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.1)
+''''''''''''''''''''
+
+Use ``-display sdl`` instead.
+
+``-curses`` (since 6.1)
+'''''''''''''''''''''''
+
+Use ``-display curses`` instead.
+
 
 QEMU Machine Protocol (QMP) commands
 ------------------------------------
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 47cd47c840..e5ed90ce11 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2881,6 +2881,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
@@ -3254,6 +3256,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 0/3] softmmu/vl: Deprecate old and crufty display options

[Peter Krempa]

On Tue, Jul 06, 2021 at 16:54:10 +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.
> 
> 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

libvirt doesn't use any of those old options any more so:

on behalf of libvirt:

ACKed-by: Peter Krempa <pkrempa@redhat.com>