Am 11.12.2018 um 17:55 hat Denis Plotnikov geschrieben: > > Why involve the AioContext at all? This could all be kept at the > > BlockBackend level without extending the layering violation that > > aio_disable_external() is. > > > > BlockBackends get notified when their root node is drained, so hooking > > things up there should be as easy, if not even easier than in > > AioContext. > > Just want to make sure that I understood correctly what you meant by > "BlockBackends get notified". Did you mean that bdrv_drain_end calls > child's role callback blk_root_drained_end by calling > bdrv_parent_drained_end?
Yes, blk_root_drained_begin/end calls are all you need. Specifically, their adjustments to blk->quiesce_counter that are already there, and in the 'if (--blk->quiesce_counter == 0)' block of blk_root_drained_end() we can resume the queued requests. > In case if it's so, it won't work if resume postponed requests in > blk_root_drained_end since we can't know if external is disabled for the > context because the counter showing that is decreased only after roles' > drained callbacks are finished at bdrv_do_drained_end. > Please correct me if I'm wrong. You don't need to know about the AioContext state, this is the whole point. blk->quiesce_counter is what tells you whether to postpone requests. > Looking at the patch again, I think that it might be useful to keep the > requests in the structure that limits their execution and also protects > the access (context acquire/release) although it's indeed the layering > violation but at least we can store the parts related at the same place > and later on move somewhere else alongside the request restrictor. You can keep everything you need in BlockBackend (and that's also where your code is that really postpones request). Kevin
