[PATCH] crypto: purge 'loaded' property that was documented as already removed

[PATCH] crypto: purge 'loaded' property that was documented as already removed

[Daniel P. Berrangé]

The 'loaded' property on TLS creds and secret objects was marked as
deprected in 6.0.0 and then marked as removed in 7.1.0.

Except it wasn't actually removed, it was just made read-only, while
claiming it was removed. Finish the long overdue removal job.

Fixes: 0310641c06dd5f7ea031b2b6170cb2edc63e4cea
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 crypto/secret_common.c          | 12 --------
 crypto/tlscredsanon.c           | 35 ----------------------
 crypto/tlscredspsk.c            | 34 ----------------------
 crypto/tlscredsx509.c           | 30 -------------------
 docs/about/removed-features.rst |  6 ++--
 qapi/crypto.json                | 51 +++------------------------------
 6 files changed, 7 insertions(+), 161 deletions(-)

diff --git a/crypto/secret_common.c b/crypto/secret_common.c
index 2c141107a5..dbda998940 100644
--- a/crypto/secret_common.c
+++ b/crypto/secret_common.c
@@ -191,15 +191,6 @@ qcrypto_secret_complete(UserCreatable *uc, Error **errp)
 }
 
 
-static bool
-qcrypto_secret_prop_get_loaded(Object *obj,
-                               Error **errp G_GNUC_UNUSED)
-{
-    QCryptoSecretCommon *secret = QCRYPTO_SECRET_COMMON(obj);
-    return secret->rawdata != NULL;
-}
-
-
 static void
 qcrypto_secret_prop_set_format(Object *obj,
                                int value,
@@ -278,9 +269,6 @@ qcrypto_secret_class_init(ObjectClass *oc, void *data)
 
     ucc->complete = qcrypto_secret_complete;
 
-    object_class_property_add_bool(oc, "loaded",
-                                   qcrypto_secret_prop_get_loaded,
-                                   NULL);
     object_class_property_add_enum(oc, "format",
                                    "QCryptoSecretFormat",
                                    &QCryptoSecretFormat_lookup,
diff --git a/crypto/tlscredsanon.c b/crypto/tlscredsanon.c
index c0d23a0ef3..476cf89c96 100644
--- a/crypto/tlscredsanon.c
+++ b/crypto/tlscredsanon.c
@@ -127,37 +127,6 @@ qcrypto_tls_creds_anon_complete(UserCreatable *uc, Error **errp)
 }
 
 
-#ifdef CONFIG_GNUTLS
-
-
-static bool
-qcrypto_tls_creds_anon_prop_get_loaded(Object *obj,
-                                       Error **errp G_GNUC_UNUSED)
-{
-    QCryptoTLSCredsAnon *creds = QCRYPTO_TLS_CREDS_ANON(obj);
-
-    if (creds->parent_obj.endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
-        return creds->data.server != NULL;
-    } else {
-        return creds->data.client != NULL;
-    }
-}
-
-
-#else /* ! CONFIG_GNUTLS */
-
-
-static bool
-qcrypto_tls_creds_anon_prop_get_loaded(Object *obj G_GNUC_UNUSED,
-                                       Error **errp G_GNUC_UNUSED)
-{
-    return false;
-}
-
-
-#endif /* ! CONFIG_GNUTLS */
-
-
 static void
 qcrypto_tls_creds_anon_finalize(Object *obj)
 {
@@ -173,10 +142,6 @@ qcrypto_tls_creds_anon_class_init(ObjectClass *oc, void *data)
     UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
 
     ucc->complete = qcrypto_tls_creds_anon_complete;
-
-    object_class_property_add_bool(oc, "loaded",
-                                   qcrypto_tls_creds_anon_prop_get_loaded,
-                                   NULL);
 }
 
 
diff --git a/crypto/tlscredspsk.c b/crypto/tlscredspsk.c
index 0d6b71a37c..aa270d7988 100644
--- a/crypto/tlscredspsk.c
+++ b/crypto/tlscredspsk.c
@@ -206,37 +206,6 @@ qcrypto_tls_creds_psk_complete(UserCreatable *uc, Error **errp)
 }
 
 
