[PATCH] tests/qemu-iotests: Check for a functional "secret" object before using it

Thomas Huth posted 1 patch 1 week, 1 day ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20251205130014.693799-1-thuth@redhat.com
Maintainers: Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@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(+)
[PATCH] tests/qemu-iotests: Check for a functional "secret" object before using it
Posted by Thomas Huth 1 week, 1 day ago
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


Re: [PATCH] tests/qemu-iotests: Check for a functional "secret" object before using it
Posted by Kevin Wolf 1 week, 1 day ago
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
> 
Re: [PATCH] tests/qemu-iotests: Check for a functional "secret" object before using it
Posted by Thomas Huth 5 days, 18 hours ago
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


Re: [PATCH] tests/qemu-iotests: Check for a functional "secret" object before using it
Posted by Daniel P. Berrangé 5 days, 17 hours ago
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 :|


Re: [PATCH] tests/qemu-iotests: Check for a functional "secret" object before using it
Posted by Alex Bennée 1 week, 1 day ago
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