Am 20.05.2025 um 12:29 hat Fiona Ebner geschrieben: > This is part of resolving the deadlock mentioned in commit "block: > move draining out of bdrv_change_aio_context() and mark GRAPH_RDLOCK". > > Convert the function to a _locked() version that has to be called with > the graph lock held and add a convenience wrapper that has to be > called with the graph unlocked, which drains and takes the lock > itself. Since bdrv_try_change_aio_context() is global state code, the > wrapper is too. > > Callers are adapted to use the appropriate variant. In the > test_set_aio_context() unit test, prior drains can be removed, because > draining already happens inside the new wrapper.
I'm not sure what's the difference between qmp_x_blockdev_set_iothread() which is converted to _locked() and e.g. qmp_blockdev_mirror(). The reason I could see in qmp_x_blockdev_set_iothread() is that (as we discussed in the RFC version of this series) draining can change the graph and that could in theory invalidate bs. But the same is true for other functions in blockdev.c. If it's just that qmp_x_blockdev_set_iothread() was easy to fix and the others are more complex, that's fine with me, but maybe worth mentioning in the commit message. > Note that bdrv_attach_child_common_abort(), bdrv_attach_child_common() > and bdrv_root_unref_child() hold the graph lock and are not actually > allowed to drain either. This will be addressed in the following > commits. > > Signed-off-by: Fiona Ebner <f.eb...@proxmox.com> Reviewed-by: Kevin Wolf <kw...@redhat.com>
