[PATCH v3 0/3] ui: Remove deprecated sdl parameters and switch to QAPI parser

[PATCH v3 0/3] ui: Remove deprecated sdl parameters and switch to QAPI parser

[Thomas Huth]

The "-display sdl" option still uses a hand-crafted parser for its
parameters since some of them used underscores which is disliked in QAPI.
Now that they've been deprecated and the deprecation period is over, we
can remove the problematic parameters and switch to use the QAPI parser
instead.

While we're at it, also remove the deprecated "-sdl" and "-curses" options.

v3:
 - Fixed some texts according to Markus' and Eric's suggestions
 - Renamed the GrabMod enum to HotKeyMod (to not confuse it so easily
   with GrabToggleKeys)
   
v2:
 - Rebase to current master branch to resolve conflicts in docs/about/*.rst
 - Use an enum for the grab-mod parameter instead of a unconstrained string

Thomas Huth (3):
  ui: Remove deprecated parameters of the "-display sdl" option
  ui: Switch "-display sdl" to use the QAPI parser
  ui: Remove deprecated options "-sdl" and "-curses"

 docs/about/deprecated.rst       |  26 -------
 docs/about/removed-features.rst |  27 +++++++
 qapi/ui.json                    |  26 ++++++-
 include/sysemu/sysemu.h         |   2 -
 softmmu/globals.c               |   2 -
 softmmu/vl.c                    | 128 +-------------------------------
 ui/sdl2.c                       |  10 +++
 qemu-options.hx                 |  56 +-------------
 8 files changed, 67 insertions(+), 210 deletions(-)

-- 
2.27.0

[PATCH v3 1/3] ui: Remove deprecated parameters of the "-display sdl" option

[Thomas Huth]

Dropping these deprecated parameters simplifies further refactoring
(e.g. QAPIfication is easier without underscores in the name).

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 docs/about/deprecated.rst       | 16 -------------
 docs/about/removed-features.rst | 17 ++++++++++++++
 softmmu/vl.c                    | 41 +--------------------------------
 qemu-options.hx                 | 32 ++-----------------------
 4 files changed, 20 insertions(+), 86 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index a92ae0f162..562a133f18 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -81,22 +81,6 @@ the process listing. This is replaced by the new ``password-secret``
 option which lets the password be securely provided on the command
 line using a ``secret`` object instance.
 
-``-display sdl,window_close=...`` (since 6.1)
-'''''''''''''''''''''''''''''''''''''''''''''
-
-Use ``-display sdl,window-close=...`` instead (i.e. with a minus instead of
-an underscore between "window" and "close").
-
-``-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.
-
 ``-sdl`` (since 6.2)
 ''''''''''''''''''''
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index eb76974347..4c9e001c35 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -370,6 +370,23 @@ The ``opened=on`` option in the command line or QMP ``object-add`` either had
 no effect (if ``opened`` was the last option) or caused errors.  The property
 is therefore useless and should simply be removed.
 
+``-display sdl,window_close=...`` (removed in 7.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Use ``-display sdl,window-close=...`` instead (i.e. with a minus instead of
+an underscore between "window" and "close").
+
+``-alt-grab`` and ``-display sdl,alt_grab=on`` (removed in 7.1)
+'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Use ``-display sdl,grab-mod=lshift-lctrl-lalt`` instead.
+
+``-ctrl-grab`` and ``-display sdl,ctrl_grab=on`` (removed in 7.1)
+'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Use ``-display sdl,grab-mod=rctrl`` instead.
+
+
 QEMU Machine Protocol (QMP) commands
 ------------------------------------
 
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 84a31eba76..57ab9d5322 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1079,32 +1079,7 @@ static void parse_display(const char *p)
                 } else {
                     goto invalid_sdl_args;
                 }
-            } else if (strstart(opts, ",alt_grab=", &nextopt)) {
-                opts = nextopt;
-                if (strstart(opts, "on", &nextopt)) {
-                    alt_grab = 1;
-                } else if (strstart(opts, "off", &nextopt)) {
-                    alt_grab = 0;
-                } 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)) {
-                    ctrl_grab = 1;
-                } else if (strstart(opts, "off", &nextopt)) {
-                    ctrl_grab = 0;
-                } 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)) {
-                    warn_report("window_close with an underscore is deprecated,"
-                                " please use window-close instead.");
-                }
+            } else if (strstart(opts, ",window-close=", &nextopt)) {
                 opts = nextopt;
                 dpy.has_window_close = true;
                 if (strstart(opts, "on", &nextopt)) {
@@ -1962,10 +1937,6 @@ static void qemu_create_early_backends(void)
     const bool use_gtk = false;
 #endif
 
-    if ((alt_grab || ctrl_grab) && !use_sdl) {
-        error_report("-alt-grab and -ctrl-grab are only valid "
-                     "for SDL, ignoring option");
-    }
     if (dpy.has_window_close && !use_gtk && !use_sdl) {
         error_report("window-close is only valid for GTK and SDL, "
                      "ignoring option");
@@ -3273,16 +3244,6 @@ void qemu_init(int argc, char **argv, char **envp)
                 dpy.has_full_screen = true;
                 dpy.full_screen = true;
                 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_sdl:
                 warn_report("-sdl is deprecated, use -display sdl instead.");
 #ifdef CONFIG_SDL
diff --git a/qemu-options.hx b/qemu-options.hx
index a664baaa18..726e437a97 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1938,8 +1938,8 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
     "-display spice-app[,gl=on|off]\n"
 #endif
 #if defined(CONFIG_SDL)
-    "-display sdl[,alt_grab=on|off][,ctrl_grab=on|off][,gl=on|core|es|off]\n"
-    "            [,grab-mod=<mod>][,show-cursor=on|off][,window-close=on|off]\n"
+    "-display sdl[,gl=on|core|es|off][,grab-mod=<mod>][,show-cursor=on|off]\n"
+    "            [,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"
@@ -2012,12 +2012,6 @@ 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.
-        This parameter is deprecated - use ``grab-mod`` instead.
-
-        ``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
 
         ``show-cursor=on|off`` :  Force showing the mouse cursor
@@ -2103,28 +2097,6 @@ SRST
     is displayed in graphical mode.
 ERST
 
-DEF("alt-grab", 0, QEMU_OPTION_alt_grab,
-    "-alt-grab       use Ctrl-Alt-Shift to grab mouse (instead of Ctrl-Alt)\n",
-    QEMU_ARCH_ALL)
-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). This option is deprecated - please use
-    ``-display sdl,grab-mod=lshift-lctrl-lalt`` instead.
-ERST
-
-DEF("ctrl-grab", 0, QEMU_OPTION_ctrl_grab,
-    "-ctrl-grab      use Right-Ctrl to grab mouse (instead of Ctrl-Alt)\n",
-    QEMU_ARCH_ALL)
-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). This option is deprecated - please use
-    ``-display sdl,grab-mod=rctrl`` instead.
-ERST
-
 DEF("sdl", 0, QEMU_OPTION_sdl,
     "-sdl            shorthand for -display sdl\n", QEMU_ARCH_ALL)
 SRST
-- 
2.27.0

[PATCH v3 2/3] ui: Switch "-display sdl" to use the QAPI parser

[Thomas Huth]

The "-display sdl" option still uses a hand-crafted parser for its
parameters since we didn't want to drag an interface we considered
somewhat flawed into the QAPI schema. Since the flaws are gone now,
it's time to QAPIfy.

This introduces the new "DisplaySDL" QAPI struct that is used to hold
the parameters that are unique to the SDL display. The only specific
parameter is currently "grab-mod" that is used to specify the required
modifier keys to escape from the mouse grabbing mode.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 qapi/ui.json            | 26 ++++++++++++++-
 include/sysemu/sysemu.h |  2 --
 softmmu/globals.c       |  2 --
 softmmu/vl.c            | 70 +----------------------------------------
 ui/sdl2.c               | 10 ++++++
 5 files changed, 36 insertions(+), 74 deletions(-)

diff --git a/qapi/ui.json b/qapi/ui.json
index 11a827d10f..413371d5e8 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1295,6 +1295,29 @@
       '*swap-opt-cmd': 'bool'
   } }
 
+##
+# @HotKeyMod:
+#
+# Set of modifier keys that need to be held for shortcut key actions.
+#
+# Since: 7.1
+##
+{ 'enum'  : 'HotKeyMod',
+  'data'  : [ 'lctrl-lalt', 'lshift-lctrl-lalt', 'rctrl' ] }
+
+##
+# @DisplaySDL:
+#
+# SDL2 display options.
+#
+# @grab-mod:  Modifier keys that should be pressed together with the
+#             "G" key to release the mouse grab.
+#
+# Since: 7.1
+##
+{ 'struct'  : 'DisplaySDL',
+  'data'    : { '*grab-mod'   : 'HotKeyMod' } }
+
 ##
 # @DisplayType:
 #
@@ -1374,7 +1397,8 @@
       'curses': { 'type': 'DisplayCurses', 'if': 'CONFIG_CURSES' },
       'egl-headless': { 'type': 'DisplayEGLHeadless',
                         'if': { 'all': ['CONFIG_OPENGL', 'CONFIG_GBM'] } },
-      'dbus': { 'type': 'DisplayDBus', 'if': 'CONFIG_DBUS_DISPLAY' }
+      'dbus': { 'type': 'DisplayDBus', 'if': 'CONFIG_DBUS_DISPLAY' },
+      'sdl': { 'type': 'DisplaySDL', 'if': 'CONFIG_SDL' }
   }
 }
 
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index b4030acd74..812f66a31a 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -42,8 +42,6 @@ extern int graphic_depth;
 extern int display_opengl;
 extern const char *keyboard_layout;
 extern int win2k_install_hack;
-extern int alt_grab;
-extern int ctrl_grab;
 extern int graphic_rotate;
 extern int old_param;
 extern uint8_t *boot_splash_filedata;
diff --git a/softmmu/globals.c b/softmmu/globals.c
index 916bc12e2b..527edbefdd 100644
--- a/softmmu/globals.c
+++ b/softmmu/globals.c
@@ -50,8 +50,6 @@ QEMUOptionRom option_rom[MAX_OPTION_ROMS];
 int nb_option_roms;
 int old_param;
 const char *qemu_name;
-int alt_grab;
-int ctrl_grab;
 unsigned int nb_prom_envs;
 const char *prom_envs[MAX_PROM_ENVS];
 uint8_t *boot_splash_filedata;
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 57ab9d5322..484e9d9921 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1056,75 +1056,7 @@ static void parse_display(const char *p)
         exit(0);
     }
 
-    if (strstart(p, "sdl", &opts)) {
-        /*
-         * sdl DisplayType needs hand-crafted parser instead of
-         * parse_display_qapi() due to some options not in
-         * DisplayOptions, specifically:
-         *   - ctrl_grab + alt_grab
-         *     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, ",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, ",window-close=", &nextopt)) {
-                opts = nextopt;
-                dpy.has_window_close = true;
-                if (strstart(opts, "on", &nextopt)) {
-                    dpy.window_close = true;
-                } else if (strstart(opts, "off", &nextopt)) {
-                    dpy.window_close = false;
-                } else {
-                    goto invalid_sdl_args;
-                }
-            } else if (strstart(opts, ",show-cursor=", &nextopt)) {
-                opts = nextopt;
-                dpy.has_show_cursor = true;
-                if (strstart(opts, "on", &nextopt)) {
-                    dpy.show_cursor = true;
-                } else if (strstart(opts, "off", &nextopt)) {
-                    dpy.show_cursor = false;
-                } else {
-                    goto invalid_sdl_args;
-                }
-            } else if (strstart(opts, ",gl=", &nextopt)) {
-                opts = nextopt;
-                dpy.has_gl = true;
-                if (strstart(opts, "on", &nextopt)) {
-                    dpy.gl = DISPLAYGL_MODE_ON;
-                } else if (strstart(opts, "core", &nextopt)) {
-                    dpy.gl = DISPLAYGL_MODE_CORE;
-                } else if (strstart(opts, "es", &nextopt)) {
-                    dpy.gl = DISPLAYGL_MODE_ES;
-                } else if (strstart(opts, "off", &nextopt)) {
-                    dpy.gl = DISPLAYGL_MODE_OFF;
-                } else {
-                    goto invalid_sdl_args;
-                }
-            } else {
-            invalid_sdl_args:
-                error_report("invalid SDL option string");
-                exit(1);
-            }
-            opts = nextopt;
-        }
-#else
-        error_report("SDL display supported is not available in this binary");
-        exit(1);
-#endif
-    } else if (strstart(p, "vnc", &opts)) {
+    if (strstart(p, "vnc", &opts)) {
         /*
          * vnc isn't a (local) DisplayType but a protocol for remote
          * display access.
diff --git a/ui/sdl2.c b/ui/sdl2.c
index d3741f9b75..8cb77416af 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -40,6 +40,8 @@ static struct sdl2_console *sdl2_console;
 
 static SDL_Surface *guest_sprite_surface;
 static int gui_grab; /* if true, all keyboard/mouse events are grabbed */