-#ifdef CONFIG_GNUTLS
-
-
-static bool
-qcrypto_tls_creds_psk_prop_get_loaded(Object *obj,
-                                      Error **errp G_GNUC_UNUSED)
-{
-    QCryptoTLSCredsPSK *creds = QCRYPTO_TLS_CREDS_PSK(obj);
-
-    if (creds->parent_obj.endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
-        return creds->data.server != NULL;
-    } else {
-        return creds->data.client != NULL;
-    }
-}
-
-
-#else /* ! CONFIG_GNUTLS */
-
-
-static bool
-qcrypto_tls_creds_psk_prop_get_loaded(Object *obj G_GNUC_UNUSED,
-                                      Error **errp G_GNUC_UNUSED)
-{
-    return false;
-}
-
-
-#endif /* ! CONFIG_GNUTLS */
-
-
 static void
 qcrypto_tls_creds_psk_finalize(Object *obj)
 {
@@ -273,9 +242,6 @@ qcrypto_tls_creds_psk_class_init(ObjectClass *oc, void *data)
 
     ucc->complete = qcrypto_tls_creds_psk_complete;
 
-    object_class_property_add_bool(oc, "loaded",
-                                   qcrypto_tls_creds_psk_prop_get_loaded,
-                                   NULL);
     object_class_property_add_str(oc, "username",
                                   qcrypto_tls_creds_psk_prop_get_username,
                                   qcrypto_tls_creds_psk_prop_set_username);
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index d14313925d..24ec584922 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -695,33 +695,6 @@ qcrypto_tls_creds_x509_complete(UserCreatable *uc, Error **errp)
 }
 
 
