qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] tests/qemu-iotests: Check for a functional "secret" object before using it
@ 2025-12-05 13:00 Thomas Huth
  2025-12-05 15:19 ` Alex Bennée
  2025-12-05 17:20 ` Kevin Wolf
  0 siblings, 2 replies; 5+ messages in thread
From: Thomas Huth @ 2025-12-05 13:00 UTC (permalink / raw)
  To: qemu-block, Kevin Wolf, Alex Bennée; +Cc: Hanna Reitz, qemu-devel

From: Thomas Huth <thuth@redhat.com>

QEMU iotests 049, 134 and 158 are currently failing if you compiled
QEMU without the crypto libraries. Thus make sure that the "secret"
object is really usable and skip the tests otherwise.

Reported-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qemu-iotests/049       |  2 ++
 tests/qemu-iotests/134       |  1 +
 tests/qemu-iotests/158       |  1 +
 tests/qemu-iotests/common.rc | 14 ++++++++++++++
 4 files changed, 18 insertions(+)

diff --git a/tests/qemu-iotests/049 b/tests/qemu-iotests/049
index ed12fa49d7f..a1b922060db 100755
--- a/tests/qemu-iotests/049
+++ b/tests/qemu-iotests/049
@@ -39,6 +39,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt qcow2
 _supported_proto file
+_require_secret
+
 
 filter_test_dir()
 {
diff --git a/tests/qemu-iotests/134 b/tests/qemu-iotests/134
index b2c3c03f08b..cc1e35eb161 100755
--- a/tests/qemu-iotests/134
+++ b/tests/qemu-iotests/134
@@ -39,6 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt qcow qcow2
 _supported_proto file
+_require_secret
 
 
 size=128M
diff --git a/tests/qemu-iotests/158 b/tests/qemu-iotests/158
index 3a9ad7eed03..8fc4e986532 100755
--- a/tests/qemu-iotests/158
+++ b/tests/qemu-iotests/158
@@ -39,6 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt qcow qcow2
 _supported_proto file
+_require_secret
 
 
 size=128M
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index e977cb4eb61..10d83d8361b 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -1053,6 +1053,20 @@ _require_one_device_of()
     _notrun "$* not available"
 }
 
+_require_secret()
+{
+    if [ -e "$TEST_IMG" ]; then
+        echo "unwilling to overwrite existing file"
+        exit 1
+    fi
+    if $QEMU_IMG create -f $IMGFMT --object secret,id=sec0,data=123 \
+                 -o encryption=on,encrypt.key-secret=sec0 "$TEST_IMG" 1M 2>&1 \
+                 | grep "Unsupported cipher" ; then
+        _notrun "missing cipher support"
+    fi
+    rm -f "$TEST_IMG"
+}
+
 _qcow2_dump_header()
 {
     if [[ "$1" == "--no-filter-compression" ]]; then
-- 
2.52.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] tests/qemu-iotests: Check for a functional "secret" object before using it
  2025-12-05 13:00 [PATCH] tests/qemu-iotests: Check for a functional "secret" object before using it Thomas Huth
@ 2025-12-05 15:19 ` Alex Bennée
  2025-12-05 17:20 ` Kevin Wolf
  1 sibling, 0 replies; 5+ messages in thread
From: Alex Bennée @ 2025-12-05 15:19 UTC (permalink / raw)
  To: Thomas Huth; +Cc: qemu-block, Kevin Wolf, Hanna Reitz, qemu-devel

Thomas Huth <thuth@redhat.com> writes:

