On Tue, 06/14 19:17, Stefan Hajnoczi wrote: > --- a/blockjob.c > +++ b/blockjob.c > @@ -247,6 +247,30 @@ void block_job_complete(BlockJob *job, Error **errp) > job->driver->complete(job, errp); > } > > +void block_job_pause_point(BlockJob *job) > +{ > + if (!block_job_is_paused(job)) {
I find this check ... > + return; > + } > + if (block_job_is_cancelled(job)) { > + return; > + } > + > + if (job->driver->pause) { > + job->driver->pause(job); > + } > + > + job->paused = true; ... and this assignment confusing. After reading more, I think we ought to rename block_job_is_paused to block_job_should_pause and mark it static, in a separate patch. > + job->busy = false; > + qemu_coroutine_yield(); /* wait for block_job_resume() */ > + job->busy = true; > + job->paused = false; Worth to "assert(!job->pause_count)" (or "assert(!block_job_should_pause(job))")? Regardless, Reviewed-by: Fam Zheng <f...@redhat.com> > + > + if (job->driver->resume) { > + job->driver->resume(job); > + } > +} > +