Am 03/06/2022 um 18:17 schrieb Kevin Wolf:
> Am 14.03.2022 um 14:36 hat Emanuele Giuseppe Esposito geschrieben:
>> In preparation to the job_lock/unlock usage, create _locked
>> duplicates of some functions, since they will be sometimes called with
>> job_mutex held (mostly within job.c),
>> and sometimes without (mostly from JobDrivers using the job API).
>>
>> Therefore create a _locked version of such function, so that it
>> can be used in both cases.
>>
>> List of functions duplicated as _locked:
>> job_is_ready (both versions are public)
>> job_is_completed (both versions are public)
>> job_is_cancelled (_locked version is public, needed by mirror.c)
>> job_pause_point (_locked version is static, purely done to simplify the code)
>> job_cancel_requested (_locked version is static)
>>
>> 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 | 25 +++++++++++++++++++++---
>> job.c | 48 ++++++++++++++++++++++++++++++++++++++++------
>> 2 files changed, 64 insertions(+), 9 deletions(-)
>>
>> diff --git a/include/qemu/job.h b/include/qemu/job.h
>> index 6000463126..aa33d091b1 100644
>> --- a/include/qemu/job.h
>> +++ b/include/qemu/job.h
>> @@ -473,21 +473,40 @@ const char *job_type_str(const Job *job);
>> /** Returns true if the job should not be visible to the management layer.
>> */
>> bool job_is_internal(Job *job);
>>
>> -/** Returns whether the job is being cancelled. */
>> +/**
>> + * Returns whether the job is being cancelled.
>> + * Called with job_mutex *not* held.
>> + */
>> bool job_is_cancelled(Job *job);
>>
>> +/** Just like job_is_cancelled, but called between job_lock and job_unlock
>> */
>> +bool job_is_cancelled_locked(Job *job);
>> +
>> /**
>> * Returns whether the job is scheduled for cancellation (at an
>> * indefinite point).
>> + * Called with job_mutex *not* held.
>> */
>> 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 with job_mutex *not* held.
>> + */
>> bool job_is_completed(Job *job);
>>
>> -/** Returns whether the job is ready to be completed. */
>> +/** Same as job_is_completed(), but assumes job_lock is held. */
>> +bool job_is_completed_locked(Job *job);
>
> Any reason why this comment is phrased differently than for
> job_is_cancelled_locked()? I don't mind which one we use, but if they
> should express the same thing, it would be better to have the same
> wording. If they should express different things, it need to be clearer
> what they are.
>
Makes sense, I will switch to the same format as job_is_cancelled_locked().
Emanuele
> Also, I assume job_mutex is meant because job_lock() is a function, not
> the lock that is held.
>
>> +/**
>> + * Returns whether the job is ready to be completed.
>> + * Called with job_mutex *not* held.
>> + */
>> bool job_is_ready(Job *job);
>>
>> +/** Same as job_is_ready(), but assumes job_lock is held. */
>> +bool job_is_ready_locked(Job *job);
>
> Same as above.
>
>> /**
>> * 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
>
> Kevin
>