Some iotests are failing with -luks

Some iotests are failing with -luks

[Thomas Huth]

  Hi,

when running "./check -luks" in the qemu-iotests directory,
some tests are failing for me:

295 296 inactive-node-nbd luks-detached-header

Is that a known problem already?

FWIW, 295 is failing with the following output:

295   fail       [17:03:01] [17:03:17]   15.7s                failed, exit status 1
--- /home/thuth/devel/qemu/tests/qemu-iotests/295.out
+++ /home/thuth/tmp/qemu-build/tests/qemu-iotests/scratch/luks-file-295/295.out.bad
@@ -1,40 +1,326 @@
-{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}}
-{"return": {}}
-{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}}
-{"return": {}}
-{"execute": "job-dismiss", "arguments": {"id": "job_erase_key"}}
-{"return": {}}
-.{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}}
-{"return": {}}
-{"execute": "job-dismiss", "arguments": {"id": "job_erase_key"}}
-{"return": {}}
-Job failed: Invalid password, cannot unlock any keyslot
-{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}}
-{"return": {}}
-{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}}
-{"return": {}}
-.{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}}
-{"return": {}}
-{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}}
-{"return": {}}
-Job failed: Refusing to overwrite active keyslot 2 - please erase it first
-{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}}
-{"return": {}}
-{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}}
-{"return": {}}
-{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}}
-{"return": {}}
-{"execute": "job-dismiss", "arguments": {"id": "job_erase_key"}}
-{"return": {}}
-{"execute": "job-dismiss", "arguments": {"id": "job_erase_key"}}
-{"return": {}}
-Job failed: All the active keyslots match the (old) password that was given and erasing them will erase all the data in the image irreversibly - refusing operation
-{"execute": "job-dismiss", "arguments": {"id": "job_erase_key"}}
-{"return": {}}
-{"execute": "job-dismiss", "arguments": {"id": "job_erase_key"}}
-{"return": {}}
-.
+EWARNING:qemu.machine.machine:qemu received signal 6; command: "/home/thuth/tmp/qemu-build/qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,fd=5 -mon chardev=mon,mode=control -chardev socket,id=qtest,fd=3 -qtest chardev:qtest -accel qtest -nodefaults -display none -accel qtest"
+EEWARNING:qemu.machine.machine:qemu received signal 6; command: "/home/thuth/tmp/qemu-build/qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,fd=6 -mon chardev=mon,mode=control -chardev socket,id=qtest,fd=3 -qtest chardev:qtest -accel qtest -nodefaults -display none -accel qtest"
+EEWARNING:qemu.machine.machine:qemu received signal 6; command: "/home/thuth/tmp/qemu-build/qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,fd=10 -mon chardev=mon,mode=control -chardev socket,id=qtest,fd=3 -qtest chardev:qtest -accel qtest -nodefaults -display none -accel qtest"
+E
+======================================================================
+ERROR: testChangeKey (__main__.EncryptionSetupTestCase.testChangeKey)
+----------------------------------------------------------------------
+Traceback (most recent call last):
+  File "/home/thuth/devel/qemu/tests/qemu-iotests/295", line 204, in testChangeKey
+    self.addKeyQmp("testdev", new_secret = self.secrets[1])
+  File "/home/thuth/devel/qemu/tests/qemu-iotests/295", line 159, in addKeyQmp
+    self.vm.cmd('x-blockdev-amend', **args)
+  File "/home/thuth/devel/qemu/python/qemu/machine/machine.py", line 735, in cmd
+    ret = self._qmp.cmd(cmd, **qmp_args)
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  File "/home/thuth/devel/qemu/python/qemu/qmp/legacy.py", line 214, in cmd
+    return self._sync(
+           ^^^^^^^^^^^
+  File "/home/thuth/devel/qemu/python/qemu/qmp/legacy.py", line 102, in _sync
+    return self._aloop.run_until_complete(
+           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  File "/usr/lib64/python3.11/asyncio/base_events.py", line 654, in run_until_complete
+    return future.result()
+           ^^^^^^^^^^^^^^^
+  File "/usr/lib64/python3.11/asyncio/tasks.py", line 452, in wait_for
+    return await fut
+           ^^^^^^^^^
+  File "/home/thuth/devel/qemu/python/qemu/qmp/qmp_client.py", line 632, in execute
+    return await self.execute_msg(msg)
+           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  File "/home/thuth/devel/qemu/python/qemu/qmp/qmp_client.py", line 574, in execute_msg
+    reply = await self._execute(msg)
+            ^^^^^^^^^^^^^^^^^^^^^^^^
+  File "/home/thuth/devel/qemu/python/qemu/qmp/qmp_client.py", line 496, in _execute
+    return await self._reply(exec_id)
+           ^^^^^^^^^^^^^^^^^^^^^^^^^^
+  File "/home/thuth/devel/qemu/python/qemu/qmp/qmp_client.py", line 463, in _reply
+    raise result
+qemu.qmp.qmp_client.ExecInterruptedError: Disconnected

etc.

296 looks very similar (also a "qemu received signal 6" error),
but the others look like this:

inactive-node-nbd   fail       [17:13:56] [17:14:04]   7.5s                 failed, exit status 1
--- /home/thuth/devel/qemu/tests/qemu-iotests/tests/inactive-node-nbd.out
+++ /home/thuth/tmp/qemu-build/tests/qemu-iotests/scratch/luks-file-inactive-node-nbd/inactive-node-nbd.out.bad
@@ -1,239 +1,64 @@
  Preparing disk...
  Launching VM...
-{"execute": "nbd-server-start", "arguments": {"addr": {"data": {"path": "SOCK_DIR/PID-nbd.sock"}, "type": "unix"}}}
-{"return": {}}
+ERROR:qemu.qmp.qmp_client.qemu-223907:Failed to receive Greeting: EOFError
+ERROR:qemu.qmp.qmp_client.qemu-223907:Failed to establish session: EOFError
+Traceback (most recent call last):
+  File "/home/thuth/devel/qemu/python/qemu/qmp/protocol.py", line 425, in _session_guard
+    await coro
+  File "/home/thuth/devel/qemu/python/qemu/qmp/qmp_client.py", line 250, in _establish_session
+    self._greeting = await self._get_greeting()
+                     ^^^^^^^^^^^^^^^^^^^^^^^^^^
+  File "/home/thuth/devel/qemu/python/qemu/qmp/qmp_client.py", line 270, in _get_greeting
+    msg = await self._recv()
+          ^^^^^^^^^^^^^^^^^^
+  File "/home/thuth/devel/qemu/python/qemu/qmp/protocol.py", line 1009, in _recv
+    message = await self._do_recv()
+              ^^^^^^^^^^^^^^^^^^^^^
+  File "/home/thuth/devel/qemu/python/qemu/qmp/qmp_client.py", line 402, in _do_recv
+    msg_bytes = await self._readline()
+                ^^^^^^^^^^^^^^^^^^^^^^
+  File "/home/thuth/devel/qemu/python/qemu/qmp/protocol.py", line 977, in _readline
+    raise EOFError
+EOFError


and:

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
+  ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━


Any ideas?

  Thomas

Re: Some iotests are failing with -luks

[Kevin Wolf]

Am 10.09.2025 um 17:16 hat Thomas Huth geschrieben:
> 
>  Hi,
> 
> when running "./check -luks" in the qemu-iotests directory,
> some tests are failing for me:
> 
> 295 296 inactive-node-nbd luks-detached-header
> 
> Is that a known problem already?

Not to me anyway.

> FWIW, 295 is failing with the following output:
> 
> 295   fail       [17:03:01] [17:03:17]   15.7s                failed, exit status 1
> [...]
> +EWARNING:qemu.machine.machine:qemu received signal 6; command: "/home/thuth/tmp/qemu-build/qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,fd=5 -mon chardev=mon,mode=control -chardev socket,id=qtest,fd=3 -qtest chardev:qtest -accel qtest -nodefaults -display none -accel qtest"
> +EEWARNING:qemu.machine.machine:qemu received signal 6; command: "/home/thuth/tmp/qemu-build/qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,fd=6 -mon chardev=mon,mode=control -chardev socket,id=qtest,fd=3 -qtest chardev:qtest -accel qtest -nodefaults -display none -accel qtest"
> +EEWARNING:qemu.machine.machine:qemu received signal 6; command: "/home/thuth/tmp/qemu-build/qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,fd=10 -mon chardev=mon,mode=control -chardev socket,id=qtest,fd=3 -qtest chardev:qtest -accel qtest -nodefaults -display none -accel qtest"
> +E
> [...]
> 
> etc.
> 
> 296 looks very similar (also a "qemu received signal 6" error),
> but the others look like this:

When it gets signal 6 (i.e. SIGABRT), that usually means that you should
have a look at the coredump.

> inactive-node-nbd   fail       [17:13:56] [17:14:04]   7.5s                 failed, exit status 1
> --- /home/thuth/devel/qemu/tests/qemu-iotests/tests/inactive-node-nbd.out
> +++ /home/thuth/tmp/qemu-build/tests/qemu-iotests/scratch/luks-file-inactive-node-nbd/inactive-node-nbd.out.bad
> @@ -1,239 +1,64 @@
>  Preparing disk...
>  Launching VM...
> -{"execute": "nbd-server-start", "arguments": {"addr": {"data": {"path": "SOCK_DIR/PID-nbd.sock"}, "type": "unix"}}}
> -{"return": {}}
> +ERROR:qemu.qmp.qmp_client.qemu-223907:Failed to receive Greeting: EOFError
> +ERROR:qemu.qmp.qmp_client.qemu-223907:Failed to establish session: EOFError
> +Traceback (most recent call last):
> +  File "/home/thuth/devel/qemu/python/qemu/qmp/protocol.py", line 425, in _session_guard
> +    await coro
> +  File "/home/thuth/devel/qemu/python/qemu/qmp/qmp_client.py", line 250, in _establish_session
> +    self._greeting = await self._get_greeting()
> +                     ^^^^^^^^^^^^^^^^^^^^^^^^^^
> +  File "/home/thuth/devel/qemu/python/qemu/qmp/qmp_client.py", line 270, in _get_greeting
> +    msg = await self._recv()
> +          ^^^^^^^^^^^^^^^^^^
> +  File "/home/thuth/devel/qemu/python/qemu/qmp/protocol.py", line 1009, in _recv
> +    message = await self._do_recv()
> +              ^^^^^^^^^^^^^^^^^^^^^
> +  File "/home/thuth/devel/qemu/python/qemu/qmp/qmp_client.py", line 402, in _do_recv
> +    msg_bytes = await self._readline()
> +                ^^^^^^^^^^^^^^^^^^^^^^
> +  File "/home/thuth/devel/qemu/python/qemu/qmp/protocol.py", line 977, in _readline
> +    raise EOFError
> +EOFError

Not sure what this is. It looks like the QEMU process failed to start,
maybe it didn't like some command line option. I would expect an error
message on stderr, but I'm not sure if qemu-iotests automatically
displays that in such cases. I thought that yes, but maybe I'm confusing
it with a different case.

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

Kevin

Re: Some iotests are failing with -luks

[Kevin Wolf]

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.

Kevin

Re: Some iotests are failing with -luks

[Yong Huang]

[-- Attachment #1: Type: text/plain, Size: 4539 bytes --]

On Thu, Sep 11, 2025 at 2:38 AM Kevin Wolf <kwolf@redhat.com> 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
>

Keeping the detached-header option is more convenient for users when

creating a detached-header image.

My inclination is to bring this optionback.  Any suggestions? cc @Daniel P.
Berrangé <berrange@redhat.com>


> 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.
>
> Kevin
>
>

-- 
Best regards

[-- Attachment #2: Type: text/html, Size: 6782 bytes --]

Re: Some iotests are failing with -luks

[Kevin Wolf]

Am 11.09.2025 um 04:33 hat Yong Huang geschrieben:
> On Thu, Sep 11, 2025 at 2:38 AM Kevin Wolf <kwolf@redhat.com> 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
> >
> 
> Keeping the detached-header option is more convenient for users when
> creating a detached-header image.
> 
> My inclination is to bring this optionback.  Any suggestions? cc @Daniel P.
> Berrangé <berrange@redhat.com>

Having it available for users in qemu-img is different from having it in
QAPI. Arguably there is no use for it in QAPI, as long as you make sure
that it's taken out of the QemuOpts before going to QAPI.

Kevin

> 
> > 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.
> >
> > Kevin
> >
> >
> 
> -- 
> Best regards

Re: Some iotests are failing with -luks

[Daniel P. Berrangé]

On Thu, Sep 11, 2025 at 12:04:24PM +0200, Kevin Wolf wrote:
> Am 11.09.2025 um 04:33 hat Yong Huang geschrieben:
> > On Thu, Sep 11, 2025 at 2:38 AM Kevin Wolf <kwolf@redhat.com> 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
> > >
> > 
> > Keeping the detached-header option is more convenient for users when
> > creating a detached-header image.
> > 
> > My inclination is to bring this optionback.  Any suggestions? cc @Daniel P.
> > Berrangé <berrange@redhat.com>
> 
> Having it available for users in qemu-img is different from having it in
> QAPI. Arguably there is no use for it in QAPI, as long as you make sure
> that it's taken out of the QemuOpts before going to QAPI.

Ah so what we actually need to fix is

diff --git a/block/crypto.c b/block/crypto.c
index 4eed3ffa6a..8b60510c61 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -792,7 +792,7 @@ block_crypto_co_create_opts_luks(BlockDriver *drv, const char *filename,
     char *buf = NULL;
     int64_t size;
     bool detached_hdr =
-        qemu_opt_get_bool(opts, "detached-header", false);
+        qemu_opt_get_bool_del(opts, "detached-header", false);
     unsigned int cflags = 0;
     int ret;
     Error *local_err = NULL;


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: Some iotests are failing with -luks

[Kevin Wolf]

Am 11.09.2025 um 12:38 hat Daniel P. Berrangé geschrieben:
> On Thu, Sep 11, 2025 at 12:04:24PM +0200, Kevin Wolf wrote:
> > Am 11.09.2025 um 04:33 hat Yong Huang geschrieben:
> > > On Thu, Sep 11, 2025 at 2:38 AM Kevin Wolf <kwolf@redhat.com> 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
> > > >
> > > 
> > > Keeping the detached-header option is more convenient for users when
> > > creating a detached-header image.
> > > 
> > > My inclination is to bring this optionback.  Any suggestions? cc @Daniel P.
> > > Berrangé <berrange@redhat.com>
> > 
> > Having it available for users in qemu-img is different from having it in
> > QAPI. Arguably there is no use for it in QAPI, as long as you make sure
> > that it's taken out of the QemuOpts before going to QAPI.
> 
> Ah so what we actually need to fix is
> 
> diff --git a/block/crypto.c b/block/crypto.c
> index 4eed3ffa6a..8b60510c61 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -792,7 +792,7 @@ block_crypto_co_create_opts_luks(BlockDriver *drv, const char *filename,
>      char *buf = NULL;
>      int64_t size;
>      bool detached_hdr =
> -        qemu_opt_get_bool(opts, "detached-header", false);
> +        qemu_opt_get_bool_del(opts, "detached-header", false);
>      unsigned int cflags = 0;
>      int ret;
>      Error *local_err = NULL;

That's what I had in mind, but we need to verify that we don't get a
scenario like what you hinted at where converting the options back to
some non-QAPI data structure (QemuOpts or QDict) would still use it
later.

Kevin

Re: Some iotests are failing with -luks

[Daniel P. Berrangé]

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

Re: Some iotests are failing with -luks

[Thomas Huth]

On 10/09/2025 18.08, Kevin Wolf wrote:
> Am 10.09.2025 um 17:16 hat Thomas Huth geschrieben:
>>
>>   Hi,
>>
>> when running "./check -luks" in the qemu-iotests directory,
>> some tests are failing for me:
>>
>> 295 296 inactive-node-nbd luks-detached-header
>>
>> Is that a known problem already?
> 
> Not to me anyway.
> 
>> FWIW, 295 is failing with the following output:
>>
>> 295   fail       [17:03:01] [17:03:17]   15.7s                failed, exit status 1
>> [...]
>> +EWARNING:qemu.machine.machine:qemu received signal 6; command: "/home/thuth/tmp/qemu-build/qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,fd=5 -mon chardev=mon,mode=control -chardev socket,id=qtest,fd=3 -qtest chardev:qtest -accel qtest -nodefaults -display none -accel qtest"
>> +EEWARNING:qemu.machine.machine:qemu received signal 6; command: "/home/thuth/tmp/qemu-build/qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,fd=6 -mon chardev=mon,mode=control -chardev socket,id=qtest,fd=3 -qtest chardev:qtest -accel qtest -nodefaults -display none -accel qtest"
>> +EEWARNING:qemu.machine.machine:qemu received signal 6; command: "/home/thuth/tmp/qemu-build/qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,fd=10 -mon chardev=mon,mode=control -chardev socket,id=qtest,fd=3 -qtest chardev:qtest -accel qtest -nodefaults -display none -accel qtest"
>> +E
>> [...]
>>
>> etc.
>>
>> 296 looks very similar (also a "qemu received signal 6" error),
>> but the others look like this:
> 
> When it gets signal 6 (i.e. SIGABRT), that usually means that you should
> have a look at the coredump.

With "-p" I additionally get this error message in the log:

qemu-system-x86_64: ../../devel/qemu/block/graph-lock.c:294:
  bdrv_graph_rdlock_main_loop: Assertion `!qemu_in_coroutine()' failed.

With -gdb I can get a back trace that looks like this:

Thread 1 "qemu-system-x86" received signal SIGABRT, Aborted.
0x00007ffff4ba7e9c in __pthread_kill_implementation () from target:/lib64/libc.so.6
--Type <RET> for more, q to quit, c to continue without paging--
#0  0x00007ffff4ba7e9c in __pthread_kill_implementation () from target:/lib64/libc.so.6
#1  0x00007ffff4b4df3e in raise () from target:/lib64/libc.so.6
#2  0x00007ffff4b356d0 in abort () from target:/lib64/libc.so.6
#3  0x00007ffff4b35639 in __assert_fail_base.cold () from target:/lib64/libc.so.6
#4  0x0000555555574eae in bdrv_graph_rdlock_main_loop () at ../../devel/qemu/block/graph-lock.c:294
#5  0x0000555555aa2f43 in graph_lockable_auto_lock_mainloop (x=<optimized out>) at /home/thuth/devel/qemu/include/block/graph-lock.h:275
#6  block_crypto_read_func (block=<optimized out>, offset=4096, buf=0x555558324100 "", buflen=256000, opaque=0x555558a259d0, errp=0x555558a8c370)
     at ../../devel/qemu/block/crypto.c:71
#7  0x0000555555a5a308 in qcrypto_block_luks_load_key (block=block@entry=0x555558686ec0, slot_idx=slot_idx@entry=0,
     password=password@entry=0x555558626050 "hunter0", masterkey=masterkey@entry=0x55555886b2a0 "",
     readfunc=readfunc@entry=0x555555aa2f10 <block_crypto_read_func>, opaque=opaque@entry=0x555558a259d0, errp=0x555558a8c370)
     at ../../devel/qemu/crypto/block-luks.c:927
#8  0x0000555555a5ba7e in qcrypto_block_luks_find_key (block=0x555558686ec0, password=0x555558626050 "hunter0", masterkey=0x55555886b2a0 "",
     readfunc=0x555555aa2f10 <block_crypto_read_func>, opaque=0x555558a259d0, errp=0x555558a8c370) at ../../devel/qemu/crypto/block-luks.c:1045
#9  qcrypto_block_luks_amend_add_keyslot (block=0x555558686ec0, readfunc=0x555555aa2f10 <block_crypto_read_func>,
     writefunc=0x555555aa2e50 <block_crypto_write_func>, opaque=0x555558a259d0, opts_luks=0x7fffec5fff38, force=<optimized out>, errp=0x555558a8c370)
     at ../../devel/qemu/crypto/block-luks.c:1673
#10 qcrypto_block_luks_amend_options (block=0x555558686ec0, readfunc=0x555555aa2f10 <block_crypto_read_func>,
     writefunc=0x555555aa2e50 <block_crypto_write_func>, opaque=0x555558a259d0, options=0x7fffec5fff30, force=<optimized out>, errp=0x555558a8c370)
     at ../../devel/qemu/crypto/block-luks.c:1865
#11 0x0000555555aa3852 in block_crypto_amend_options_generic_luks (bs=<optimized out>, amend_options=<optimized out>, force=<optimized out>,
     errp=<optimized out>) at ../../devel/qemu/block/crypto.c:949
#12 0x0000555555aa38e9 in block_crypto_co_amend_luks (bs=<optimized out>, opts=<optimized out>, force=<optimized out>, errp=<optimized out>)
     at ../../devel/qemu/block/crypto.c:1008
#13 0x0000555555a96030 in blockdev_amend_run (job=0x555558a8c2b0, errp=0x555558a8c370) at ../../devel/qemu/block/amend.c:52
#14 0x0000555555a874ad in job_co_entry (opaque=0x555558a8c2b0) at ../../devel/qemu/job.c:1112
#15 0x0000555555bdc41b in coroutine_trampoline (i0=<optimized out>, i1=<optimized out>) at ../../devel/qemu/util/coroutine-ucontext.c:175
#16 0x00007ffff4b68f70 in ?? () from target:/lib64/libc.so.6
#17 0x00007fffffffc310 in ?? ()
#18 0x0000000000000000 in ?? ()

>> inactive-node-nbd   fail       [17:13:56] [17:14:04]   7.5s                 failed, exit status 1
>> --- /home/thuth/devel/qemu/tests/qemu-iotests/tests/inactive-node-nbd.out
>> +++ /home/thuth/tmp/qemu-build/tests/qemu-iotests/scratch/luks-file-inactive-node-nbd/inactive-node-nbd.out.bad
>> @@ -1,239 +1,64 @@
>>   Preparing disk...
>>   Launching VM...
>> -{"execute": "nbd-server-start", "arguments": {"addr": {"data": {"path": "SOCK_DIR/PID-nbd.sock"}, "type": "unix"}}}
>> -{"return": {}}
>> +ERROR:qemu.qmp.qmp_client.qemu-223907:Failed to receive Greeting: EOFError
>> +ERROR:qemu.qmp.qmp_client.qemu-223907:Failed to establish session: EOFError
>> +Traceback (most recent call last):
>> +  File "/home/thuth/devel/qemu/python/qemu/qmp/protocol.py", line 425, in _session_guard
>> +    await coro
>> +  File "/home/thuth/devel/qemu/python/qemu/qmp/qmp_client.py", line 250, in _establish_session
>> +    self._greeting = await self._get_greeting()
>> +                     ^^^^^^^^^^^^^^^^^^^^^^^^^^
>> +  File "/home/thuth/devel/qemu/python/qemu/qmp/qmp_client.py", line 270, in _get_greeting
>> +    msg = await self._recv()
>> +          ^^^^^^^^^^^^^^^^^^
>> +  File "/home/thuth/devel/qemu/python/qemu/qmp/protocol.py", line 1009, in _recv
>> +    message = await self._do_recv()
>> +              ^^^^^^^^^^^^^^^^^^^^^
>> +  File "/home/thuth/devel/qemu/python/qemu/qmp/qmp_client.py", line 402, in _do_recv
>> +    msg_bytes = await self._readline()
>> +                ^^^^^^^^^^^^^^^^^^^^^^
>> +  File "/home/thuth/devel/qemu/python/qemu/qmp/protocol.py", line 977, in _readline
>> +    raise EOFError
>> +EOFError
> 
> Not sure what this is. It looks like the QEMU process failed to start,
> maybe it didn't like some command line option. I would expect an error
> message on stderr, but I'm not sure if qemu-iotests automatically
> displays that in such cases. I thought that yes, but maybe I'm confusing
> it with a different case.

With "-p" I'm getting this additional error message:

qemu-system-x86_64: -blockdev luks,file=disk-file,node-name=disk-fmt,active=off: Parameter 'key-secret' is required for cipher

  HTH,
   Thomas

Re: Some iotests are failing with -luks

[Kevin Wolf]

Am 11.09.2025 um 13:21 hat Thomas Huth geschrieben:
> On 10/09/2025 18.08, Kevin Wolf wrote:
> > Am 10.09.2025 um 17:16 hat Thomas Huth geschrieben:
> > > 
> > >   Hi,
> > > 
> > > when running "./check -luks" in the qemu-iotests directory,
> > > some tests are failing for me:
> > > 
> > > 295 296 inactive-node-nbd luks-detached-header
> > > 
> > > Is that a known problem already?
> > 
> > Not to me anyway.
> > 
> > > FWIW, 295 is failing with the following output:
> > > 
> > > 295   fail       [17:03:01] [17:03:17]   15.7s                failed, exit status 1
> > > [...]
> > > +EWARNING:qemu.machine.machine:qemu received signal 6; command: "/home/thuth/tmp/qemu-build/qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,fd=5 -mon chardev=mon,mode=control -chardev socket,id=qtest,fd=3 -qtest chardev:qtest -accel qtest -nodefaults -display none -accel qtest"
> > > +EEWARNING:qemu.machine.machine:qemu received signal 6; command: "/home/thuth/tmp/qemu-build/qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,fd=6 -mon chardev=mon,mode=control -chardev socket,id=qtest,fd=3 -qtest chardev:qtest -accel qtest -nodefaults -display none -accel qtest"
> > > +EEWARNING:qemu.machine.machine:qemu received signal 6; command: "/home/thuth/tmp/qemu-build/qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,fd=10 -mon chardev=mon,mode=control -chardev socket,id=qtest,fd=3 -qtest chardev:qtest -accel qtest -nodefaults -display none -accel qtest"
> > > +E
> > > [...]
> > > 
> > > etc.
> > > 
> > > 296 looks very similar (also a "qemu received signal 6" error),
> > > but the others look like this:
> > 
> > When it gets signal 6 (i.e. SIGABRT), that usually means that you should
> > have a look at the coredump.
> 
> With "-p" I additionally get this error message in the log:
> 
> qemu-system-x86_64: ../../devel/qemu/block/graph-lock.c:294:
>  bdrv_graph_rdlock_main_loop: Assertion `!qemu_in_coroutine()' failed.
> 
> With -gdb I can get a back trace that looks like this:
> 
> Thread 1 "qemu-system-x86" received signal SIGABRT, Aborted.
> 0x00007ffff4ba7e9c in __pthread_kill_implementation () from target:/lib64/libc.so.6
> --Type <RET> for more, q to quit, c to continue without paging--
> #0  0x00007ffff4ba7e9c in __pthread_kill_implementation () from target:/lib64/libc.so.6
> #1  0x00007ffff4b4df3e in raise () from target:/lib64/libc.so.6
> #2  0x00007ffff4b356d0 in abort () from target:/lib64/libc.so.6
> #3  0x00007ffff4b35639 in __assert_fail_base.cold () from target:/lib64/libc.so.6
> #4  0x0000555555574eae in bdrv_graph_rdlock_main_loop () at ../../devel/qemu/block/graph-lock.c:294
> #5  0x0000555555aa2f43 in graph_lockable_auto_lock_mainloop (x=<optimized out>) at /home/thuth/devel/qemu/include/block/graph-lock.h:275
> #6  block_crypto_read_func (block=<optimized out>, offset=4096, buf=0x555558324100 "", buflen=256000, opaque=0x555558a259d0, errp=0x555558a8c370)
>     at ../../devel/qemu/block/crypto.c:71
> #7  0x0000555555a5a308 in qcrypto_block_luks_load_key (block=block@entry=0x555558686ec0, slot_idx=slot_idx@entry=0,
>     password=password@entry=0x555558626050 "hunter0", masterkey=masterkey@entry=0x55555886b2a0 "",
>     readfunc=readfunc@entry=0x555555aa2f10 <block_crypto_read_func>, opaque=opaque@entry=0x555558a259d0, errp=0x555558a8c370)
>     at ../../devel/qemu/crypto/block-luks.c:927
> #8  0x0000555555a5ba7e in qcrypto_block_luks_find_key (block=0x555558686ec0, password=0x555558626050 "hunter0", masterkey=0x55555886b2a0 "",
>     readfunc=0x555555aa2f10 <block_crypto_read_func>, opaque=0x555558a259d0, errp=0x555558a8c370) at ../../devel/qemu/crypto/block-luks.c:1045
> #9  qcrypto_block_luks_amend_add_keyslot (block=0x555558686ec0, readfunc=0x555555aa2f10 <block_crypto_read_func>,
>     writefunc=0x555555aa2e50 <block_crypto_write_func>, opaque=0x555558a259d0, opts_luks=0x7fffec5fff38, force=<optimized out>, errp=0x555558a8c370)
>     at ../../devel/qemu/crypto/block-luks.c:1673
> #10 qcrypto_block_luks_amend_options (block=0x555558686ec0, readfunc=0x555555aa2f10 <block_crypto_read_func>,
>     writefunc=0x555555aa2e50 <block_crypto_write_func>, opaque=0x555558a259d0, options=0x7fffec5fff30, force=<optimized out>, errp=0x555558a8c370)
>     at ../../devel/qemu/crypto/block-luks.c:1865
> #11 0x0000555555aa3852 in block_crypto_amend_options_generic_luks (bs=<optimized out>, amend_options=<optimized out>, force=<optimized out>,
>     errp=<optimized out>) at ../../devel/qemu/block/crypto.c:949
> #12 0x0000555555aa38e9 in block_crypto_co_amend_luks (bs=<optimized out>, opts=<optimized out>, force=<optimized out>, errp=<optimized out>)
>     at ../../devel/qemu/block/crypto.c:1008
> #13 0x0000555555a96030 in blockdev_amend_run (job=0x555558a8c2b0, errp=0x555558a8c370) at ../../devel/qemu/block/amend.c:52
> #14 0x0000555555a874ad in job_co_entry (opaque=0x555558a8c2b0) at ../../devel/qemu/job.c:1112
> #15 0x0000555555bdc41b in coroutine_trampoline (i0=<optimized out>, i1=<optimized out>) at ../../devel/qemu/util/coroutine-ucontext.c:175
> #16 0x00007ffff4b68f70 in ?? () from target:/lib64/libc.so.6
> #17 0x00007fffffffc310 in ?? ()
> #18 0x0000000000000000 in ?? ()

Hm, so block_crypto_read_func() isn't prepared to be called in coroutine
context, but block_crypto_co_amend_luks() still calls it from a
coroutine. The indirection of going through QCrypto won't make it easier
to fix this properly.

It seems to me that while block_crypto_read/write_func are effectively
no_coroutine_fn, qcrypto_block_amend_options() should really take
function pointers that can be called from coroutines. It is called from
both coroutine and non-coroutine code paths, so should the function
pointers be coroutine_mixed_fn or do we want to change the callers?

Either way, we should add the appropriate coroutine markers to the
QCrypto interfaces to show the intention at least.

> > > inactive-node-nbd   fail       [17:13:56] [17:14:04]   7.5s                 failed, exit status 1
> > > [...]
> 
> With "-p" I'm getting this additional error message:
> 
> qemu-system-x86_64: -blockdev luks,file=disk-file,node-name=disk-fmt,active=off: Parameter 'key-secret' is required for cipher

The test case just isn't made for luks. iotests.py has special code for
luks in VM.add_drive(), but not in VM.add_blockdev().

For now, the test should probably just mark luks as unsupported.

Kevin

Re: Some iotests are failing with -luks

[Daniel P. Berrangé]

On Thu, Sep 11, 2025 at 02:13:47PM +0200, Kevin Wolf wrote:
> Am 11.09.2025 um 13:21 hat Thomas Huth geschrieben:
> > On 10/09/2025 18.08, Kevin Wolf wrote:
> > > Am 10.09.2025 um 17:16 hat Thomas Huth geschrieben:
> > > > 
> > > >   Hi,
> > > > 
> > > > when running "./check -luks" in the qemu-iotests directory,
> > > > some tests are failing for me:
> > > > 
> > > > 295 296 inactive-node-nbd luks-detached-header
> > > > 
> > > > Is that a known problem already?
> > > 
> > > Not to me anyway.
> > > 
> > > > FWIW, 295 is failing with the following output:
> > > > 
> > > > 295   fail       [17:03:01] [17:03:17]   15.7s                failed, exit status 1
> > > > [...]
> > > > +EWARNING:qemu.machine.machine:qemu received signal 6; command: "/home/thuth/tmp/qemu-build/qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,fd=5 -mon chardev=mon,mode=control -chardev socket,id=qtest,fd=3 -qtest chardev:qtest -accel qtest -nodefaults -display none -accel qtest"
> > > > +EEWARNING:qemu.machine.machine:qemu received signal 6; command: "/home/thuth/tmp/qemu-build/qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,fd=6 -mon chardev=mon,mode=control -chardev socket,id=qtest,fd=3 -qtest chardev:qtest -accel qtest -nodefaults -display none -accel qtest"
> > > > +EEWARNING:qemu.machine.machine:qemu received signal 6; command: "/home/thuth/tmp/qemu-build/qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,fd=10 -mon chardev=mon,mode=control -chardev socket,id=qtest,fd=3 -qtest chardev:qtest -accel qtest -nodefaults -display none -accel qtest"
> > > > +E
> > > > [...]
> > > > 
> > > > etc.
> > > > 
> > > > 296 looks very similar (also a "qemu received signal 6" error),
> > > > but the others look like this:
> > > 
> > > When it gets signal 6 (i.e. SIGABRT), that usually means that you should
> > > have a look at the coredump.
> > 
> > With "-p" I additionally get this error message in the log:
> > 
> > qemu-system-x86_64: ../../devel/qemu/block/graph-lock.c:294:
> >  bdrv_graph_rdlock_main_loop: Assertion `!qemu_in_coroutine()' failed.
> > 
> > With -gdb I can get a back trace that looks like this:
> > 
> > Thread 1 "qemu-system-x86" received signal SIGABRT, Aborted.
> > 0x00007ffff4ba7e9c in __pthread_kill_implementation () from target:/lib64/libc.so.6
> > --Type <RET> for more, q to quit, c to continue without paging--
> > #0  0x00007ffff4ba7e9c in __pthread_kill_implementation () from target:/lib64/libc.so.6
> > #1  0x00007ffff4b4df3e in raise () from target:/lib64/libc.so.6
> > #2  0x00007ffff4b356d0 in abort () from target:/lib64/libc.so.6
> > #3  0x00007ffff4b35639 in __assert_fail_base.cold () from target:/lib64/libc.so.6
> > #4  0x0000555555574eae in bdrv_graph_rdlock_main_loop () at ../../devel/qemu/block/graph-lock.c:294
> > #5  0x0000555555aa2f43 in graph_lockable_auto_lock_mainloop (x=<optimized out>) at /home/thuth/devel/qemu/include/block/graph-lock.h:275
> > #6  block_crypto_read_func (block=<optimized out>, offset=4096, buf=0x555558324100 "", buflen=256000, opaque=0x555558a259d0, errp=0x555558a8c370)
> >     at ../../devel/qemu/block/crypto.c:71
> > #7  0x0000555555a5a308 in qcrypto_block_luks_load_key (block=block@entry=0x555558686ec0, slot_idx=slot_idx@entry=0,
> >     password=password@entry=0x555558626050 "hunter0", masterkey=masterkey@entry=0x55555886b2a0 "",
> >     readfunc=readfunc@entry=0x555555aa2f10 <block_crypto_read_func>, opaque=opaque@entry=0x555558a259d0, errp=0x555558a8c370)
> >     at ../../devel/qemu/crypto/block-luks.c:927
> > #8  0x0000555555a5ba7e in qcrypto_block_luks_find_key (block=0x555558686ec0, password=0x555558626050 "hunter0", masterkey=0x55555886b2a0 "",
> >     readfunc=0x555555aa2f10 <block_crypto_read_func>, opaque=0x555558a259d0, errp=0x555558a8c370) at ../../devel/qemu/crypto/block-luks.c:1045
> > #9  qcrypto_block_luks_amend_add_keyslot (block=0x555558686ec0, readfunc=0x555555aa2f10 <block_crypto_read_func>,
> >     writefunc=0x555555aa2e50 <block_crypto_write_func>, opaque=0x555558a259d0, opts_luks=0x7fffec5fff38, force=<optimized out>, errp=0x555558a8c370)
> >     at ../../devel/qemu/crypto/block-luks.c:1673
> > #10 qcrypto_block_luks_amend_options (block=0x555558686ec0, readfunc=0x555555aa2f10 <block_crypto_read_func>,
> >     writefunc=0x555555aa2e50 <block_crypto_write_func>, opaque=0x555558a259d0, options=0x7fffec5fff30, force=<optimized out>, errp=0x555558a8c370)
> >     at ../../devel/qemu/crypto/block-luks.c:1865
> > #11 0x0000555555aa3852 in block_crypto_amend_options_generic_luks (bs=<optimized out>, amend_options=<optimized out>, force=<optimized out>,
> >     errp=<optimized out>) at ../../devel/qemu/block/crypto.c:949
> > #12 0x0000555555aa38e9 in block_crypto_co_amend_luks (bs=<optimized out>, opts=<optimized out>, force=<optimized out>, errp=<optimized out>)
> >     at ../../devel/qemu/block/crypto.c:1008
> > #13 0x0000555555a96030 in blockdev_amend_run (job=0x555558a8c2b0, errp=0x555558a8c370) at ../../devel/qemu/block/amend.c:52
> > #14 0x0000555555a874ad in job_co_entry (opaque=0x555558a8c2b0) at ../../devel/qemu/job.c:1112
> > #15 0x0000555555bdc41b in coroutine_trampoline (i0=<optimized out>, i1=<optimized out>) at ../../devel/qemu/util/coroutine-ucontext.c:175
> > #16 0x00007ffff4b68f70 in ?? () from target:/lib64/libc.so.6
> > #17 0x00007fffffffc310 in ?? ()
> > #18 0x0000000000000000 in ?? ()
> 
> Hm, so block_crypto_read_func() isn't prepared to be called in coroutine
> context, but block_crypto_co_amend_luks() still calls it from a
> coroutine. The indirection of going through QCrypto won't make it easier
> to fix this properly.

Historically block_crypto_read_func() didn't care/know whether it
was in a coroutine or not. Bisect tells me the regression was caused
by

  commit 1f051dcbdf2e4b6f518db731c84e304b2b9d15ce
  Author: Kevin Wolf <kwolf@redhat.com>
  Date:   Fri Oct 27 17:53:33 2023 +0200

    block: Protect bs->file with graph_lock

which added
 
    GLOBAL_STATE_CODE();
    GRAPH_RDLOCK_GUARD_MAINLOOP();

> It seems to me that while block_crypto_read/write_func are effectively
> no_coroutine_fn, qcrypto_block_amend_options() should really take
> function pointers that can be called from coroutines. It is called from
> both coroutine and non-coroutine code paths, so should the function
> pointers be coroutine_mixed_fn or do we want to change the callers?
>
> Either way, we should add the appropriate coroutine markers to the
> QCrypto interfaces to show the intention at least.

I'm unclear why QCrypto needs to know about coroutines at all ?
It just wants a function pointer that will send or recv a blob
of data. In the case of the block layer these functions end up
doing I/O via the block APIs, but QCrypto doesn't care about
this impl detail.


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: Some iotests are failing with -luks

[Kevin Wolf]

Am 12.09.2025 um 16:23 hat Daniel P. Berrangé geschrieben:
> On Thu, Sep 11, 2025 at 02:13:47PM +0200, Kevin Wolf wrote:
> > Am 11.09.2025 um 13:21 hat Thomas Huth geschrieben:
> > > On 10/09/2025 18.08, Kevin Wolf wrote:
> > > > Am 10.09.2025 um 17:16 hat Thomas Huth geschrieben:
> > > > > 
> > > > >   Hi,
> > > > > 
> > > > > when running "./check -luks" in the qemu-iotests directory,
> > > > > some tests are failing for me:
> > > > > 
> > > > > 295 296 inactive-node-nbd luks-detached-header
> > > > > 
> > > > > Is that a known problem already?
> > > > 
> > > > Not to me anyway.
> > > > 
> > > > > FWIW, 295 is failing with the following output:
> > > > > 
> > > > > 295   fail       [17:03:01] [17:03:17]   15.7s                failed, exit status 1
> > > > > [...]
> > > > > +EWARNING:qemu.machine.machine:qemu received signal 6; command: "/home/thuth/tmp/qemu-build/qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,fd=5 -mon chardev=mon,mode=control -chardev socket,id=qtest,fd=3 -qtest chardev:qtest -accel qtest -nodefaults -display none -accel qtest"
> > > > > +EEWARNING:qemu.machine.machine:qemu received signal 6; command: "/home/thuth/tmp/qemu-build/qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,fd=6 -mon chardev=mon,mode=control -chardev socket,id=qtest,fd=3 -qtest chardev:qtest -accel qtest -nodefaults -display none -accel qtest"
> > > > > +EEWARNING:qemu.machine.machine:qemu received signal 6; command: "/home/thuth/tmp/qemu-build/qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,fd=10 -mon chardev=mon,mode=control -chardev socket,id=qtest,fd=3 -qtest chardev:qtest -accel qtest -nodefaults -display none -accel qtest"
> > > > > +E
> > > > > [...]
> > > > > 
> > > > > etc.
> > > > > 
> > > > > 296 looks very similar (also a "qemu received signal 6" error),
> > > > > but the others look like this:
> > > > 
> > > > When it gets signal 6 (i.e. SIGABRT), that usually means that you should
> > > > have a look at the coredump.
> > > 
> > > With "-p" I additionally get this error message in the log:
> > > 
> > > qemu-system-x86_64: ../../devel/qemu/block/graph-lock.c:294:
> > >  bdrv_graph_rdlock_main_loop: Assertion `!qemu_in_coroutine()' failed.
> > > 
> > > With -gdb I can get a back trace that looks like this:
> > > 
> > > Thread 1 "qemu-system-x86" received signal SIGABRT, Aborted.
> > > 0x00007ffff4ba7e9c in __pthread_kill_implementation () from target:/lib64/libc.so.6
> > > --Type <RET> for more, q to quit, c to continue without paging--
> > > #0  0x00007ffff4ba7e9c in __pthread_kill_implementation () from target:/lib64/libc.so.6
> > > #1  0x00007ffff4b4df3e in raise () from target:/lib64/libc.so.6
> > > #2  0x00007ffff4b356d0 in abort () from target:/lib64/libc.so.6
> > > #3  0x00007ffff4b35639 in __assert_fail_base.cold () from target:/lib64/libc.so.6
> > > #4  0x0000555555574eae in bdrv_graph_rdlock_main_loop () at ../../devel/qemu/block/graph-lock.c:294
> > > #5  0x0000555555aa2f43 in graph_lockable_auto_lock_mainloop (x=<optimized out>) at /home/thuth/devel/qemu/include/block/graph-lock.h:275
> > > #6  block_crypto_read_func (block=<optimized out>, offset=4096, buf=0x555558324100 "", buflen=256000, opaque=0x555558a259d0, errp=0x555558a8c370)
> > >     at ../../devel/qemu/block/crypto.c:71
> > > #7  0x0000555555a5a308 in qcrypto_block_luks_load_key (block=block@entry=0x555558686ec0, slot_idx=slot_idx@entry=0,
> > >     password=password@entry=0x555558626050 "hunter0", masterkey=masterkey@entry=0x55555886b2a0 "",
> > >     readfunc=readfunc@entry=0x555555aa2f10 <block_crypto_read_func>, opaque=opaque@entry=0x555558a259d0, errp=0x555558a8c370)
> > >     at ../../devel/qemu/crypto/block-luks.c:927
> > > #8  0x0000555555a5ba7e in qcrypto_block_luks_find_key (block=0x555558686ec0, password=0x555558626050 "hunter0", masterkey=0x55555886b2a0 "",
> > >     readfunc=0x555555aa2f10 <block_crypto_read_func>, opaque=0x555558a259d0, errp=0x555558a8c370) at ../../devel/qemu/crypto/block-luks.c:1045
> > > #9  qcrypto_block_luks_amend_add_keyslot (block=0x555558686ec0, readfunc=0x555555aa2f10 <block_crypto_read_func>,
> > >     writefunc=0x555555aa2e50 <block_crypto_write_func>, opaque=0x555558a259d0, opts_luks=0x7fffec5fff38, force=<optimized out>, errp=0x555558a8c370)
> > >     at ../../devel/qemu/crypto/block-luks.c:1673
> > > #10 qcrypto_block_luks_amend_options (block=0x555558686ec0, readfunc=0x555555aa2f10 <block_crypto_read_func>,
> > >     writefunc=0x555555aa2e50 <block_crypto_write_func>, opaque=0x555558a259d0, options=0x7fffec5fff30, force=<optimized out>, errp=0x555558a8c370)
> > >     at ../../devel/qemu/crypto/block-luks.c:1865
> > > #11 0x0000555555aa3852 in block_crypto_amend_options_generic_luks (bs=<optimized out>, amend_options=<optimized out>, force=<optimized out>,
> > >     errp=<optimized out>) at ../../devel/qemu/block/crypto.c:949
> > > #12 0x0000555555aa38e9 in block_crypto_co_amend_luks (bs=<optimized out>, opts=<optimized out>, force=<optimized out>, errp=<optimized out>)
> > >     at ../../devel/qemu/block/crypto.c:1008
> > > #13 0x0000555555a96030 in blockdev_amend_run (job=0x555558a8c2b0, errp=0x555558a8c370) at ../../devel/qemu/block/amend.c:52
> > > #14 0x0000555555a874ad in job_co_entry (opaque=0x555558a8c2b0) at ../../devel/qemu/job.c:1112
> > > #15 0x0000555555bdc41b in coroutine_trampoline (i0=<optimized out>, i1=<optimized out>) at ../../devel/qemu/util/coroutine-ucontext.c:175
> > > #16 0x00007ffff4b68f70 in ?? () from target:/lib64/libc.so.6
> > > #17 0x00007fffffffc310 in ?? ()
> > > #18 0x0000000000000000 in ?? ()
> > 
> > Hm, so block_crypto_read_func() isn't prepared to be called in coroutine
> > context, but block_crypto_co_amend_luks() still calls it from a
> > coroutine. The indirection of going through QCrypto won't make it easier
> > to fix this properly.
> 
> Historically block_crypto_read_func() didn't care/know whether it
> was in a coroutine or not. Bisect tells me the regression was caused
> by
> 
>   commit 1f051dcbdf2e4b6f518db731c84e304b2b9d15ce
>   Author: Kevin Wolf <kwolf@redhat.com>
>   Date:   Fri Oct 27 17:53:33 2023 +0200
> 
>     block: Protect bs->file with graph_lock
> 
> which added
>  
>     GLOBAL_STATE_CODE();
>     GRAPH_RDLOCK_GUARD_MAINLOOP();
> 
> > It seems to me that while block_crypto_read/write_func are effectively
> > no_coroutine_fn, qcrypto_block_amend_options() should really take
> > function pointers that can be called from coroutines. It is called from
> > both coroutine and non-coroutine code paths, so should the function
> > pointers be coroutine_mixed_fn or do we want to change the callers?
> >
> > Either way, we should add the appropriate coroutine markers to the
> > QCrypto interfaces to show the intention at least.
> 
> I'm unclear why QCrypto needs to know about coroutines at all ?
> It just wants a function pointer that will send or recv a blob
> of data. In the case of the block layer these functions end up
> doing I/O via the block APIs, but QCrypto doesn't care about
> this impl detail.

