Re: [RFC PATCH 3/6] job: minor changes to simplify locking

[Emanuele Giuseppe Esposito]

On 08/07/2021 12:55, Stefan Hajnoczi wrote:
> On Wed, Jul 07, 2021 at 06:58:10PM +0200, Emanuele Giuseppe Esposito wrote:
> > @@ -406,15 +410,18 @@ void *job_create(const char *job_id, const JobDriver 
> > *driver, JobTxn *txn,
> >               error_setg(errp, "Invalid job ID '%s'", job_id);
> >               return NULL;
> >           }
> > -        if (job_get(job_id)) {
> > -            error_setg(errp, "Job ID '%s' already in use", job_id);
> > -            return NULL;
> > -        }
> >       } else if (!(flags & JOB_INTERNAL)) {
> >           error_setg(errp, "An explicit job ID is required");
> >           return NULL;
> >       }
> >   
> > +    job_lock();
> > +    if (job_get(job_id)) {
> > +        error_setg(errp, "Job ID '%s' already in use", job_id);
> > +        job_unlock();
> > +        return NULL;
> > +    }
> > +
>
> Where is the matching job_unlock() in the success case? Please consider
> lock guard macros like QEMU_LOCK_GUARD()/WITH_QEMU_LOCK_GUARD() to
> prevent common errors.
Again this is a dumb mistake, the job_lock/unlock lines should go on 
patch 5, not here. Apologies.

I did not use QEMU_LOCK_GUARD()/WITH_QEMU_LOCK_GUARD() yet because I 
added some assertions to make sure I also didn't create nested locking 
situations. The final version will certainly use them.

Emanuele