Fabian Ebner <f.eb...@proxmox.com> writes:
> From: Stefan Reiter <s.rei...@proxmox.com>
>
> 'protocol' and 'connected' are better suited as enums than as strings,
> make use of that. No functional change intended.
>
> Suggested-by: Markus Armbruster <arm...@redhat.com>
> Signed-off-by: Stefan Reiter <s.rei...@proxmox.com>
> [FE: update "Since: " from 6.2 to 7.0
> put 'keep' first in enum to ease use as a default]
> Signed-off-by: Fabian Ebner <f.eb...@proxmox.com>
> ---
>
> v7 -> v8:
> * drop if conditionals for DisplayProtocol enum, so compilation with
> --disable-{spice,vnc} works
>
> monitor/hmp-cmds.c | 29 +++++++++++++++++++++++++++--
> monitor/qmp-cmds.c | 37 ++++++++++++-------------------------
> qapi/ui.json | 36 ++++++++++++++++++++++++++++++++++--
> 3 files changed, 73 insertions(+), 29 deletions(-)
>
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 8c384dc1b2..ff78741b75 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -1398,8 +1398,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);
> }
>
> @@ -1408,8 +1424,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 db4d186448..b6e8b57fcc 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -168,33 +168,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");
> @@ -203,10 +197,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) {
> @@ -214,7 +204,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;
> @@ -230,17 +220,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 9354f4c467..e112409211 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -9,6 +9,34 @@
> { 'include': 'common.json' }
> { 'include': 'sockets.json' }
>
> +##
> +# @DisplayProtocol:
> +#
> +# Display protocols which support changing password options.
This is correct now: the enum is only used in that role. If we ever use
DisplayReloadType for other purposes, the comment will become wrong.
Let's not worry about that now.
> +#
> +# Since: 7.0
> +#
> +##
> +{ 'enum': 'DisplayProtocol',
> + 'data': [ 'vnc', 'spice' ] }
> +
> +##
> +# @SetPasswordAction:
> +#
> +# An action to take on changing a password on a connection with active
> clients.
> +#
> +# @keep: maintain existing clients
> +#
> +# @fail: fail the command if clients are connected
> +#
> +# @disconnect: disconnect existing clients
> +#
> +# Since: 7.0
> +#
> +##
> +{ 'enum': 'SetPasswordAction',
> + 'data': [ 'keep', 'fail', 'disconnect' ] }
> +
> ##
> # @set_password:
> #
> @@ -38,7 +66,9 @@
> #
> ##
> { 'command': 'set_password',
> - 'data': {'protocol': 'str', 'password': 'str', '*connected': 'str'} }
> + 'data': { 'protocol': 'DisplayProtocol',
> + 'password': 'str',
> + '*connected': 'SetPasswordAction' } }
>
> ##
> # @expire_password:
> @@ -71,7 +101,9 @@
> # <- { "return": {} }
> #
> ##
> -{ 'command': 'expire_password', 'data': {'protocol': 'str', 'time': 'str'} }
> +{ 'command': 'expire_password',
> + 'data': { 'protocol': 'DisplayProtocol',
> + 'time': 'str' } }
>
> ##
> # @screendump:
Reviewed-by: Markus Armbruster <arm...@redhat.com>