Does a case where it's not in the context of the block layer even exist?
The only callers of qcrypto_block_amend_options() are in block/crypto.c
and block/qcow2.c. Apart from a test case, qcrypto_block_open() is the
same.

And even ignoring the block layer, doing synchronous I/O outside of
coroutines is arguably a bug anyway because that's a blocking operation
that stops the mainloop from making progress.

But if we don't want to fix it at the QCrypto level, that makes the
function pointer implicitly coroutine_mixed_fn and the function needs to
be rewritten to check qemu_in_coroutine() and probably take the graph
lock internally before calling bdrv_co_pread() in the coroutine case,
unless we can prove that all callers already hold it. Unfortunately,
function pointers still defeat TSA, so this proof will have to be
manual.

Kevin

Re: Some iotests are failing with -luks

[Daniel P. Berrangé]

On Fri, Sep 12, 2025 at 04:53:19PM +0200, Kevin Wolf wrote:
> Am 12.09.2025 um 16:23 hat Daniel P. Berrangé geschrieben:
> > On Thu, Sep 11, 2025 at 02:13:47PM +0200, Kevin Wolf wrote:
> > > Hm, so block_crypto_read_func() isn't prepared to be called in coroutine
> > > context, but block_crypto_co_amend_luks() still calls it from a
> > > coroutine. The indirection of going through QCrypto won't make it easier
> > > to fix this properly.
> > 
> > Historically block_crypto_read_func() didn't care/know whether it
> > was in a coroutine or not. Bisect tells me the regression was caused
> > by
> > 
> >   commit 1f051dcbdf2e4b6f518db731c84e304b2b9d15ce
> >   Author: Kevin Wolf <kwolf@redhat.com>
> >   Date:   Fri Oct 27 17:53:33 2023 +0200
> > 
> >     block: Protect bs->file with graph_lock
> > 
> > which added
> >  
> >     GLOBAL_STATE_CODE();
> >     GRAPH_RDLOCK_GUARD_MAINLOOP();
> > 
> > > It seems to me that while block_crypto_read/write_func are effectively
> > > no_coroutine_fn, qcrypto_block_amend_options() should really take
> > > function pointers that can be called from coroutines. It is called from
> > > both coroutine and non-coroutine code paths, so should the function
> > > pointers be coroutine_mixed_fn or do we want to change the callers?
> > >
> > > Either way, we should add the appropriate coroutine markers to the
> > > QCrypto interfaces to show the intention at least.
> > 
> > I'm unclear why QCrypto needs to know about coroutines at all ?
> > It just wants a function pointer that will send or recv a blob
> > of data. In the case of the block layer these functions end up
> > doing I/O via the block APIs, but QCrypto doesn't care about
> > this impl detail.
> 
> Does a case where it's not in the context of the block layer even exist?

