[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-6.2 v3 05/12] job: @force parameter for job_cancel_sync{,
From: |
Eric Blake |
Subject: |
Re: [PATCH for-6.2 v3 05/12] job: @force parameter for job_cancel_sync{, _all}() |
Date: |
Fri, 6 Aug 2021 14:39:38 -0500 |
User-agent: |
NeoMutt/20210205-687-0ed190 |
On Fri, Aug 06, 2021 at 11:38:52AM +0200, Max Reitz wrote:
> Callers should be able to specify whether they want job_cancel_sync() to
> force-cancel the job or not.
>
> In fact, almost all invocations do not care about consistency of the
> result and just want the job to terminate as soon as possible, so they
> should pass force=true. The replication block driver is the exception.
>
> This changes some iotest outputs, because quitting qemu while a mirror
> job is active will now lead to it being cancelled instead of completed,
> which is what we want. (Cancelling a READY mirror job with force=false
> may take an indefinite amount of time, which we do not want when
> quitting. If users want consistent results, they must have all jobs be
> done before they quit qemu.)
Feels somewhat like a bug fix, but I also understand why you'd prefer
to delay this to 6.2 (it is not a fresh regression, but a longstanding
issue).
>
> Buglink: https://gitlab.com/qemu-project/qemu/-/issues/462
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> +++ b/job.c
> @@ -982,12 +982,24 @@ static void job_cancel_err(Job *job, Error **errp)
> job_cancel(job, false);
> }
>
> -int job_cancel_sync(Job *job)
> +/**
> + * Same as job_cancel_err(), but force-cancel.
> + */
> +static void job_force_cancel_err(Job *job, Error **errp)
> {
> - return job_finish_sync(job, &job_cancel_err, NULL);
> + job_cancel(job, true);
> +}
In isolation, it looks odd that errp is passed but not used. But
looking further, it's because this is a callback that must have a
given signature, so it's okay.
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
- [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
- Re: [PATCH for-6.2 v3 05/12] job: @force parameter for job_cancel_sync{, _all}(),
Eric Blake <=
- [PATCH for-6.2 v3 07/12] job: Add job_cancel_requested(), Max Reitz, 2021/08/06
- [PATCH for-6.2 v3 08/12] mirror: Use job_is_cancelled(), Max Reitz, 2021/08/06
- [PATCH for-6.2 v3 09/12] mirror: Check job_is_cancelled() earlier, Max Reitz, 2021/08/06
- [PATCH for-6.2 v3 10/12] mirror: Stop active mirroring after force-cancel, Max Reitz, 2021/08/06
- [PATCH for-6.2 v3 11/12] mirror: Do not clear .cancelled, Max Reitz, 2021/08/06