Re: Some iotests are failing with -luks

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Sep 10, 2025 at 08:38:36PM +0200, Kevin Wolf wrote:
> Am 10.09.2025 um 18:08 hat Kevin Wolf geschrieben:
> > Am 10.09.2025 um 17:16 hat Thomas Huth geschrieben:
> > > luks-detached-header   fail       [17:15:26] [17:15:38]   12.2s                failed, exit status 1
> > > --- /home/thuth/devel/qemu/tests/qemu-iotests/tests/luks-detached-header.out
> > > +++ /home/thuth/tmp/qemu-build/tests/qemu-iotests/scratch/luks-file-luks-detached-header/luks-detached-header.out.bad
> > > @@ -1,5 +1,55 @@
> > > -..
> > > +EE
> > > +======================================================================
> > > +ERROR: test_detached_luks_header (__main__.TestDetachedLUKSHeader.test_detached_luks_header)
> > > +----------------------------------------------------------------------
> > > +Traceback (most recent call last):
> > > +  File "/home/thuth/devel/qemu/tests/qemu-iotests/tests/luks-detached-header", line 139, in setUp
> > > +    res = qemu_img_create(
> > > +          ^^^^^^^^^^^^^^^^
> > > +  File "/home/thuth/devel/qemu/tests/qemu-iotests/iotests.py", line 278, in qemu_img_create
> > > +    return qemu_img('create', *args)
> > > +           ^^^^^^^^^^^^^^^^^^^^^^^^^
> > > +  File "/home/thuth/devel/qemu/tests/qemu-iotests/iotests.py", line 261, in qemu_img
> > > +    return qemu_tool(*full_args, check=check, combine_stdio=combine_stdio)
> > > +           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > +  File "/home/thuth/devel/qemu/tests/qemu-iotests/iotests.py", line 241, in qemu_tool
> > > +    raise VerboseProcessError(
> > > +qemu.utils.VerboseProcessError: Command '('/home/thuth/tmp/qemu-build/qemu-img', 'create', '-f', 'luks', '-o', 'iter-time=10', '-o', 'key-secret=sec0', '-o', 'detached-header=true', '--object', 'secret,id=sec0,data=foo', '/home/thuth/tmp/qemu-build/tests/qemu-iotests/scratch/luks-file-luks-detached-header/detached_header.img2')' returned non-zero exit status 1.
> > > +  ┏━ output ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
> > > +  ┃ Formatting '/home/thuth/tmp/qemu-build/tests/qemu-iotests/scratch/l
> > > +  ┃ uks-file-luks-detached-header/detached_header.img2', fmt=luks
> > > +  ┃ size=-1 key-secret=sec0 iter-time=10 detached-header=true
> > > +  ┃ qemu-img: /home/thuth/tmp/qemu-build/tests/qemu-iotests/scratch/luk
> > > +  ┃ s-file-luks-detached-header/detached_header.img2: Parameter
> > > +  ┃ 'detached-header' is unexpected
> > > +  ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
> > 
> > This one is surprising. I don't think anything relevant in the luks
> > driver has changed since the test was introduced. At the same time, the
> > code clearly has a problem when it tries to convert a QemuOpts
> > containing a "detached-header" option into a QAPI object when the schema
> > doesn't even have this option. Was this broken from the beginning? Would
> > have been for a year and half.
> 
> I bisected this one because I was curious how this could happen, and it
> was broken quite explicitly by commit e818c01a:
> 
> commit e818c01ae6e7c54c7019baaf307be59d99ce80b9 (HEAD)
> Author: Daniel P. Berrangé <berrange@redhat.com>
> Date:   Mon Feb 19 15:12:59 2024 +0000
> 
>     qapi: drop unused QCryptoBlockCreateOptionsLUKS.detached-header
> 
>     The 'detached-header' field in QCryptoBlockCreateOptionsLUKS
>     was left over from earlier patch iterations.
> 
>     Acked-by: Markus Armbruster <armbru@redhat.com>
>     Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> The test case demonstrates that it actually wasn't unused.
> 
> If we don't want to reintroduce the field in QAPI, we need to explicitly
> delete it from the QemuOpts in block_crypto_co_create_opts_luks() before
> block_crypto_create_opts_init() creates a QCryptoBlockCreateOptions from
> the given options and fails now that this option doesn't exist any more.

Sigh, that was rather a awful mistake.  I've been misled by fact that
the code does not use the QAPI generated structs directly, but instead
converts the QAPI struct back into a QemuOpts and then accesses the
opts by name. So I saw no reference to 'detached_header' or
'has_detached_header' struct fields, but failed to remember that the
qemu_opt_get_bool relied on the QAPI field. We need to revert that
commit.


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