+static bool alt_grab;
+static bool ctrl_grab;
 
 static int gui_saved_grab;
 static int gui_fullscreen;
@@ -853,6 +855,14 @@ static void sdl2_display_init(DisplayState *ds, DisplayOptions *o)
 
     gui_fullscreen = o->has_full_screen && o->full_screen;
 
+    if (o->u.sdl.has_grab_mod) {
+        if (o->u.sdl.grab_mod == HOT_KEY_MOD_LSHIFT_LCTRL_LALT) {
+            alt_grab = true;
+        } else if (o->u.sdl.grab_mod == HOT_KEY_MOD_RCTRL) {
+            ctrl_grab = true;
+        }
+    }
+
     for (i = 0;; i++) {
         QemuConsole *con = qemu_console_lookup_by_index(i);
         if (!con) {
-- 
2.27.0

Re: [PATCH v3 2/3] ui: Switch "-display sdl" to use the QAPI parser

[Markus Armbruster]

Thomas Huth <thuth@redhat.com> writes:

> The "-display sdl" option still uses a hand-crafted parser for its
> parameters since we didn't want to drag an interface we considered
> somewhat flawed into the QAPI schema. Since the flaws are gone now,
> it's time to QAPIfy.
>
> This introduces the new "DisplaySDL" QAPI struct that is used to hold
> the parameters that are unique to the SDL display. The only specific
> parameter is currently "grab-mod" that is used to specify the required
> modifier keys to escape from the mouse grabbing mode.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  qapi/ui.json            | 26 ++++++++++++++-
>  include/sysemu/sysemu.h |  2 --
>  softmmu/globals.c       |  2 --
>  softmmu/vl.c            | 70 +----------------------------------------
>  ui/sdl2.c               | 10 ++++++
>  5 files changed, 36 insertions(+), 74 deletions(-)
>
> diff --git a/qapi/ui.json b/qapi/ui.json
> index 11a827d10f..413371d5e8 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1295,6 +1295,29 @@
>        '*swap-opt-cmd': 'bool'
>    } }
>  
> +##
> +# @HotKeyMod:
> +#
> +# Set of modifier keys that need to be held for shortcut key actions.
> +#
> +# Since: 7.1
> +##
> +{ 'enum'  : 'HotKeyMod',
> +  'data'  : [ 'lctrl-lalt', 'lshift-lctrl-lalt', 'rctrl' ] }