-#ifdef CONFIG_GNUTLS
-
-
-static bool
-qcrypto_tls_creds_x509_prop_get_loaded(Object *obj,
-                                       Error **errp G_GNUC_UNUSED)
-{
-    QCryptoTLSCredsX509 *creds = QCRYPTO_TLS_CREDS_X509(obj);
-
-    return creds->data != NULL;
-}
-
-
-#else /* ! CONFIG_GNUTLS */
-
-
-static bool
-qcrypto_tls_creds_x509_prop_get_loaded(Object *obj G_GNUC_UNUSED,
-                                       Error **errp G_GNUC_UNUSED)
-{
-    return false;
-}
-
-
-#endif /* ! CONFIG_GNUTLS */
-
-
 static void
 qcrypto_tls_creds_x509_prop_set_sanity(Object *obj,
                                        bool value,
@@ -838,9 +811,6 @@ qcrypto_tls_creds_x509_class_init(ObjectClass *oc, void *data)
 
     ucc->complete = qcrypto_tls_creds_x509_complete;
 
-    object_class_property_add_bool(oc, "loaded",
-                                   qcrypto_tls_creds_x509_prop_get_loaded,
-                                   NULL);
     object_class_property_add_bool(oc, "sanity-check",
                                    qcrypto_tls_creds_x509_prop_get_sanity,
                                    qcrypto_tls_creds_x509_prop_set_sanity);
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 912e0a1fcf..59a132b8ce 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -355,13 +355,13 @@ The ``-writeconfig`` option was not able to serialize the entire contents
 of the QEMU command line.  It is thus considered a failed experiment
 and removed without a replacement.
 
-``loaded`` property of ``secret`` and ``secret_keyring`` objects (removed in 7.1)
-'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+``loaded`` property of secret and TLS credential objects (removed in 7.1)
+'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
 
 The ``loaded=on`` option in the command line or QMP ``object-add`` either had
 no effect (if ``loaded`` was the last option) or caused options to be
 effectively ignored as if they were not given.  The property is therefore
-useless and should simply be removed.
+useless and has been removed.
 
 ``opened`` property of ``rng-*`` objects (removed in 7.1)
 '''''''''''''''''''''''''''''''''''''''''''''''''''''''''
diff --git a/qapi/crypto.json b/qapi/crypto.json
index 9431522768..7c30df9e31 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -419,11 +419,6 @@
 #
 # 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)
 #
@@ -436,16 +431,10 @@
 #     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',
+  'data': { '*format': 'QCryptoSecretFormat',
             '*keyid': 'str',
             '*iv': 'str' } }
 
@@ -512,58 +501,32 @@
 #
 # 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'] } } }
+  'data': { } }
 
 ##
 # @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' } }
+  'data': { '*username': 'str' } }
 
 ##
 # @TlsCredsX509Properties:
 #
 # Properties for tls-creds-x509 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)
-#
 # @sanity-check: if true, perform some sanity checks before using the
 #     credentials (default: true)
 #
@@ -573,17 +536,11 @@
 #     provides the ID of a previously created secret object containing
 #     the password for decryption.
 #
-# Features:
-#
-# @deprecated: Member @loaded is deprecated.  Setting true doesn't
-#     make sense, and false is already the default.
-#
 # Since: 2.5
 ##
 { 'struct': 'TlsCredsX509Properties',
   'base': 'TlsCredsProperties',
-  'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] },
-            '*sanity-check': 'bool',
+  'data': { '*sanity-check': 'bool',
             '*passwordid': 'str' } }
 ##
 # @QCryptoAkCipherAlgo:
-- 
2.46.0

Re: [PATCH] crypto: purge 'loaded' property that was documented as already removed

[Philippe Mathieu-Daudé]

On 30/10/24 05:47, Daniel P. Berrangé wrote:
> The 'loaded' property on TLS creds and secret objects was marked as
> deprected in 6.0.0 and then marked as removed in 7.1.0.
> 
> Except it wasn't actually removed, it was just made read-only, while
> claiming it was removed. Finish the long overdue removal job.
> 
> Fixes: 0310641c06dd5f7ea031b2b6170cb2edc63e4cea
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   crypto/secret_common.c          | 12 --------
>   crypto/tlscredsanon.c           | 35 ----------------------
>   crypto/tlscredspsk.c            | 34 ----------------------
>   crypto/tlscredsx509.c           | 30 -------------------
>   docs/about/removed-features.rst |  6 ++--
>   qapi/crypto.json                | 51 +++------------------------------
>   6 files changed, 7 insertions(+), 161 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH] crypto: purge 'loaded' property that was documented as already removed

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> The 'loaded' property on TLS creds and secret objects was marked as
> deprected in 6.0.0 and then marked as removed in 7.1.0.

deprecated

Regarding "marked as removed": not quite.  Its entry was moved from
docs/about/deprecated.rst to docs/about/removed-features.rst, but the
text there is "should simply be removed."

>
> Except it wasn't actually removed, it was just made read-only, while
> claiming it was removed. Finish the long overdue removal job.
>
> Fixes: 0310641c06dd5f7ea031b2b6170cb2edc63e4cea

I'm not sure it fixes something that was broken.  Commit 0310641c06d
(crypto: make loaded property read-only) did what it said on the tin.
What it did was unusual, and maybe a bad idea.

Anyway, not important now.

> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH] crypto: purge 'loaded' property that was documented as already removed

[Daniel P. Berrangé]

On Mon, Nov 04, 2024 at 09:09:40AM +0100, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > The 'loaded' property on TLS creds and secret objects was marked as
> > deprected in 6.0.0 and then marked as removed in 7.1.0.
> 
> deprecated
> 
> Regarding "marked as removed": not quite.  Its entry was moved from
> docs/about/deprecated.rst to docs/about/removed-features.rst, but the
> text there is "should simply be removed."
> 
> >
> > Except it wasn't actually removed, it was just made read-only, while
> > claiming it was removed. Finish the long overdue removal job.
> >
> > Fixes: 0310641c06dd5f7ea031b2b6170cb2edc63e4cea
> 
> I'm not sure it fixes something that was broken.  Commit 0310641c06d
> (crypto: make loaded property read-only) did what it said on the tin.
> What it did was unusual, and maybe a bad idea.

I'm re-wording the commit message to:

  The 'loaded' property on TLS creds and secret objects was marked as
  deprecated in 6.0.0. In 7.1.0 the deprecation info was moved into
  the 'removed-features.rst' file, but the property was not actually
  removed, just made read-only. This was a highly unusual practice,
  so finish the long overdue removal job.

and removing the "Fixes" tag

> 
> Anyway, not important now.
> 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] crypto: purge 'loaded' property that was documented as already removed

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Mon, Nov 04, 2024 at 09:09:40AM +0100, Markus Armbruster wrote:
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>> 
>> > The 'loaded' property on TLS creds and secret objects was marked as
>> > deprected in 6.0.0 and then marked as removed in 7.1.0.
>> 
>> deprecated
>> 
>> Regarding "marked as removed": not quite.  Its entry was moved from
>> docs/about/deprecated.rst to docs/about/removed-features.rst, but the
>> text there is "should simply be removed."
>> 
>> >
>> > Except it wasn't actually removed, it was just made read-only, while
>> > claiming it was removed. Finish the long overdue removal job.
>> >
>> > Fixes: 0310641c06dd5f7ea031b2b6170cb2edc63e4cea
>> 
>> I'm not sure it fixes something that was broken.  Commit 0310641c06d
>> (crypto: make loaded property read-only) did what it said on the tin.
>> What it did was unusual, and maybe a bad idea.
>
> I'm re-wording the commit message to:
>
>   The 'loaded' property on TLS creds and secret objects was marked as
>   deprecated in 6.0.0. In 7.1.0 the deprecation info was moved into
>   the 'removed-features.rst' file, but the property was not actually
>   removed, just made read-only. This was a highly unusual practice,
>   so finish the long overdue removal job.
>
> and removing the "Fixes" tag

Thanks!

[...]