Re: [PATCH v2 11/31] qapi/qom: Add ObjectOptions for tls-*, deprecate 'loaded'

On 2/24/21 7:52 AM, Kevin Wolf wrote: > This adds a QAPI schema for the properties of the tls-* objects. > > The 'loaded' property doesn't seem to make sense as an external > interface: It is automatically set to true in ucc->complete, and > explicitly setting it to true earlier just means that additional options > will be silently ignored. > > In other words, the 'loaded' property is useless. Mark it as deprecated > in the schema from the start. > > Signed-off-by: Kevin Wolf <kwolf(a)redhat.com&gt; > --- > qapi/crypto.json | 98 ++++++++++++++++++++++++++++++++++++++++++++++++ > qapi/qom.json | 12 +++++- > 2 files changed, 108 insertions(+), 2 deletions(-) > > diff --git a/qapi/crypto.json b/qapi/crypto.json > index 0fef3de66d..7116ae9a46 100644 > --- a/qapi/crypto.json > +++ b/qapi/crypto.json > @@ -442,3 +442,101 @@ > { 'struct': 'SecretKeyringProperties', > 'base': 'SecretCommonProperties', > 'data': { 'serial': 'int32' } } > + > +## > +# @TlsCredsProperties: > +# > +# Properties for objects of classes derived from tls-creds. > +# > +# @verify-peer: if true the peer credentials will be verified once the > +# handshake is completed. This is a no-op for anonymous > +# credentials. (default: true) > +# > +# @dir: the path of the directory that contains the credential files > +# > +# @endpoint: whether the QEMU network backend that uses the credentials will be > +# acting as a client or as a server (default: client) > +# > +# @priority: a gnutls priority string as described at > +# https://gnutls.org/manual/html_node/Priority-Strings.html > +# > +# Since: 2.5 > +## > +{ 'struct': 'TlsCredsProperties', > + 'data': { '*verify-peer': 'bool', > + '*dir': 'str', > + '*endpoint': 'QCryptoTLSCredsEndpoint', > + '*priority': 'str' } } Matches crypto/tlscreds.c:qcrypto_tls_creds_class_init(). > + > +## > +# @TlsCredsAnonProperties: > +# > +# Properties for tls-creds-anon objects. > +# > +# @loaded: if true, the credentials are loaded immediately when applying this > +# option and will ignore options that are processed later. Don't use; > +# only provided for compatibility. (default: false) > +# > +# Features: > +# @deprecated: Member @loaded is deprecated. Setting true doesn't make sense, > +# and false is already the default. > +# > +# Since: 2.5 > +## > +{ 'struct': 'TlsCredsAnonProperties', > + 'base': 'TlsCredsProperties', > + 'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] } } } Since we documented that 'verify-peer' is a no-op for this struct, is it worth altering our type hierarchy to make it explicit, as in: TlsCredsCommonProperties - dir, endpoint, priority TlsCredsProperties - TlsCredsCommonProperties + verify-peer TlsCredsAnonProperties - TlsCredsCommonProperties + loaded TlsCredsPskProperties - TlsCredsProperties + loaded, username But even if not, this matches crypto/tlscredsanon.c:qcrypto_tls_creds_anon_class_init().

> + > +## > +# @TlsCredsPskProperties: > +# > +# Properties for tls-creds-psk objects. > +# > +# @loaded: if true, the credentials are loaded immediately when applying this > +# option and will ignore options that are processed later. Don't use; > +# only provided for compatibility. (default: false) > +# > +# @username: the username which will be sent to the server. For clients only. > +# If absent, "qemu" is sent and the property will read back as an > +# empty string. > +# > +# Features: > +# @deprecated: Member @loaded is deprecated. Setting true doesn't make sense, > +# and false is already the default. > +# > +# Since: 3.0 > +## > +{ 'struct': 'TlsCredsPskProperties', > + 'base': 'TlsCredsProperties', > + 'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] }, > + '*username': 'str' } } This matches crypto/tlscredspsk.c:qcrypto_tls_creds_psk_class_init(). Do we want to use QAPI type inheritance to declare a union where 'endpoint' is the union discriminator, and 'username' is only present for 'endpoint':'client'? (Hmm, we'd have to improve the QAPI code generator to allow a flat union as the branch of yet another flat union...)