On 07/23/2017 10:36 AM, Paolo Bonzini wrote: > On 21/07/2017 17:47, Stefan Hajnoczi wrote: >> Hmm...BlockDriverState still has bdrv_is_inserted() even though >> BlockBackend->root can be NULL? CCing Markus in case he has thoughts on >> the BB/BDS split. >> >> I find it weird that block-backend.c calls bdrv_inc_in_flight() and then >> bdrv_co_flush() will call it again. Perhaps we need to do this so that >> blk_drain() works correctly but it's odd that they share the same >> counter variable. > > See here: > https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg02305.html >
>> I need to count requests at the BB level because the blk_aio_* >> operations have a separate bottom half that is invoked if either 1) they >> never reach BDS (because of an error); or 2) the bdrv_co_* call >> completes without yielding. The count must be >0 when blk_aio_* >> returns, or bdrv_drain (and thus blk_drain) won't loop. Because >> bdrv_drain and blk_drain are conflated, the counter must be the BDS one. >> >> In turn, the BDS counter is needed because of the lack of isolated >> sections. The right design would be for blk_isolate_begin to call >> blk_drain on *other* BlockBackends reachable in a child-to-parent visit. >> Instead, until that is implemented, we have bdrv_drained_begin that >> emulates that through the same-named callback, followed by a >> parent-to-child bdrv_drain that is almost always unnecessary. OK, so you have some justifications for why it needs to be at the BB level... > > The full explanation of the long-term plans and what "isolated section" > means is at > https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg02016.html. > (Unfortunately, since then we've reintroduced the "double aio_poll" hack > in BDRV_POLL_WHILE...). > > Paolo > I may need some nudging towards understanding what the right solution here is, though. Should the blk_aio_flush assume that there always is a root BDS? should it not assume that? It's difficult for me to understand right now if the bug is in the expectation for the blk_ functions and the caller should be amended, or if you need changes to the way the blk_ functions are trying to increment a counter that doesn't exist. I can handle the former fairly easily; if it's the latter, I'm afraid it's stuck in the middle of some of your changes and I'd need a stronger hint. John