[PATCH 0/3] vnc: remove "change vnc TARGET" and QMP change command, support "-vnc help"

[PATCH 0/3] vnc: remove "change vnc TARGET" and QMP change command, support "-vnc help"

[Paolo Bonzini]

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

[PATCH 1/3] hmp: remove "change vnc TARGET" command

[Paolo Bonzini]

The HMP command \"change vnc TARGET\" is messy:

- it takes an ugly shortcut to determine if the option has an "id",
with incorrect results if "id=" is not preceded by an unescaped
comma.

- it deletes the existing QemuOpts and does not try to rollback
if the parsing fails (which is not causing problems, but only due to
how VNC options are parsed)

- because it uses the same parsing function as "-vnc", it forces
the latter to not support "-vnc help".

On top of this, it uses a deprecated QMP command, thus getting in
the way of removing the QMP command.  Since the usecase for the
command is not clear, just remove it and send "change vnc password"
directly to the QMP "change-vnc-password" command.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 docs/system/removed-features.rst | 6 ++++++
 hmp-commands.hx                  | 6 ------
 monitor/hmp-cmds.c               | 7 +++++--
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index 430fc33ca1..5b0ff6ab1f 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -68,6 +68,12 @@ The ``[hub_id name]`` parameter tuple of the 'hostfwd_add' and
 Use ``device_add`` for hotplugging vCPUs instead of ``cpu-add``.  See
 documentation of ``query-hotpluggable-cpus`` for additional details.
 
+``change vnc TARGET`` (removed in 6.0)
+''''''''''''''''''''''''''''''''''''''
+
+No replacement.  The ``change vnc password`` and ``change DEVICE MEDIUM``
+commands are not affected.
+
 Guest Emulator ISAs
 -------------------
 
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 73e0832ea1..d4001f9c5d 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -231,12 +231,6 @@ SRST
     read-write
       Makes the device writable.
 
-  ``change vnc`` *display*,\ *options*
-    Change the configuration of the VNC server. The valid syntax for *display*
-    and *options* are described at :ref:`sec_005finvocation`. eg::
-
-      (qemu) change vnc localhost:1
-
   ``change vnc password`` [*password*]
 
     Change the password associated with the VNC server. If the new password
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index fd4d77e246..499647a578 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1521,13 +1521,16 @@ void hmp_change(Monitor *mon, const QDict *qdict)
         }
         if (strcmp(target, "passwd") == 0 ||
             strcmp(target, "password") == 0) {
-            if (!arg) {
+            if (arg) {
                 MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, common);
                 monitor_read_password(hmp_mon, hmp_change_read_arg, NULL);
                 return;
+            } else {
+                qmp_change_vnc_password(arg, &err);
             }
+        } else {
+            monitor_printf(mon, "Expected 'password' after 'vnc'\n");
         }
-        qmp_change("vnc", target, !!arg, arg, &err);
     } else
 #endif
     {
-- 
2.29.2

Re: [PATCH 1/3] hmp: remove "change vnc TARGET" command

[Eric Blake]

On 1/20/21 8:42 AM, Paolo Bonzini wrote:
> The HMP command \"change vnc TARGET\" is messy:
> 
> - it takes an ugly shortcut to determine if the option has an "id",
> with incorrect results if "id=" is not preceded by an unescaped
> comma.
> 
> - it deletes the existing QemuOpts and does not try to rollback
> if the parsing fails (which is not causing problems, but only due to
> how VNC options are parsed)
> 
> - because it uses the same parsing function as "-vnc", it forces
> the latter to not support "-vnc help".
> 
> On top of this, it uses a deprecated QMP command, thus getting in
> the way of removing the QMP command.  Since the usecase for the
> command is not clear, just remove it and send "change vnc password"
> directly to the QMP "change-vnc-password" command.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  docs/system/removed-features.rst | 6 ++++++
>  hmp-commands.hx                  | 6 ------
>  monitor/hmp-cmds.c               | 7 +++++--
>  3 files changed, 11 insertions(+), 8 deletions(-)

HMP is not promised to be stable, so no deprecation period required.
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

[PATCH 2/3] qmp: remove deprecated "change" command

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 docs/system/deprecated.rst       |  5 ----
 docs/system/removed-features.rst |  5 ++++
 monitor/qmp-cmds.c               | 51 --------------------------------
 qapi/misc.json                   | 49 ------------------------------
 4 files changed, 5 insertions(+), 105 deletions(-)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index e20bfcb17a..651182b2df 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -131,11 +131,6 @@ devices.  It is possible to use drives the board doesn't pick up with
 QEMU Machine Protocol (QMP) commands
 ------------------------------------
 
-``change`` (since 2.5.0)
-''''''''''''''''''''''''
-
-Use ``blockdev-change-medium`` or ``change-vnc-password`` instead.
-
 ``blockdev-open-tray``, ``blockdev-close-tray`` argument ``device`` (since 2.8.0)
 '''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
 
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index 5b0ff6ab1f..c48266724f 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -53,6 +53,11 @@ are automatically loaded from qcow2 images.
 Use ``device_add`` for hotplugging vCPUs instead of ``cpu-add``.  See
 documentation of ``query-hotpluggable-cpus`` for additional details.
 
+``change`` (removed in 6.0)
+'''''''''''''''''''''''''''
+
+Use ``blockdev-change-medium`` or ``change-vnc-password`` instead.
+
 Human Monitor Protocol (HMP) commands
 -------------------------------------
 
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 34f7e75b7b..990936136c 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -251,58 +251,7 @@ void qmp_change_vnc_password(const char *password, Error **errp)
         error_setg(errp, "Could not set password");
     }
 }
