| Message ID | 20210224135255.253837-11-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qapi/qom: QAPIfy --object and object-add | expand |
On 2/24/21 7:52 AM, Kevin Wolf wrote: > This adds a QAPI schema for the properties of the secret* 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@redhat.com> > --- > qapi/crypto.json | 61 ++++++++++++++++++++++++++++++++++++++ > qapi/qom.json | 5 ++++ > docs/system/deprecated.rst | 11 +++++++ > 3 files changed, 77 insertions(+) > > diff --git a/qapi/crypto.json b/qapi/crypto.json > index 2aebe6fa20..0fef3de66d 100644 > --- a/qapi/crypto.json > +++ b/qapi/crypto.json > @@ -381,3 +381,64 @@ > 'discriminator': 'format', > 'data': { > 'luks': 'QCryptoBlockAmendOptionsLUKS' } } > + > +## > +# @SecretCommonProperties: > +# > +# Properties for objects of classes derived from secret-common. > +# > +# @loaded: if true, the secret is loaded immediately when applying this option > +# and will probably fail when processing the next option. Don't use; > +# only provided for compatibility. (default: false) > +# > +# @format: the data format that the secret is provided in (default: raw) > +# > +# @keyid: the name of another secret that should be used to decrypt the > +# provided data. If not present, the data is assumed to be unencrypted. > +# > +# @iv: the random initialization vector used for encryption of this particular > +# secret. Should be a base64 encrypted string of the 16-byte IV. Mandatory > +# if @keyid is given. Ignored if @keyid is absent. > +# > +# Features: > +# @deprecated: Member @loaded is deprecated. Setting true doesn't make sense, > +# and false is already the default. > +# > +# Since: 2.6 > +## > +{ 'struct': 'SecretCommonProperties', > + 'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] }, > + '*format': 'QCryptoSecretFormat', > + '*keyid': 'str', > + '*iv': 'str' } } Matches crypto/secret_common.c:qcrypto_secret_class_init(), and I concur with the deprecation. > + > +## > +# @SecretProperties: > +# > +# Properties for secret objects. > +# > +# Either @data or @file must be provided, but not both. > +# > +# @data: the associated with the secret from > +# > +# @file: the filename to load the data associated with the secret from > +# > +# Since: 2.6 > +## > +{ 'struct': 'SecretProperties', > + 'base': 'SecretCommonProperties', > + 'data': { '*data': 'str', > + '*file': 'str' } } Matches crypto/secret.c:qcrypto_secret_class_init() (ugh, we really do reuse the same static function name in two different files, but not your fault) > + > +## > +# @SecretKeyringProperties: > +# > +# Properties for secret_keyring objects. > +# > +# @serial: serial number that identifies a key to get from the kernel > +# > +# Since: 5.1 > +## > +{ 'struct': 'SecretKeyringProperties', > + 'base': 'SecretCommonProperties', > + 'data': { 'serial': 'int32' } } Matches crypto/secret_keyring.c:qcrypto_secret_keyring_class_init(). > diff --git a/qapi/qom.json b/qapi/qom.json > index 449dca8ec5..2668ad8369 100644 > --- a/qapi/qom.json > +++ b/qapi/qom.json > @@ -7,6 +7,7 @@ > { 'include': 'authz.json' } > { 'include': 'block-core.json' } > { 'include': 'common.json' } > +{ 'include': 'crypto.json' } > > ## > # = QEMU Object Model (QOM) > @@ -449,6 +450,8 @@ > 'rng-builtin', > 'rng-egd', > 'rng-random', > + 'secret', > + 'secret_keyring', What is stopping us from naming this 'secret-keyring'? > 'throttle-group' > ] } > > @@ -483,6 +486,8 @@ > 'rng-builtin': 'RngProperties', > 'rng-egd': 'RngEgdProperties', > 'rng-random': 'RngRandomProperties', > + 'secret': 'SecretProperties', > + 'secret_keyring': 'SecretKeyringProperties', > 'throttle-group': 'ThrottleGroupProperties' > } } > > diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst > index 79991c2893..78b175cb59 100644 > --- a/docs/system/deprecated.rst > +++ b/docs/system/deprecated.rst > @@ -155,6 +155,17 @@ other options have been processed. This will either have no effect (if > ``opened`` was the last option) or cause errors. The property is therefore > useless and should not be specified. > > +``loaded`` property of ``secret`` and ``secret_keyring`` objects (since 6.0.0) > +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' > + > +The only effect of specifying ``loaded=on`` in the command line or QMP > +``object-add`` is that the secret is loaded immediately, possibly before all > +other options have been processed. This will either have no effect (if > +``loaded`` was the last option) or cause options to be effectively ignored as > +if they were not given. The property is therefore useless and should not be > +specified. May be impacted if we rename to secret-keyring (in fact, if we rename, the new name wouldn't even need the deprecated field), but that may be trickier to coordinate. So with regards to just the mechanical conversion, Reviewed-by: Eric Blake <eblake@redhat.com>
Am 26.02.2021 um 20:17 hat Eric Blake geschrieben: > On 2/24/21 7:52 AM, Kevin Wolf wrote: > > diff --git a/qapi/qom.json b/qapi/qom.json > > index 449dca8ec5..2668ad8369 100644 > > --- a/qapi/qom.json > > +++ b/qapi/qom.json > > @@ -7,6 +7,7 @@ > > { 'include': 'authz.json' } > > { 'include': 'block-core.json' } > > { 'include': 'common.json' } > > +{ 'include': 'crypto.json' } > > > > ## > > # = QEMU Object Model (QOM) > > @@ -449,6 +450,8 @@ > > 'rng-builtin', > > 'rng-egd', > > 'rng-random', > > + 'secret', > > + 'secret_keyring', > > What is stopping us from naming this 'secret-keyring'? That it's not the name of the QOM type, so it wouldn't be possible to create an object from it any more ('secret_keyring' would be rejected by QAPI, but 'secret-keyring' would be rejected by QOM). If we ever want to rename the type, this might be a case where QAPI aliases could help. But I'm almost sure that there would be more compatibility concerns than just with object creation for renaming a user creatable QOM type. Kevin
diff --git a/qapi/crypto.json b/qapi/crypto.json index 2aebe6fa20..0fef3de66d 100644 --- a/qapi/crypto.json +++ b/qapi/crypto.json @@ -381,3 +381,64 @@ 'discriminator': 'format', 'data': { 'luks': 'QCryptoBlockAmendOptionsLUKS' } } + +## +# @SecretCommonProperties: +# +# Properties for objects of classes derived from secret-common. +# +# @loaded: if true, the secret is loaded immediately when applying this option +# and will probably fail when processing the next option. Don't use; +# only provided for compatibility. (default: false) +# +# @format: the data format that the secret is provided in (default: raw) +# +# @keyid: the name of another secret that should be used to decrypt the +# provided data. If not present, the data is assumed to be unencrypted. +# +# @iv: the random initialization vector used for encryption of this particular +# secret. Should be a base64 encrypted string of the 16-byte IV. Mandatory +# if @keyid is given. Ignored if @keyid is absent. +# +# Features: +# @deprecated: Member @loaded is deprecated. Setting true doesn't make sense, +# and false is already the default. +# +# Since: 2.6 +## +{ 'struct': 'SecretCommonProperties', + 'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] }, + '*format': 'QCryptoSecretFormat', + '*keyid': 'str', + '*iv': 'str' } } + +## +# @SecretProperties: +# +# Properties for secret objects. +# +# Either @data or @file must be provided, but not both. +# +# @data: the associated with the secret from +# +# @file: the filename to load the data associated with the secret from +# +# Since: 2.6 +## +{ 'struct': 'SecretProperties', + 'base': 'SecretCommonProperties', + 'data': { '*data': 'str', + '*file': 'str' } } + +## +# @SecretKeyringProperties: +# +# Properties for secret_keyring objects. +# +# @serial: serial number that identifies a key to get from the kernel +# +# Since: 5.1 +## +{ 'struct': 'SecretKeyringProperties', + 'base': 'SecretCommonProperties', + 'data': { 'serial': 'int32' } } diff --git a/qapi/qom.json b/qapi/qom.json index 449dca8ec5..2668ad8369 100644 --- a/qapi/qom.json +++ b/qapi/qom.json @@ -7,6 +7,7 @@ { 'include': 'authz.json' } { 'include': 'block-core.json' } { 'include': 'common.json' } +{ 'include': 'crypto.json' } ## # = QEMU Object Model (QOM) @@ -449,6 +450,8 @@ 'rng-builtin', 'rng-egd', 'rng-random', + 'secret', + 'secret_keyring', 'throttle-group' ] } @@ -483,6 +486,8 @@ 'rng-builtin': 'RngProperties', 'rng-egd': 'RngEgdProperties', 'rng-random': 'RngRandomProperties', + 'secret': 'SecretProperties', + 'secret_keyring': 'SecretKeyringProperties', 'throttle-group': 'ThrottleGroupProperties' } } diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 79991c2893..78b175cb59 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -155,6 +155,17 @@ other options have been processed. This will either have no effect (if ``opened`` was the last option) or cause errors. The property is therefore useless and should not be specified. +``loaded`` property of ``secret`` and ``secret_keyring`` objects (since 6.0.0) +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' + +The only effect of specifying ``loaded=on`` in the command line or QMP +``object-add`` is that the secret is loaded immediately, possibly before all +other options have been processed. This will either have no effect (if +``loaded`` was the last option) or cause options to be effectively ignored as +if they were not given. The property is therefore useless and should not be +specified. + + QEMU Machine Protocol (QMP) commands ------------------------------------
This adds a QAPI schema for the properties of the secret* 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@redhat.com> --- qapi/crypto.json | 61 ++++++++++++++++++++++++++++++++++++++ qapi/qom.json | 5 ++++ docs/system/deprecated.rst | 11 +++++++ 3 files changed, 77 insertions(+)