Am 11/07/2022 um 16:44 schrieb Vladimir Sementsov-Ogievskiy:
> On 7/6/22 23:15, Emanuele Giuseppe Esposito wrote:
>> The same job lock is being used also to protect some of blockjob fields.
>> Categorize them just as done in job.h.
>
> Thanks!
>
>>
>> Signed-off-by: Emanuele Giuseppe Esposito <eespo...@redhat.com>
>> ---
>> include/block/blockjob.h | 17 ++++++++++++++---
>> 1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/block/blockjob.h b/include/block/blockjob.h
>> index 8b65d3949d..912e10b083 100644
>> --- a/include/block/blockjob.h
>> +++ b/include/block/blockjob.h
>> @@ -40,10 +40,16 @@ typedef struct BlockJobDriver BlockJobDriver;
>> * Long-running operation on a BlockDriverState.
>> */
>> typedef struct BlockJob {
>> - /** Data belonging to the generic Job infrastructure */
>> + /**
>> + * Data belonging to the generic Job infrastructure.
>> + * Protected by job mutex.
>> + */
>> Job job;
>> - /** Status that is published by the query-block-jobs QMP API */
>> + /**
>> + * Status that is published by the query-block-jobs QMP API.
>> + * Protected by job mutex.
>> + */
>> BlockDeviceIoStatus iostatus;
>> /** Speed that was set with @block_job_set_speed. */
>> @@ -55,6 +61,8 @@ typedef struct BlockJob {
>> /** Block other operations when block job is running */
>> Error *blocker;
>> + /** All notifiers are set once in block_job_create() and never
>> modified. */
>> +
>> /** Called when a cancelled job is finalised. */
>> Notifier finalize_cancelled_notifier;
>> @@ -70,7 +78,10 @@ typedef struct BlockJob {
>> /** Called when the job coroutine yields or terminates */
>> Notifier idle_notifier;
>> - /** BlockDriverStates that are involved in this block job */
>> + /**
>> + * BlockDriverStates that are involved in this block job.
>> + * Always modified and read under QEMU global mutex
>> (GLOBAL_STATE_CODE)
>> + */
>> GSList *nodes;
>> } BlockJob;
>>
>
> Can we also add GLOBAL_STATE_CODE(); marker into
> child_job_can_set_aio_ctx() and child_job_set_aio_ctx() ?
Usually for callbacks I feel redundant to add assertions there. It is
enough to look at their definition in the header to understand which
category are they in.
Emanuele
>
> Anyway:
>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
>