-
-static void qmp_change_vnc_listen(const char *target, Error **errp)
-{
-    QemuOptsList *olist = qemu_find_opts("vnc");
-    QemuOpts *opts;
-
-    if (strstr(target, "id=")) {
-        error_setg(errp, "id not supported");
-        return;
-    }
-
-    opts = qemu_opts_find(olist, "default");
-    if (opts) {
-        qemu_opts_del(opts);
-    }
-    opts = vnc_parse(target, errp);
-    if (!opts) {
-        return;
-    }
-
-    vnc_display_open("default", errp);
-}
-
-static void qmp_change_vnc(const char *target, bool has_arg, const char *arg,
-                           Error **errp)
-{
-    if (strcmp(target, "passwd") == 0 || strcmp(target, "password") == 0) {
-        if (!has_arg) {
-            error_setg(errp, QERR_MISSING_PARAMETER, "password");
-        } else {
-            qmp_change_vnc_password(arg, errp);
-        }
-    } else {
-        qmp_change_vnc_listen(target, errp);
-    }
-}
-#endif /* !CONFIG_VNC */
-
-void qmp_change(const char *device, const char *target,
-                bool has_arg, const char *arg, Error **errp)
-{
-    if (strcmp(device, "vnc") == 0) {
-#ifdef CONFIG_VNC
-        qmp_change_vnc(target, has_arg, arg, errp);
-#else
-        error_setg(errp, QERR_FEATURE_DISABLED, "vnc");
 #endif
-    } else {
-        qmp_blockdev_change_medium(true, device, false, NULL, target,
-                                   has_arg, arg, false, 0, errp);
-    }
-}
 
 void qmp_add_client(const char *protocol, const char *fdname,
                     bool has_skipauth, bool skipauth, bool has_tls, bool tls,
diff --git a/qapi/misc.json b/qapi/misc.json
index 27ccd7385f..156f98203e 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -238,55 +238,6 @@
   'returns': 'str',
   'features': [ 'savevm-monitor-nodes' ] }
 
-##
-# @change:
-#
-# This command is multiple commands multiplexed together.
-#
-# @device: This is normally the name of a block device but it may also be 'vnc'.
-#          when it's 'vnc', then sub command depends on @target
-#
-# @target: If @device is a block device, then this is the new filename.
-#          If @device is 'vnc', then if the value 'password' selects the vnc
-#          change password command.   Otherwise, this specifies a new server URI
-#          address to listen to for VNC connections.
-#
-# @arg: If @device is a block device, then this is an optional format to open
-#       the device with.
-#       If @device is 'vnc' and @target is 'password', this is the new VNC
-#       password to set.  See change-vnc-password for additional notes.
-#
-# Features:
-# @deprecated: This command is deprecated.  For changing block
-#              devices, use 'blockdev-change-medium' instead; for changing VNC
-#              parameters, use 'change-vnc-password' instead.
-#
-# Returns: - Nothing on success.
-#          - If @device is not a valid block device, DeviceNotFound
-#
-# Since: 0.14
-#
-# Example:
-#
-# 1. Change a removable medium
-#
-# -> { "execute": "change",
-#      "arguments": { "device": "ide1-cd0",
-#                     "target": "/srv/images/Fedora-12-x86_64-DVD.iso" } }
-# <- { "return": {} }
-#
-# 2. Change VNC password
-#
-# -> { "execute": "change",
-#      "arguments": { "device": "vnc", "target": "password",
-#                     "arg": "foobar1" } }
-# <- { "return": {} }
-#
-##
-{ 'command': 'change',
-  'data': {'device': 'str', 'target': 'str', '*arg': 'str'},
-  'features': [ 'deprecated' ] }
-
 ##
 # @getfd:
 #
