Re: [PATCH v3 05/16] block/mirror.c: use of job helpers in drivers to avoid TOC/TOU

[Paolo Bonzini]

On 1/5/22 15:01, Emanuele Giuseppe Esposito wrote:
> +    WITH_JOB_LOCK_GUARD() {
> +        abort = job->ret < 0;
> +    }
> +
>      if (s->prepared) {
>          return 0;
>      }

At this point I think job->ret is stable and can be accessed without 
guard.  The question however is what serializes calls to job_prepare. 
Is it the BQL?  Can we say that job->ret is only written under BQL, just 
like job->aio_context.

> @@ -1161,8 +1165,10 @@ static void mirror_complete(Job *job, Error **errp)
>      s->should_complete = true;
>  
>      /* If the job is paused, it will be re-entered when it is resumed */
> -    if (!job->paused) {
> -        job_enter(job);
> +    WITH_JOB_LOCK_GUARD() {
> +        if (!job->paused) {
> +            job_enter_cond_locked(job, NULL);
> +        }
>      }

I don't want to open a can of worms, but does it ever make sense to call 
job_enter while the job is paused?  Should this condition be moved to 
job_enter_cond_locked?

Paolo