These functions assume that the job lock is held by the
caller, to avoid TOC/TOU conditions.
Introduce also additional helpers that define _locked
functions (useful when the job_mutex is globally applied).
Note: at this stage, job_{lock/unlock} and job lock guard macros
are *nop*.
Signed-off-by: Emanuele Giuseppe Esposito <eespo...@redhat.com>
---
include/qemu/job.h | 66 ++++++++++++++++++++++++++++++++++++++++++----
job.c | 18 +++++++++++--
2 files changed, 77 insertions(+), 7 deletions(-)
diff --git a/include/qemu/job.h b/include/qemu/job.h
index f7036ac6b3..c7a6bcad1b 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -355,6 +355,8 @@ void job_txn_unref(JobTxn *txn);
* the reference that is automatically grabbed here.
*
* If @txn is NULL, the function does nothing.
+ *
+ * Called between job_lock and job_unlock.
*/
void job_txn_add_job(JobTxn *txn, Job *job);
@@ -377,12 +379,16 @@ void *job_create(const char *job_id, const JobDriver
*driver, JobTxn *txn,
/**
* Add a reference to Job refcnt, it will be decreased with job_unref, and then
* be freed if it comes to be the last reference.
+ *
+ * Called between job_lock and job_unlock.
*/
void job_ref(Job *job);
/**
* Release a reference that was previously acquired with job_ref() or
* job_create(). If it's the last reference to the object, it will be freed.
+ *
+ * Called between job_lock and job_unlock.
*/
void job_unref(Job *job);
@@ -429,6 +435,8 @@ void job_event_completed(Job *job);
* Conditionally enter the job coroutine if the job is ready to run, not
* already busy and fn() returns true. fn() is called while under the job_lock
* critical section.
+ *
+ * Called between job_lock and job_unlock, but it releases the lock temporarly.
*/
void job_enter_cond(Job *job, bool(*fn)(Job *job));
@@ -490,34 +498,52 @@ bool job_is_cancelled(Job *job);
*/
bool job_cancel_requested(Job *job);
-/** Returns whether the job is in a completed state. */
+/**
+ * Returns whether the job is in a completed state.
+ * Called between job_lock and job_unlock.
+ */
bool job_is_completed(Job *job);
/** Returns whether the job is ready to be completed. */
bool job_is_ready(Job *job);
+/** Same as job_is_ready(), but assumes job_lock is held. */
+bool job_is_ready_locked(Job *job);
+
/**
* Request @job to pause at the next pause point. Must be paired with
* job_resume(). If the job is supposed to be resumed by user action, call
* job_user_pause() instead.
+ *
+ * Called between job_lock and job_unlock.
*/
void job_pause(Job *job);
-/** Resumes a @job paused with job_pause. */
+/**
+ * Resumes a @job paused with job_pause.
+ * Called between job_lock and job_unlock.
+ */
void job_resume(Job *job);
/**
* Asynchronously pause the specified @job.
* Do not allow a resume until a matching call to job_user_resume.
+ *
+ * Called between job_lock and job_unlock.
*/
void job_user_pause(Job *job, Error **errp);
-/** Returns true if the job is user-paused. */
+/**
+ * Returns true if the job is user-paused.
+ * Called between job_lock and job_unlock.
+ */
bool job_user_paused(Job *job);
/**
* Resume the specified @job.
* Must be paired with a preceding job_user_pause.
+ *
+ * Called between job_lock and job_unlock.
*/
void job_user_resume(Job *job, Error **errp);
@@ -526,6 +552,8 @@ void job_user_resume(Job *job, Error **errp);
* first one if @job is %NULL.
*
* Returns the requested job, or %NULL if there are no more jobs left.
+ *
+ * Called between job_lock and job_unlock.
*/
Job *job_next(Job *job);
@@ -533,6 +561,8 @@ Job *job_next(Job *job);
* Get the job identified by @id (which must not be %NULL).
*
* Returns the requested job, or %NULL if it doesn't exist.
+ *
+ * Called between job_lock and job_unlock.
*/
Job *job_get(const char *id);
@@ -540,27 +570,39 @@ Job *job_get(const char *id);
* Check whether the verb @verb can be applied to @job in its current state.
* Returns 0 if the verb can be applied; otherwise errp is set and -EPERM
* returned.
+ *
+ * Called between job_lock and job_unlock.
*/
int job_apply_verb(Job *job, JobVerb verb, Error **errp);
/** The @job could not be started, free it. */
void job_early_fail(Job *job);
+/** Same as job_early_fail(), but assumes job_lock is held. */
+void job_early_fail_locked(Job *job);
+
/** Moves the @job from RUNNING to READY */
void job_transition_to_ready(Job *job);
-/** Asynchronously complete the specified @job. */
+/**
+ * Asynchronously complete the specified @job.
+ * Called between job_lock and job_unlock, but it releases the lock temporarly.
+ */
void job_complete(Job *job, Error **errp);
/**
* Asynchronously cancel the specified @job. If @force is true, the job should
* be cancelled immediately without waiting for a consistent state.
+ *
+ * Called between job_lock and job_unlock.
*/
void job_cancel(Job *job, bool force);
/**
* Cancels the specified job like job_cancel(), but may refuse to do so if the
* operation isn't meaningful in the current state of the job.
+ *
+ * Called between job_lock and job_unlock.
*/
void job_user_cancel(Job *job, bool force, Error **errp);
@@ -577,7 +619,13 @@ void job_user_cancel(Job *job, bool force, Error **errp);
*/
int job_cancel_sync(Job *job, bool force);
-/** Synchronously force-cancels all jobs using job_cancel_sync(). */
+/**
+ * Synchronously force-cancels all jobs using job_cancel_sync().
+ *
+ * Called with job_lock *not* held, unlike most other APIs consumed
+ * by the monitor! This is primarly to avoid adding unnecessary lock-unlock
+ * patterns in the caller.
+ */
void job_cancel_sync_all(void);
/**
@@ -593,6 +641,8 @@ void job_cancel_sync_all(void);
* Returns the return value from the job.
*
* Callers must hold the AioContext lock of job->aio_context.
+ *
+ * Called between job_lock and job_unlock.
*/
int job_complete_sync(Job *job, Error **errp);
@@ -603,12 +653,16 @@ int job_complete_sync(Job *job, Error **errp);
* FIXME: Make the below statement universally true:
* For jobs that support the manual workflow mode, all graph changes that occur
* as a result will occur after this command and before a successful reply.
+ *
+ * Called between job_lock and job_unlock.
*/
void job_finalize(Job *job, Error **errp);
/**
* Remove the concluded @job from the query list and resets the passed pointer
* to %NULL. Returns an error if the job is not actually concluded.
+ *
+ * Called between job_lock and job_unlock.
*/
void job_dismiss(Job **job, Error **errp);
@@ -620,6 +674,8 @@ void job_dismiss(Job **job, Error **errp);
* cancelled before completing, and -errno in other error cases.
*
* Callers must hold the AioContext lock of job->aio_context.
+ *
+ * Called between job_lock and job_unlock.
*/
int job_finish_sync(Job *job, void (*finish)(Job *, Error **errp), Error
**errp);
diff --git a/job.c b/job.c
index 0e4dacf028..e393c1222f 100644
--- a/job.c
+++ b/job.c
@@ -242,7 +242,8 @@ bool job_cancel_requested(Job *job)
return job->cancelled;
}
-bool job_is_ready(Job *job)
+/* Called with job_mutex held. */
+bool job_is_ready_locked(Job *job)
{
switch (job->status) {
case JOB_STATUS_UNDEFINED:
@@ -264,6 +265,13 @@ bool job_is_ready(Job *job)
return false;
}
+/* Called with job_mutex lock *not* held */
+bool job_is_ready(Job *job)
+{
+ JOB_LOCK_GUARD();
+ return job_is_ready_locked(job);
+}
+
bool job_is_completed(Job *job)
{
switch (job->status) {
@@ -659,12 +667,18 @@ void job_dismiss(Job **jobptr, Error **errp)
*jobptr = NULL;
}
-void job_early_fail(Job *job)
+void job_early_fail_locked(Job *job)
{
assert(job->status == JOB_STATUS_CREATED);
job_do_dismiss(job);
}
+void job_early_fail(Job *job)
+{
+ JOB_LOCK_GUARD();
+ job_early_fail_locked(job);
+}
+
static void job_conclude(Job *job)
{
job_state_transition(job, JOB_STATUS_CONCLUDED);
--
2.27.0
