Re: [PATCH] qemu-options: Remove the deprecated -no-quit option

[Thomas Huth <thuth@redhat.com>]

On 15/12/2021 10.11, Markus Armbruster wrote:
> Thomas Huth <thuth@redhat.com> writes:
> 
>> This option was just a wrapper around the -display ...,window-close=off
>> parameter, and the name "no-quit" is rather confusing compared to
>> "window-close" (since there are still other means to quit the emulator),
>> so let's remove this now.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   docs/about/deprecated.rst       | 6 ------
>>   docs/about/removed-features.rst | 7 +++++++
>>   qemu-options.hx                 | 8 --------
>>   softmmu/vl.c                    | 8 +-------
>>   4 files changed, 8 insertions(+), 21 deletions(-)
>>
>> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
>> index 7f12f53713..6f85afdee4 100644
>> --- a/docs/about/deprecated.rst
>> +++ b/docs/about/deprecated.rst
>> @@ -134,12 +134,6 @@ specified.
>>   Use ``-display sdl,window-close=...`` instead (i.e. with a minus instead of
>>   an underscore between "window" and "close").
>>   
>> -``-no-quit`` (since 6.1)
>> -''''''''''''''''''''''''
>> -
>> -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)
>>   ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
>>   
>> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
>> index f92b8bd738..380a1b30ea 100644
>> --- a/docs/about/removed-features.rst
>> +++ b/docs/about/removed-features.rst
>> @@ -330,6 +330,13 @@ RISC-V firmware not booted by default (removed in 5.1)
>>   QEMU 5.1 changes the default behaviour from ``-bios none`` to ``-bios default``
>>   for the RISC-V ``virt`` machine and ``sifive_u`` machine.
>>   
>> +``-no-quit`` (removed in 7.0)
>> +'''''''''''''''''''''''''''''
>> +
>> +The ``-no-quit`` was a synonym for ``-display ...,window-close=off`` which
>> +should be used instead.
>> +
>> +
>>   QEMU Machine Protocol (QMP) commands
>>   ------------------------------------
>>   
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index ae2c6dbbfc..e2b73f6e5d 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -2002,14 +2002,6 @@ SRST
>>       ``-display sdl,grab-mod=rctrl`` instead.
>>   ERST
>>   
>> -DEF("no-quit", 0, QEMU_OPTION_no_quit,
>> -    "-no-quit        disable SDL/GTK window close capability (deprecated)\n", QEMU_ARCH_ALL)
>> -SRST
>> -``-no-quit``
>> -    Disable window close capability (SDL and GTK only). This option is
>> -    deprecated, please use ``-display ...,window-close=off`` instead.
>> -ERST
>> -
>>   DEF("sdl", 0, QEMU_OPTION_sdl,
>>       "-sdl            shorthand for -display sdl\n", QEMU_ARCH_ALL)
>>   SRST
>> diff --git a/softmmu/vl.c b/softmmu/vl.c
>> index 620a1f1367..163ab355f5 100644
>> --- a/softmmu/vl.c
>> +++ b/softmmu/vl.c
>> @@ -1938,7 +1938,7 @@ static void qemu_create_early_backends(void)
>>                        "for SDL, ignoring option");
>>       }
>>       if (dpy.has_window_close && !use_gtk && !use_sdl) {
>> -        error_report("-no-quit is only valid for GTK and SDL, "
>> +        error_report("window-close is only valid for GTK and SDL, "
>>                        "ignoring option");
>>       }
>>   
> 
> Bonus: the error message in now in terms of the non-deprecated
> interface.  Suggest to mention this in the commit message.
> 
> Sadly, it lacks context:
> 
>      qemu-system-x86_64: window-close is only valid for GTK and SDL, ignoring option
> 
> If we check in parse_display() instead, we should be able to get
> something like
> 
>      qemu-system-x86_64: -display none,window-close=off: window-close is only valid for GTK and SDL, ignoring option
> 
> Funny: we get a different error with VNC:
> 
>      qemu-system-x86_64: -display vnc=:0,window-close=off: Invalid parameter 'window-close'
> 
> That's because parse_display() uses separate parsers for SDL, VNC and
> the rest, and only SDL and the rest accept window-close.
> 
> Finally, why isn't this an error?  Invalid configuration should be...
> 
> If we decide not to error out in this case, we should use warn_report()
> instead of error_report().
> 
> All this can be addressed on top.

I'm planning to get this mess cleaned up in QEMU 7.1 - once the ugly SDL 
parameters with the underscore can finally be removed after their 
deprecation period is over. Then we can switch the SDL display to proper 
QAPI parsing and remove all the legacy cruft there.

  Thomas