|
From: | Emanuele Giuseppe Esposito |
Subject: | Re: [RFC PATCH 3/6] job: minor changes to simplify locking |
Date: | Mon, 12 Jul 2021 10:43:23 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 |
On 08/07/2021 12:55, Stefan Hajnoczi wrote:
Again this is a dumb mistake, the job_lock/unlock lines should go on patch 5, not here. Apologies.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.
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
[Prev in Thread] | Current Thread | [Next in Thread] |