[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-6.2 v3 01/12] job: Context changes in job_completed_txn_a
From: |
Eric Blake |
Subject: |
Re: [PATCH for-6.2 v3 01/12] job: Context changes in job_completed_txn_abort() |
Date: |
Fri, 6 Aug 2021 14:16:04 -0500 |
User-agent: |
NeoMutt/20210205-687-0ed190 |
On Fri, Aug 06, 2021 at 11:38:48AM +0200, Max Reitz wrote:
> Finalizing the job may cause its AioContext to change. This is noted by
> job_exit(), which points at job_txn_apply() to take this fact into
> account.
>
> However, job_completed() does not necessarily invoke job_txn_apply()
> (through job_completed_txn_success()), but potentially also
> job_completed_txn_abort(). The latter stores the context in a local
> variable, and so always acquires the same context at its end that it has
> released in the beginning -- which may be a different context from the
> one that job_exit() releases at its end. If it is different, qemu
> aborts ("qemu_mutex_unlock_impl: Operation not permitted").
Is this a bug fix that needs to make it into 6.1?
>
> Drop the local @outer_ctx variable from job_completed_txn_abort(), and
> instead re-acquire the actual job's context at the end of the function,
> so job_exit() will release the same.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> job.c | 23 ++++++++++++++++++-----
> 1 file changed, 18 insertions(+), 5 deletions(-)
The commit message makes sense, and does a good job at explaining the
change. I'm still a bit fuzzy on how jobs are supposed to play nice
with contexts, but since your patch matches the commit message, I'm
happy to give:
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
- [PATCH for-6.2 v3 00/12] mirror: Handle errors after READY cancel, Max Reitz, 2021/08/06
- [PATCH for-6.2 v3 01/12] job: Context changes in job_completed_txn_abort(), Max Reitz, 2021/08/06
- Re: [PATCH for-6.2 v3 01/12] job: Context changes in job_completed_txn_abort(),
Eric Blake <=
- [PATCH for-6.2 v3 02/12] mirror: Keep s->synced on error, Max Reitz, 2021/08/06
- [PATCH for-6.2 v3 03/12] mirror: Drop s->synced, Max Reitz, 2021/08/06
- [PATCH for-6.2 v3 04/12] job: Force-cancel jobs in a failed transaction, Max Reitz, 2021/08/06
- [PATCH for-6.2 v3 06/12] jobs: Give Job.force_cancel more meaning, Max Reitz, 2021/08/06
- [PATCH for-6.2 v3 05/12] job: @force parameter for job_cancel_sync{, _all}(), Max Reitz, 2021/08/06
- [PATCH for-6.2 v3 07/12] job: Add job_cancel_requested(), Max Reitz, 2021/08/06