> From: Thomas Huth <thuth@redhat.com>
>
> QEMU iotests 049, 134 and 158 are currently failing if you compiled
> QEMU without the crypto libraries. Thus make sure that the "secret"
> object is really usable and skip the tests otherwise.
>
> Reported-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Queued to pr/051225-10.2-final-fixes-1, thanks.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] tests/qemu-iotests: Check for a functional "secret" object before using it
  2025-12-05 13:00 [PATCH] tests/qemu-iotests: Check for a functional "secret" object before using it Thomas Huth
  2025-12-05 15:19 ` Alex Bennée
@ 2025-12-05 17:20 ` Kevin Wolf
  2025-12-08  8:15   ` Thomas Huth
  1 sibling, 1 reply; 5+ messages in thread
From: Kevin Wolf @ 2025-12-05 17:20 UTC (permalink / raw)
  To: Thomas Huth; +Cc: qemu-block, Alex Bennée, Hanna Reitz, qemu-devel

Am 05.12.2025 um 14:00 hat Thomas Huth geschrieben:
> From: Thomas Huth <thuth@redhat.com>
> 
> QEMU iotests 049, 134 and 158 are currently failing if you compiled
> QEMU without the crypto libraries. Thus make sure that the "secret"
> object is really usable and skip the tests otherwise.
> 
> Reported-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
> index e977cb4eb61..10d83d8361b 100644
> --- a/tests/qemu-iotests/common.rc
> +++ b/tests/qemu-iotests/common.rc
> @@ -1053,6 +1053,20 @@ _require_one_device_of()
>      _notrun "$* not available"
>  }
>  
> +_require_secret()
> +{
> +    if [ -e "$TEST_IMG" ]; then
> +        echo "unwilling to overwrite existing file"
> +        exit 1
> +    fi
> +    if $QEMU_IMG create -f $IMGFMT --object secret,id=sec0,data=123 \
> +                 -o encryption=on,encrypt.key-secret=sec0 "$TEST_IMG" 1M 2>&1 \
> +                 | grep "Unsupported cipher" ; then
> +        _notrun "missing cipher support"
> +    fi

What is the thing that you're checking here? If it's really the secret,
then just running 'qemu-io --object secret,data=123,id=sec0 -c ""' would
be enough. If it's not the secret, but encryption support, then the
function is a misnomer.

_require_working_luks() looks pretty similar, though it requires
specifically a working luks driver. Could something be unified? (The
answer might be no, but it would be good to explicitly say it.)

Kevin

> +    rm -f "$TEST_IMG"
> +}
> +
>  _qcow2_dump_header()
>  {
>      if [[ "$1" == "--no-filter-compression" ]]; then
> -- 
> 2.52.0
> 



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] tests/qemu-iotests: Check for a functional "secret" object before using it
  2025-12-05 17:20 ` Kevin Wolf
@ 2025-12-08  8:15   ` Thomas Huth
  2025-12-08  9:18     ` Daniel P. Berrangé
  0 siblings, 1 reply; 5+ messages in thread
From: Thomas Huth @ 2025-12-08  8:15 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: qemu-block, Alex Bennée, Hanna Reitz, qemu-devel

On 05/12/2025 18.20, Kevin Wolf wrote:
> Am 05.12.2025 um 14:00 hat Thomas Huth geschrieben:
>> From: Thomas Huth <thuth@redhat.com>
>>
>> QEMU iotests 049, 134 and 158 are currently failing if you compiled
>> QEMU without the crypto libraries. Thus make sure that the "secret"
>> object is really usable and skip the tests otherwise.
>>
>> Reported-by: Alex Bennée <alex.bennee@linaro.org>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
> 
>> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
>> index e977cb4eb61..10d83d8361b 100644
>> --- a/tests/qemu-iotests/common.rc
>> +++ b/tests/qemu-iotests/common.rc
>> @@ -1053,6 +1053,20 @@ _require_one_device_of()
>>       _notrun "$* not available"
>>   }
>>   
>> +_require_secret()
>> +{
>> +    if [ -e "$TEST_IMG" ]; then
>> +        echo "unwilling to overwrite existing file"
>> +        exit 1
>> +    fi
>> +    if $QEMU_IMG create -f $IMGFMT --object secret,id=sec0,data=123 \
>> +                 -o encryption=on,encrypt.key-secret=sec0 "$TEST_IMG" 1M 2>&1 \
>> +                 | grep "Unsupported cipher" ; then
>> +        _notrun "missing cipher support"
>> +    fi
> 
> What is the thing that you're checking here? If it's really the secret,
> then just running 'qemu-io --object secret,data=123,id=sec0 -c ""' would
> be enough. If it's not the secret, but encryption support, then the
> function is a misnomer.

The "qemu-io" statement seems to work fine in that case, so you're right, 
it's apparently not the "secret" object, but rather the "encryption" part 
that is failing.

So shall I rename it to "_require_encryption" ?

> _require_working_luks() looks pretty similar, though it requires
> specifically a working luks driver. Could something be unified? (The
> answer might be no, but it would be good to explicitly say it.)

While it looks a little bit similar, at least for me it still looks too 
distinct for unification - or is "-o key-secret=sec0" doing exactly the same 
as "-o encryption=on,encrypt.key-secret=sec0" ? ... I lack the deeper 
understanding of the parameters here to judge on that topic.

  Thomas



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] tests/qemu-iotests: Check for a functional "secret" object before using it
  2025-12-08  8:15   ` Thomas Huth
@ 2025-12-08  9:18     ` Daniel P. Berrangé
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel P. Berrangé @ 2025-12-08  9:18 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Kevin Wolf, qemu-block, Alex Bennée, Hanna Reitz, qemu-devel

On Mon, Dec 08, 2025 at 09:15:38AM +0100, Thomas Huth wrote:
> On 05/12/2025 18.20, Kevin Wolf wrote:
> > Am 05.12.2025 um 14:00 hat Thomas Huth geschrieben:
> > > From: Thomas Huth <thuth@redhat.com>
> > > 
> > > QEMU iotests 049, 134 and 158 are currently failing if you compiled
> > > QEMU without the crypto libraries. Thus make sure that the "secret"
> > > object is really usable and skip the tests otherwise.
> > > 
> > > Reported-by: Alex Bennée <alex.bennee@linaro.org>
> > > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > 
> > > diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
> > > index e977cb4eb61..10d83d8361b 100644
> > > --- a/tests/qemu-iotests/common.rc
> > > +++ b/tests/qemu-iotests/common.rc
> > > @@ -1053,6 +1053,20 @@ _require_one_device_of()
> > >       _notrun "$* not available"
> > >   }
> > > +_require_secret()
> > > +{
> > > +    if [ -e "$TEST_IMG" ]; then
> > > +        echo "unwilling to overwrite existing file"
> > > +        exit 1
> > > +    fi
> > > +    if $QEMU_IMG create -f $IMGFMT --object secret,id=sec0,data=123 \
> > > +                 -o encryption=on,encrypt.key-secret=sec0 "$TEST_IMG" 1M 2>&1 \
> > > +                 | grep "Unsupported cipher" ; then
> > > +        _notrun "missing cipher support"
> > > +    fi
> > 
> > What is the thing that you're checking here? If it's really the secret,
> > then just running 'qemu-io --object secret,data=123,id=sec0 -c ""' would
> > be enough. If it's not the secret, but encryption support, then the
> > function is a misnomer.
> 
> The "qemu-io" statement seems to work fine in that case, so you're right,
> it's apparently not the "secret" object, but rather the "encryption" part
> that is failing.
> 
> So shall I rename it to "_require_encryption" ?
>
> > _require_working_luks() looks pretty similar, though it requires
> > specifically a working luks driver. Could something be unified? (The
> > answer might be no, but it would be good to explicitly say it.)
> 
> While it looks a little bit similar, at least for me it still looks too
> distinct for unification - or is "-o key-secret=sec0" doing exactly the same
> as "-o encryption=on,encrypt.key-secret=sec0" ? ... I lack the deeper
> understanding of the parameters here to judge on that topic.

Specifically these three tests are all relying on QCow2 traditional
built-in AES encryption which pre-dated LUKS. Just name it for what
it tests:

  _require_qcow2_aes

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 :|



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-12-08  9:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-05 13:00 [PATCH] tests/qemu-iotests: Check for a functional "secret" object before using it Thomas Huth
2025-12-05 15:19 ` Alex Bennée
2025-12-05 17:20 ` Kevin Wolf
2025-12-08  8:15   ` Thomas Huth
2025-12-08  9:18     ` Daniel P. Berrangé

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).