Re: [Qemu-devel] [PATCH] blk: postpone request execution on a context protected with "drained section"

[Kevin Wolf]

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