| Message ID | 20220204101220.343526-4-f.ebner@proxmox.com |
|---|---|
| State | New |
| Headers | show |
| Series | VNC-related HMP/QMP fixes | expand |
Fabian Ebner <f.ebner@proxmox.com> writes: > From: Stefan Reiter <s.reiter@proxmox.com> > > 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, 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: Markus Armbruster <armbru@redhat.com> Did I suggest this feature? I don't remember... Most likely, I merely suggested using a union. Mind if I drop this tag? > Signed-off-by: Stefan Reiter <s.reiter@proxmox.com> > [FE: update "Since: " from 6.2 to 7.0 > set {has_}connected for VNC in hmp_set_password] > Signed-off-by: Fabian Ebner <f.ebner@proxmox.com> > --- > > v7 -> v8: > * add missing # in the description for @ExpirePasswordOptions > * other changes are already mentioned above > > hmp-commands.hx | 24 +++++----- > monitor/hmp-cmds.c | 39 ++++++++++++---- > monitor/qmp-cmds.c | 34 ++++++-------- > qapi/ui.json | 110 ++++++++++++++++++++++++++++++++++++--------- > 4 files changed, 145 insertions(+), 62 deletions(-) > > diff --git a/hmp-commands.hx b/hmp-commands.hx > index 70a9136ac2..cc2f4bdeba 100644 > --- a/hmp-commands.hx > +++ b/hmp-commands.hx > @@ -1514,33 +1514,35 @@ 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:-dV,connected:s?", > + .params = "protocol password [-d 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. *action-if-connected* specifies what > - should happen in case a connection is established: *fail* makes the > - password change fail. *disconnect* changes the password and > +``set_password [ vnc | spice ] password [ -d display ] [ action-if-connected ]`` This is the first flag with an argument in HMP. The alternative is another optional argument. PRO optional argument: no need for PATCH 1. PRO flag with argument: can specify the display without action-if-connected. Dave, this is your call to make. > + Change spice/vnc password. *display* 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. > ERST > > { > .name = "expire_password", > - .args_type = "protocol:s,time:s", > - .params = "protocol time", > + .args_type = "protocol:s,time:s,display:-dV", > + .params = "protocol time [-d 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 [ -d 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 ff78741b75..be0d919b64 100644 > --- a/monitor/hmp-cmds.c > +++ b/monitor/hmp-cmds.c > @@ -1396,13 +1396,17 @@ 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; > SetPasswordAction conn; > > - proto = qapi_enum_parse(&DisplayProtocol_lookup, protocol, > - DISPLAY_PROTOCOL_VNC, &err); > + SetPasswordOptions opts = { > + .password = (char *)password, > + }; > + > + opts.protocol = qapi_enum_parse(&DisplayProtocol_lookup, protocol, > + DISPLAY_PROTOCOL_VNC, &err); > if (err) { > goto out; > } > @@ -1413,7 +1417,17 @@ void hmp_set_password(Monitor *mon, const QDict *qdict) > goto out; > } > > - qmp_set_password(proto, password, !!connected, conn, &err); > + if (opts.protocol == DISPLAY_PROTOCOL_VNC) { > + opts.u.vnc.has_display = !!display; > + opts.u.vnc.display = (char *)display; > + opts.u.vnc.has_connected = !!connected; > + opts.u.vnc.connected = conn; > + } else if (opts.protocol == DISPLAY_PROTOCOL_SPICE) { > + opts.u.spice.has_connected = !!connected; > + opts.u.spice.connected = conn; > + } > + > + qmp_set_password(&opts, &err); > > out: > hmp_handle_error(mon, err); > @@ -1423,16 +1437,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; > > - proto = qapi_enum_parse(&DisplayProtocol_lookup, protocol, > - DISPLAY_PROTOCOL_VNC, &err); > + ExpirePasswordOptions opts = { > + .time = (char *)whenstr, > + }; > + > + 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 = (char *)display; > + } > + > + qmp_expire_password(&opts, &err); > > out: > hmp_handle_error(mon, err); > diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c > index b6e8b57fcc..37db941fd3 100644 > --- a/monitor/qmp-cmds.c > +++ b/monitor/qmp-cmds.c > @@ -168,35 +168,27 @@ void qmp_system_wakeup(Error **errp) > qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER, 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; > - int fail_if_connected = 0; > int rc; > > - if (has_connected) { > - fail_if_connected = connected == SET_PASSWORD_ACTION_FAIL; > - disconnect_if_connected = connected == SET_PASSWORD_ACTION_DISCONNECT; > - } > - > - 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); > + 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) { > + 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(NULL, password); > + rc = vnc_display_password(opts->u.vnc.display, opts->password); > } > > if (rc != 0) { > @@ -204,11 +196,11 @@ void qmp_set_password(DisplayProtocol protocol, const char *password, > } > } > > -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; > > if (strcmp(whenstr, "now") == 0) { > when = 0; > @@ -220,14 +212,14 @@ void qmp_expire_password(DisplayProtocol protocol, const char *whenstr, > when = strtoull(whenstr, NULL, 10); > } > > - 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 { > - assert(protocol == DISPLAY_PROTOCOL_VNC); > - rc = vnc_display_pw_expire(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 e112409211..089f05c702 100644 > --- a/qapi/ui.json > +++ b/qapi/ui.json > @@ -38,20 +38,61 @@ > 'data': [ 'keep', 'fail', 'disconnect' ] } > > ## > -# @set_password: > +# @SetPasswordOptions: > # > -# Sets the password of a remote display session. > +# General options for set_password. Actually, all the options there are. Let's drop "General". > # > # @protocol: - 'vnc' to modify the VNC server password > # - 'spice' to modify the Spice server 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 > +# Since: 7.0 > +# > +## > +{ 'union': 'SetPasswordOptions', > + 'base': { 'protocol': 'DisplayProtocol', > + 'password': 'str' }, > + 'discriminator': 'protocol', > + 'data': { 'vnc': 'SetPasswordOptionsVnc', > + 'spice': 'SetPasswordOptionsSpice' } } > + > +## > +# @SetPasswordOptionsSpice: > +# > +# 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'. > +# > +# Since: 7.0 > +# > +## > +{ 'struct': 'SetPasswordOptionsSpice', > + 'data': { '*connected': 'SetPasswordAction' } } > + > +## > +# @SetPasswordOptionsVnc: > +# > +# Options for set_password specific to the VNC procotol. > +# > +# @display: The id of the display where the password should be changed. > +# Defaults to the first. > +# > +# @connected: How to handle existing clients when changing the > +# password. Neglects to document the default, unlike SetPasswordOptionsSpice above. > +# > +# Since: 7.0 > +# > +## > +{ 'struct': 'SetPasswordOptionsVnc', > + 'data': { '*display': 'str', > + '*connected': 'SetPasswordAction' }} @connected could be made a common member. Untested incremental patch appended for your consideration. > + > +## > +# @set_password: > +# > +# Set the password of a remote display server. > # > # Returns: - Nothing on success > # - If Spice is not enabled, DeviceNotFound > @@ -65,17 +106,15 @@ > # <- { "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. > +# General options for expire_password. > # > -# @protocol: the name of the remote display protocol 'vnc' or 'spice' > +# @protocol: - 'vnc' to modify the VNC server expiration > +# - 'spice' to modify the Spice server expiration > # > # @time: when to expire the password. > # > @@ -84,16 +123,45 @@ > # - '+INT' where INT is the number of seconds from now (integer) > # - 'INT' where INT is the absolute time in seconds > # > -# Returns: - Nothing on success > -# - If @protocol is 'spice' and Spice is not active, DeviceNotFound > -# > -# Since: 0.14 > -# > # Notes: Time is relative to the server and currently there is no way to > # coordinate server time with client time. It is not recommended to > # use the absolute time version of the @time parameter unless you're > # sure you are on the same machine as the QEMU instance. > # > +# Since: 7.0 > +# > +## > +{ 'union': 'ExpirePasswordOptions', > + 'base': { 'protocol': 'DisplayProtocol', > + 'time': 'str' }, > + 'discriminator': 'protocol', > + 'data': { 'vnc': 'ExpirePasswordOptionsVnc' } } > + > +## > +# @ExpirePasswordOptionsVnc: > +# > +# Options for expire_password specific to the VNC procotol. > +# > +# @display: The id of the display where the expiration should be changed. > +# Defaults to the first. > +# > +# Since: 7.0 > +# > +## > + > +{ '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", > @@ -101,9 +169,7 @@ > # <- { "return": {} } > # > ## > -{ 'command': 'expire_password', > - 'data': { 'protocol': 'DisplayProtocol', > - 'time': 'str' } } > +{ 'command': 'expire_password', 'boxed': true, 'data': 'ExpirePasswordOptions' } > > ## > # @screendump: diff --git a/qapi/ui.json b/qapi/ui.json index 089f05c702..bcc69896ed 100644 --- a/qapi/ui.json +++ b/qapi/ui.json @@ -47,29 +47,18 @@ # # @password: the new password # +# @connected: How to handle existing clients when changing the +# password. +# # Since: 7.0 # ## { 'union': 'SetPasswordOptions', 'base': { 'protocol': 'DisplayProtocol', - 'password': 'str' }, + 'password': 'str', + '*connected': 'SetPasswordAction' }, 'discriminator': 'protocol', - 'data': { 'vnc': 'SetPasswordOptionsVnc', - 'spice': 'SetPasswordOptionsSpice' } } - -## -# @SetPasswordOptionsSpice: -# -# 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'. -# -# Since: 7.0 -# -## -{ 'struct': 'SetPasswordOptionsSpice', - 'data': { '*connected': 'SetPasswordAction' } } + 'data': { 'vnc': 'SetPasswordOptionsVnc' } } ## # @SetPasswordOptionsVnc: @@ -79,15 +68,11 @@ # @display: The id of the display where the password should be changed. # Defaults to the first. # -# @connected: How to handle existing clients when changing the -# password. -# # Since: 7.0 # ## { 'struct': 'SetPasswordOptionsVnc', - 'data': { '*display': 'str', - '*connected': 'SetPasswordAction' }} + 'data': { '*display': 'str' } } ## # @set_password: diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c index be0d919b64..54762a9396 100644 --- a/monitor/hmp-cmds.c +++ b/monitor/hmp-cmds.c @@ -1401,8 +1401,16 @@ void hmp_set_password(Monitor *mon, const QDict *qdict) Error *err = NULL; SetPasswordAction conn; + conn = qapi_enum_parse(&SetPasswordAction_lookup, connected, + SET_PASSWORD_ACTION_KEEP, &err); + if (err) { + goto out; + } + SetPasswordOptions opts = { .password = (char *)password, + .has_connected = !!connected, + .connected = conn, }; opts.protocol = qapi_enum_parse(&DisplayProtocol_lookup, protocol, @@ -1411,20 +1419,9 @@ void hmp_set_password(Monitor *mon, const QDict *qdict) 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 = (char *)display; - opts.u.vnc.has_connected = !!connected; - opts.u.vnc.connected = conn; - } else if (opts.protocol == DISPLAY_PROTOCOL_SPICE) { - opts.u.spice.has_connected = !!connected; - opts.u.spice.connected = conn; } qmp_set_password(&opts, &err); diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c index 37db941fd3..df97582dd4 100644 --- a/monitor/qmp-cmds.c +++ b/monitor/qmp-cmds.c @@ -177,11 +177,11 @@ void qmp_set_password(SetPasswordOptions *opts, Error **errp) return; } rc = qemu_spice.set_passwd(opts->password, - opts->u.spice.connected == SET_PASSWORD_ACTION_FAIL, - opts->u.spice.connected == SET_PASSWORD_ACTION_DISCONNECT); + opts->connected == SET_PASSWORD_ACTION_FAIL, + opts->connected == SET_PASSWORD_ACTION_DISCONNECT); } else { assert(opts->protocol == DISPLAY_PROTOCOL_VNC); - if (opts->u.vnc.connected != SET_PASSWORD_ACTION_KEEP) { + if (opts->connected != SET_PASSWORD_ACTION_KEEP) { /* vnc supports "connected=keep" only */ error_setg(errp, QERR_INVALID_PARAMETER, "connected"); return;
Am 09.02.22 um 15:07 schrieb Markus Armbruster: > Fabian Ebner <f.ebner@proxmox.com> writes: > >> From: Stefan Reiter <s.reiter@proxmox.com> >> >> 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, 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: Markus Armbruster <armbru@redhat.com> > > Did I suggest this feature? I don't remember... Most likely, I merely > suggested using a union. Mind if I drop this tag? > Yes, Stefan might've put the tag because of the suggested approach. I'll drop it. >> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com> >> [FE: update "Since: " from 6.2 to 7.0 >> set {has_}connected for VNC in hmp_set_password] >> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com> >> --- >> >> v7 -> v8: >> * add missing # in the description for @ExpirePasswordOptions >> * other changes are already mentioned above >> >> hmp-commands.hx | 24 +++++----- >> monitor/hmp-cmds.c | 39 ++++++++++++---- >> monitor/qmp-cmds.c | 34 ++++++-------- >> qapi/ui.json | 110 ++++++++++++++++++++++++++++++++++++--------- >> 4 files changed, 145 insertions(+), 62 deletions(-) >> >> diff --git a/hmp-commands.hx b/hmp-commands.hx >> index 70a9136ac2..cc2f4bdeba 100644 >> --- a/hmp-commands.hx >> +++ b/hmp-commands.hx >> @@ -1514,33 +1514,35 @@ 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:-dV,connected:s?", >> + .params = "protocol password [-d 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. *action-if-connected* specifies what >> - should happen in case a connection is established: *fail* makes the >> - password change fail. *disconnect* changes the password and >> +``set_password [ vnc | spice ] password [ -d display ] [ action-if-connected ]`` > > This is the first flag with an argument in HMP. The alternative is > another optional argument. > > PRO optional argument: no need for PATCH 1. > > PRO flag with argument: can specify the display without > action-if-connected. > > Dave, this is your call to make. > I'll go ahead with v9 once the decision is made. ----8<---- >> diff --git a/qapi/ui.json b/qapi/ui.json >> index e112409211..089f05c702 100644 >> --- a/qapi/ui.json >> +++ b/qapi/ui.json >> @@ -38,20 +38,61 @@ >> 'data': [ 'keep', 'fail', 'disconnect' ] } >> >> ## >> -# @set_password: >> +# @SetPasswordOptions: >> # >> -# Sets the password of a remote display session. >> +# General options for set_password. > > Actually, all the options there are. Let's drop "General". > Ok. >> # >> # @protocol: - 'vnc' to modify the VNC server password >> # - 'spice' to modify the Spice server 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 >> +# Since: 7.0 >> +# >> +## >> +{ 'union': 'SetPasswordOptions', >> + 'base': { 'protocol': 'DisplayProtocol', >> + 'password': 'str' }, >> + 'discriminator': 'protocol', >> + 'data': { 'vnc': 'SetPasswordOptionsVnc', >> + 'spice': 'SetPasswordOptionsSpice' } } >> + >> +## >> +# @SetPasswordOptionsSpice: >> +# >> +# 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'. >> +# >> +# Since: 7.0 >> +# >> +## >> +{ 'struct': 'SetPasswordOptionsSpice', >> + 'data': { '*connected': 'SetPasswordAction' } } >> + >> +## >> +# @SetPasswordOptionsVnc: >> +# >> +# Options for set_password specific to the VNC procotol. >> +# >> +# @display: The id of the display where the password should be changed. >> +# Defaults to the first. >> +# >> +# @connected: How to handle existing clients when changing the >> +# password. > > Neglects to document the default, unlike SetPasswordOptionsSpice above. > Will add it in v9. >> +# >> +# Since: 7.0 >> +# >> +## >> +{ 'struct': 'SetPasswordOptionsVnc', >> + 'data': { '*display': 'str', >> + '*connected': 'SetPasswordAction' }} > > @connected could be made a common member. Untested incremental patch > appended for your consideration. > Yes, sounds good.
Fabian Ebner <f.ebner@proxmox.com> writes: > Am 09.02.22 um 15:07 schrieb Markus Armbruster: >> Fabian Ebner <f.ebner@proxmox.com> writes: >> >>> From: Stefan Reiter <s.reiter@proxmox.com> >>> >>> 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, 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. [...] >>> diff --git a/hmp-commands.hx b/hmp-commands.hx >>> index 70a9136ac2..cc2f4bdeba 100644 >>> --- a/hmp-commands.hx >>> +++ b/hmp-commands.hx >>> @@ -1514,33 +1514,35 @@ 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:-dV,connected:s?", >>> + .params = "protocol password [-d 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. *action-if-connected* specifies what >>> - should happen in case a connection is established: *fail* makes the >>> - password change fail. *disconnect* changes the password and >>> +``set_password [ vnc | spice ] password [ -d display ] [ action-if-connected ]`` >> >> This is the first flag with an argument in HMP. The alternative is >> another optional argument. >> >> PRO optional argument: no need for PATCH 1. >> >> PRO flag with argument: can specify the display without >> action-if-connected. >> >> Dave, this is your call to make. >> > > I'll go ahead with v9 once the decision is made. Dave? [...]
* Markus Armbruster (armbru@redhat.com) wrote: > Fabian Ebner <f.ebner@proxmox.com> writes: > > > Am 09.02.22 um 15:07 schrieb Markus Armbruster: > >> Fabian Ebner <f.ebner@proxmox.com> writes: > >> > >>> From: Stefan Reiter <s.reiter@proxmox.com> > >>> > >>> 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, 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. > > [...] > > >>> diff --git a/hmp-commands.hx b/hmp-commands.hx > >>> index 70a9136ac2..cc2f4bdeba 100644 > >>> --- a/hmp-commands.hx > >>> +++ b/hmp-commands.hx > >>> @@ -1514,33 +1514,35 @@ 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:-dV,connected:s?", > >>> + .params = "protocol password [-d 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. *action-if-connected* specifies what > >>> - should happen in case a connection is established: *fail* makes the > >>> - password change fail. *disconnect* changes the password and > >>> +``set_password [ vnc | spice ] password [ -d display ] [ action-if-connected ]`` > >> > >> This is the first flag with an argument in HMP. The alternative is > >> another optional argument. > >> > >> PRO optional argument: no need for PATCH 1. > >> > >> PRO flag with argument: can specify the display without > >> action-if-connected. > >> > >> Dave, this is your call to make. > >> > > > > I'll go ahead with v9 once the decision is made. > > Dave? I think the flag with argument is clearer; HMP has a problem of having a lot of optional arguments that get very order dependent which is messy; so I'd go with the flag with argument. Dave > [...] >
diff --git a/hmp-commands.hx b/hmp-commands.hx index 70a9136ac2..cc2f4bdeba 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1514,33 +1514,35 @@ 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:-dV,connected:s?", + .params = "protocol password [-d 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. *action-if-connected* specifies what - should happen in case a connection is established: *fail* makes the - password change fail. *disconnect* changes the password and +``set_password [ vnc | spice ] password [ -d display ] [ action-if-connected ]`` + Change spice/vnc password. *display* 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. ERST { .name = "expire_password", - .args_type = "protocol:s,time:s", - .params = "protocol time", + .args_type = "protocol:s,time:s,display:-dV", + .params = "protocol time [-d 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 [ -d 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 ff78741b75..be0d919b64 100644 --- a/monitor/hmp-cmds.c +++ b/monitor/hmp-cmds.c @@ -1396,13 +1396,17 @@ 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; SetPasswordAction conn; - proto = qapi_enum_parse(&DisplayProtocol_lookup, protocol, - DISPLAY_PROTOCOL_VNC, &err); + SetPasswordOptions opts = { + .password = (char *)password, + }; + + opts.protocol = qapi_enum_parse(&DisplayProtocol_lookup, protocol, + DISPLAY_PROTOCOL_VNC, &err); if (err) { goto out; } @@ -1413,7 +1417,17 @@ void hmp_set_password(Monitor *mon, const QDict *qdict) goto out; } - qmp_set_password(proto, password, !!connected, conn, &err); + if (opts.protocol == DISPLAY_PROTOCOL_VNC) { + opts.u.vnc.has_display = !!display; + opts.u.vnc.display = (char *)display; + opts.u.vnc.has_connected = !!connected; + opts.u.vnc.connected = conn; + } else if (opts.protocol == DISPLAY_PROTOCOL_SPICE) { + opts.u.spice.has_connected = !!connected; + opts.u.spice.connected = conn; + } + + qmp_set_password(&opts, &err); out: hmp_handle_error(mon, err); @@ -1423,16 +1437,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; - proto = qapi_enum_parse(&DisplayProtocol_lookup, protocol, - DISPLAY_PROTOCOL_VNC, &err); + ExpirePasswordOptions opts = { + .time = (char *)whenstr, + }; + + 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 = (char *)display; + } + + qmp_expire_password(&opts, &err); out: hmp_handle_error(mon, err); diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c index b6e8b57fcc..37db941fd3 100644 --- a/monitor/qmp-cmds.c +++ b/monitor/qmp-cmds.c @@ -168,35 +168,27 @@ void qmp_system_wakeup(Error **errp) qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER, 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; - int fail_if_connected = 0; int rc; - if (has_connected) { - fail_if_connected = connected == SET_PASSWORD_ACTION_FAIL; - disconnect_if_connected = connected == SET_PASSWORD_ACTION_DISCONNECT; - } - - 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); + 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) { + 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(NULL, password); + rc = vnc_display_password(opts->u.vnc.display, opts->password); } if (rc != 0) { @@ -204,11 +196,11 @@ void qmp_set_password(DisplayProtocol protocol, const char *password, } } -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; if (strcmp(whenstr, "now") == 0) { when = 0; @@ -220,14 +212,14 @@ void qmp_expire_password(DisplayProtocol protocol, const char *whenstr, when = strtoull(whenstr, NULL, 10); } - 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 { - assert(protocol == DISPLAY_PROTOCOL_VNC); - rc = vnc_display_pw_expire(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 e112409211..089f05c702 100644 --- a/qapi/ui.json +++ b/qapi/ui.json @@ -38,20 +38,61 @@ 'data': [ 'keep', 'fail', 'disconnect' ] } ## -# @set_password: +# @SetPasswordOptions: # -# Sets the password of a remote display session. +# General options for set_password. # # @protocol: - 'vnc' to modify the VNC server password # - 'spice' to modify the Spice server 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 +# Since: 7.0 +# +## +{ 'union': 'SetPasswordOptions', + 'base': { 'protocol': 'DisplayProtocol', + 'password': 'str' }, + 'discriminator': 'protocol', + 'data': { 'vnc': 'SetPasswordOptionsVnc', + 'spice': 'SetPasswordOptionsSpice' } } + +## +# @SetPasswordOptionsSpice: +# +# 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'. +# +# Since: 7.0 +# +## +{ 'struct': 'SetPasswordOptionsSpice', + 'data': { '*connected': 'SetPasswordAction' } } + +## +# @SetPasswordOptionsVnc: +# +# Options for set_password specific to the VNC procotol. +# +# @display: The id of the display where the password should be changed. +# Defaults to the first. +# +# @connected: How to handle existing clients when changing the +# password. +# +# Since: 7.0 +# +## +{ 'struct': 'SetPasswordOptionsVnc', + 'data': { '*display': 'str', + '*connected': 'SetPasswordAction' }} + +## +# @set_password: +# +# Set the password of a remote display server. # # Returns: - Nothing on success # - If Spice is not enabled, DeviceNotFound @@ -65,17 +106,15 @@ # <- { "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. +# General options for expire_password. # -# @protocol: the name of the remote display protocol 'vnc' or 'spice' +# @protocol: - 'vnc' to modify the VNC server expiration +# - 'spice' to modify the Spice server expiration # # @time: when to expire the password. # @@ -84,16 +123,45 @@ # - '+INT' where INT is the number of seconds from now (integer) # - 'INT' where INT is the absolute time in seconds # -# Returns: - Nothing on success -# - If @protocol is 'spice' and Spice is not active, DeviceNotFound -# -# Since: 0.14 -# # Notes: Time is relative to the server and currently there is no way to # coordinate server time with client time. It is not recommended to # use the absolute time version of the @time parameter unless you're # sure you are on the same machine as the QEMU instance. # +# Since: 7.0 +# +## +{ 'union': 'ExpirePasswordOptions', + 'base': { 'protocol': 'DisplayProtocol', + 'time': 'str' }, + 'discriminator': 'protocol', + 'data': { 'vnc': 'ExpirePasswordOptionsVnc' } } + +## +# @ExpirePasswordOptionsVnc: +# +# Options for expire_password specific to the VNC procotol. +# +# @display: The id of the display where the expiration should be changed. +# Defaults to the first. +# +# Since: 7.0 +# +## + +{ '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", @@ -101,9 +169,7 @@ # <- { "return": {} } # ## -{ 'command': 'expire_password', - 'data': { 'protocol': 'DisplayProtocol', - 'time': 'str' } } +{ 'command': 'expire_password', 'boxed': true, 'data': 'ExpirePasswordOptions' } ## # @screendump: