Re: [PATCH v5 09/20] jobs: add job lock in find_* functions

[Stefan Hajnoczi]

On Tue, Feb 08, 2022 at 09:35:02AM -0500, Emanuele Giuseppe Esposito wrote:
> diff --git a/blockdev.c b/blockdev.c
> index c5fba4d157..08408cd44b 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -3311,7 +3311,10 @@ out:
>      aio_context_release(aio_context);
>  }
>  
> -/* Get a block job using its ID and acquire its AioContext */
> +/*
> + * Get a block job using its ID and acquire its AioContext.
> + * Returns with job_lock held on success.

The caller needs to deal with unlocking anyway, so maybe ask the caller
to acquire the lock too? That would make the function simpler to reason
about.

> @@ -60,6 +65,7 @@ void qmp_job_cancel(const char *id, Error **errp)
>      trace_qmp_job_cancel(job);
>      job_user_cancel(job, true, errp);
>      aio_context_release(aio_context);
> +    job_unlock();
>  }

Is job_mutex -> AioContext lock ordering correct? I thought the
AioContext must be held before taking the job lock according to "jobs:
remove aiocontext locks since the functions are under BQL"?

signature.asc
Description: PGP signature