Am 22.05.2025 um 11:56 hat Fiona Ebner geschrieben: > Am 21.05.25 um 18:36 schrieb Kevin Wolf: > > 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(). > > For deciding on the variant, I only looked at whether the caller holds > the graph lock or not.
Oh, I see. I was thinking much too complicated once again. Maybe just add "the appropriate variant, depending on whether the caller already holds the lock"? > > 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. > > Right, my patches are not enough to address that kind of problem. > > > 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. > > Yes, I can mention something in the commit message, maybe: > > Functions like qmp_blockdev_mirror() query the nodes to act on before > draining and locking. In theory, draining could invalidate those nodes. > This kind of issue is not addressed by these commits. Doesn't hurt, but it's also not really needed. Kevin
