Maxim Levitsky <mlevi...@redhat.com> writes: > On Sat, 2020-02-15 at 15:51 +0100, Markus Armbruster wrote: >> Review of this patch led to a lengthy QAPI schema design discussion. >> Let me try to condense it into a concrete proposal. >> >> This is about the QAPI schema, and therefore about QMP. The >> human-friendly interface is out of scope. Not because it's not >> important (it clearly is!), only because we need to *focus* to have a >> chance at success. > 100% agree. >> >> I'm going to include a few design options. I'll mark them "Option:". >> >> The proposed "amend" interface takes a specification of desired state, >> and figures out how to get from here to there by itself. LUKS keyslots >> are one part of desired state. >> >> We commonly have eight LUKS keyslots. Each keyslot is either active or >> inactive. An active keyslot holds a secret. >> >> Goal: a QAPI type for specifying desired state of LUKS keyslots. >> >> Proposal: >> >> { 'enum': 'LUKSKeyslotState', >> 'data': [ 'active', 'inactive' ] } >> >> { 'struct': 'LUKSKeyslotActive', >> 'data': { 'secret': 'str', >> '*iter-time': 'int } } >> >> { 'struct': 'LUKSKeyslotInactive', >> 'data': { '*old-secret': 'str' } } >> >> { 'union': 'LUKSKeyslotAmend', >> 'base': { '*keyslot': 'int', >> 'state': 'LUKSKeyslotState' } >> 'discriminator': 'state', >> 'data': { 'active': 'LUKSKeyslotActive', >> 'inactive': 'LUKSKeyslotInactive' } } >> >> LUKSKeyslotAmend specifies desired state for a set of keyslots. >> >> Four cases: >> >> * @state is "active" >> >> Desired state is active holding the secret given by @secret. Optional >> @iter-time tweaks key stretching. >> >> The keyslot is chosen either by the user or by the system, as follows: >> >> - @keyslot absent >> >> One inactive keyslot chosen by the system. If none exists, error. >> >> - @keyslot present >> >> The keyslot given by @keyslot. >> >> If it's already active holding @secret, no-op. Rationale: the >> current state is the desired state. >> >> If it's already active holding another secret, error. Rationale: >> update in place is unsafe. >> >> Option: delete the "already active holding @secret" case. Feels >> inelegant to me. Okay if it makes things substantially simpler. > I didn't really understand this, since in state=active we shouldn't > delete anything. Looks OK otherwise.
Let me try to clarify. Option: make the "already active holding @secret" case an error like the "already active holding another secret" case. In longhand: - @keyslot present The keyslot given by @keyslot. If it's already active, error. It feels inelegant to me, because it deviates from "specify desired state" paradigm: the specified desired state is fine, the way to get there from current state is obvious (do nothing), yet it's still an error. >> * @state is "inactive" >> >> Desired state is inactive. >> >> Error if the current state has active keyslots, but the desired state >> has none. >> >> The user choses the keyslot by number and/or by the secret it holds, >> as follows: >> >> - @keyslot absent, @old-secret present >> >> All active keyslots holding @old-secret. If none exists, error. >> >> - @keyslot present, @old-secret absent >> >> The keyslot given by @keyslot. >> >> If it's already inactive, no-op. Rationale: the current state is >> the desired state. >> >> - both @keyslot and @old-secret present >> >> The keyslot given by keyslot. >> >> If it's inactive or holds a secret other than @old-secret, error. > Yea, that would be very nice to have. >> >> Option: error regardless of @old-secret, if that makes things >> simpler. >> >> - neither @keyslot not @old-secret present >> >> All keyslots. Note that this will error out due to "desired state >> has no active keyslots" unless the current state has none, either. >> >> Option: error out unconditionally. > Yep, that the best IMHO. It's a matter of taste. If we interpret "both absent" as "all keyslots", then all cases flow out of the following simple spec: 0. Start with the set of all keyslots 1. If @old-secret is present, interset it with the set of slots holding that secret. 2. If @keyslots is present, intersect it with the set of slots with that slot number. The order of steps 1 and 2 doesn't matter. To error out unconditionally, we have to make "both absent" a special case. A good way to resolve such matters of taste is to try writing doc comments for all proposals. If you find you hate one of them much less, you have a winner :) [...]