[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v9 3/3] qapi/monitor: allow VNC display id in set/expire_pass
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v9 3/3] qapi/monitor: allow VNC display id in set/expire_password |
Date: |
Fri, 25 Feb 2022 13:44:39 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) |
Fabian Ebner <f.ebner@proxmox.com> writes:
> Am 25.02.22 um 12:34 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.
>>>
>>> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
>>> [FE: update "Since: " from 6.2 to 7.0
>>> make @connected a common member of @SetPasswordOptions]
>>> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
>>
>> [...]
>>
>>> diff --git a/qapi/ui.json b/qapi/ui.json
>>> index e112409211..4a13f883a3 100644
>>> --- a/qapi/ui.json
>>> +++ b/qapi/ui.json
>>> @@ -38,20 +38,47 @@
>>> 'data': [ 'keep', 'fail', 'disconnect' ] }
>>>
>>> ##
>>> -# @set_password:
>>> +# @SetPasswordOptions:
>>> #
>>> -# Sets the password of a remote display session.
>>> +# 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
>>> +# @connected: How to handle existing clients when changing the
>>> +# password. If nothing is specified, defaults to 'keep'.
>>> +# For VNC, only 'keep' is currently implemented.
>>> +#
>>> +# Since: 7.0
>>> +#
>>> +##
>>> +{ 'union': 'SetPasswordOptions',
>>> + 'base': { 'protocol': 'DisplayProtocol',
>>> + 'password': 'str',
>>> + '*connected': 'SetPasswordAction' },
>>> + 'discriminator': 'protocol',
>>> + 'data': { 'vnc': 'SetPasswordOptionsVnc' } }
>>> +
>>> +##
>>> +# @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.
>>
>> Is this default equivalent to any value? "The first" suggests it's not.
>>
>
> The value will be NULL and QTAILQ_FIRST(&vnc_displays) is picked, which
> means the display defaults to the first display. But yeah, the value
> doesn't actually default to the id of the first display, it just behaves
> as if it did.
QAPI lets you give "absent" a meaning different from any value (because
it lets you distinguish "absent" from any value). I prefer not to make
use of it. But it's not wrong. My Acked-by stands.