Re: [PATCH v9 17/21] blockjob.h: categorize fields in struct BlockJob

[Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>]

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 <eesposit@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() ?

Anyway:

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

-- 
Best regards,
Vladimir