diff mbox series

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

Message ID 20210825092023.81396-4-thuth@redhat.com
State New
Headers show
Series softmmu/vl: Deprecate old and crufty display ui options | expand

Commit Message

Thomas Huth Aug. 25, 2021, 9:20 a.m. UTC
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(+)

Comments

Paolo Bonzini Aug. 31, 2021, 1:53 p.m. UTC | #1
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
>
>
Thomas Huth Sept. 2, 2021, 10:51 a.m. UTC | #2
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
Daniel P. Berrangé Sept. 2, 2021, 10:58 a.m. UTC | #3
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
Thomas Huth Sept. 2, 2021, 11:21 a.m. UTC | #4
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
Markus Armbruster Sept. 2, 2021, 11:37 a.m. UTC | #5
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.
Gerd Hoffmann Sept. 2, 2021, 1:21 p.m. UTC | #6
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
diff mbox series

Patch

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;