Only the unit tests.

> The only callers of qcrypto_block_amend_options() are in block/crypto.c
> and block/qcow2.c. Apart from a test case, qcrypto_block_open() is the
> same.

Yep

> And even ignoring the block layer, doing synchronous I/O outside of
> coroutines is arguably a bug anyway because that's a blocking operation
> that stops the mainloop from making progress.

More generally, simply opening a LUKS volume can also impose a significant
delay because key validation is intentionally slow in wallclock time. So we
should get a minimum of 1 second delay, but if given an image created on a
significantly faster machine (or a malicious image), the larger 'iterations'
count could make us take way longer to open the image.

I guess that's a potential problem too ?

Amending the keys has the same performance penalty too as that involves
same intentionally slow crypto

> But if we don't want to fix it at the QCrypto level, that makes the
> function pointer implicitly coroutine_mixed_fn and the function needs to
> be rewritten to check qemu_in_coroutine() and probably take the graph
> lock internally before calling bdrv_co_pread() in the coroutine case,
> unless we can prove that all callers already hold it. Unfortunately,
> function pointers still defeat TSA, so this proof will have to be
> manual.

So IIUC the 'open' operation is not in a coroutine, while the
'amend' operation is in a coroutine ?

IIUC the coroutine_mixed_fn is expanding to a no-op. What is the
actual functional fix needed to stop the crash ?

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: Some iotests are failing with -luks

