The real driver for these patches is to send all QemuOpts user input
to qemu_opts_parse_noisily, for consistency in the command line
parsing code and to effectively outlaw "help" and "?" QemuOpts
suboptions.  vnc_parse is the only function that is still using
qemu_opts_parse.

In order to remove the non-command-line callers of vnc_parse,
I am removing the deprecated QMP change command but also its HMP
veneer "change vnc TARGET", whose usecase is somewhat unclear to
me.  "change vnc password" is still supported.

Finally, by switching to qemu_opts_parse_noisily, it is easy to
print a help message on "-vnc help".


Paolo Bonzini (3):
  hmp: remove "change vnc TARGET" command
  qmp: remove deprecated "change" command
  vnc: support "-vnc help"

 docs/system/deprecated.rst       |  5 ----
 docs/system/removed-features.rst | 11 +++++++
 hmp-commands.hx                  |  6 ----
 include/ui/console.h             |  2 +-
 monitor/hmp-cmds.c               |  7 +++--
 monitor/qmp-cmds.c               | 51 --------------------------------
 qapi/misc.json                   | 49 ------------------------------
 softmmu/vl.c                     |  6 ++--
 ui/vnc-stubs.c                   |  7 ++---
 ui/vnc.c                         |  8 ++---
 10 files changed, 27 insertions(+), 125 deletions(-)

-- 
2.29.2

On Wed, Jan 20, 2021 at 03:42:32PM +0100, Paolo Bonzini wrote:
> The real driver for these patches is to send all QemuOpts user input
> to qemu_opts_parse_noisily, for consistency in the command line
> parsing code and to effectively outlaw "help" and "?" QemuOpts
> suboptions.  vnc_parse is the only function that is still using
> qemu_opts_parse.

Should we maybe move vnc to qapi cmd line parsing instead?

> In order to remove the non-command-line callers of vnc_parse,
> I am removing the deprecated QMP change command but also its HMP
> veneer "change vnc TARGET", whose usecase is somewhat unclear to
> me.

Hmm.  It's been a few years ...

IIRC back when this was added the main use case was having a way to
enable/disable the vnc server.  Not sure this is still needed/useful.
These days you can effectively disable vnc access by expiring the
password (or not setting one in the first place) without re-configuring
the vnc server.  Also the race where qemu allowed passwordless connects
between start and password being set via monitor is long gone.

So, all in all I feel a bit uncomfortable dropping this without the
usual deprecation period.  No strong objections though.

take care,
  Gerd

On Thu, Jan 21, 2021 at 11:38:31AM +0100, Gerd Hoffmann wrote:
> On Wed, Jan 20, 2021 at 03:42:32PM +0100, Paolo Bonzini wrote:
> > The real driver for these patches is to send all QemuOpts user input
> > to qemu_opts_parse_noisily, for consistency in the command line
> > parsing code and to effectively outlaw "help" and "?" QemuOpts
> > suboptions.  vnc_parse is the only function that is still using
> > qemu_opts_parse.
> 
> Should we maybe move vnc to qapi cmd line parsing instead?
> 
> > In order to remove the non-command-line callers of vnc_parse,
> > I am removing the deprecated QMP change command but also its HMP
> > veneer "change vnc TARGET", whose usecase is somewhat unclear to
> > me.
> 
> Hmm.  It's been a few years ...
> 
> IIRC back when this was added the main use case was having a way to
> enable/disable the vnc server.  Not sure this is still needed/useful.
> These days you can effectively disable vnc access by expiring the
> password (or not setting one in the first place) without re-configuring
> the vnc server.  Also the race where qemu allowed passwordless connects
> between start and password being set via monitor is long gone.

Yep, it was my patch back here:

  https://lists.gnu.org/archive/html/qemu-devel/2007-08/msg00151.html

The original justification or design was not especially compelling
and somewhat hackish in retrospect.

These days we really ought to change VNC so that it integrates with
"-object secret" for getting its password.

Being able to live add/remove display backends is somewhat
interesting, but if we want that it should be done generically
and use qapi modelling, and covering at least SPICE and VNC.

> So, all in all I feel a bit uncomfortable dropping this without the
> usual deprecation period.  No strong objections though.

Well we did deprecate the "change" command in general in 2.5.0.

  https://qemu.readthedocs.io/en/latest/system/deprecated.html#change-since-2-5-0

We gave illustrations for replacement for vnc password and CD
media change, but no replacement was provided for changing
VNC server config.  That's ok though, as there's no requirement
that we provide a replacement when deprecating stuff. It would
have been nice if we explicitly mentioned we were dropping the
vnc target change funcitonality, but we have none the less
followed deprecation process for the 'change' command and so
can remove it now if desired.

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 :|

  Hi,

> > So, all in all I feel a bit uncomfortable dropping this without the
> > usual deprecation period.  No strong objections though.
> 
> Well we did deprecate the "change" command in general in 2.5.0.
> 
>   https://qemu.readthedocs.io/en/latest/system/deprecated.html#change-since-2-5-0
> 
> We gave illustrations for replacement for vnc password and CD
> media change, but no replacement was provided for changing
> VNC server config.  That's ok though, as there's no requirement
> that we provide a replacement when deprecating stuff.

Fair enough.

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

(assuming you want queue that with other qemu-opts changes,
 but I can also take this through ui queue if you want).

take care,
  Gerd

On 21/01/21 12:13, Gerd Hoffmann wrote:
>    Hi,
> 
>>> So, all in all I feel a bit uncomfortable dropping this without the
>>> usual deprecation period.  No strong objections though.
>>
>> Well we did deprecate the "change" command in general in 2.5.0.
>>
>>    https://qemu.readthedocs.io/en/latest/system/deprecated.html#change-since-2-5-0
>>
>> We gave illustrations for replacement for vnc password and CD
>> media change, but no replacement was provided for changing
>> VNC server config.  That's ok though, as there's no requirement
>> that we provide a replacement when deprecating stuff.
> 
> Fair enough.
> 
> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
> 
> (assuming you want queue that with other qemu-opts changes,
>   but I can also take this through ui queue if you want).

I'll queue it myself, thanks!

Paolo