On Mon, Feb 01, 2016 at 10:49:00AM +0800, Fam Zheng wrote:
> On Fri, 01/29 11:31, Stefan Hajnoczi wrote:
> > On Fri, Jan 29, 2016 at 10:19:49AM +0800, Fam Zheng wrote:
> > > @@ -402,6 +407,10 @@ typedef void BlockJobDeferToMainLoopFn(BlockJob
> > > *job, void *opaque);
> > > * AioContext acquired. Block jobs must call bdrv_unref(),
> > > bdrv_close(), and
> > > * anything that uses bdrv_drain_all() in the main loop.
> > > *
> > > + * The job->deferred_to_main_loop flag will be set. Caller must clear it
> > > once
> > > + * the deferred work is done and the block job coroutine continues,
> > > unless it's
> > > + * completing immediately.
> > > + *
> >
> > It's not necessary to expose job->deferred_to_main_loop to the user.
> > Just clear it:
> >
> > static void block_job_defer_to_main_loop_bh(void *opaque)
> > {
> > BlockJobDeferToMainLoopData *data = opaque;
> > AioContext *aio_context;
> >
> > qemu_bh_delete(data->bh);
> >
> > /* Prevent race with block_job_defer_to_main_loop() */
> > aio_context_acquire(data->aio_context);
> >
> > /* Fetch BDS AioContext again, in case it has changed */
> > aio_context = bdrv_get_aio_context(data->job->bs);
> > aio_context_acquire(aio_context);
> >
> > data->fn(data->job, data->opaque);
> > job->deferred_to_main_loop = false; /* <----- HERE */
>
> Maybe move one line above in case data->fn() does another
> block_job_defer_to_main_loop()?Yes, good point. Thanks for spotting the bug. It's safe to clear the boolean as soon as we acquire aio_context. Stefan
signature.asc
Description: PGP signature
