| Message ID | 20240319153908.194975-1-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [for-9.0] qapi: drop unused QCryptoBlockCreateOptionsLUKS.detached-header | expand |
Daniel P. Berrangé <berrange@redhat.com> writes: > The 'detached-header' field in QCryptoBlockCreateOptionsLUKS > was left over from earlier patch iterations. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > qapi/crypto.json | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/qapi/crypto.json b/qapi/crypto.json > index 931c88e688..ad9e3d0297 100644 > --- a/qapi/crypto.json > +++ b/qapi/crypto.json > @@ -226,8 +226,6 @@ > # @iter-time: number of milliseconds to spend in PBKDF passphrase > # processing. Currently defaults to 2000. (since 2.8) > # > -# @detached-header: create a detached LUKS header. (since 9.0) > -# > # Since: 2.6 > ## > { 'struct': 'QCryptoBlockCreateOptionsLUKS', > @@ -237,8 +235,7 @@ > '*ivgen-alg': 'QCryptoIVGenAlgorithm', > '*ivgen-hash-alg': 'QCryptoHashAlgorithm', > '*hash-alg': 'QCryptoHashAlgorithm', > - '*iter-time': 'int', > - '*detached-header': 'bool'}} > + '*iter-time': 'int' }} > > ## > # @QCryptoBlockOpenOptions: Glad you caught this in time. Acked-by: Markus Armbruster <armbru@redhat.com>
On Tue, Mar 19, 2024 at 05:06:07PM +0100, Markus Armbruster wrote: > Daniel P. Berrangé <berrange@redhat.com> writes: > > > The 'detached-header' field in QCryptoBlockCreateOptionsLUKS > > was left over from earlier patch iterations. > > > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > > --- > > qapi/crypto.json | 5 +---- > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > diff --git a/qapi/crypto.json b/qapi/crypto.json > > index 931c88e688..ad9e3d0297 100644 > > --- a/qapi/crypto.json > > +++ b/qapi/crypto.json > > @@ -226,8 +226,6 @@ > > # @iter-time: number of milliseconds to spend in PBKDF passphrase > > # processing. Currently defaults to 2000. (since 2.8) > > # > > -# @detached-header: create a detached LUKS header. (since 9.0) > > -# > > # Since: 2.6 > > ## > > { 'struct': 'QCryptoBlockCreateOptionsLUKS', > > @@ -237,8 +235,7 @@ > > '*ivgen-alg': 'QCryptoIVGenAlgorithm', > > '*ivgen-hash-alg': 'QCryptoHashAlgorithm', > > '*hash-alg': 'QCryptoHashAlgorithm', > > - '*iter-time': 'int', > > - '*detached-header': 'bool'}} > > + '*iter-time': 'int' }} > > > > ## > > # @QCryptoBlockOpenOptions: > > Glad you caught this in time. > > Acked-by: Markus Armbruster <armbru@redhat.com> Urgh, it appears this patch got lost and was not in fact included in 9.0. What's your opinion on removing it from 9.1 with no deprecation period ? Functionally it did absolutely nothing even if used, so removing it at most would cause an incorrect field to now be rejected. With regards, Daniel
Daniel P. Berrangé <berrange@redhat.com> writes: > On Tue, Mar 19, 2024 at 05:06:07PM +0100, Markus Armbruster wrote: >> Daniel P. Berrangé <berrange@redhat.com> writes: >> >> > The 'detached-header' field in QCryptoBlockCreateOptionsLUKS >> > was left over from earlier patch iterations. >> > >> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> >> > --- >> > qapi/crypto.json | 5 +---- >> > 1 file changed, 1 insertion(+), 4 deletions(-) >> > >> > diff --git a/qapi/crypto.json b/qapi/crypto.json >> > index 931c88e688..ad9e3d0297 100644 >> > --- a/qapi/crypto.json >> > +++ b/qapi/crypto.json >> > @@ -226,8 +226,6 @@ >> > # @iter-time: number of milliseconds to spend in PBKDF passphrase >> > # processing. Currently defaults to 2000. (since 2.8) >> > # >> > -# @detached-header: create a detached LUKS header. (since 9.0) >> > -# >> > # Since: 2.6 >> > ## >> > { 'struct': 'QCryptoBlockCreateOptionsLUKS', >> > @@ -237,8 +235,7 @@ >> > '*ivgen-alg': 'QCryptoIVGenAlgorithm', >> > '*ivgen-hash-alg': 'QCryptoHashAlgorithm', >> > '*hash-alg': 'QCryptoHashAlgorithm', >> > - '*iter-time': 'int', >> > - '*detached-header': 'bool'}} >> > + '*iter-time': 'int' }} >> > >> > ## >> > # @QCryptoBlockOpenOptions: >> >> Glad you caught this in time. >> >> Acked-by: Markus Armbruster <armbru@redhat.com> > > Urgh, it appears this patch got lost and was not in fact included in > 9.0. What's your opinion on removing it from 9.1 with no deprecation > period ? Functionally it did absolutely nothing even if used, so > removing it at most would cause an incorrect field to now be rejected. Doc section "Deprecated features": In general features are intended to be supported indefinitely once introduced into QEMU. In the event that a feature needs to be removed, it will be listed in this section. The feature will remain functional for the release in which it was deprecated and one further release. After these two releases, the feature is liable to be removed. Deprecated features may also generate warnings on the console when QEMU starts up, or if activated via a monitor command, however, this is not a mandatory requirement. Note this talks about *features*. The software industry has a longstanding habit of declaring bugs features. By removing @detached-header without a deprecation period, we effectively do the opposite. The opposite of something bad must be good, right? ;-) Seriously, it's a judgement call. I'm happy to defer to your judgement here.
On Mon, Jul 22, 2024 at 04:07:28PM +0200, Markus Armbruster wrote: > Daniel P. Berrangé <berrange@redhat.com> writes: > > > On Tue, Mar 19, 2024 at 05:06:07PM +0100, Markus Armbruster wrote: > >> Daniel P. Berrangé <berrange@redhat.com> writes: > >> > >> > The 'detached-header' field in QCryptoBlockCreateOptionsLUKS > >> > was left over from earlier patch iterations. > >> > > >> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > >> > --- > >> > qapi/crypto.json | 5 +---- > >> > 1 file changed, 1 insertion(+), 4 deletions(-) > >> > > >> > diff --git a/qapi/crypto.json b/qapi/crypto.json > >> > index 931c88e688..ad9e3d0297 100644 > >> > --- a/qapi/crypto.json > >> > +++ b/qapi/crypto.json > >> > @@ -226,8 +226,6 @@ > >> > # @iter-time: number of milliseconds to spend in PBKDF passphrase > >> > # processing. Currently defaults to 2000. (since 2.8) > >> > # > >> > -# @detached-header: create a detached LUKS header. (since 9.0) > >> > -# > >> > # Since: 2.6 > >> > ## > >> > { 'struct': 'QCryptoBlockCreateOptionsLUKS', > >> > @@ -237,8 +235,7 @@ > >> > '*ivgen-alg': 'QCryptoIVGenAlgorithm', > >> > '*ivgen-hash-alg': 'QCryptoHashAlgorithm', > >> > '*hash-alg': 'QCryptoHashAlgorithm', > >> > - '*iter-time': 'int', > >> > - '*detached-header': 'bool'}} > >> > + '*iter-time': 'int' }} > >> > > >> > ## > >> > # @QCryptoBlockOpenOptions: > >> > >> Glad you caught this in time. > >> > >> Acked-by: Markus Armbruster <armbru@redhat.com> > > > > Urgh, it appears this patch got lost and was not in fact included in > > 9.0. What's your opinion on removing it from 9.1 with no deprecation > > period ? Functionally it did absolutely nothing even if used, so > > removing it at most would cause an incorrect field to now be rejected. > > Doc section "Deprecated features": > > In general features are intended to be supported indefinitely once > introduced into QEMU. In the event that a feature needs to be removed, > it will be listed in this section. The feature will remain functional for the > release in which it was deprecated and one further release. After these two > releases, the feature is liable to be removed. Deprecated features may also > generate warnings on the console when QEMU starts up, or if activated via a > monitor command, however, this is not a mandatory requirement. > > Note this talks about *features*. > > The software industry has a longstanding habit of declaring bugs > features. By removing @detached-header without a deprecation period, we > effectively do the opposite. The opposite of something bad must be > good, right? ;-) > > Seriously, it's a judgement call. I'm happy to defer to your judgement > here. Ok, I'm going to include this in 9.1. The chances of something already using the new 'detached header' feature from 9.0 is small. Out of that small set, the chances of something setting the (useless) 'detached-header' QAPI field is even smaller. With regards, Daniel
diff --git a/qapi/crypto.json b/qapi/crypto.json index 931c88e688..ad9e3d0297 100644 --- a/qapi/crypto.json +++ b/qapi/crypto.json @@ -226,8 +226,6 @@ # @iter-time: number of milliseconds to spend in PBKDF passphrase # processing. Currently defaults to 2000. (since 2.8) # -# @detached-header: create a detached LUKS header. (since 9.0) -# # Since: 2.6 ## { 'struct': 'QCryptoBlockCreateOptionsLUKS', @@ -237,8 +235,7 @@ '*ivgen-alg': 'QCryptoIVGenAlgorithm', '*ivgen-hash-alg': 'QCryptoHashAlgorithm', '*hash-alg': 'QCryptoHashAlgorithm', - '*iter-time': 'int', - '*detached-header': 'bool'}} + '*iter-time': 'int' }} ## # @QCryptoBlockOpenOptions:
The 'detached-header' field in QCryptoBlockCreateOptionsLUKS was left over from earlier patch iterations. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- qapi/crypto.json | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)