[pve-devel] [PATCH qemu] extra: update VNC 'display' patches to latest from list

Stefan Reiter s.reiter at proxmox.com
Thu Oct 28 10:56:52 CEST 2021


Functionally the same, but now with less crashbugs and memory leaks.
Also includes git tags from upstream, now that it has officially been
included in a PR.

Cleanly rebased from upstream master to v6.1.0, only change is
- "if: CONFIG_VNC"
+ "if: defined(CONFIG_VNC)"
(same for CONFIG_SPICE)
in the QAPI JSON as that semantic was renamed in the meantime.

Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
---

See: https://lists.nongnu.org/archive/html/qemu-devel/2021-10/msg06676.html

 ...atch => 0003-qxl-fix-pre-save-logic.patch} |   0
 ...upport-for-flag-argument-with-value.patch} |  41 +-
 ...ectly-invert-password-argument-detec.patch |  36 --
 ...actor-set-expire_password-with-enums.patch | 218 +++++++++++
 ...w-VNC-display-id-in-set-expire_pass.patch} | 362 +++++++-----------
 ...y-allow-keep-SetPasswordAction-for-V.patch | 101 +++++
 debian/patches/series                         |   9 +-
 7 files changed, 503 insertions(+), 264 deletions(-)
 rename debian/patches/extra/{0006-qxl-fix-pre-save-logic.patch => 0003-qxl-fix-pre-save-logic.patch} (100%)
 rename debian/patches/extra/{0003-monitor-hmp-add-support-for-flag-argument-with-value.patch => 0004-monitor-hmp-add-support-for-flag-argument-with-value.patch} (55%)
 delete mode 100644 debian/patches/extra/0005-monitor-hmp-correctly-invert-password-argument-detec.patch
 create mode 100644 debian/patches/extra/0005-qapi-monitor-refactor-set-expire_password-with-enums.patch
 rename debian/patches/extra/{0004-monitor-refactor-set-expire_password-and-allow-VNC-d.patch => 0006-qapi-monitor-allow-VNC-display-id-in-set-expire_pass.patch} (57%)
 create mode 100644 debian/patches/extra/0007-qapi-monitor-only-allow-keep-SetPasswordAction-for-V.patch

diff --git a/debian/patches/extra/0006-qxl-fix-pre-save-logic.patch b/debian/patches/extra/0003-qxl-fix-pre-save-logic.patch
similarity index 100%
rename from debian/patches/extra/0006-qxl-fix-pre-save-logic.patch
rename to debian/patches/extra/0003-qxl-fix-pre-save-logic.patch
diff --git a/debian/patches/extra/0003-monitor-hmp-add-support-for-flag-argument-with-value.patch b/debian/patches/extra/0004-monitor-hmp-add-support-for-flag-argument-with-value.patch
similarity index 55%
rename from debian/patches/extra/0003-monitor-hmp-add-support-for-flag-argument-with-value.patch
rename to debian/patches/extra/0004-monitor-hmp-add-support-for-flag-argument-with-value.patch
index ca1fbaf..d2e9c0b 100644
--- a/debian/patches/extra/0003-monitor-hmp-add-support-for-flag-argument-with-value.patch
+++ b/debian/patches/extra/0004-monitor-hmp-add-support-for-flag-argument-with-value.patch
@@ -1,23 +1,27 @@
 From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
 From: Stefan Reiter <s.reiter at proxmox.com>
-Date: Wed, 1 Sep 2021 16:51:04 +0200
+Date: Thu, 28 Oct 2021 07:29:52 +0200
 Subject: [PATCH] monitor/hmp: add support for flag argument with value
 
-Adds support for the "-xS" parameter type, where "-x" denotes a flag
-name and the "S" suffix indicates that this flag is supposed to take an
+Adds support for the "-xV" parameter type, where "-x" denotes a flag
+name and the "V" suffix indicates that this flag is supposed to take an
 arbitrary string parameter.
 
 These parameters are always optional, the entry in the qdict will be
 omitted if the flag is not given.
 
-Reviewed-by: Eric Blake <eblake at redhat.com>
 Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
+Message-Id: <20211021100135.4146766-2-s.reiter at proxmox.com>
+Reviewed-by: Dr. David Alan Gilbert <dgilbert at redhat.com>
+Acked-by: Gerd Hoffmann <kraxel at redhat.com>
+Signed-off-by: Markus Armbruster <armbru at redhat.com>
 ---
- monitor/hmp.c | 17 ++++++++++++++++-
- 1 file changed, 16 insertions(+), 1 deletion(-)
+ monitor/hmp.c              | 19 ++++++++++++++++++-
+ monitor/monitor-internal.h |  3 ++-
+ 2 files changed, 20 insertions(+), 2 deletions(-)
 
 diff --git a/monitor/hmp.c b/monitor/hmp.c
