On Tue, Apr 26, 2022 at 04:51:06AM -0400, Emanuele Giuseppe Esposito wrote: > This is a new attempt to replace the need to take the AioContext lock to > protect graph modifications. In particular, we aim to remove > (or better, substitute) the AioContext around bdrv_replace_child_noperm, > since this function changes BlockDriverState's ->parents and ->children > lists. > > In the previous version, we decided to discard using subtree_drains to > protect the nodes, for different reasons: for those unfamiliar with it, > please see > https://patchew.org/QEMU/20220301142113.163174-1-eespo...@redhat.com/
I reread the thread and it's unclear to me why drain is the wrong mechanism for protecting graph modifications. We theorized a lot but ultimately is this new mechanism sufficiently different from bdrv_drained_begin()/end() to make it worth implementing? Instead of invoking .drained_begin() callbacks to stop further I/O, we're now queuing coroutines (without backpressure information that whoever is spawning I/O needs so they can stop). The writer still waits for in-flight I/O to finish, including I/O not associated with the bdrv graph we wish to modify (because rdlock is per-AioContext and unrelated to a specific graph). Is this really more lightweight than drain? If I understand correctly, the original goal was to avoid the need to hold the AioContext lock across bdrv_replace_child_noperm(). I would focus on that and use drain for now. Maybe I've missed an important point about why the new mechanism is needed? Stefan
signature.asc
Description: PGP signature
