On Thu, 01/28 11:53, Stefan Hajnoczi wrote: > On Thu, Jan 28, 2016 at 03:02:50PM +0800, Fam Zheng wrote: > > diff --git a/include/block/blockjob.h b/include/block/blockjob.h > > index d84ccd8..de59fc2 100644 > > --- a/include/block/blockjob.h > > +++ b/include/block/blockjob.h > > @@ -393,18 +393,20 @@ BlockErrorAction block_job_error_action(BlockJob > > *job, BlockDriverState *bs, > > typedef void BlockJobDeferToMainLoopFn(BlockJob *job, void *opaque); > > > > /** > > - * block_job_defer_to_main_loop: > > + * block_job_coroutine_complete: > > * @job: The job > > * @fn: The function to run in the main loop > > * @opaque: The opaque value that is passed to @fn > > * > > - * Execute a given function in the main loop with the BlockDriverState > > - * AioContext acquired. Block jobs must call bdrv_unref(), bdrv_close(), > > and > > - * anything that uses bdrv_drain_all() in the main loop. > > + * Complete the block job coroutine and execute a given function in the > > main > > This function does not "complete the block job coroutine" so this seems > confusing to me. How about "Call this function to schedule clean up > code to run in the main loop when completing a block job"?
Sounds good. > > That said, I'm not sure if changing the scope of this function is > necessary at all. If the next patch adds a job->aio_context pointer > then it could set the pointer back after calling the user's function > from the main loop. Then this function could also be used in cases > where the job/coroutine stays alive. Yes, that can be done. Fam