| Message ID | 20210120144235.345983-1-pbonzini@redhat.com |
|---|---|
| Headers | show |
| Series | vnc: remove "change vnc TARGET" and QMP change command, support "-vnc help" | expand |
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
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