I have a somewhat uneasy feeling about encoding what is essentially a
subset of the sets of modifier keys as an enumeration, but it's what we
have to do to QAPIfy existing grab-mod.

> +
> +##
> +# @DisplaySDL:
> +#
> +# SDL2 display options.
> +#
> +# @grab-mod:  Modifier keys that should be pressed together with the
> +#             "G" key to release the mouse grab.
> +#
> +# Since: 7.1
> +##
> +{ 'struct'  : 'DisplaySDL',
> +  'data'    : { '*grab-mod'   : 'HotKeyMod' } }
> +
>  ##
>  # @DisplayType:
>  #
> @@ -1374,7 +1397,8 @@
>        'curses': { 'type': 'DisplayCurses', 'if': 'CONFIG_CURSES' },
>        'egl-headless': { 'type': 'DisplayEGLHeadless',
>                          'if': { 'all': ['CONFIG_OPENGL', 'CONFIG_GBM'] } },
> -      'dbus': { 'type': 'DisplayDBus', 'if': 'CONFIG_DBUS_DISPLAY' }
> +      'dbus': { 'type': 'DisplayDBus', 'if': 'CONFIG_DBUS_DISPLAY' },
> +      'sdl': { 'type': 'DisplaySDL', 'if': 'CONFIG_SDL' }
>    }
>  }
>  
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index b4030acd74..812f66a31a 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -42,8 +42,6 @@ extern int graphic_depth;
>  extern int display_opengl;
>  extern const char *keyboard_layout;
>  extern int win2k_install_hack;
> -extern int alt_grab;
> -extern int ctrl_grab;
>  extern int graphic_rotate;
>  extern int old_param;
>  extern uint8_t *boot_splash_filedata;
> diff --git a/softmmu/globals.c b/softmmu/globals.c
> index 916bc12e2b..527edbefdd 100644
> --- a/softmmu/globals.c
> +++ b/softmmu/globals.c
> @@ -50,8 +50,6 @@ QEMUOptionRom option_rom[MAX_OPTION_ROMS];
>  int nb_option_roms;
>  int old_param;
>  const char *qemu_name;
> -int alt_grab;
> -int ctrl_grab;
>  unsigned int nb_prom_envs;
>  const char *prom_envs[MAX_PROM_ENVS];
>  uint8_t *boot_splash_filedata;
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 57ab9d5322..484e9d9921 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -1056,75 +1056,7 @@ static void parse_display(const char *p)
>          exit(0);
>      }
>  
> -    if (strstart(p, "sdl", &opts)) {
> -        /*
> -         * sdl DisplayType needs hand-crafted parser instead of
> -         * parse_display_qapi() due to some options not in
> -         * DisplayOptions, specifically:
> -         *   - ctrl_grab + alt_grab
> -         *     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, ",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, ",window-close=", &nextopt)) {
> -                opts = nextopt;
> -                dpy.has_window_close = true;
> -                if (strstart(opts, "on", &nextopt)) {
> -                    dpy.window_close = true;
> -                } else if (strstart(opts, "off", &nextopt)) {
> -                    dpy.window_close = false;
> -                } else {
> -                    goto invalid_sdl_args;
> -                }
> -            } else if (strstart(opts, ",show-cursor=", &nextopt)) {
> -                opts = nextopt;
> -                dpy.has_show_cursor = true;
> -                if (strstart(opts, "on", &nextopt)) {
> -                    dpy.show_cursor = true;
> -                } else if (strstart(opts, "off", &nextopt)) {
> -                    dpy.show_cursor = false;
> -                } else {
> -                    goto invalid_sdl_args;
> -                }
> -            } else if (strstart(opts, ",gl=", &nextopt)) {
> -                opts = nextopt;
> -                dpy.has_gl = true;
> -                if (strstart(opts, "on", &nextopt)) {
> -                    dpy.gl = DISPLAYGL_MODE_ON;
> -                } else if (strstart(opts, "core", &nextopt)) {
> -                    dpy.gl = DISPLAYGL_MODE_CORE;
> -                } else if (strstart(opts, "es", &nextopt)) {
> -                    dpy.gl = DISPLAYGL_MODE_ES;
> -                } else if (strstart(opts, "off", &nextopt)) {
> -                    dpy.gl = DISPLAYGL_MODE_OFF;
> -                } else {
> -                    goto invalid_sdl_args;
> -                }
> -            } else {
> -            invalid_sdl_args:
> -                error_report("invalid SDL option string");
> -                exit(1);
> -            }
> -            opts = nextopt;
> -        }
> -#else
> -        error_report("SDL display supported is not available in this binary");
> -        exit(1);
> -#endif

When CONFIG_SDL is off, the error message changes from

    qemu-system-x86_64: -display sdl: SDL display supported is not available in this binary

to

    qemu-system-x86_64: -display sdl: Parameter 'type' does not accept value 'sdl'

I don't mind, but I'd suggest to mention it in the commit message.

> -    } else if (strstart(p, "vnc", &opts)) {
> +    if (strstart(p, "vnc", &opts)) {
>          /*
>           * vnc isn't a (local) DisplayType but a protocol for remote
>           * display access.
> diff --git a/ui/sdl2.c b/ui/sdl2.c
> index d3741f9b75..8cb77416af 100644
> --- a/ui/sdl2.c
> +++ b/ui/sdl2.c
> @@ -40,6 +40,8 @@ static struct sdl2_console *sdl2_console;
>  
>  static SDL_Surface *guest_sprite_surface;
>  static int gui_grab; /* if true, all keyboard/mouse events are grabbed */
> +static bool alt_grab;
> +static bool ctrl_grab;

I like that these become internal to sdl2.c.

>  
>  static int gui_saved_grab;
>  static int gui_fullscreen;
> @@ -853,6 +855,14 @@ static void sdl2_display_init(DisplayState *ds, DisplayOptions *o)
>  
>      gui_fullscreen = o->has_full_screen && o->full_screen;
>  
> +    if (o->u.sdl.has_grab_mod) {
> +        if (o->u.sdl.grab_mod == HOT_KEY_MOD_LSHIFT_LCTRL_LALT) {
> +            alt_grab = true;
> +        } else if (o->u.sdl.grab_mod == HOT_KEY_MOD_RCTRL) {
> +            ctrl_grab = true;
> +        }
> +    }
> +
>      for (i = 0;; i++) {
>          QemuConsole *con = qemu_console_lookup_by_index(i);
>          if (!con) {

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH v3 2/3] ui: Switch "-display sdl" to use the QAPI parser

[Thomas Huth]

On 23/05/2022 15.45, Markus Armbruster wrote:
> Thomas Huth <thuth@redhat.com> writes:
> 
>> The "-display sdl" option still uses a hand-crafted parser for its
>> parameters since we didn't want to drag an interface we considered
>> somewhat flawed into the QAPI schema. Since the flaws are gone now,
>> it's time to QAPIfy.
>>
>> This introduces the new "DisplaySDL" QAPI struct that is used to hold
>> the parameters that are unique to the SDL display. The only specific
>> parameter is currently "grab-mod" that is used to specify the required
>> modifier keys to escape from the mouse grabbing mode.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   qapi/ui.json            | 26 ++++++++++++++-
>>   include/sysemu/sysemu.h |  2 --
>>   softmmu/globals.c       |  2 --
>>   softmmu/vl.c            | 70 +----------------------------------------
>>   ui/sdl2.c               | 10 ++++++
>>   5 files changed, 36 insertions(+), 74 deletions(-)
>>
>> diff --git a/qapi/ui.json b/qapi/ui.json
>> index 11a827d10f..413371d5e8 100644
>> --- a/qapi/ui.json
>> +++ b/qapi/ui.json
>> @@ -1295,6 +1295,29 @@
>>         '*swap-opt-cmd': 'bool'
>>     } }
>>   
>> +##
>> +# @HotKeyMod:
>> +#
>> +# Set of modifier keys that need to be held for shortcut key actions.
>> +#
>> +# Since: 7.1
>> +##
>> +{ 'enum'  : 'HotKeyMod',
>> +  'data'  : [ 'lctrl-lalt', 'lshift-lctrl-lalt', 'rctrl' ] }
> 
> I have a somewhat uneasy feeling about encoding what is essentially a
> subset of the sets of modifier keys as an enumeration, but it's what we
> have to do to QAPIfy existing grab-mod.

