diff mbox

[1/4] block: Use drained section in bdrv_set_aio_context

Message ID 1458123018-18651-2-git-send-email-famz@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Fam Zheng March 16, 2016, 10:10 a.m. UTC
An empty begin/end pair is almost the same as a bare bdrv_drain except
the aio_poll inside is wrapped by
aio_disable_external/aio_enable_external.

This is safer, and is the only way to achieve quiescence in this
aio_poll(), because bdrv_drained_begin/end pair cannot span across
context detach/attach options, so it's not possible to do by the caller.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Paolo Bonzini March 16, 2016, 10:27 a.m. UTC | #1
On 16/03/2016 11:10, Fam Zheng wrote:
> An empty begin/end pair is almost the same as a bare bdrv_drain except
> the aio_poll inside is wrapped by
> aio_disable_external/aio_enable_external.
> 
> This is safer, and is the only way to achieve quiescence in this
> aio_poll(), because bdrv_drained_begin/end pair cannot span across
> context detach/attach options, so it's not possible to do by the caller.

I'm still not sure about this patch.

When starting dataplane, the ioeventfd is registered with iohandler.c so
bdrv_drained_begin/end is not necessary.

Likewise when stopping dataplane bdrv_set_aio_context is called after
the thread has been stopped and thus the ioeventfd is not registered
anymore as an external client.

Paolo
Fam Zheng March 16, 2016, 10:51 a.m. UTC | #2
On Wed, 03/16 11:27, Paolo Bonzini wrote:
> 
> 
> On 16/03/2016 11:10, Fam Zheng wrote:
> > An empty begin/end pair is almost the same as a bare bdrv_drain except
> > the aio_poll inside is wrapped by
> > aio_disable_external/aio_enable_external.
> > 
> > This is safer, and is the only way to achieve quiescence in this
> > aio_poll(), because bdrv_drained_begin/end pair cannot span across
> > context detach/attach options, so it's not possible to do by the caller.
> 
> I'm still not sure about this patch.
> 
> When starting dataplane, the ioeventfd is registered with iohandler.c so
> bdrv_drained_begin/end is not necessary.

You are right, and looks like the k->set_host_notifier() above
blk_set_aio_context would disable the fd anyway.

> 
> Likewise when stopping dataplane bdrv_set_aio_context is called after
> the thread has been stopped and thus the ioeventfd is not registered
> anymore as an external client.

Right.

Fam
diff mbox

Patch

diff --git a/block.c b/block.c
index 59a18a3..31f4a9f 100644
--- a/block.c
+++ b/block.c
@@ -3747,7 +3747,9 @@  void bdrv_attach_aio_context(BlockDriverState *bs,
 
 void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context)
 {
-    bdrv_drain(bs); /* ensure there are no in-flight requests */
+    /* ensure there are no in-flight requests */
+    bdrv_drained_begin(bs);
+    bdrv_drained_end(bs);
 
     bdrv_detach_aio_context(bs);