[Kevin Wolf]

Am 12.09.2025 um 20:35 hat Daniel P. Berrangé geschrieben:
> On Fri, Sep 12, 2025 at 04:53:19PM +0200, Kevin Wolf wrote:
> > Am 12.09.2025 um 16:23 hat Daniel P. Berrangé geschrieben:
> > > On Thu, Sep 11, 2025 at 02:13:47PM +0200, Kevin Wolf wrote:
> > > > Hm, so block_crypto_read_func() isn't prepared to be called in coroutine
> > > > context, but block_crypto_co_amend_luks() still calls it from a
> > > > coroutine. The indirection of going through QCrypto won't make it easier
> > > > to fix this properly.
> > > 
> > > Historically block_crypto_read_func() didn't care/know whether it
> > > was in a coroutine or not. Bisect tells me the regression was caused
> > > by
> > > 
> > >   commit 1f051dcbdf2e4b6f518db731c84e304b2b9d15ce
> > >   Author: Kevin Wolf <kwolf@redhat.com>
> > >   Date:   Fri Oct 27 17:53:33 2023 +0200
> > > 
> > >     block: Protect bs->file with graph_lock
> > > 
> > > which added
> > >  
> > >     GLOBAL_STATE_CODE();
> > >     GRAPH_RDLOCK_GUARD_MAINLOOP();
> > > 
> > > > It seems to me that while block_crypto_read/write_func are effectively
> > > > no_coroutine_fn, qcrypto_block_amend_options() should really take
> > > > function pointers that can be called from coroutines. It is called from
> > > > both coroutine and non-coroutine code paths, so should the function
> > > > pointers be coroutine_mixed_fn or do we want to change the callers?
> > > >
> > > > Either way, we should add the appropriate coroutine markers to the
> > > > QCrypto interfaces to show the intention at least.
> > > 
> > > I'm unclear why QCrypto needs to know about coroutines at all ?
> > > It just wants a function pointer that will send or recv a blob
> > > of data. In the case of the block layer these functions end up
> > > doing I/O via the block APIs, but QCrypto doesn't care about
> > > this impl detail.
> > 
> > Does a case where it's not in the context of the block layer even exist?
> 
> Only the unit tests.
> 
> > The only callers of qcrypto_block_amend_options() are in block/crypto.c
> > and block/qcow2.c. Apart from a test case, qcrypto_block_open() is the
> > same.
> 
> Yep
> 
> > And even ignoring the block layer, doing synchronous I/O outside of
> > coroutines is arguably a bug anyway because that's a blocking operation
> > that stops the mainloop from making progress.
> 
> More generally, simply opening a LUKS volume can also impose a significant
> delay because key validation is intentionally slow in wallclock time. So we
> should get a minimum of 1 second delay, but if given an image created on a
> significantly faster machine (or a malicious image), the larger 'iterations'
> count could make us take way longer to open the image.
> 
> I guess that's a potential problem too ?
> 
> Amending the keys has the same performance penalty too as that involves
> same intentionally slow crypto

