Am 05.12.2017 um 16:02 hat Paolo Bonzini geschrieben: > On 05/12/2017 15:54, Kevin Wolf wrote: > > } > > > > + bdrv_drain_invoke(bs, true); > > bdrv_drain_recurse(bs, true); > > } > > > > @@ -294,6 +298,7 @@ void bdrv_drained_end(BlockDriverState *bs) > > } > > > > bdrv_parent_drained_end(bs); > > + bdrv_drain_invoke(bs, false); > > bdrv_drain_recurse(bs, false); > > aio_enable_external(bdrv_get_aio_context(bs)); > > I think invoke should be done after recurse from bdrv_drain*end. In the > end aio_enable_external is a special kind of drain_end callback, so > bdrv_drain_invoke should go together with it.
Yes and no. Calling them together makes sense to me. Not for this patch, though, which is just mechanical refactoring. bdrv_drain_invoke() was at the beginning of bdrv_drain_recurse(), so it ends up before it after the refactoring. But there is really no reason at all to call bdrv_drain_recurse() in bdrv_drained_end(). We only called it because it happened to involve bdrv_drain_invoke(). After this patch, the call can be removed. I can do that in a v2 (in a separate patch). What we probably also should do is move bdrv_parent_drained_end() after bdrv_drain_invoke() so that children are ready to accept new requests when the parent callback is called. There is also some inconsistency between bdrv_drain() and bdrv_drain_all() about the order of these operations. I hope that in the end they would just call the same helper and only the actual polling loop stays separate (as long as needed). Kevin
