On Thu, 04/13 14:51, Paolo Bonzini wrote:
>
>
> On 11/04/2017 00:04, Stefan Hajnoczi wrote:
> > On Mon, Apr 10, 2017 at 10:26:34AM +0800, Fam Zheng wrote:
> >> v3: Use bdrv_parent_drained_begin/end. [Kevin]
> >> Do it before releasing new_context. [Stefan]
> >>
> >> Fam Zheng (2):
> >> block: Make bdrv_parent_drained_begin/end public
> >> block: Quiesce old aio context during bdrv_set_aio_context
> >>
> >> block.c | 7 +++++--
> >> block/io.c | 4 ++--
> >> include/block/block.h | 16 ++++++++++++++++
> >> 3 files changed, 23 insertions(+), 4 deletions(-)
> >
> > Thanks, applied to my block tree for 2.10:
> > https://github.com/stefanha/qemu/commits/block
> >
> > If you'd like this in a stable release please CC qemu-stable.
>
> Stefan, can you squash this documentation patch in please?
>
> diff --git a/block.c b/block.c
> index 3b20a29..cb1c5c9 100644
> --- a/block.c
> +++ b/block.c
> @@ -4392,6 +4392,18 @@ void bdrv_set_aio_context(BlockDriverState *bs,
> AioContext *new_context)
> {
> AioContext *ctx = bdrv_get_aio_context(bs);
>
> + /* FIXME: This is the same as bdrv_drained_begin, only done
> + * manually. At the end of this function, aio_enable_external
> + * must be done on ctx, not on new_context, so we cannot call
> + * bdrv_drained_end there. And because bs->quiesce_counter will
> + * not be decreased there, we have to "explode" bdrv_drained_begin
> + * here too.
> + *
> + * Once aio_disable_external and aio_enable_external are replaced
> + * by BlockBackend device ops (called by .drained_begin/end
> + * BdrvChildRole callbacks), this can be replaced by a simple
> + * bdrv_drained_begin/end pair.
> + */
> aio_disable_external(ctx);
> bdrv_parent_drained_begin(bs);
> bdrv_drain(bs); /* ensure there are no in-flight requests */
This patch was necessary to fix the earlier crash so it is already merged with
my last pull request for -rc4:
commit aabf591007a83dc6520329bce929570115849286
Author: Fam Zheng <f...@redhat.com>
Date: Wed Apr 5 14:44:24 2017 +0800
block: Quiesce old aio context during bdrv_set_aio_context
The fact that the bs->aio_context is changing can confuse the dataplane
iothread, because of the now fine granularity aio context lock.
bdrv_drain should rather be a bdrv_drained_begin/end pair, but since
bs->aio_context is changing, we can just use aio_disable_external and
bdrv_parent_drained_begin.
Reported-by: Ed Swierk <eswi...@skyportsystems.com>
Signed-off-by: Fam Zheng <f...@redhat.com>
Acked-by: Stefan Hajnoczi <stefa...@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com>
Reviewed-by: Kevin Wolf <kw...@redhat.com>
Paolo, maybe you want to submit a formal patch for 2.10? (Sorry for the
confusion, these series of mine went a bit messy.)
Fam
