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
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
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
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
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 :|
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
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 :|
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
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
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 :|
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
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
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 :|
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
© 2016 - 2025 Red Hat, Inc.