> So this looks good under two conditions that I haven't checked yet: That > bdrv_detach_aio_context() is actually safe when called with the "wrong" > AioContext lock held, and that the .change_aio_context() callbacks are > implemented correctly in a later patch.
So regarding bdrv_detach_aio_context(), I added a aio_context_lock_acquire/release pair around that function in commit(). The callbacks should be implemented correctly, as you didn't point out anything later on :) > > To reiterate my initial point, reviewing this took me some effort to > match the new functions with the existing ones they duplicate and then > manual diffing of each, which is kind of error prone. I feel the better > approach would have been adding a tran parameter (with empty commit and > abort) to the existing functions in a first patch and then change stuff > in a second patch (in the real code, not dead code to be enabled later), > so that you would actually see the real changes instead of having to > find them between lots of unchanged copied code. > Yes, I see what you mean. I'll change my approach on the next series, thanks for the suggestion! Thank you, Emanuele