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