qemu-block
[Top][All Lists]
Advanced

[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: Max Reitz
Subject: Re: [PATCH for-6.2 v3 05/12] job: @force parameter for job_cancel_sync{, _all}()
Date: Mon, 9 Aug 2021 12:09:23 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0

On 06.08.21 21:39, Eric Blake wrote:
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).

It is, hence the “Buglink” tag below.  However, only all of this series together really fixes that bug (or at least patches 5+7+9 together), just taking one wouldn’t help much.  And together, it’s just too much for 6.2 at this point.

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>





reply via email to

[Prev in Thread] Current Thread [Next in Thread]