Re: [PULL v3 00/50] Block layer patches

[Fabiano Rosas]

Kevin Wolf <kwolf@redhat.com> writes:

> The following changes since commit 48804eebd4a327e4b11f902ba80a00876ee53a43:
>
>   Merge tag 'pull-misc-2022-12-14' of https://repo.or.cz/qemu/armbru into 
> staging (2022-12-15 10:13:46 +0000)
>
> are available in the Git repository at:
>
>   https://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to 1b3ff9feb942c2ad0b01ac931e99ad451ab0ef39:
>
>   block: GRAPH_RDLOCK for functions only called by co_wrappers (2022-12-15 
> 16:08:23 +0100)
>
> v3:
> - Dropped "configure: Enable -Wthread-safety if present" because FreeBSD
>   has TSA annotations in its pthread locking functions, so we would have
>   to annotate the use of every lock in QEMU first before we can enable
>   it.
>
> v2:
> - Changed TSA capability name to "mutex" to work with older clang
>   versions. The tsan-build CI job succeeds now.
>
> ----------------------------------------------------------------
> Block layer patches
>
> - Code cleanups around block graph modification
> - Simplify drain
> - coroutine_fn correctness fixes, including splitting generated
>   coroutine wrappers into co_wrapper (to be called only from
>   non-coroutine context) and co_wrapper_mixed (both coroutine and
>   non-coroutine context)
> - Introduce a block graph rwlock
>
> ----------------------------------------------------------------
> Emanuele Giuseppe Esposito (21):
>       block-io: introduce coroutine_fn duplicates for 
> bdrv_common_block_status_above callers
>       block-copy: add coroutine_fn annotations
>       nbd/server.c: add coroutine_fn annotations
>       block-backend: replace bdrv_*_above with blk_*_above
>       block/vmdk: add coroutine_fn annotations
>       block: avoid duplicating filename string in bdrv_create
>       block: distinguish between bdrv_create running in coroutine and not
>       block: bdrv_create_file is a coroutine_fn
>       block: rename generated_co_wrapper in co_wrapper_mixed
>       block-coroutine-wrapper.py: introduce co_wrapper
>       block-coroutine-wrapper.py: support functions without bs arg
>       block-coroutine-wrapper.py: support also basic return types
>       block: convert bdrv_create to co_wrapper
>       block/dirty-bitmap: convert coroutine-only functions to co_wrapper
>       graph-lock: Implement guard macros
>       async: Register/unregister aiocontext in graph lock list
>       block: wrlock in bdrv_replace_child_noperm
>       block: remove unnecessary assert_bdrv_graph_writable()
>       block: assert that graph read and writes are performed correctly
>       block-coroutine-wrapper.py: introduce annotations that take the graph 
> rdlock
>       block: use co_wrapper_mixed_bdrv_rdlock in functions taking the rdlock
>
> Kevin Wolf (24):
>       qed: Don't yield in bdrv_qed_co_drain_begin()
>       test-bdrv-drain: Don't yield in .bdrv_co_drained_begin/end()
>       block: Revert .bdrv_drained_begin/end to non-coroutine_fn
>       block: Remove drained_end_counter
>       block: Inline bdrv_drain_invoke()
>       block: Fix locking for bdrv_reopen_queue_child()
>       block: Drain individual nodes during reopen
>       block: Don't use subtree drains in bdrv_drop_intermediate()
>       stream: Replace subtree drain with a single node drain
>       block: Remove subtree drains
>       block: Call drain callbacks only once
>       block: Remove ignore_bds_parents parameter from drain_begin/end.
>       block: Drop out of coroutine in bdrv_do_drained_begin_quiesce()
>       block: Don't poll in bdrv_replace_child_noperm()
>       block: Remove poll parameter from bdrv_parent_drained_begin_single()
>       block: Factor out bdrv_drain_all_begin_nopoll()

Hi, 

With today's master at c15dc499cc (Merge tag 'pull-misc-20221218'
of https://gitlab.com/rth7680/qemu into staging, 2022-12-19),

I get a test failure:

$ make check-avocado 
AVOCADO_TESTS=../tests/avocado/reverse_debugging.py:ReverseDebugging_X86_64.test_x86_64_pc
...
Output: qemu-system-x86_64: ../block/block-backend.c:2572:
blk_root_drained_poll: Assertion `blk->quiesce_counter' failed.

Bisect points to this patch
da0bd74434 (block: Factor out bdrv_drain_all_begin_nopoll(), 2022-12-07)

Command from avocado logs (paths cut to save space):

./qemu-system-x86_64 -display none -vga none \
-chardev socket,id=mon,path=monitor.sock \
-mon chardev=mon,mode=control -machine pc \
-chardev socket,id=console,path=console.sock,server=on,wait=off \
-serial chardev:console -icount 
shift=7,rr=record,rrfile=replay.bin,rrsnapshot=init \
-net none -drive file=disk.qcow2,if=none

Happens with arm as well:

./qemu-system-aarch64 -display none -vga none \
-chardev socket,id=mon,path=monitor.sock \
-mon chardev=mon,mode=control -machine virt \
-chardev socket,id=console,path=console.sock,server=on,wait=off \
-serial chardev:console -cpu cortex-a53 -icount \
 shift=7,rr=record,rrfile=replay.bin,rrsnapshot=init \
-net none -drive file=disk.qcow2,if=none \
-kernel vmlinuz

>       Import clang-tsa.h
>       clang-tsa: Add TSA_ASSERT() macro
>       clang-tsa: Add macros for shared locks
>       test-bdrv-drain: Fix incorrrect drain assumptions
>       block: Fix locking in external_snapshot_prepare()
>       graph-lock: TSA annotations for lock/unlock functions
>       Mark assert_bdrv_graph_readable/writable() GRAPH_RD/WRLOCK
>       block: GRAPH_RDLOCK for functions only called by co_wrappers
>
> Paolo Bonzini (1):
>       graph-lock: Introduce a lock to protect block graph operations
>
> Vladimir Sementsov-Ogievskiy (4):
>       block: Inline bdrv_detach_child()
>       block: drop bdrv_remove_filter_or_cow_child
>       block: bdrv_refresh_perms(): allow external tran
>       block: refactor bdrv_list_refresh_perms to allow any list of nodes
>