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