Yes, that could be very noticable for the guest.

bdrv_open() being synchronous is a problem in general. It is somewhat
mitigated in some block drivers like qcow2 by creating a coroutine that
does the real open and an AIO_WAIT_WHILE_UNLOCKED() call that waits for
the coroutine, but processes other events, too (though it doesn't
process events in iohandler_ctx, like new QMP commands). But at least in
the common case, bdrv_open() is relatively fast anyway.

I guess the easiest way to deal with it in the luks block driver would
be offloading the calculations to a task in the thread pool and using
AIO_WAIT_WHILE_UNLOCKED() to wait for its completion. We could combine
it with the qcow2 approach to cover the read request(s?) for the image
file, too.

> > But if we don't want to fix it at the QCrypto level, that makes the
> > function pointer implicitly coroutine_mixed_fn and the function needs to
> > be rewritten to check qemu_in_coroutine() and probably take the graph
> > lock internally before calling bdrv_co_pread() in the coroutine case,
> > unless we can prove that all callers already hold it. Unfortunately,
> > function pointers still defeat TSA, so this proof will have to be
> > manual.
> 
> So IIUC the 'open' operation is not in a coroutine, while the
> 'amend' operation is in a coroutine ?

Yes.

> IIUC the coroutine_mixed_fn is expanding to a no-op. What is the
> actual functional fix needed to stop the crash ?

The problem is that block_crypto_read_func() calls bdrv_pread().

When called outside of a coroutine, it creates a coroutine, takes the
graph reader lock and calls bdrv_co_pread().

When called from a coroutine, it's just a thin wrapper that calls
bdrv_co_pread() directly without creating a new coroutine and without
taking the graph lock, but calling bdrv_co_pread() still requires that
you hold the lock. So we need to make sure that either the graph lock is
already taken when the function is called from coroutine context, or we
must take it here.

Kevin