Hi
On Wed, Aug 25, 2021 at 1:39 PM Stefan Reiter <s.rei...@proxmox.com> wrote:
> It is possible to specify more than one VNC server on the command line,
> either with an explicit ID or the auto-generated ones à la "default",
> "vnc2", "vnc3", ...
>
> It is not possible to change the password on one of these extra VNC
> displays though. Fix this by adding a "display" parameter to the
> 'set_password' and 'expire_password' QMP and HMP commands.
>
> For HMP, this is a bit trickier, since at least 'set_password' already
> has the 'connected' parameter following the mandatory 'password' one, so
> we need to prefix the display ID with "id=" to allow correct parsing.
>
It's not something done with other commands afaik, feels a bit awkward (the
"connected = display"...).
Is it really necessary to add support for HMP?
With this prefix, no existing command or workflow should be affected.
>
> While rewriting the descriptions, also remove the line "Use zero to make
> the password stay valid forever." from 'set_password', I believe this was
> intended for 'expire_password', but would even be wrong there.
>
> Signed-off-by: Stefan Reiter <s.rei...@proxmox.com>
> ---
> hmp-commands.hx | 28 +++++++++++++++-------------
> monitor/hmp-cmds.c | 20 ++++++++++++++++++--
> monitor/qmp-cmds.c | 9 +++++----
> qapi/ui.json | 12 ++++++++++--
> 4 files changed, 48 insertions(+), 21 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e01ca13ca8..0b5abcfb8a 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1541,34 +1541,36 @@ ERST
>
> {
> .name = "set_password",
> - .args_type = "protocol:s,password:s,connected:s?",
> - .params = "protocol password action-if-connected",
> + .args_type = "protocol:s,password:s,display:s?,connected:s?",
> + .params = "protocol password [id=display]
> [action-if-connected]",
> .help = "set spice/vnc password",
> .cmd = hmp_set_password,
> },
>
> SRST
> -``set_password [ vnc | spice ] password [ action-if-connected ]``
> - Change spice/vnc password. Use zero to make the password stay valid
> - forever. *action-if-connected* specifies what should happen in
> +``set_password [ vnc | spice ] password [ id=display ] [
> action-if-connected ]``
> + Change spice/vnc password. *display* (must be prefixed with
> + 'id=') can be used with 'vnc' to specify which display to set the
> + password on. *action-if-connected* specifies what should happen in
> case a connection is established: *fail* makes the password change
> - fail. *disconnect* changes the password and disconnects the
> - client. *keep* changes the password and keeps the connection up.
> - *keep* is the default.
> + fail. *disconnect* changes the password and disconnects the client.
> + *keep* changes the password and keeps the connection up. *keep* is
> + the default.
> ERST
>
> {
> .name = "expire_password",
> - .args_type = "protocol:s,time:s",
> - .params = "protocol time",
> + .args_type = "protocol:s,time:s,display:s?",
> + .params = "protocol time [id=display]",
> .help = "set spice/vnc password expire-time",
> .cmd = hmp_expire_password,
> },
>
> SRST
> -``expire_password [ vnc | spice ]`` *expire-time*
> - Specify when a password for spice/vnc becomes
> - invalid. *expire-time* accepts:
> +``expire_password [ vnc | spice ] expire-time [ id=display ]``
> + Specify when a password for spice/vnc becomes invalid.
> + *display* behaves the same as in ``set_password``.
> + *expire-time* accepts:
>
> ``now``
> Invalidate password instantly.
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 31366e6331..30f5b2c3e3 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -1546,10 +1546,20 @@ 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;
>
> - qmp_set_password(protocol, password, !!connected, connected, &err);
> + if (display && strncmp(display, "id=", 3)) {
> + connected = display;
> + display = NULL;
> + } else if (display) {
> + /* skip "id=" */
> + display = display + 3;
> + }
> +
> + qmp_set_password(protocol, password, !!connected, connected,
> !!display,
> + display, &err);
> hmp_handle_error(mon, err);
> }
>
> @@ -1557,9 +1567,15 @@ 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;
>
> - qmp_expire_password(protocol, whenstr, &err);
> + if (display && !strncmp(display, "id=", 3)) {
> + /* skip "id=" */
> + display = display + 3;
> + }
> +
> + qmp_expire_password(protocol, whenstr, !!display, display, &err);
> hmp_handle_error(mon, err);
> }
>
> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> index f7d64a6457..a9ded90a41 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -165,7 +165,8 @@ void qmp_system_wakeup(Error **errp)
> }
>
> void qmp_set_password(const char *protocol, const char *password,
> - bool has_connected, const char *connected, Error
> **errp)
> + bool has_connected, const char *connected,
> + bool has_display, const char *display, Error **errp)
> {
> int disconnect_if_connected = 0;
> int fail_if_connected = 0;
> @@ -198,7 +199,7 @@ 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);
> + rc = vnc_display_password(has_display ? display : NULL, password);
> } else {
> error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "protocol",
> "'vnc' or 'spice'");
> @@ -211,7 +212,7 @@ void qmp_set_password(const char *protocol, const char
> *password,
> }
>
> void qmp_expire_password(const char *protocol, const char *whenstr,
> - Error **errp)
> + bool has_display, const char *display, Error
> **errp)
> {
> time_t when;
> int rc;
> @@ -232,7 +233,7 @@ void qmp_expire_password(const char *protocol, const
> char *whenstr,
> }
> rc = qemu_spice.set_pw_expire(when);
> } else if (strcmp(protocol, "vnc") == 0) {
> - rc = vnc_display_pw_expire(NULL, when);
> + rc = vnc_display_pw_expire(has_display ? display : NULL, when);
> } else {
> error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "protocol",
> "'vnc' or 'spice'");
> diff --git a/qapi/ui.json b/qapi/ui.json
> index 16bf03224f..24dca811f8 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -25,6 +25,9 @@
> # 'disconnect' to disconnect existing clients
> # 'keep' to maintain existing clients
> #
> +# @display: In case of VNC, the id of the display where the password
> +# should be changed. Defaults to the first.
> +#
> # Returns: - Nothing on success
> # - If Spice is not enabled, DeviceNotFound
> #
> @@ -38,7 +41,8 @@
> #
> ##
> { 'command': 'set_password',
> - 'data': {'protocol': 'str', 'password': 'str', '*connected': 'str'} }
> + 'data': {'protocol': 'str', 'password': 'str', '*connected': 'str',
> + '*display': 'str'} }
>
> ##
> # @expire_password:
> @@ -54,6 +58,9 @@
> # - '+INT' where INT is the number of seconds from now (integer)
> # - 'INT' where INT is the absolute time in seconds
> #
> +# @display: In case of VNC, the id of the display where the password
> +# should be set to expire. Defaults to the first.
> +#
> # Returns: - Nothing on success
> # - If @protocol is 'spice' and Spice is not active,
> DeviceNotFound
> #
> @@ -71,7 +78,8 @@
> # <- { "return": {} }
> #
> ##
> -{ 'command': 'expire_password', 'data': {'protocol': 'str', 'time':
> 'str'} }
> +{ 'command': 'expire_password',
> + 'data': {'protocol': 'str', 'time': 'str', '*display': 'str'} }
>
> ##
> # @screendump:
> --
> 2.30.2
>
>
>
>
--
Marc-André Lureau