-- 
2.29.2

Re: [PATCH 2/3] qmp: remove deprecated "change" command

[Philippe Mathieu-Daudé]

On 1/20/21 3:42 PM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  docs/system/deprecated.rst       |  5 ----
>  docs/system/removed-features.rst |  5 ++++
>  monitor/qmp-cmds.c               | 51 --------------------------------
>  qapi/misc.json                   | 49 ------------------------------
>  4 files changed, 5 insertions(+), 105 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH 2/3] qmp: remove deprecated "change" command

[Eric Blake]

On 1/20/21 8:42 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  docs/system/deprecated.rst       |  5 ----
>  docs/system/removed-features.rst |  5 ++++
>  monitor/qmp-cmds.c               | 51 --------------------------------
>  qapi/misc.json                   | 49 ------------------------------
>  4 files changed, 5 insertions(+), 105 deletions(-)

>  
> -``change`` (since 2.5.0)
> -''''''''''''''''''''''''
> -
> -Use ``blockdev-change-medium`` or ``change-vnc-password`` instead.

We didn't document a fallback for changing the vnc listener...


> +++ b/docs/system/removed-features.rst
> @@ -53,6 +53,11 @@ are automatically loaded from qcow2 images.
>  Use ``device_add`` for hotplugging vCPUs instead of ``cpu-add``.  See
>  documentation of ``query-hotpluggable-cpus`` for additional details.
>  
> +``change`` (removed in 6.0)
> +'''''''''''''''''''''''''''
> +
> +Use ``blockdev-change-medium`` or ``change-vnc-password`` instead.

...and still don't,...


> -static void qmp_change_vnc(const char *target, bool has_arg, const char *arg,
> -                           Error **errp)
> -{
> -    if (strcmp(target, "passwd") == 0 || strcmp(target, "password") == 0) {
> -        if (!has_arg) {
> -            error_setg(errp, QERR_MISSING_PARAMETER, "password");
> -        } else {
> -            qmp_change_vnc_password(arg, errp);
> -        }
> -    } else {
> -        qmp_change_vnc_listen(target, errp);

...and now we're entirely removing that QMP ability, without
deprecation.  Is that intended?

> +++ b/qapi/misc.json
> @@ -238,55 +238,6 @@
>    'returns': 'str',
>    'features': [ 'savevm-monitor-nodes' ] }
>  
> -##
> -# @change:
> -#
> -# This command is multiple commands multiplexed together.
> -#
> -# @device: This is normally the name of a block device but it may also be 'vnc'.
> -#          when it's 'vnc', then sub command depends on @target
> -#
> -# @target: If @device is a block device, then this is the new filename.
> -#          If @device is 'vnc', then if the value 'password' selects the vnc
> -#          change password command.   Otherwise, this specifies a new server URI
> -#          address to listen to for VNC connections.

That is, I'm wondering if we should be adding a change-vnc-listener QMP
command, and extending the deprecation for 2 more releases to allow this
final use of 'change vnc' to still work.  Or are we really at the point
where we have no known users?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

[PATCH 3/3] vnc: support "-vnc help"

[Paolo Bonzini]

Use qemu_opts_parse_noisily now that HMP does not call
vnc_parse anymore.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/ui/console.h | 2 +-
 softmmu/vl.c         | 6 +++---
 ui/vnc-stubs.c       | 7 +++----
 ui/vnc.c             | 8 ++++----
 4 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index 5dd21976a3..7a3fc11abf 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -439,7 +439,7 @@ void vnc_display_open(const char *id, Error **errp);
 void vnc_display_add_client(const char *id, int csock, bool skipauth);
 int vnc_display_password(const char *id, const char *password);
 int vnc_display_pw_expire(const char *id, time_t expires);
-QemuOpts *vnc_parse(const char *str, Error **errp);
+void vnc_parse(const char *str);
 int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp);
 
 /* input.c */
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 59304261cf..a8876b8965 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1113,7 +1113,7 @@ static void parse_display(const char *p)
          * display access.
          */
         if (*opts == '=') {
-            vnc_parse(opts + 1, &error_fatal);
+            vnc_parse(opts + 1);
         } else {
             error_report("VNC requires a display argument vnc=<display>");
             exit(1);
@@ -1402,7 +1402,7 @@ static void qemu_create_default_devices(void)
         if (!qemu_display_find_default(&dpy)) {
             dpy.type = DISPLAY_TYPE_NONE;
 #if defined(CONFIG_VNC)
-            vnc_parse("localhost:0,to=99,id=default", &error_abort);
+            vnc_parse("localhost:0,to=99,id=default");
 #endif
         }
     }
@@ -3186,7 +3186,7 @@ void qemu_init(int argc, char **argv, char **envp)
                 }
                 break;
             case QEMU_OPTION_vnc:
-                vnc_parse(optarg, &error_fatal);
+                vnc_parse(optarg);
                 break;
             case QEMU_OPTION_no_acpi:
                 olist = qemu_find_opts("machine");
diff --git a/ui/vnc-stubs.c b/ui/vnc-stubs.c
index c6b737dcec..0fd4e0846a 100644
--- a/ui/vnc-stubs.c
+++ b/ui/vnc-stubs.c
@@ -10,13 +10,12 @@ int vnc_display_pw_expire(const char *id, time_t expires)
 {
     return -ENODEV;
 };
-QemuOpts *vnc_parse(const char *str, Error **errp)
+void vnc_parse(const char *str)
 {
     if (strcmp(str, "none") == 0) {
-        return NULL;
+        return;
     }
-    error_setg(errp, "VNC support is disabled");
-    return NULL;
+    error_setg(&error_fatal, "VNC support is disabled");
 }
 int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp)
 {
diff --git a/ui/vnc.c b/ui/vnc.c
index d429bfee5a..66f7c1b936 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -50,6 +50,7 @@
 #include "crypto/random.h"
 #include "qom/object_interfaces.h"
 #include "qemu/cutils.h"
+#include "qemu/help_option.h"
 #include "io/dns-resolver.h"
 
 #define VNC_REFRESH_INTERVAL_BASE GUI_REFRESH_INTERVAL_DEFAULT
@@ -4211,14 +4212,14 @@ static void vnc_auto_assign_id(QemuOptsList *olist, QemuOpts *opts)
     qemu_opts_set_id(opts, id);
 }
 
-QemuOpts *vnc_parse(const char *str, Error **errp)
+void vnc_parse(const char *str)
 {
     QemuOptsList *olist = qemu_find_opts("vnc");
-    QemuOpts *opts = qemu_opts_parse(olist, str, true, errp);
+    QemuOpts *opts = qemu_opts_parse_noisily(olist, str, !is_help_option(str));
     const char *id;
 
     if (!opts) {
-        return NULL;
+        exit(1);
     }
 
     id = qemu_opts_id(opts);
@@ -4226,7 +4227,6 @@ QemuOpts *vnc_parse(const char *str, Error **errp)
         /* auto-assign id if not present */
         vnc_auto_assign_id(olist, opts);
     }
-    return opts;
 }
 
 int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp)
-- 
2.29.2

Re: [PATCH 3/3] vnc: support "-vnc help"

[Eric Blake]

On 1/20/21 8:42 AM, Paolo Bonzini wrote:
> Use qemu_opts_parse_noisily now that HMP does not call
> vnc_parse anymore.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/ui/console.h | 2 +-
>  softmmu/vl.c         | 6 +++---
>  ui/vnc-stubs.c       | 7 +++----
>  ui/vnc.c             | 8 ++++----
>  4 files changed, 11 insertions(+), 12 deletions(-)

Unless something about my comments on 2/3 causes us to change,

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 0/3] vnc: remove "change vnc TARGET" and QMP change command, support "-vnc help"

[Gerd Hoffmann]

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

Re: [PATCH 0/3] vnc: remove "change vnc TARGET" and QMP change command, support "-vnc help"

[Daniel P. Berrangé]

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

Re: [PATCH 0/3] vnc: remove "change vnc TARGET" and QMP change command, support "-vnc help"

[Gerd Hoffmann]

  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

Re: [PATCH 0/3] vnc: remove "change vnc TARGET" and QMP change command, support "-vnc help"

[Paolo Bonzini]

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