Re: [RFC PATCH v2 05/14] block/mirror.c: use of job helpers in drivers to avoid TOC/TOU

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

04.11.2021 17:53, Emanuele Giuseppe Esposito wrote:
> Once job lock is used and aiocontext is removed, mirror has
> to perform job operations under the same critical section,
> using the helpers prepared in previous commit.
> 
> Note: at this stage, job_{lock/unlock} and job lock guard macros
> are *nop*.
> 
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
>   block/mirror.c | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/block/mirror.c b/block/mirror.c
> index 00089e519b..f22fa7da6e 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -653,7 +653,7 @@ static int mirror_exit_common(Job *job)
>       BlockDriverState *target_bs;
>       BlockDriverState *mirror_top_bs;
>       Error *local_err = NULL;
> -    bool abort = job->ret < 0;
> +    bool abort = job_has_failed(job);
>       int ret = 0;
>   
>       if (s->prepared) {
> @@ -1161,9 +1161,7 @@ static void mirror_complete(Job *job, Error **errp)
>       s->should_complete = true;
>   
>       /* If the job is paused, it will be re-entered when it is resumed */
> -    if (!job->paused) {
> -        job_enter(job);
> -    }
> +    job_enter_not_paused(job);
>   }
>   
>   static void coroutine_fn mirror_pause(Job *job)
> @@ -1182,7 +1180,7 @@ static bool mirror_drained_poll(BlockJob *job)
>        * from one of our own drain sections, to avoid a deadlock waiting for
>        * ourselves.
>        */
> -    if (!s->common.job.paused && !job_is_cancelled(&job->job) && !s->in_drain) {
> +    if (job_not_paused_nor_cancelled(&s->common.job) && !s->in_drain) {
>           return true;
>       }
>   
> 

Why to introduce a separate API function for every use case?

Could we instead just use WITH_JOB_LOCK_GUARD() ?


-- 
Best regards,
Vladimir