From b660a84bbb0eb1a76b505648d31d5e82594fb75e Mon Sep 17 00:00:00 2001 From: Stefan Reiter Date: Tue, 7 Apr 2020 13:56:49 +0200 Subject: job: take each job's lock individually in job_txn_apply All callers of job_txn_apply hold a single job's lock, but different jobs within a transaction can have different contexts, thus we need to lock each one individually before applying the callback function. Similar to job_completed_txn_abort this also requires releasing the caller's context before and reacquiring it after to avoid recursive locks which might break AIO_WAIT_WHILE in the callback. This is safe, since existing code would already have to take this into account, lest job_completed_txn_abort might have broken. This also brings to light a different issue: When a callback function in job_txn_apply moves it's job to a different AIO context, callers will try to release the wrong lock (now that we re-acquire the lock correctly, previously it would just continue with the old lock, leaving the job unlocked for the rest of the return path). Fix this by not caching the job's context. This is only necessary for qmp_block_job_finalize, qmp_job_finalize and job_exit, since everyone else calls through job_exit. One test needed adapting, since it calls job_finalize directly, so it manually needs to acquire the correct context. Signed-off-by: Stefan Reiter Message-Id: <20200407115651.69472-2-s.reiter@proxmox.com> Signed-off-by: Kevin Wolf --- job-qmp.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'job-qmp.c') diff --git a/job-qmp.c b/job-qmp.c index fecc939..f9a5883 100644 --- a/job-qmp.c +++ b/job-qmp.c @@ -114,7 +114,16 @@ void qmp_job_finalize(const char *id, Error **errp) } trace_qmp_job_finalize(job); + job_ref(job); job_finalize(job, errp); + + /* + * Job's context might have changed via job_finalize (and job_txn_apply + * automatically acquires the new one), so make sure we release the correct + * one. + */ + aio_context = job->aio_context; + job_unref(job); aio_context_release(aio_context); } -- cgit v1.1