-index d50c3124e1..a32dce7a35 100644
+index d50c3124e1..899e0c990f 100644
 --- a/monitor/hmp.c
 +++ b/monitor/hmp.c
 @@ -980,6 +980,7 @@ static QDict *monitor_parse_arguments(Monitor *mon,
@@ -28,11 +32,11 @@ index d50c3124e1..a32dce7a35 100644
                  /* option */
  
                  c = *typestr++;
-@@ -1002,8 +1003,22 @@ static QDict *monitor_parse_arguments(Monitor *mon,
+@@ -1002,11 +1003,27 @@ static QDict *monitor_parse_arguments(Monitor *mon,
                      }
                      if (skip_key) {
                          p = tmp;
-+                    } else if (*typestr == 'S') {
++                    } else if (*typestr == 'V') {
 +                        /* has option with string value */
 +                        typestr++;
 +                        tmp = p++;
@@ -52,3 +56,22 @@ index d50c3124e1..a32dce7a35 100644
                          p++;
                          qdict_put_bool(qdict, key, true);
                      }
++                } else if (*typestr == 'V') {
++                    typestr++;
+                 }
+             }
+             break;
+diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
+index 9c3a09cb01..9e708b329d 100644
+--- a/monitor/monitor-internal.h
++++ b/monitor/monitor-internal.h
+@@ -63,7 +63,8 @@
+  * '.'          other form of optional type (for 'i' and 'l')
+  * 'b'          boolean
+  *              user mode accepts "on" or "off"
+- * '-'          optional parameter (eg. '-f')
++ * '-'          optional parameter (eg. '-f'); if followed by an 'V', it
++ *              specifies an optional string param (e.g. '-fV' allows '-f foo')
+  *
+  */
+ 
diff --git a/debian/patches/extra/0005-monitor-hmp-correctly-invert-password-argument-detec.patch b/debian/patches/extra/0005-monitor-hmp-correctly-invert-password-argument-detec.patch
deleted file mode 100644
index 3d79a35..0000000
--- a/debian/patches/extra/0005-monitor-hmp-correctly-invert-password-argument-detec.patch
+++ /dev/null
@@ -1,36 +0,0 @@
-From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
-From: Stefan Reiter <s.reiter at proxmox.com>
-Date: Wed, 25 Aug 2021 11:08:41 +0200
-Subject: [PATCH] monitor/hmp: correctly invert password argument detection
- again
-MIME-Version: 1.0
-Content-Type: text/plain; charset=UTF-8
-Content-Transfer-Encoding: 8bit
-
-Commit cfb5387a1d 'hmp: remove "change vnc TARGET" command' claims to
-remove the HMP "change vnc" command, but doesn't actually do that.
-Instead it rewires it to use 'qmp_change_vnc_password', and in the
-process inverts the argument detection - ignoring the first issue, this
-inversion is wrong, as this will now ask the user for a password if one
-is already provided, and simply fail if none is given.
-
-Fixes: cfb5387a1d ("hmp: remove "change vnc TARGET" command")
-Reviewed-by: Marc-André Lureau <marcandre.lureau at redhat.com>
-Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
----
- monitor/hmp-cmds.c | 2 +-
- 1 file changed, 1 insertion(+), 1 deletion(-)
-
-diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
-index 047294e1ed..f4ef58d257 100644
---- a/monitor/hmp-cmds.c
-+++ b/monitor/hmp-cmds.c
-@@ -1549,7 +1549,7 @@ 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;
diff --git a/debian/patches/extra/0005-qapi-monitor-refactor-set-expire_password-with-enums.patch b/debian/patches/extra/0005-qapi-monitor-refactor-set-expire_password-with-enums.patch
new file mode 100644
index 0000000..91a0d67
--- /dev/null
+++ b/debian/patches/extra/0005-qapi-monitor-refactor-set-expire_password-with-enums.patch
@@ -0,0 +1,218 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Stefan Reiter <s.reiter at proxmox.com>
+Date: Thu, 28 Oct 2021 07:29:53 +0200
+Subject: [PATCH] qapi/monitor: refactor set/expire_password with enums
+
+'protocol' and 'connected' are better suited as enums than as strings,
+make use of that. No functional change intended.
+
+Suggested-by: Markus Armbruster <armbru at redhat.com>
+Reviewed-by: Markus Armbruster <armbru at redhat.com>
+Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
+Message-Id: <20211021100135.4146766-3-s.reiter at proxmox.com>
+Acked-by: Gerd Hoffmann <kraxel at redhat.com>
+Signed-off-by: Markus Armbruster <armbru at redhat.com>
+---
+ monitor/hmp-cmds.c | 29 +++++++++++++++++++++++++++--
+ monitor/qmp-cmds.c | 37 ++++++++++++-------------------------
+ qapi/ui.json       | 37 +++++++++++++++++++++++++++++++++++--
+ 3 files changed, 74 insertions(+), 29 deletions(-)
+
+diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
+index e00255f7ee..716f125393 100644
+--- a/monitor/hmp-cmds.c
++++ b/monitor/hmp-cmds.c
+@@ -1453,8 +1453,24 @@ void hmp_set_password(Monitor *mon, const QDict *qdict)
+     const char *password  = qdict_get_str(qdict, "password");
+     const char *connected = qdict_get_try_str(qdict, "connected");
+     Error *err = NULL;
++    DisplayProtocol proto;
++    SetPasswordAction conn;
+ 
+-    qmp_set_password(protocol, password, !!connected, connected, &err);
++    proto = qapi_enum_parse(&DisplayProtocol_lookup, protocol,
++                            DISPLAY_PROTOCOL_VNC, &err);
++    if (err) {
++        goto out;
++    }
++
++    conn = qapi_enum_parse(&SetPasswordAction_lookup, connected,
++                           SET_PASSWORD_ACTION_KEEP, &err);
++    if (err) {
++        goto out;
++    }
++
++    qmp_set_password(proto, password, !!connected, conn, &err);
++
++out:
+     hmp_handle_error(mon, err);
+ }
+ 
+@@ -1463,8 +1479,17 @@ void hmp_expire_password(Monitor *mon, const QDict *qdict)
+     const char *protocol  = qdict_get_str(qdict, "protocol");
+     const char *whenstr = qdict_get_str(qdict, "time");
+     Error *err = NULL;
++    DisplayProtocol proto;
++
++    proto = qapi_enum_parse(&DisplayProtocol_lookup, protocol,
++                            DISPLAY_PROTOCOL_VNC, &err);
++    if (err) {
++        goto out;
++    }
+ 
+-    qmp_expire_password(protocol, whenstr, &err);
++    qmp_expire_password(proto, whenstr, &err);
++
++out:
+     hmp_handle_error(mon, err);
+ }
+ 
+diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
+index f7d64a6457..9d9085c909 100644
+--- a/monitor/qmp-cmds.c
++++ b/monitor/qmp-cmds.c
+@@ -164,33 +164,27 @@ void qmp_system_wakeup(Error **errp)
+     qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER, errp);
+ }
+ 
+-void qmp_set_password(const char *protocol, const char *password,
+-                      bool has_connected, const char *connected, Error **errp)
++void qmp_set_password(DisplayProtocol protocol, const char *password,
++                      bool has_connected, SetPasswordAction connected,
++                      Error **errp)
+ {
+     int disconnect_if_connected = 0;
+     int fail_if_connected = 0;
+     int rc;
+ 
+     if (has_connected) {
+-        if (strcmp(connected, "fail") == 0) {
+-            fail_if_connected = 1;
+-        } else if (strcmp(connected, "disconnect") == 0) {
+-            disconnect_if_connected = 1;
+-        } else if (strcmp(connected, "keep") == 0) {
+-            /* nothing */
+-        } else {
+-            error_setg(errp, QERR_INVALID_PARAMETER, "connected");
+-            return;
+-        }
++        fail_if_connected = connected == SET_PASSWORD_ACTION_FAIL;
++        disconnect_if_connected = connected == SET_PASSWORD_ACTION_DISCONNECT;
+     }
+ 
+-    if (strcmp(protocol, "spice") == 0) {
++    if (protocol == DISPLAY_PROTOCOL_SPICE) {
+         if (!qemu_using_spice(errp)) {
+             return;
+         }
+         rc = qemu_spice.set_passwd(password, fail_if_connected,
+                                    disconnect_if_connected);
+-    } else if (strcmp(protocol, "vnc") == 0) {
++    } else {
++        assert(protocol == DISPLAY_PROTOCOL_VNC);
+         if (fail_if_connected || disconnect_if_connected) {
+             /* vnc supports "connected=keep" only */
+             error_setg(errp, QERR_INVALID_PARAMETER, "connected");
+@@ -199,10 +193,6 @@ void qmp_set_password(const char *protocol, const char *password,
+         /* Note that setting an empty password will not disable login through
+          * this interface. */
+         rc = vnc_display_password(NULL, password);
+-    } else {
+-        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "protocol",
+-                   "'vnc' or 'spice'");
+-        return;
+     }
+ 
+     if (rc != 0) {
+@@ -210,7 +200,7 @@ void qmp_set_password(const char *protocol, const char *password,
+     }
+ }
+ 
+-void qmp_expire_password(const char *protocol, const char *whenstr,
++void qmp_expire_password(DisplayProtocol protocol, const char *whenstr,
+                          Error **errp)
+ {
+     time_t when;
+@@ -226,17 +216,14 @@ void qmp_expire_password(const char *protocol, const char *whenstr,
+         when = strtoull(whenstr, NULL, 10);
+     }
+ 
+-    if (strcmp(protocol, "spice") == 0) {
++    if (protocol == DISPLAY_PROTOCOL_SPICE) {
+         if (!qemu_using_spice(errp)) {
+             return;
+         }
+         rc = qemu_spice.set_pw_expire(when);
+-    } else if (strcmp(protocol, "vnc") == 0) {
+-        rc = vnc_display_pw_expire(NULL, when);
+     } else {
+-        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "protocol",
+-                   "'vnc' or 'spice'");
+-        return;
++        assert(protocol == DISPLAY_PROTOCOL_VNC);
++        rc = vnc_display_pw_expire(NULL, when);
+     }
+ 
+     if (rc != 0) {
+diff --git a/qapi/ui.json b/qapi/ui.json
+index fd9677d48e..7e7be154c5 100644
+--- a/qapi/ui.json
++++ b/qapi/ui.json
+@@ -9,6 +9,35 @@
+ { 'include': 'common.json' }
+ { 'include': 'sockets.json' }
+ 
++##
++# @DisplayProtocol:
++#
++# Display protocols which support changing password options.
++#
++# Since: 6.2
++#
++##
++{ 'enum': 'DisplayProtocol',
++  'data': [ { 'name': 'vnc', 'if': 'defined(CONFIG_VNC)' },
++            { 'name': 'spice', 'if': 'defined(CONFIG_SPICE)' } ] }
++
++##
++# @SetPasswordAction:
++#
++# An action to take on changing a password on a connection with active clients.
++#
++# @fail: fail the command if clients are connected
++#
++# @disconnect: disconnect existing clients
++#
++# @keep: maintain existing clients
++#
++# Since: 6.2
++#
++##
++{ 'enum': 'SetPasswordAction',
++  'data': [ 'fail', 'disconnect', 'keep' ] }
++
+ ##
+ # @set_password:
+ #
+@@ -38,7 +67,9 @@
+ #
+ ##
+ { 'command': 'set_password',
+-  'data': {'protocol': 'str', 'password': 'str', '*connected': 'str'} }
++  'data': { 'protocol': 'DisplayProtocol',
++            'password': 'str',
++            '*connected': 'SetPasswordAction' } }
+ 
+ ##
+ # @expire_password:
+@@ -71,7 +102,9 @@
+ # <- { "return": {} }
+ #
+ ##
+-{ 'command': 'expire_password', 'data': {'protocol': 'str', 'time': 'str'} }
++{ 'command': 'expire_password',
++  'data': { 'protocol': 'DisplayProtocol',
++            'time': 'str' } }
+ 
+ ##
+ # @screendump:
diff --git a/debian/patches/extra/0004-monitor-refactor-set-expire_password-and-allow-VNC-d.patch b/debian/patches/extra/0006-qapi-monitor-allow-VNC-display-id-in-set-expire_pass.patch
similarity index 57%
rename from debian/patches/extra/0004-monitor-refactor-set-expire_password-and-allow-VNC-d.patch
rename to debian/patches/extra/0006-qapi-monitor-allow-VNC-display-id-in-set-expire_pass.patch
index 8d8b20a..f412246 100644
--- a/debian/patches/extra/0004-monitor-refactor-set-expire_password-and-allow-VNC-d.patch
+++ b/debian/patches/extra/0006-qapi-monitor-allow-VNC-display-id-in-set-expire_pass.patch
@@ -1,8 +1,7 @@
 From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
 From: Stefan Reiter <s.reiter at proxmox.com>
-Date: Wed, 25 Aug 2021 11:14:13 +0200
-Subject: [PATCH] monitor: refactor set/expire_password and allow VNC display
- id
+Date: Thu, 28 Oct 2021 07:29:54 +0200
+Subject: [PATCH] qapi/monitor: allow VNC display id in set/expire_password
 MIME-Version: 1.0
 Content-Type: text/plain; charset=UTF-8
 Content-Transfer-Encoding: 8bit
@@ -20,18 +19,21 @@ For HMP, the display is specified using the "-d" value flag.
 For QMP, the schema is updated to explicitly express the supported
 variants of the commands with protocol-discriminated unions.
 
-Suggested-by: Eric Blake <eblake at redhat.com>
 Suggested-by: Markus Armbruster <armbru at redhat.com>
 Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
+Message-Id: <20211021100135.4146766-4-s.reiter at proxmox.com>
+Reviewed-by: Markus Armbruster <armbru at redhat.com>
+Acked-by: Gerd Hoffmann <kraxel at redhat.com>
+Signed-off-by: Markus Armbruster <armbru at redhat.com>
 ---
- hmp-commands.hx    |  29 ++++----
- monitor/hmp-cmds.c |  57 +++++++++++++++-
- monitor/qmp-cmds.c |  62 ++++++-----------
- qapi/ui.json       | 163 ++++++++++++++++++++++++++++++++++++++-------
- 4 files changed, 232 insertions(+), 79 deletions(-)
+ hmp-commands.hx    |  29 ++++++------
+ monitor/hmp-cmds.c |  45 ++++++++++++------
+ monitor/qmp-cmds.c |  36 ++++++---------
+ qapi/ui.json       | 112 +++++++++++++++++++++++++++++++++++----------
+ 4 files changed, 150 insertions(+), 72 deletions(-)
 
 diff --git a/hmp-commands.hx b/hmp-commands.hx
-index 8e45bce2cd..d78e4cfc47 100644
+index 8e45bce2cd..9fbb207b35 100644
 --- a/hmp-commands.hx
 +++ b/hmp-commands.hx
 @@ -1514,34 +1514,35 @@ ERST
@@ -40,7 +42,7 @@ index 8e45bce2cd..d78e4cfc47 100644
          .name       = "set_password",
 -        .args_type  = "protocol:s,password:s,connected:s?",
 -        .params     = "protocol password action-if-connected",
-+        .args_type  = "protocol:s,password:s,display:-dS,connected:s?",
++        .args_type  = "protocol:s,password:s,display:-dV,connected:s?",
 +        .params     = "protocol password [-d display] [action-if-connected]",
          .help       = "set spice/vnc password",
          .cmd        = hmp_set_password,
@@ -67,7 +69,7 @@ index 8e45bce2cd..d78e4cfc47 100644
          .name       = "expire_password",
 -        .args_type  = "protocol:s,time:s",
 -        .params     = "protocol time",
-+        .args_type  = "protocol:s,time:s,display:-dS",
++        .args_type  = "protocol:s,time:s,display:-dV",
 +        .params     = "protocol time [-d display]",
          .help       = "set spice/vnc password expire-time",
          .cmd        = hmp_expire_password,
@@ -85,95 +87,94 @@ index 8e45bce2cd..d78e4cfc47 100644
    ``now``
      Invalidate password instantly.
 diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
-index e00255f7ee..047294e1ed 100644
+index 716f125393..c164321ed6 100644
 --- a/monitor/hmp-cmds.c
 +++ b/monitor/hmp-cmds.c
-@@ -1451,10 +1451,41 @@ void hmp_set_password(Monitor *mon, const QDict *qdict)
+@@ -1451,24 +1451,34 @@ void hmp_set_password(Monitor *mon, const QDict *qdict)
  {
      const char *protocol  = qdict_get_str(qdict, "protocol");
      const char *password  = qdict_get_str(qdict, "password");
 +    const char *display = qdict_get_try_str(qdict, "display");
      const char *connected = qdict_get_try_str(qdict, "connected");
      Error *err = NULL;
-+    DisplayProtocol proto;
+-    DisplayProtocol proto;
+-    SetPasswordAction conn;
  
--    qmp_set_password(protocol, password, !!connected, connected, &err);
+-    proto = qapi_enum_parse(&DisplayProtocol_lookup, protocol,
+-                            DISPLAY_PROTOCOL_VNC, &err);
 +    SetPasswordOptions opts = {
-+        .password = g_strdup(password),
-+        .u.vnc.display = NULL,
++        .password = (char *)password,
 +    };
 +
-+    proto = qapi_enum_parse(&DisplayProtocol_lookup, protocol,
-+                            DISPLAY_PROTOCOL_VNC, &err);
-+    if (err) {
-+        hmp_handle_error(mon, err);
-+        return;
-+    }
-+    opts.protocol = proto;
-+
-+    if (proto == DISPLAY_PROTOCOL_VNC) {
++    opts.protocol = qapi_enum_parse(&DisplayProtocol_lookup, protocol,
++                                    DISPLAY_PROTOCOL_VNC, &err);
+     if (err) {
+         goto out;
+     }
+ 
+-    conn = qapi_enum_parse(&SetPasswordAction_lookup, connected,
+-                           SET_PASSWORD_ACTION_KEEP, &err);
+-    if (err) {
+-        goto out;
++    if (opts.protocol == DISPLAY_PROTOCOL_VNC) {
 +        opts.u.vnc.has_display = !!display;
-+        opts.u.vnc.display = g_strdup(display);
-+    } else if (proto == DISPLAY_PROTOCOL_SPICE) {
++        opts.u.vnc.display = (char *)display;
++    } else if (opts.protocol == DISPLAY_PROTOCOL_SPICE) {
 +        opts.u.spice.has_connected = !!connected;
 +        opts.u.spice.connected =
 +            qapi_enum_parse(&SetPasswordAction_lookup, connected,
 +                            SET_PASSWORD_ACTION_KEEP, &err);
 +        if (err) {
-+            hmp_handle_error(mon, err);
-+            return;
++            goto out;
 +        }
-+    }
-+
-+    qmp_set_password(&opts, &err);
-+    g_free(opts.password);
-+    g_free(opts.u.vnc.display);
-     hmp_handle_error(mon, err);
- }
+     }
  
-@@ -1462,9 +1493,31 @@ void hmp_expire_password(Monitor *mon, const QDict *qdict)
+-    qmp_set_password(proto, password, !!connected, conn, &err);
++    qmp_set_password(&opts, &err);
+ 
+ out:
+     hmp_handle_error(mon, err);
+@@ -1478,16 +1488,25 @@ void hmp_expire_password(Monitor *mon, const QDict *qdict)
  {
      const char *protocol  = qdict_get_str(qdict, "protocol");
      const char *whenstr = qdict_get_str(qdict, "time");
 +    const char *display = qdict_get_try_str(qdict, "display");
      Error *err = NULL;
-+    DisplayProtocol proto;
-+
+-    DisplayProtocol proto;
+ 
+-    proto = qapi_enum_parse(&DisplayProtocol_lookup, protocol,
+-                            DISPLAY_PROTOCOL_VNC, &err);
 +    ExpirePasswordOptions opts = {
-+        .time = g_strdup(whenstr),
-+        .u.vnc.display = NULL,
++        .time = (char *)whenstr,
 +    };
 +
-+    proto = qapi_enum_parse(&DisplayProtocol_lookup, protocol,
-+                            DISPLAY_PROTOCOL_VNC, &err);
-+    if (err) {
-+        hmp_handle_error(mon, err);
-+        return;
-+    }
-+    opts.protocol = proto;
-+
-+    if (proto == DISPLAY_PROTOCOL_VNC) {
++    opts.protocol = qapi_enum_parse(&DisplayProtocol_lookup, protocol,
++                                    DISPLAY_PROTOCOL_VNC, &err);
+     if (err) {
+         goto out;
+     }
+ 
+-    qmp_expire_password(proto, whenstr, &err);
++    if (opts.protocol == DISPLAY_PROTOCOL_VNC) {
 +        opts.u.vnc.has_display = !!display;
-+        opts.u.vnc.display = g_strdup(display);
++        opts.u.vnc.display = (char *)display;
 +    }
- 
--    qmp_expire_password(protocol, whenstr, &err);
++
 +    qmp_expire_password(&opts, &err);
-+    g_free(opts.time);
-+    g_free(opts.u.vnc.display);
-     hmp_handle_error(mon, err);
- }
  
+ out:
+     hmp_handle_error(mon, err);
 diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
-index f7d64a6457..65882b5997 100644
+index 9d9085c909..6802b45ead 100644
 --- a/monitor/qmp-cmds.c
 +++ b/monitor/qmp-cmds.c
-@@ -164,45 +164,30 @@ void qmp_system_wakeup(Error **errp)
+@@ -164,35 +164,27 @@ void qmp_system_wakeup(Error **errp)
      qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER, errp);
  }
  
--void qmp_set_password(const char *protocol, const char *password,
--                      bool has_connected, const char *connected, Error **errp)
+-void qmp_set_password(DisplayProtocol protocol, const char *password,
+-                      bool has_connected, SetPasswordAction connected,
+-                      Error **errp)
 +void qmp_set_password(SetPasswordOptions *opts, Error **errp)
  {
 -    int disconnect_if_connected = 0;
@@ -181,145 +182,93 @@ index f7d64a6457..65882b5997 100644
 -    int rc;
 -
 -    if (has_connected) {
--        if (strcmp(connected, "fail") == 0) {
--            fail_if_connected = 1;
--        } else if (strcmp(connected, "disconnect") == 0) {
--            disconnect_if_connected = 1;
--        } else if (strcmp(connected, "keep") == 0) {
--            /* nothing */
--        } else {
--            error_setg(errp, QERR_INVALID_PARAMETER, "connected");
--            return;
--        }
+-        fail_if_connected = connected == SET_PASSWORD_ACTION_FAIL;
+-        disconnect_if_connected = connected == SET_PASSWORD_ACTION_DISCONNECT;
 -    }
-+    bool disconnect_if_connected = false;
-+    bool fail_if_connected = false;
 +    int rc = 0;
  
--    if (strcmp(protocol, "spice") == 0) {
+-    if (protocol == DISPLAY_PROTOCOL_SPICE) {
 +    if (opts->protocol == DISPLAY_PROTOCOL_SPICE) {
          if (!qemu_using_spice(errp)) {
              return;
          }
 -        rc = qemu_spice.set_passwd(password, fail_if_connected,
 -                                   disconnect_if_connected);
--    } else if (strcmp(protocol, "vnc") == 0) {
++        rc = qemu_spice.set_passwd(opts->password,
++                opts->u.spice.connected == SET_PASSWORD_ACTION_FAIL,
++                opts->u.spice.connected == SET_PASSWORD_ACTION_DISCONNECT);
+     } else {
+-        assert(protocol == DISPLAY_PROTOCOL_VNC);
 -        if (fail_if_connected || disconnect_if_connected) {
--            /* vnc supports "connected=keep" only */
--            error_setg(errp, QERR_INVALID_PARAMETER, "connected");
--            return;
-+        if (opts->u.spice.has_connected) {
-+            fail_if_connected =
-+                opts->u.spice.connected == SET_PASSWORD_ACTION_FAIL;
-+            disconnect_if_connected =
-+                opts->u.spice.connected == SET_PASSWORD_ACTION_DISCONNECT;
++        assert(opts->protocol == DISPLAY_PROTOCOL_VNC);
++        if (opts->u.vnc.connected != SET_PASSWORD_ACTION_KEEP) {
+             /* vnc supports "connected=keep" only */
+             error_setg(errp, QERR_INVALID_PARAMETER, "connected");
+             return;
          }
-+        rc = qemu_spice.set_passwd(opts->password, fail_if_connected,
-+                                   disconnect_if_connected);
-+    } else if (opts->protocol == DISPLAY_PROTOCOL_VNC) {
          /* Note that setting an empty password will not disable login through
           * this interface. */
 -        rc = vnc_display_password(NULL, password);
--    } else {
--        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "protocol",
--                   "'vnc' or 'spice'");
--        return;
-+        rc = vnc_display_password(
-+                opts->u.vnc.has_display ? opts->u.vnc.display : NULL,
-+                opts->password);
++        rc = vnc_display_password(opts->u.vnc.display, opts->password);
      }
  
      if (rc != 0) {
-@@ -210,11 +195,11 @@ void qmp_set_password(const char *protocol, const char *password,
+@@ -200,11 +192,11 @@ void qmp_set_password(DisplayProtocol protocol, const char *password,
      }
  }
  
--void qmp_expire_password(const char *protocol, const char *whenstr,
+-void qmp_expire_password(DisplayProtocol protocol, const char *whenstr,
 -                         Error **errp)
 +void qmp_expire_password(ExpirePasswordOptions *opts, Error **errp)
  {
      time_t when;
      int rc;
-+    const char* whenstr = opts->time;
++    const char *whenstr = opts->time;
  
      if (strcmp(whenstr, "now") == 0) {
          when = 0;
-@@ -226,17 +211,14 @@ void qmp_expire_password(const char *protocol, const char *whenstr,
+@@ -216,14 +208,14 @@ void qmp_expire_password(DisplayProtocol protocol, const char *whenstr,
          when = strtoull(whenstr, NULL, 10);
      }
  
--    if (strcmp(protocol, "spice") == 0) {
+-    if (protocol == DISPLAY_PROTOCOL_SPICE) {
 +    if (opts->protocol == DISPLAY_PROTOCOL_SPICE) {
          if (!qemu_using_spice(errp)) {
              return;
          }
          rc = qemu_spice.set_pw_expire(when);
--    } else if (strcmp(protocol, "vnc") == 0) {
+     } else {
+-        assert(protocol == DISPLAY_PROTOCOL_VNC);
 -        rc = vnc_display_pw_expire(NULL, when);
--    } else {
--        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "protocol",
--                   "'vnc' or 'spice'");
--        return;
-+    } else if (opts->protocol == DISPLAY_PROTOCOL_VNC) {
-+        rc = vnc_display_pw_expire(
-+                opts->u.vnc.has_display ? opts->u.vnc.display : NULL, when);
++        assert(opts->protocol == DISPLAY_PROTOCOL_VNC);
++        rc = vnc_display_pw_expire(opts->u.vnc.display, when);
      }
  
      if (rc != 0) {
 diff --git a/qapi/ui.json b/qapi/ui.json
-index fd9677d48e..0177cf4ee9 100644
+index 7e7be154c5..0ce06124b1 100644
 --- a/qapi/ui.json
 +++ b/qapi/ui.json
-@@ -10,20 +10,21 @@
- { 'include': 'sockets.json' }
+@@ -39,20 +39,61 @@
+   'data': [ 'fail', 'disconnect', 'keep' ] }
  
  ##
 -# @set_password:
-+# @DisplayProtocol:
++# @SetPasswordOptions:
  #
 -# Sets the password of a remote display session.
-+# Display protocols which support changing password options.
++# General options for set_password.
  #
--# @protocol: - 'vnc' to modify the VNC server password
--#            - 'spice' to modify the Spice server password
-+# Since: 6.2
+ # @protocol: - 'vnc' to modify the VNC server password
+ #            - 'spice' to modify the Spice server password
  #
--# @password: the new password
-+##
-+{ 'enum': 'DisplayProtocol',
-+  'data': [ { 'name': 'vnc', 'if': 'defined(CONFIG_VNC)' },
-+            { 'name': 'spice', 'if': 'defined(CONFIG_SPICE)' } ] }
-+
-+##
-+# @set_password:
+ # @password: the new password
  #
 -# @connected: how to handle existing clients when changing the
 -#             password.  If nothing is specified, defaults to 'keep'
 -#             'fail' to fail the command if clients are connected
 -#             'disconnect' to disconnect existing clients
 -#             'keep' to maintain existing clients
-+# Sets the password of a remote display session.
- #
- # Returns: - Nothing on success
- #          - If Spice is not enabled, DeviceNotFound
-@@ -37,16 +38,123 @@
- # <- { "return": {} }
- #
- ##
--{ 'command': 'set_password',
--  'data': {'protocol': 'str', 'password': 'str', '*connected': 'str'} }
-+{ 'command': 'set_password', 'boxed': true, 'data': 'SetPasswordOptions' }
-+
-+##
-+# @SetPasswordOptions:
-+#
-+# Data required to set a new password on a display server protocol.
-+#
-+# @protocol: - 'vnc' to modify the VNC server password
-+#            - 'spice' to modify the Spice server password
-+#
-+# @password: the new password
-+#
 +# Since: 6.2
 +#
 +##
@@ -331,40 +280,9 @@ index fd9677d48e..0177cf4ee9 100644
 +            'spice': 'SetPasswordOptionsSpice' } }
 +
 +##
-+# @SetPasswordAction:
-+#
-+# An action to take on changing a password on a connection with active clients.
-+#
-+# @fail: fail the command if clients are connected
-+#
-+# @disconnect: disconnect existing clients
-+#
-+# @keep: maintain existing clients
-+#
-+# Since: 6.2
-+#
-+##
-+{ 'enum': 'SetPasswordAction',
-+  'data': [ 'fail', 'disconnect', 'keep' ] }
-+
-+##
-+# @SetPasswordActionVnc:
-+#
-+# See @SetPasswordAction. VNC only supports the keep action. 'connection'
-+# should just be omitted for VNC, this is kept for backwards compatibility.
-+#
-+# @keep: maintain existing clients
-+#
-+# Since: 6.2
-+#
-+##
-+{ 'enum': 'SetPasswordActionVnc',
-+  'data': [ 'keep' ] }
-+
-+##
 +# @SetPasswordOptionsSpice:
 +#
-+# Options for set_password specific to the VNC procotol.
++# Options for set_password specific to the SPICE procotol.
 +#
 +# @connected: How to handle existing clients when changing the
 +#             password. If nothing is specified, defaults to 'keep'.
@@ -386,42 +304,38 @@ index fd9677d48e..0177cf4ee9 100644
 +# @connected: How to handle existing clients when changing the
 +#             password.
 +#
-+# Features:
-+# @deprecated: For VNC, @connected will always be 'keep', parameter should be
-+#              omitted.
-+#
 +# Since: 6.2
 +#
 +##
 +{ 'struct': 'SetPasswordOptionsVnc',
 +  'data': { '*display': 'str',
-+            '*connected': { 'type': 'SetPasswordActionVnc',
-+                            'features': ['deprecated'] } } }
- 
- ##
- # @expire_password:
- #
- # Expire the password of a remote display server.
- #
--# @protocol: the name of the remote display protocol 'vnc' or 'spice'
-+# Returns: - Nothing on success
-+#          - If @protocol is 'spice' and Spice is not active, DeviceNotFound
-+#
-+# Since: 0.14
-+#
-+# Example:
-+#
-+# -> { "execute": "expire_password", "arguments": { "protocol": "vnc",
-+#                                                   "time": "+60" } }
-+# <- { "return": {} }
-+#
-+##
-+{ 'command': 'expire_password', 'boxed': true, 'data': 'ExpirePasswordOptions' }
++            '*connected': 'SetPasswordAction' }}
 +
 +##
-+# @ExpirePasswordOptions:
++# @set_password:
 +#
-+# Data required to set password expiration on a display server protocol.
++# Set the password of a remote display server.
+ #
+ # Returns: - Nothing on success
+ #          - If Spice is not enabled, DeviceNotFound
+@@ -66,18 +107,16 @@
+ # <- { "return": {} }
+ #
+ ##
+-{ 'command': 'set_password',
+-  'data': { 'protocol': 'DisplayProtocol',
+-            'password': 'str',
+-            '*connected': 'SetPasswordAction' } }
++{ 'command': 'set_password', 'boxed': true, 'data': 'SetPasswordOptions' }
+ 
+ ##
+-# @expire_password:
++# @ExpirePasswordOptions:
+ #
+-# Expire the password of a remote display server.
+-#
+-# @protocol: the name of the remote display protocol 'vnc' or 'spice'
++# General options for expire_password.
  #
 +# @protocol: - 'vnc' to modify the VNC server expiration
 +#            - 'spice' to modify the Spice server expiration
@@ -429,7 +343,7 @@ index fd9677d48e..0177cf4ee9 100644
  # @time: when to expire the password.
  #
  #        - 'now' to expire the password immediately
-@@ -54,24 +162,33 @@
+@@ -85,16 +124,45 @@
  #        - '+INT' where INT is the number of seconds from now (integer)
  #        - 'INT' where INT is the absolute time in seconds
  #
@@ -443,12 +357,8 @@ index fd9677d48e..0177cf4ee9 100644
  #        use the absolute time version of the @time parameter unless you're
  #        sure you are on the same machine as the QEMU instance.
  #
--# Example:
 +# Since: 6.2
- #
--# -> { "execute": "expire_password", "arguments": { "protocol": "vnc",
--#                                                   "time": "+60" } }
--# <- { "return": {} }
++#
 +##
 +{ 'union': 'ExpirePasswordOptions',
 +  'base': { 'protocol': 'DisplayProtocol',
@@ -465,11 +375,33 @@ index fd9677d48e..0177cf4ee9 100644
 +#           Defaults to the first.
 +#
 +# Since: 6.2
- #
- ##
--{ 'command': 'expire_password', 'data': {'protocol': 'str', 'time': 'str'} }
++#
++##
++
 +{ 'struct': 'ExpirePasswordOptionsVnc',
 +  'data': { '*display': 'str' } }
++
++##
++# @expire_password:
++#
++# Expire the password of a remote display server.
++#
++# Returns: - Nothing on success
++#          - If @protocol is 'spice' and Spice is not active, DeviceNotFound
++#
++# Since: 0.14
++#
+ # Example:
+ #
+ # -> { "execute": "expire_password", "arguments": { "protocol": "vnc",
+@@ -102,9 +170,7 @@
+ # <- { "return": {} }
+ #
+ ##
+-{ 'command': 'expire_password',
+-  'data': { 'protocol': 'DisplayProtocol',
+-            'time': 'str' } }
++{ 'command': 'expire_password', 'boxed': true, 'data': 'ExpirePasswordOptions' }
  
  ##
  # @screendump:
diff --git a/debian/patches/extra/0007-qapi-monitor-only-allow-keep-SetPasswordAction-for-V.patch b/debian/patches/extra/0007-qapi-monitor-only-allow-keep-SetPasswordAction-for-V.patch
new file mode 100644
index 0000000..4c7be7d
--- /dev/null
+++ b/debian/patches/extra/0007-qapi-monitor-only-allow-keep-SetPasswordAction-for-V.patch
@@ -0,0 +1,101 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Stefan Reiter <s.reiter at proxmox.com>
+Date: Thu, 28 Oct 2021 07:29:55 +0200
+Subject: [PATCH] qapi/monitor: only allow 'keep' SetPasswordAction for VNC and
+ deprecate
+
+VNC only supports 'keep' here, enforce this via a seperate
+SetPasswordActionVnc enum and mark the option 'deprecated' (as it is
+useless with only one value possible).
+
+Also add a deprecation note to docs.
+
+Suggested-by: Eric Blake <eblake at redhat.com>
+Reviewed-by: Markus Armbruster <armbru at redhat.com>
+Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
+Message-Id: <20211021100135.4146766-5-s.reiter at proxmox.com>
+Acked-by: Gerd Hoffmann <kraxel at redhat.com>
+Signed-off-by: Markus Armbruster <armbru at redhat.com>
+---
+ docs/about/deprecated.rst |  6 ++++++
+ monitor/qmp-cmds.c        |  5 -----
+ qapi/ui.json              | 21 ++++++++++++++++++++-
+ 3 files changed, 26 insertions(+), 6 deletions(-)
+
+diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
+index 6d438f1c8d..34bdc0db2f 100644
+--- a/docs/about/deprecated.rst
++++ b/docs/about/deprecated.rst
+@@ -179,6 +179,12 @@ Use the more generic commands ``block-export-add`` and ``block-export-del``
+ instead.  As part of this deprecation, where ``nbd-server-add`` used a
+ single ``bitmap``, the new ``block-export-add`` uses a list of ``bitmaps``.
+ 
++``set_password`` argument ``connected`` for VNC protocol (since 6.2)
++''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
++
++Only the value ``keep`` is and was ever supported for VNC. The (useless)
++argument will be dropped in a future version of QEMU.
++
+ System accelerators
+ -------------------
+ 
+diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
+index 6802b45ead..8c7d179ec1 100644
+--- a/monitor/qmp-cmds.c
++++ b/monitor/qmp-cmds.c
+@@ -177,11 +177,6 @@ void qmp_set_password(SetPasswordOptions *opts, Error **errp)
+                 opts->u.spice.connected == SET_PASSWORD_ACTION_DISCONNECT);
+     } else {
+         assert(opts->protocol == DISPLAY_PROTOCOL_VNC);
+-        if (opts->u.vnc.connected != SET_PASSWORD_ACTION_KEEP) {
+-            /* vnc supports "connected=keep" only */
+-            error_setg(errp, QERR_INVALID_PARAMETER, "connected");
+-            return;
+-        }
+         /* Note that setting an empty password will not disable login through
+          * this interface. */
+         rc = vnc_display_password(opts->u.vnc.display, opts->password);
+diff --git a/qapi/ui.json b/qapi/ui.json
+index 0ce06124b1..009b4c3f98 100644
+--- a/qapi/ui.json
++++ b/qapi/ui.json
+@@ -38,6 +38,20 @@
+ { 'enum': 'SetPasswordAction',
+   'data': [ 'fail', 'disconnect', 'keep' ] }
+ 
++##
++# @SetPasswordActionVnc:
++#
++# See @SetPasswordAction. VNC only supports the keep action. 'connection'
++# should just be omitted for VNC, this is kept for backwards compatibility.
++#
++# @keep: maintain existing clients
++#
++# Since: 6.2
++#
++##
++{ 'enum': 'SetPasswordActionVnc',
++  'data': [ 'keep' ] }
++
+ ##
+ # @SetPasswordOptions:
+ #
+@@ -83,12 +97,17 @@
+ # @connected: How to handle existing clients when changing the
+ #             password.
+ #
++# Features:
++# @deprecated: For VNC, @connected will always be 'keep', parameter should be
++#              omitted.
++#
+ # Since: 6.2
+ #
+ ##
+ { 'struct': 'SetPasswordOptionsVnc',
+   'data': { '*display': 'str',
+-            '*connected': 'SetPasswordAction' }}
++            '*connected': { 'type': 'SetPasswordActionVnc',
++                            'features': ['deprecated'] } } }
+ 
+ ##
+ # @set_password:
diff --git a/debian/patches/series b/debian/patches/series
index c1eecec..fb5c213 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1,9 +1,10 @@
 extra/0001-qemu-sockets-fix-unix-socket-path-copy-again.patch
 extra/0002-monitor-qmp-fix-race-with-clients-disconnecting-earl.patch
-extra/0003-monitor-hmp-add-support-for-flag-argument-with-value.patch
-extra/0004-monitor-refactor-set-expire_password-and-allow-VNC-d.patch
-extra/0005-monitor-hmp-correctly-invert-password-argument-detec.patch
-extra/0006-qxl-fix-pre-save-logic.patch
+extra/0003-qxl-fix-pre-save-logic.patch
+extra/0004-monitor-hmp-add-support-for-flag-argument-with-value.patch
+extra/0005-qapi-monitor-refactor-set-expire_password-with-enums.patch
+extra/0006-qapi-monitor-allow-VNC-display-id-in-set-expire_pass.patch
+extra/0007-qapi-monitor-only-allow-keep-SetPasswordAction-for-V.patch
 bitmap-mirror/0001-drive-mirror-add-support-for-sync-bitmap-mode-never.patch
 bitmap-mirror/0002-drive-mirror-add-support-for-conditional-and-always-.patch
 bitmap-mirror/0003-mirror-add-check-for-bitmap-mode-without-bitmap.patch
-- 
2.30.2






More information about the pve-devel mailing list