Re: [PATCH v4 00/25] block layer: split block APIs in global state and I/O

[Philippe Mathieu-Daudé]

On 10/25/21 12:17, Emanuele Giuseppe Esposito wrote:
[...]

> Each function in the GS API will have an assertion, checking
> that it is always running under BQL.
> I/O functions are instead thread safe (or so should be), meaning
> that they *can* run under BQL, but also in an iothread in another
> AioContext. Therefore they do not provide any assertion, and
> need to be audited manually to verify the correctness.
> 
> Adding assetions has helped finding 2 bugs already, as shown in
> my series "Migration: fix missing iothread locking".
> 
> Tested this series by running unit tests, qemu-iotests and qtests
> (x86_64).
> Some functions in the GS API are used everywhere but not
> properly tested. Therefore their assertion is never actually run in
> the tests, so despite my very careful auditing, it is not impossible
> to exclude that some will trigger while actually using QEMU.
> 
> Patch 1 introduces qemu_in_main_thread(), the function used in
> all assertions. This had to be introduced otherwise all unit tests
> would fail, since they run in the main loop but use the code in
> stubs/iothread.c
> Patches 2-14 and 19-25 (with the exception of patch 9, that is an additional
> assert) are all structured in the same way: first we split the header
> and in the next (even) patch we add assertions.
> The rest of the patches ontain either both assertions and split,
> or have no assertions.

This seems a lot of assertions added in hot-path code.

Does it makes sense to use a BLOCK_ASSERT() macro instead,
only expanded when configure with --enable-debug?