Re: [Qemu-devel] [PATCH 4/4] blockjob: reimplement block_job_sleep_ns to allow cancellation

[Jeff Cody <jcody@redhat.com>]

On Wed, Nov 29, 2017 at 01:54:39PM +0100, Kevin Wolf wrote:
> Am 29.11.2017 um 11:25 hat Paolo Bonzini geschrieben:
> > This reverts the effects of commit 4afeffc857 ("blockjob: do not allow
> > coroutine double entry or entry-after-completion", 2017-11-21)
> > 
> > This fixed the symptom of a bug rather than the root cause. Canceling the
> > wait on a sleeping blockjob coroutine is generally fine, we just need to
> > make it work correctly across AioContexts.  To do so, use a QEMUTimer
> > that calls block_job_enter.  Use a mutex to ensure that block_job_enter
> > synchronizes correctly with block_job_sleep_ns.
> > 
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >  blockjob.c                   | 57 +++++++++++++++++++++++++++++++++++---------
> >  include/block/blockjob.h     |  5 +++-
> >  include/block/blockjob_int.h |  4 ++--
> >  3 files changed, 52 insertions(+), 14 deletions(-)
> > 
> > diff --git a/blockjob.c b/blockjob.c
> > index 4d22b7d2fb..3fdaabbc1f 100644
> > --- a/blockjob.c
> > +++ b/blockjob.c
> > @@ -37,6 +37,26 @@
> >  #include "qemu/timer.h"
> >  #include "qapi-event.h"
> >  
> > +/* Right now, this mutex is only needed to synchronize accesses to job->busy,
> > + * especially concurrent calls to block_job_enter.
> > + */
> 
> As discussed with Paolo on IRC, I'll replace the second line of this
> comment, which he had in a different place originally and became prone
> to misunderstanding now. The new version is:
> 
> /* Right now, this mutex is only needed to synchronize accesses to job->busy,
>  * such as concurrent calls to block_job_do_yield and block_job_enter.
>  */
> 

By synchronizing job->busy, job->sleep_timer is also protected.  But
job->sleep_timer needs synchronization too, it just gets it since job->busy
needs it too.  Maybe worth saying:


 /* Right now, this mutex is only needed to synchronize accesses to
  * job->busy and job->sleep_timer, such as concurrent calls to
  * block_job_do_yield and block_job_enter.
  */