Re: [Qemu-devel] [PATCH] block: for jobs, do not clear user_paused until after the resume

[Jeff Cody <jcody@redhat.com>]

On Wed, Aug 15, 2018 at 05:23:43PM -0400, John Snow wrote:
> 
> 
> On 08/15/2018 11:59 AM, Jeff Cody wrote:
> > The function job_cancel_async() will always cause an assert for blockjob
> > user resume.  We set job->user_paused to false, and then call
> > job->driver->user_resume().  In the case of blockjobs, this is the
> > block_job_user_resume() function.
> > 
> > In that function, we assert that job.user_paused is set to true.
> > Unfortunately, right before calling this function, it has explicitly
> > been set to false.
> > 
> > The fix is pretty simple: set job->user_paused to false only after the
> > job user_resume() function has been called.
> > 
> > Signed-off-by: Jeff Cody <jcody@redhat.com>
> > ---
> >  job.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/job.c b/job.c
> > index fa671b431a..e36ebaafd8 100644
> > --- a/job.c
> > +++ b/job.c
> > @@ -732,10 +732,10 @@ static void job_cancel_async(Job *job, bool force)
> >  {
> >      if (job->user_paused) {
> >          /* Do not call job_enter here, the caller will handle it.  */
> > -        job->user_paused = false;
> >          if (job->driver->user_resume) {
> >              job->driver->user_resume(job);
> >          }
> > +        job->user_paused = false;
> >          assert(job->pause_count > 0);
> >          job->pause_count--;
> >      }
> > 
> 
> Looks right to me; are you going to make good on your threat of a v2
> with a unit test?
>

Yep!

> Reviewed-by: John Snow <jsnow@redhat.com>

Thanks

-Jeff