Well, that's exactly what you suggested here:

  https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg03401.html

So I really don't understand your uneasy feeling now?

...
>> diff --git a/softmmu/vl.c b/softmmu/vl.c
>> index 57ab9d5322..484e9d9921 100644
>> --- a/softmmu/vl.c
>> +++ b/softmmu/vl.c
>> @@ -1056,75 +1056,7 @@ static void parse_display(const char *p)
>>           exit(0);
>>       }
>>   
>> -    if (strstart(p, "sdl", &opts)) {
>> -        /*
>> -         * sdl DisplayType needs hand-crafted parser instead of
>> -         * parse_display_qapi() due to some options not in
>> -         * DisplayOptions, specifically:
>> -         *   - ctrl_grab + alt_grab
>> -         *     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, ",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, ",window-close=", &nextopt)) {
>> -                opts = nextopt;
>> -                dpy.has_window_close = true;
>> -                if (strstart(opts, "on", &nextopt)) {
>> -                    dpy.window_close = true;
>> -                } else if (strstart(opts, "off", &nextopt)) {
>> -                    dpy.window_close = false;
>> -                } else {
>> -                    goto invalid_sdl_args;
>> -                }
>> -            } else if (strstart(opts, ",show-cursor=", &nextopt)) {
>> -                opts = nextopt;
>> -                dpy.has_show_cursor = true;
>> -                if (strstart(opts, "on", &nextopt)) {
>> -                    dpy.show_cursor = true;
>> -                } else if (strstart(opts, "off", &nextopt)) {
>> -                    dpy.show_cursor = false;
>> -                } else {
>> -                    goto invalid_sdl_args;
>> -                }
>> -            } else if (strstart(opts, ",gl=", &nextopt)) {
>> -                opts = nextopt;
>> -                dpy.has_gl = true;
>> -                if (strstart(opts, "on", &nextopt)) {
>> -                    dpy.gl = DISPLAYGL_MODE_ON;
>> -                } else if (strstart(opts, "core", &nextopt)) {
>> -                    dpy.gl = DISPLAYGL_MODE_CORE;
>> -                } else if (strstart(opts, "es", &nextopt)) {
>> -                    dpy.gl = DISPLAYGL_MODE_ES;
>> -                } else if (strstart(opts, "off", &nextopt)) {
>> -                    dpy.gl = DISPLAYGL_MODE_OFF;
>> -                } else {
>> -                    goto invalid_sdl_args;
>> -                }
>> -            } else {
>> -            invalid_sdl_args:
>> -                error_report("invalid SDL option string");
>> -                exit(1);
>> -            }
>> -            opts = nextopt;
>> -        }
>> -#else
>> -        error_report("SDL display supported is not available in this binary");
>> -        exit(1);
>> -#endif
> 
> When CONFIG_SDL is off, the error message changes from
> 
>      qemu-system-x86_64: -display sdl: SDL display supported is not available in this binary
> 
> to
> 
>      qemu-system-x86_64: -display sdl: Parameter 'type' does not accept value 'sdl'
> 
> I don't mind, but I'd suggest to mention it in the commit message.

I can do that if I have to respin this series for some other reasons. 
Otherwise, I don't think this really severe enough to justify a v4.

> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 

Thanks!

  Thomas

Re: [PATCH v3 2/3] ui: Switch "-display sdl" to use the QAPI parser

[Daniel P. Berrangé]

On Mon, May 23, 2022 at 09:23:48PM +0200, Thomas Huth wrote:
> On 23/05/2022 15.45, Markus Armbruster wrote:
> > Thomas Huth <thuth@redhat.com> writes:
> > 
> > > The "-display sdl" option still uses a hand-crafted parser for its
> > > parameters since we didn't want to drag an interface we considered
> > > somewhat flawed into the QAPI schema. Since the flaws are gone now,
> > > it's time to QAPIfy.
> > > 
> > > This introduces the new "DisplaySDL" QAPI struct that is used to hold
> > > the parameters that are unique to the SDL display. The only specific
> > > parameter is currently "grab-mod" that is used to specify the required
> > > modifier keys to escape from the mouse grabbing mode.
> > > 
> > > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > > ---
> > >   qapi/ui.json            | 26 ++++++++++++++-
> > >   include/sysemu/sysemu.h |  2 --
> > >   softmmu/globals.c       |  2 --
> > >   softmmu/vl.c            | 70 +----------------------------------------
> > >   ui/sdl2.c               | 10 ++++++
> > >   5 files changed, 36 insertions(+), 74 deletions(-)
> > > 
> > > diff --git a/qapi/ui.json b/qapi/ui.json
> > > index 11a827d10f..413371d5e8 100644
> > > --- a/qapi/ui.json
> > > +++ b/qapi/ui.json
> > > @@ -1295,6 +1295,29 @@
> > >         '*swap-opt-cmd': 'bool'
> > >     } }
> > > +##
> > > +# @HotKeyMod:
> > > +#
> > > +# Set of modifier keys that need to be held for shortcut key actions.
> > > +#
> > > +# Since: 7.1
> > > +##
> > > +{ 'enum'  : 'HotKeyMod',
> > > +  'data'  : [ 'lctrl-lalt', 'lshift-lctrl-lalt', 'rctrl' ] }
> > 
> > I have a somewhat uneasy feeling about encoding what is essentially a
> > subset of the sets of modifier keys as an enumeration, but it's what we
> > have to do to QAPIfy existing grab-mod.
> 
> Well, that's exactly what you suggested here:
> 
>  https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg03401.html
> 
> So I really don't understand your uneasy feeling now?

I think we should consider what happens if we want to allow arbitrary
key combinations in future, consisting of one or more modifiers together
witha non-modifier key. For example CtrlL+ShiftL+F12.  We won't want
to be expressing the combinatorial expansion of all possible key
combinations as an enum.  Instead I think we would want to have an
enum for keycode names and accept an array of them. We already
have QKeyCode, so for SDL grab sequence we need to accept an
arrray of QKeyCode.

  { 'struct'  : 'DisplaySDL',
    'data'    : { '*grab-mod'   : [ 'QKeyCode' ] }

Good for QMP but not entirely pretty on the CLI. But we need back
compat for existing CLI syntax too, so we would have to use an
alternate allowing plain string for the legacy key codes combinations.

IOW we end up needing

   { 'alternate': 'SDLGrabSeq',
     'data': { 'grab-mod': 'HotKeyMod',
               'grab-keys: [ 'QKeyCode' ] } }

  { 'struct'  : 'DisplaySDL',
    'data'    : { '*grab-mod' : [ 'SDLGrabSeq' ] }

deprecating 'grab-mod' for a while, eventually leaving just the
first example above.

Since  IIUC, we can retrofit the alternate after the fact if we
decide to allow arbitrary key combos, it means we could safely
start with what Thomas proposes

  { 'struct'  : 'DisplaySDL',
    'data'    : { '*grab-mod' : 'HotKeyMod' }

and worry about the more general solution another day/month/year

> 
> ...
> > > diff --git a/softmmu/vl.c b/softmmu/vl.c
> > > index 57ab9d5322..484e9d9921 100644
> > > --- a/softmmu/vl.c
> > > +++ b/softmmu/vl.c
> > > @@ -1056,75 +1056,7 @@ static void parse_display(const char *p)
> > >           exit(0);
> > >       }
> > > -    if (strstart(p, "sdl", &opts)) {
> > > -        /*
> > > -         * sdl DisplayType needs hand-crafted parser instead of
> > > -         * parse_display_qapi() due to some options not in
> > > -         * DisplayOptions, specifically:
> > > -         *   - ctrl_grab + alt_grab
> > > -         *     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, ",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, ",window-close=", &nextopt)) {
> > > -                opts = nextopt;
> > > -                dpy.has_window_close = true;
> > > -                if (strstart(opts, "on", &nextopt)) {
> > > -                    dpy.window_close = true;
> > > -                } else if (strstart(opts, "off", &nextopt)) {
> > > -                    dpy.window_close = false;
> > > -                } else {
> > > -                    goto invalid_sdl_args;
> > > -                }
> > > -            } else if (strstart(opts, ",show-cursor=", &nextopt)) {
> > > -                opts = nextopt;
> > > -                dpy.has_show_cursor = true;
> > > -                if (strstart(opts, "on", &nextopt)) {
> > > -                    dpy.show_cursor = true;
> > > -                } else if (strstart(opts, "off", &nextopt)) {
> > > -                    dpy.show_cursor = false;
> > > -                } else {
> > > -                    goto invalid_sdl_args;
> > > -                }
> > > -            } else if (strstart(opts, ",gl=", &nextopt)) {
> > > -                opts = nextopt;
> > > -                dpy.has_gl = true;
> > > -                if (strstart(opts, "on", &nextopt)) {
> > > -                    dpy.gl = DISPLAYGL_MODE_ON;
> > > -                } else if (strstart(opts, "core", &nextopt)) {
> > > -                    dpy.gl = DISPLAYGL_MODE_CORE;
> > > -                } else if (strstart(opts, "es", &nextopt)) {
> > > -                    dpy.gl = DISPLAYGL_MODE_ES;
> > > -                } else if (strstart(opts, "off", &nextopt)) {
> > > -                    dpy.gl = DISPLAYGL_MODE_OFF;
> > > -                } else {
> > > -                    goto invalid_sdl_args;
> > > -                }
> > > -            } else {
> > > -            invalid_sdl_args:
> > > -                error_report("invalid SDL option string");
> > > -                exit(1);
> > > -            }
> > > -            opts = nextopt;
> > > -        }
> > > -#else
> > > -        error_report("SDL display supported is not available in this binary");
> > > -        exit(1);
> > > -#endif
> > 
> > When CONFIG_SDL is off, the error message changes from
> > 
> >      qemu-system-x86_64: -display sdl: SDL display supported is not available in this binary
> > 
> > to
> > 
> >      qemu-system-x86_64: -display sdl: Parameter 'type' does not accept value 'sdl'
> > 
> > I don't mind, but I'd suggest to mention it in the commit message.
> 
> I can do that if I have to respin this series for some other reasons.
> Otherwise, I don't think this really severe enough to justify a v4.
> 
> > 
> > Reviewed-by: Markus Armbruster <armbru@redhat.com>
> > 
> 
> Thanks!
> 
>  Thomas
> 
> 

With 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 v3 2/3] ui: Switch "-display sdl" to use the QAPI parser

[Markus Armbruster]

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

> On Mon, May 23, 2022 at 09:23:48PM +0200, Thomas Huth wrote:
>> On 23/05/2022 15.45, Markus Armbruster wrote:
>> > Thomas Huth <thuth@redhat.com> writes:
>> > 
>> > > The "-display sdl" option still uses a hand-crafted parser for its
>> > > parameters since we didn't want to drag an interface we considered
>> > > somewhat flawed into the QAPI schema. Since the flaws are gone now,
>> > > it's time to QAPIfy.
>> > > 
>> > > This introduces the new "DisplaySDL" QAPI struct that is used to hold
>> > > the parameters that are unique to the SDL display. The only specific
>> > > parameter is currently "grab-mod" that is used to specify the required
>> > > modifier keys to escape from the mouse grabbing mode.
>> > > 
>> > > Signed-off-by: Thomas Huth <thuth@redhat.com>
>> > > ---
>> > >   qapi/ui.json            | 26 ++++++++++++++-
>> > >   include/sysemu/sysemu.h |  2 --
>> > >   softmmu/globals.c       |  2 --
>> > >   softmmu/vl.c            | 70 +----------------------------------------
>> > >   ui/sdl2.c               | 10 ++++++
>> > >   5 files changed, 36 insertions(+), 74 deletions(-)
>> > > 
>> > > diff --git a/qapi/ui.json b/qapi/ui.json
>> > > index 11a827d10f..413371d5e8 100644
>> > > --- a/qapi/ui.json
>> > > +++ b/qapi/ui.json
>> > > @@ -1295,6 +1295,29 @@
>> > >         '*swap-opt-cmd': 'bool'
>> > >     } }
>> > > +##
>> > > +# @HotKeyMod:
>> > > +#
>> > > +# Set of modifier keys that need to be held for shortcut key actions.
>> > > +#
>> > > +# Since: 7.1
>> > > +##
>> > > +{ 'enum'  : 'HotKeyMod',
>> > > +  'data'  : [ 'lctrl-lalt', 'lshift-lctrl-lalt', 'rctrl' ] }
>> > 
>> > I have a somewhat uneasy feeling about encoding what is essentially a
>> > subset of the sets of modifier keys as an enumeration, but it's what we
>> > have to do to QAPIfy existing grab-mod.
>> 
>> Well, that's exactly what you suggested here:
>> 
>>  https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg03401.html
>> 
>> So I really don't understand your uneasy feeling now?

I did mention the set of modifiers (encoded as list) design alternative.
You pointed out that you're merely QAPIfying what we have.  Valid point,
well taken, I don't want to block that.

> I think we should consider what happens if we want to allow arbitrary
> key combinations in future, consisting of one or more modifiers together
> witha non-modifier key. For example CtrlL+ShiftL+F12.  We won't want
> to be expressing the combinatorial expansion of all possible key
> combinations as an enum.  Instead I think we would want to have an
> enum for keycode names and accept an array of them. We already
> have QKeyCode, so for SDL grab sequence we need to accept an
> arrray of QKeyCode.
>
>   { 'struct'  : 'DisplaySDL',
>     'data'    : { '*grab-mod'   : [ 'QKeyCode' ] }
>
> Good for QMP but not entirely pretty on the CLI. But we need back
> compat for existing CLI syntax too, so we would have to use an
> alternate allowing plain string for the legacy key codes combinations.
>
> IOW we end up needing
>
>    { 'alternate': 'SDLGrabSeq',
>      'data': { 'grab-mod': 'HotKeyMod',
>                'grab-keys: [ 'QKeyCode' ] } }
>
>   { 'struct'  : 'DisplaySDL',
>     'data'    : { '*grab-mod' : [ 'SDLGrabSeq' ] }
>
> deprecating 'grab-mod' for a while, eventually leaving just the
> first example above.
>
> Since  IIUC, we can retrofit the alternate after the fact if we
> decide to allow arbitrary key combos, it means we could safely
> start with what Thomas proposes
>
>   { 'struct'  : 'DisplaySDL',
>     'data'    : { '*grab-mod' : 'HotKeyMod' }
>
> and worry about the more general solution another day/month/year

Right.

[...]

[PATCH v3 3/3] ui: Remove deprecated options "-sdl" and "-curses"

[Thomas Huth]

We have "-sdl" and "-curses", but no "-gtk" and no "-cocoa" ...
these old-style options are rather confusing than helpful nowadays.
Now that the deprecation period is over, let's remove them, so we
get a cleaner interface (where "-display" is the only way to select
the user interface).

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 docs/about/deprecated.rst       | 10 ----------
 docs/about/removed-features.rst | 10 ++++++++++
 softmmu/vl.c                    | 19 -------------------
 qemu-options.hx                 | 24 ++----------------------
 4 files changed, 12 insertions(+), 51 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 562a133f18..e19bcba242 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -81,16 +81,6 @@ the process listing. This is replaced by the new ``password-secret``
 option which lets the password be securely provided on the command
 line using a ``secret`` object instance.
 
-``-sdl`` (since 6.2)
-''''''''''''''''''''
-
-Use ``-display sdl`` instead.
-
-``-curses`` (since 6.2)
-'''''''''''''''''''''''
-
-Use ``-display curses`` instead.
-
 ``-watchdog`` (since 6.2)
 '''''''''''''''''''''''''
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 4c9e001c35..c7b9dadd5d 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -386,6 +386,16 @@ Use ``-display sdl,grab-mod=lshift-lctrl-lalt`` instead.
 
 Use ``-display sdl,grab-mod=rctrl`` instead.
 
+``-sdl`` (removed in 7.1)
+'''''''''''''''''''''''''
+
+Use ``-display sdl`` instead.
+
+``-curses`` (removed in 7.1)
+''''''''''''''''''''''''''''
+
+Use ``-display curses`` instead.
+
 
 QEMU Machine Protocol (QMP) commands
 ------------------------------------
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 484e9d9921..4c1e94b00e 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2800,16 +2800,6 @@ void qemu_init(int argc, char **argv, char **envp)
                 nographic = true;
                 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
-                error_report("curses or iconv support is disabled");
-                exit(1);
-#endif
-                break;
             case QEMU_OPTION_portrait:
                 graphic_rotate = 90;
                 break;
@@ -3176,15 +3166,6 @@ void qemu_init(int argc, char **argv, char **envp)
                 dpy.has_full_screen = true;
                 dpy.full_screen = true;
                 break;
-            case QEMU_OPTION_sdl:
-                warn_report("-sdl is deprecated, use -display sdl instead.");
-#ifdef CONFIG_SDL
-                dpy.type = DISPLAY_TYPE_SDL;
-                break;
-#else
-                error_report("SDL support is disabled");
-                exit(1);
-#endif
             case QEMU_OPTION_pidfile:
                 pid_file = optarg;
                 break;
diff --git a/qemu-options.hx b/qemu-options.hx
index 726e437a97..60cf188da4 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1981,9 +1981,8 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
     , QEMU_ARCH_ALL)
 SRST
 ``-display type``
-    Select type of display to use. This option is a replacement for the
-    old style -sdl/-curses/... options. Use ``-display help`` to list
-    the available display types. Valid values for type are
+    Select type of display to use. Use ``-display help`` to list the available
+    display types. Valid values for type are
 
     ``spice-app[,gl=on|off]``
         Start QEMU as a Spice server and launch the default Spice client
@@ -2085,25 +2084,6 @@ SRST
     Use C-a h for help on switching between the console and monitor.
 ERST
 
-DEF("curses", 0, QEMU_OPTION_curses,
-    "-curses         shorthand for -display curses\n",
-    QEMU_ARCH_ALL)
-SRST
-``-curses``
-    Normally, if QEMU is compiled with graphical window support, it
-    displays output such as guest graphics, guest console, and the QEMU
-    monitor in a window. With this option, QEMU can display the VGA
-    output when in text mode using a curses/ncurses interface. Nothing
-    is displayed in graphical mode.
-ERST
-
-DEF("sdl", 0, QEMU_OPTION_sdl,
-    "-sdl            shorthand for -display sdl\n", QEMU_ARCH_ALL)
-SRST
-``-sdl``
-    Enable SDL.
-ERST
-
 #ifdef CONFIG_SPICE
 DEF("spice", HAS_ARG, QEMU_OPTION_spice,
     "-spice [port=port][,tls-port=secured-port][,x509-dir=<dir>]\n"
-- 
2.27.0