On Mon, Apr 04, 2022 at 11:41:04AM +0200, Paolo Bonzini wrote: > On Mon, Apr 4, 2022 at 11:25 AM Stefan Hajnoczi <stefa...@redhat.com> wrote: > > - The new API still needs to be combined with bdrv_drained_begin/end() > > to ensure in-flight requests are done. > > > > I don't think so, because in-flight requests would take the lock for > reading. The write side would not start until those in-flight requests > release the lock.
Good point! > - It's not obvious to me whether the new API obsoletes is_external. I think > > it probably doesn't. > > > > I agree that it doesn't. This new lock is only protecting ->parents and > ->children. bdrv_drained_begin()/end() remains necessary, for example, when > you need to send a request during the drained section. An example is > block_resize. > > In addition, bdrv_drained_begin()/end() ensures that the callback of > blk_aio_*() functions has been invoked (see commit 46aaf2a566, > "block-backend: Decrease in_flight only after callback", 2018-09-25). This > new lock would not ensure that. > > As an aside, instead of is_external, QEMU could remove/add the ioeventfd > handler in the blk->dev_ops->drained_begin and blk->dev_ops->drained_end > callbacks respectively. But that's just a code cleanup. Interesting idea. If is_external is a block layer-specific feature that nothing else in QEMU uses then I like the idea because it's cleaner and more obvious than is_external. Stefan
signature.asc
Description: PGP signature
