[PATCH for-6.2 v3 00/12] mirror: Handle errors after READY cancel

[Max Reitz]

Hi,

v1 cover letter:
https://lists.nongnu.org/archive/html/qemu-block/2021-07/msg00705.html

v2 cover letter:
https://lists.nongnu.org/archive/html/qemu-block/2021-07/msg00747.html

Changes in v3:
- Patch 1: After adding patch 11, I got a failed assertion in
  tests/unit/test-block-iothread (failing qemu_mutex_unlock_impl()).
  That is because before patch 11, for zero-length source devices,
  mirror clears .cancelled unconditionally before exiting.  So even
  force-cancelled jobs are considered to be completed normally, which
  doesn’t seem quite right.
  Anyway, test-block-iothread does some iothread switching, and
  cancelling jobs is not really prepared for that.  This patch fixes
  that (I hope...).

- Patch 4: Split off from patch 5

- Patch 7:
  - Added a long section in the commit message detailing every choice
for every job_is_cancelled() invocation
  - Use job_cancel_requested() in the assertion in
job_completed_txn_abort(), because it is not quite clear whether
soft-cancelled mirror jobs can end up in this path (it seems like a
bug if that happens, but I think that’s something to fix in some
other series)

- Patch 8: Added: This is kind of preparation for patch 9, but also just
  a bug fix in itself, I believe

- Patch 9: Moved the job_is_cancelled() check after the last yield point
  before the mirror_iteration() call

- Patch 10: Added: If force-cancelled jobs should not generate new I/O
  requests at all (except for forwarding something to the source
  device), then we need to stop doing active mirroring once the mirror
  job is force-cancelled

- Patch 11: Added: Clearing .cancelled seemed like a hack, so getting
  rid of it seems like a good thing to do
  (And only with this patch, I can assert that .force_cancel can only be
  true when .cancelled is true also; if we tried it before this patch,
  tests/unit/test-block-iothread would fail.)


The discussion around v2 has shown that there are probably more bugs in
the job code, but I think this series is becoming long enough that we
should tackle those in a different series.


git-backport-diff against v1:

Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/12:[down] 'job: Context changes in job_completed_txn_abort()'
002/12:[] [--] 'mirror: Keep s->synced on error'
003/12:[] [--] 'mirror: Drop s->synced'
004/12:[down] 'job: Force-cancel jobs in a failed transaction'
005/12:[0007] [FC] 'job: @force parameter for job_cancel_sync{,_all}()'
006/12:[] [--] 'jobs: Give Job.force_cancel more meaning'
007/12:[0002] [FC] 'job: Add job_cancel_requested()'
008/12:[down] 'mirror: Use job_is_cancelled()'
009/12:[0007] [FC] 'mirror: Check job_is_cancelled() earlier'
010/12:[down] 'mirror: Stop active mirroring after force-cancel'
011/12:[down] 'mirror: Do not clear .cancelled'
012/12:[] [--] 'iotests: Add mirror-ready-cancel-error test'


Max Reitz (12):
  job: Context changes in job_completed_txn_abort()
  mirror: Keep s->synced on error
  mirror: Drop s->synced
  job: Force-cancel jobs in a failed transaction
  job: @force parameter for job_cancel_sync{,_all}()
  jobs: Give Job.force_cancel more meaning
  job: Add job_cancel_requested()
  mirror: Use job_is_cancelled()
  mirror: Check job_is_cancelled() earlier
  mirror: Stop active mirroring after force-cancel
  mirror: Do not clear .cancelled
  iotests: Add mirror-ready-cancel-error test

 include/qemu/job.h|  29 +++-
 block/backup.c|   3 +-
 block/mirror.c|  56 ---
 block/replication.c   |   4 +-
 blockdev.c|   4 +-
 job.c |  67 ++--
 qemu-nbd.c|   2 +-
 softmmu/runstate.c|   2 +-
 storage-daemon/qemu-storage-daemon.c  |   2 +-
 tests/unit/test-block-iothread.c  |   2 +-
 tests/unit/test-blockjob.c|   2 +-
 tests/qemu-iotests/109.out|  60 +++-
 .../tests/mirror-ready-cancel-error   | 143 ++
 .../tests/mirror-ready-cancel-error.out   |   5 +
 tests/qemu-iotests/tests/qsd-jobs.out |   2 +-
 15 files changed, 292 insertions(+), 91 deletions(-)
 create mode 100755 tests/qemu-iotests/tests/mirror-ready-cancel-error
 create mode 100644 tests/qemu-iotests/tests/mirror-ready-cancel-error.out

-- 
2.31.1

[PATCH for-6.2 v3 01/12] job: Context changes in job_completed_txn_abort()

[Max Reitz]

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").

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 
---
 job.c | 23 ++-
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/job.c b/job.c
index e7a5d28854..3fe23bb77e 100644
--- a/job.c
+++ b/job.c
@@ -737,7 +737,6 @@ static void job_cancel_async(Job *job, bool force)
 
 static void job_completed_txn_abort(Job *job)
 {
-AioContext *outer_ctx = job->aio_context;
 AioContext *ctx;
 JobTxn *txn = job->txn;
 Job *other_job;
@@ -751,10 +750,14 @@ static void job_completed_txn_abort(Job *job)
 txn->aborting = true;
 job_txn_ref(txn);
 
-/* We can only hold the single job's AioContext lock while calling
+/*
+ * We can only hold the single job's AioContext lock while calling
  * job_finalize_single() because the finalization callbacks can involve
- * calls of AIO_WAIT_WHILE(), which could deadlock otherwise. */
-aio_context_release(outer_ctx);
+ * calls of AIO_WAIT_WHILE(), which could deadlock otherwise.
+ * Note that the job's AioContext may change when it is finalized.
+ */
+job_ref(job);
+aio_context_release(job->aio_context);
 
 /* Other jobs are effectively cancelled by us, set the status for
  * them; this job, however, may or may not be cancelled, depending
@@ -769,6 +772,10 @@ static void job_completed_txn_abort(Job *job)
 }
 while (!QLIST_EMPTY(&txn->jobs)) {
 other_job = QLIST_FIRST(&txn->jobs);
+/*
+ * The job's AioContext may change, so store it in @ctx so we
+ * release the same context that we have acquired before.
+ */
 ctx = other_job->aio_context;
 aio_context_acquire(ctx);
 if (!job_is_completed(other_job)) {
@@ -779,7 +786,13 @@ static void job_completed_txn_abort(Job *job)
 aio_context_release(ctx);
 }
 
-aio_context_acquire(outer_ctx);
+/*
+ * Use job_ref()/job_unref() so we can read the AioContext here
+ * even if the job went away during job_finalize_single().
+ */
+ctx = job->aio_context;
+job_unref(job);
+aio_context_acquire(ctx);
 
 job_txn_unref(txn);
 }
-- 
2.31.1

[PATCH for-6.2 v3 02/12] mirror: Keep s->synced on error

[Max Reitz]

An error does not take us out of the READY phase, which is what
s->synced signifies.  It does of course mean that source and target are
no longer in sync, but that is what s->actively_sync is for -- s->synced
never meant that source and target are in sync, only that they were at
some point (and at that point we transitioned into the READY phase).

The tangible problem is that we transition to READY once we are in sync
and s->synced is false.  By resetting s->synced here, we will transition
from READY to READY once the error is resolved (if the job keeps
running), and that transition is not allowed.

Signed-off-by: Max Reitz 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Kevin Wolf 
---
 block/mirror.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/block/mirror.c b/block/mirror.c
index 98fc66eabf..d73b704473 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -121,7 +121,6 @@ typedef enum MirrorMethod {
 static BlockErrorAction mirror_error_action(MirrorBlockJob *s, bool read,
 int error)
 {
-s->synced = false;
 s->actively_synced = false;
 if (read) {
 return block_job_error_action(&s->common, s->on_source_error,
-- 
2.31.1

[PATCH for-6.2 v3 03/12] mirror: Drop s->synced

[Max Reitz]

As of HEAD^, there is no meaning to s->synced other than whether the job
is READY or not.  job_is_ready() gives us that information, too.

Suggested-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Kevin Wolf 
---
 block/mirror.c | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index d73b704473..fcb7b65f93 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -56,7 +56,6 @@ typedef struct MirrorBlockJob {
 bool zero_target;
 MirrorCopyMode copy_mode;
 BlockdevOnError on_source_error, on_target_error;
-bool synced;
 /* Set when the target is synced (dirty bitmap is clean, nothing
  * in flight) and the job is running in active mode */
 bool actively_synced;
@@ -936,7 +935,6 @@ static int coroutine_fn mirror_run(Job *job, Error **errp)
 if (s->bdev_length == 0) {
 /* Transition to the READY state and wait for complete. */
 job_transition_to_ready(&s->common.job);
-s->synced = true;
 s->actively_synced = true;
 while (!job_is_cancelled(&s->common.job) && !s->should_complete) {
 job_yield(&s->common.job);
@@ -1028,7 +1026,7 @@ static int coroutine_fn mirror_run(Job *job, Error **errp)
 should_complete = false;
 if (s->in_flight == 0 && cnt == 0) {
 trace_mirror_before_flush(s);
-if (!s->synced) {
+if (!job_is_ready(&s->common.job)) {
 if (mirror_flush(s) < 0) {
 /* Go check s->ret.  */
 continue;
@@ -1039,7 +1037,6 @@ static int coroutine_fn mirror_run(Job *job, Error **errp)
  * the target in a consistent state.
  */
 job_transition_to_ready(&s->common.job);
-s->synced = true;
 if (s->copy_mode != MIRROR_COPY_MODE_BACKGROUND) {
 s->actively_synced = true;
 }
@@ -1083,14 +1080,15 @@ static int coroutine_fn mirror_run(Job *job, Error 
**errp)
 
 ret = 0;
 
-if (s->synced && !should_complete) {
+if (job_is_ready(&s->common.job) && !should_complete) {
 delay_ns = (s->in_flight == 0 &&
 cnt == 0 ? BLOCK_JOB_SLICE_TIME : 0);
 }
-trace_mirror_before_sleep(s, cnt, s->synced, delay_ns);
+trace_mirror_before_sleep(s, cnt, job_is_ready(&s->common.job),
+  delay_ns);
 job_sleep_ns(&s->common.job, delay_ns);
 if (job_is_cancelled(&s->common.job) &&
-(!s->synced || s->common.job.force_cancel))
+(!job_is_ready(&s->common.job) || s->common.job.force_cancel))
 {
 break;
 }
@@ -1103,8 +1101,9 @@ immediate_exit:
  * or it was cancelled prematurely so that we do not guarantee that
  * the target is a copy of the source.
  */
-assert(ret < 0 || ((s->common.job.force_cancel || !s->synced) &&
-   job_is_cancelled(&s->common.job)));
+assert(ret < 0 ||
+   ((s->common.job.force_cancel || !job_is_ready(&s->common.job)) 
&&
+job_is_cancelled(&s->common.job)));
 assert(need_drain);
 mirror_wait_for_all_io(s);
 }
@@ -1127,7 +1126,7 @@ static void mirror_complete(Job *job, Error **errp)
 {
 MirrorBlockJob *s = container_of(job, MirrorBlockJob, common.job);
 
-if (!s->synced) {
+if (!job_is_ready(job)) {
 error_setg(errp, "The active block job '%s' cannot be completed",
job->id);
 return;
-- 
2.31.1

[PATCH for-6.2 v3 06/12] jobs: Give Job.force_cancel more meaning

[Max Reitz]

We largely have two cancel modes for jobs:

First, there is actual cancelling.  The job is terminated as soon as
possible, without trying to reach a consistent result.

Second, we have mirror in the READY state.  Technically, the job is not
really cancelled, but it just is a different completion mode.  The job
can still run for an indefinite amount of time while it tries to reach a
consistent result.

We want to be able to clearly distinguish which cancel mode a job is in
(when it has been cancelled).  We can use Job.force_cancel for this, but
right now it only reflects cancel requests from the user with
force=true, but clearly, jobs that do not even distinguish between
force=false and force=true are effectively always force-cancelled.

So this patch has Job.force_cancel signify whether the job will
terminate as soon as possible (force_cancel=true) or whether it will
effectively remain running despite being "cancelled"
(force_cancel=false).

To this end, we let jobs that provide JobDriver.cancel() tell the
generic job code whether they will terminate as soon as possible or not,
and for jobs that do not provide that method we assume they will.

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Kevin Wolf 
---
 include/qemu/job.h | 11 ++-
 block/backup.c |  3 ++-
 block/mirror.c | 24 ++--
 job.c  |  6 +-
 4 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/include/qemu/job.h b/include/qemu/job.h
index 5e8edbc2c8..8aa90f7395 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -253,8 +253,17 @@ struct JobDriver {
 
 /**
  * If the callback is not NULL, it will be invoked in job_cancel_async
+ *
+ * This function must return true if the job will be cancelled
+ * immediately without any further I/O (mandatory if @force is
+ * true), and false otherwise.  This lets the generic job layer
+ * know whether a job has been truly (force-)cancelled, or whether
+ * it is just in a special completion mode (like mirror after
+ * READY).
+ * (If the callback is NULL, the job is assumed to terminate
+ * without I/O.)
  */
-void (*cancel)(Job *job, bool force);
+bool (*cancel)(Job *job, bool force);
 
 
 /** Called when the job is freed */
diff --git a/block/backup.c b/block/backup.c
index bd3614ce70..513e1c8a0b 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -331,11 +331,12 @@ static void coroutine_fn backup_set_speed(BlockJob *job, 
int64_t speed)
 }
 }
 
-static void backup_cancel(Job *job, bool force)
+static bool backup_cancel(Job *job, bool force)
 {
 BackupBlockJob *s = container_of(job, BackupBlockJob, common.job);
 
 bdrv_cancel_in_flight(s->target_bs);
+return true;
 }
 
 static const BlockJobDriver backup_job_driver = {
diff --git a/block/mirror.c b/block/mirror.c
index fcb7b65f93..e93631a9f6 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1087,9 +1087,7 @@ static int coroutine_fn mirror_run(Job *job, Error **errp)
 trace_mirror_before_sleep(s, cnt, job_is_ready(&s->common.job),
   delay_ns);
 job_sleep_ns(&s->common.job, delay_ns);
-if (job_is_cancelled(&s->common.job) &&
-(!job_is_ready(&s->common.job) || s->common.job.force_cancel))
-{
+if (job_is_cancelled(&s->common.job) && s->common.job.force_cancel) {
 break;
 }
 s->last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
@@ -1102,7 +1100,7 @@ immediate_exit:
  * the target is a copy of the source.
  */
 assert(ret < 0 ||
-   ((s->common.job.force_cancel || !job_is_ready(&s->common.job)) 
&&
+   (s->common.job.force_cancel &&
 job_is_cancelled(&s->common.job)));
 assert(need_drain);
 mirror_wait_for_all_io(s);
@@ -1188,14 +1186,27 @@ static bool mirror_drained_poll(BlockJob *job)
 return !!s->in_flight;
 }
 
-static void mirror_cancel(Job *job, bool force)
+static bool mirror_cancel(Job *job, bool force)
 {
 MirrorBlockJob *s = container_of(job, MirrorBlockJob, common.job);
 BlockDriverState *target = blk_bs(s->target);
 
-if (force || !job_is_ready(job)) {
+/*
+ * Before the job is READY, we treat any cancellation like a
+ * force-cancellation.
+ */
+force = force || !job_is_ready(job);
+
+if (force) {
 bdrv_cancel_in_flight(target);
 }
+return force;
+}
+
+static bool commit_active_cancel(Job *job, bool force)
+{
+/* Same as above in mirror_cancel() */
+return force || !job_is_ready(job);
 }
 
 static const BlockJobDriver mirror_job_driver = {
@@ -1225,6 +1236,7 @@ static const BlockJobDriver commit_active_job_driver = {
 .abort  = mirror_abort,
 .pause  = mirror_pause,
 .complete   = mirror_complete,
+.cancel

[PATCH for-6.2 v3 07/12] job: Add job_cancel_requested()

[Max Reitz]

Most callers of job_is_cancelled() actually want to know whether the job
is on its way to immediate termination.  For example, we refuse to pause
jobs that are cancelled; but this only makes sense for jobs that are
really actually cancelled.

A mirror job that is cancelled during READY with force=false should
absolutely be allowed to pause.  This "cancellation" (which is actually
a kind of completion) may take an indefinite amount of time, and so
should behave like any job during normal operation.  For example, with
on-target-error=stop, the job should stop on write errors.  (In
contrast, force-cancelled jobs should not get write errors, as they
should just terminate and not do further I/O.)

Therefore, redefine job_is_cancelled() to only return true for jobs that
are force-cancelled (which as of HEAD^ means any job that interprets the
cancellation request as a request for immediate termination), and add
job_cancel_requested() as the general variant, which returns true for
any jobs which have been requested to be cancelled, whether it be
immediately or after an arbitrarily long completion phase.

Finally, here is a justification for how different job_is_cancelled()
invocations are treated by this patch:

- block/mirror.c (mirror_run()):
  - The first invocation is a while loop that should loop until the job
has been cancelled or scheduled for completion.  What kind of cancel
does not matter, only the fact that the job is supposed to end.

  - The second invocation wants to know whether the job has been
soft-cancelled.  Calling job_cancel_requested() is a bit too broad,
but if the job were force-cancelled, we should leave the main loop
as soon as possible anyway, so this should not matter here.

  - The last two invocations already check force_cancel, so they should
continue to use job_is_cancelled().

- block/backup.c, block/commit.c, block/stream.c, anything in tests/:
  These jobs know only force-cancel, so there is no difference between
  job_is_cancelled() and job_cancel_requested().  We can continue using
  job_is_cancelled().

- job.c:
  - job_pause_point(), job_yield(), job_sleep_ns(): Only force-cancelled
jobs should be prevented from being paused.  Continue using 
job_is_cancelled().

  - job_update_rc(), job_finalize_single(), job_finish_sync(): These
functions are all called after the job has left its main loop.  The
mirror job (the only job that can be soft-cancelled) will clear
.cancelled before leaving the main loop if it has been
soft-cancelled.  Therefore, these functions will observe .cancelled
to be true only if the job has been force-cancelled.  We can
continue to use job_is_cancelled().
(Furthermore, conceptually, a soft-cancelled mirror job should not
report to have been cancelled.  It should report completion (see
also the block-job-cancel QAPI documentation).  Therefore, it makes
sense for these functions not to distinguish between a
soft-cancelled mirror job and a job that has completed as normal.)

  - job_completed_txn_abort(): All jobs other than @job have been
force-cancelled.  job_is_cancelled() must be true for them.
Regarding @job itself: job_completed_txn_abort() is mostly called
when the job's return value is not 0.  A soft-cancelled mirror has a
return value of 0, and so will not end up here then.
However, job_cancel() invokes job_completed_txn_abort() if the job
has been deferred to the main loop, which is mostly the case for
completed jobs (which skip the assertion), but not for sure.
To be safe, use job_cancel_requested() in this assertion.

  - job_complete(): This is function eventually invoked by the user
(through qmp_block_job_complete() or qmp_job_complete(), or
job_complete_sync(), which comes from qemu-img).  The intention here
is to prevent a user from invoking job-complete after the job has
been cancelled.  This should also apply to soft cancelling: After a
mirror job has been soft-cancelled, the user should not be able to
decide otherwise and have it complete as normal (i.e. pivoting to
the target).

Buglink: https://gitlab.com/qemu-project/qemu/-/issues/462
Signed-off-by: Max Reitz 
---
 include/qemu/job.h |  8 +++-
 block/mirror.c | 10 --
 job.c  |  9 +++--
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/include/qemu/job.h b/include/qemu/job.h
index 8aa90f7395..032edf3c5f 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -436,9 +436,15 @@ const char *job_type_str(const Job *job);
 /** Returns true if the job should not be visible to the management layer. */
 bool job_is_internal(Job *job);
 
-/** Returns whether the job is scheduled for cancellation. */
+/** Returns whether the job is being cancelled. */
 bool job_is_cancelled(Job *job);
 
+/**
+ * Returns whether the job is scheduled for cancellation (at an
+ * indefinite point).
+ */
+bool job_cancel_requested(Job *job

[PATCH for-6.2 v3 08/12] mirror: Use job_is_cancelled()

[Max Reitz]

mirror_drained_poll() returns true whenever the job is cancelled,
because "we [can] be sure that it won't issue more requests".  However,
this is only true for force-cancelled jobs, so use job_is_cancelled().

Signed-off-by: Max Reitz 
---
 block/mirror.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/mirror.c b/block/mirror.c
index 72e02fa34e..024fa2dcea 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1177,7 +1177,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 && !s->common.job.cancelled && !s->in_drain) {
+if (!s->common.job.paused && !job_is_cancelled(&job->job) && !s->in_drain) 
{
 return true;
 }
 
-- 
2.31.1

[PATCH for-6.2 v3 04/12] job: Force-cancel jobs in a failed transaction

[Max Reitz]

When a transaction is aborted, no result matters, and so all jobs within
should be force-cancelled.

Signed-off-by: Max Reitz 
---
 job.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/job.c b/job.c
index 3fe23bb77e..24e7c4fcb7 100644
--- a/job.c
+++ b/job.c
@@ -766,7 +766,12 @@ static void job_completed_txn_abort(Job *job)
 if (other_job != job) {
 ctx = other_job->aio_context;
 aio_context_acquire(ctx);
-job_cancel_async(other_job, false);
+/*
+ * This is a transaction: If one job failed, no result will matter.
+ * Therefore, pass force=true to terminate all other jobs as 
quickly
+ * as possible.
+ */
+job_cancel_async(other_job, true);
 aio_context_release(ctx);
 }
 }
-- 
2.31.1

[PATCH for-6.2 v3 05/12] job: @force parameter for job_cancel_sync{, _all}()

[Max Reitz]

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.)

Buglink: https://gitlab.com/qemu-project/qemu/-/issues/462
Signed-off-by: Max Reitz 
---
 include/qemu/job.h| 10 ++---
 block/replication.c   |  4 +-
 blockdev.c|  4 +-
 job.c | 20 +++--
 qemu-nbd.c|  2 +-
 softmmu/runstate.c|  2 +-
 storage-daemon/qemu-storage-daemon.c  |  2 +-
 tests/unit/test-block-iothread.c  |  2 +-
 tests/unit/test-blockjob.c|  2 +-
 tests/qemu-iotests/109.out| 60 +++
 tests/qemu-iotests/tests/qsd-jobs.out |  2 +-
 11 files changed, 55 insertions(+), 55 deletions(-)

diff --git a/include/qemu/job.h b/include/qemu/job.h
index 41162ed494..5e8edbc2c8 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -506,19 +506,19 @@ void job_user_cancel(Job *job, bool force, Error **errp);
 
 /**
  * Synchronously cancel the @job.  The completion callback is called
- * before the function returns.  The job may actually complete
- * instead of canceling itself; the circumstances under which this
- * happens depend on the kind of job that is active.
+ * before the function returns.  If @force is false, the job may
+ * actually complete instead of canceling itself; the circumstances
+ * under which this happens depend on the kind of job that is active.
  *
  * Returns the return value from the job if the job actually completed
  * during the call, or -ECANCELED if it was canceled.
  *
  * Callers must hold the AioContext lock of job->aio_context.
  */
-int job_cancel_sync(Job *job);
+int job_cancel_sync(Job *job, bool force);
 
 /** Synchronously cancels all jobs using job_cancel_sync(). */
-void job_cancel_sync_all(void);
+void job_cancel_sync_all(bool force);
 
 /**
  * @job: The job to be completed.
diff --git a/block/replication.c b/block/replication.c
index 32444b9a8f..e7a9327b12 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -149,7 +149,7 @@ static void replication_close(BlockDriverState *bs)
 if (s->stage == BLOCK_REPLICATION_FAILOVER) {
 commit_job = &s->commit_job->job;
 assert(commit_job->aio_context == qemu_get_current_aio_context());
-job_cancel_sync(commit_job);
+job_cancel_sync(commit_job, false);
 }
 
 if (s->mode == REPLICATION_MODE_SECONDARY) {
@@ -726,7 +726,7 @@ static void replication_stop(ReplicationState *rs, bool 
failover, Error **errp)
  * disk, secondary disk in backup_job_completed().
  */
 if (s->backup_job) {
-job_cancel_sync(&s->backup_job->job);
+job_cancel_sync(&s->backup_job->job, false);
 }
 
 if (!failover) {
diff --git a/blockdev.c b/blockdev.c
index 3d8ac368a1..aa95918c02 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1848,7 +1848,7 @@ static void drive_backup_abort(BlkActionState *common)
 aio_context = bdrv_get_aio_context(state->bs);
 aio_context_acquire(aio_context);
 
-job_cancel_sync(&state->job->job);
+job_cancel_sync(&state->job->job, true);
 
 aio_context_release(aio_context);
 }
@@ -1949,7 +1949,7 @@ static void blockdev_backup_abort(BlkActionState *common)
 aio_context = bdrv_get_aio_context(state->bs);
 aio_context_acquire(aio_context);
 
-job_cancel_sync(&state->job->job);
+job_cancel_sync(&state->job->job, true);
 
 aio_context_release(aio_context);
 }
diff --git a/job.c b/job.c
index 24e7c4fcb7..1b68a7a983 100644
--- a/job.c
+++ 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);
+}
+
+int job_cancel_sync(Job *job, bool force)
+{
+if (force) {
+return job_finish_sync(job, &job_force_cancel_err, NULL);
+} else {
+return job_finish_sync(job, &job_cancel_err, NULL);
+}
 }
 
-void job_cancel_sync_all(void)
+void job_cancel_sync_all(bool force)
 {
 Job *job;
 AioContext *aio_context;
@@ -995,7 +1007,7 @@ void job_cancel_sync_all(void)

[PATCH for-6.2 v3 10/12] mirror: Stop active mirroring after force-cancel

[Max Reitz]

Once the mirror job is force-cancelled (job_is_cancelled() is true), we
should not generate new I/O requests.  This applies to active mirroring,
too, so stop it once the job is cancelled.

(We must still forward all I/O requests to the source, though, of
course, but those are not really I/O requests generated by the job, so
this is fine.)

Signed-off-by: Max Reitz 
---
 block/mirror.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/mirror.c b/block/mirror.c
index bf1d50ff1c..af89c1716a 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1418,6 +1418,7 @@ static int coroutine_fn 
bdrv_mirror_top_do_write(BlockDriverState *bs,
 bool copy_to_target;
 
 copy_to_target = s->job->ret >= 0 &&
+ !job_is_cancelled(&s->job->common.job) &&
  s->job->copy_mode == MIRROR_COPY_MODE_WRITE_BLOCKING;
 
 if (copy_to_target) {
@@ -1466,6 +1467,7 @@ static int coroutine_fn 
bdrv_mirror_top_pwritev(BlockDriverState *bs,
 bool copy_to_target;
 
 copy_to_target = s->job->ret >= 0 &&
+ !job_is_cancelled(&s->job->common.job) &&
  s->job->copy_mode == MIRROR_COPY_MODE_WRITE_BLOCKING;
 
 if (copy_to_target) {
-- 
2.31.1

[PATCH for-6.2 v3 12/12] iotests: Add mirror-ready-cancel-error test

[Max Reitz]

Test what happens when there is an I/O error after a mirror job in the
READY phase has been cancelled.

Signed-off-by: Max Reitz 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Tested-by: Vladimir Sementsov-Ogievskiy 
---
 .../tests/mirror-ready-cancel-error   | 143 ++
 .../tests/mirror-ready-cancel-error.out   |   5 +
 2 files changed, 148 insertions(+)
 create mode 100755 tests/qemu-iotests/tests/mirror-ready-cancel-error
 create mode 100644 tests/qemu-iotests/tests/mirror-ready-cancel-error.out

diff --git a/tests/qemu-iotests/tests/mirror-ready-cancel-error 
b/tests/qemu-iotests/tests/mirror-ready-cancel-error
new file mode 100755
index 00..f2dc1f
--- /dev/null
+++ b/tests/qemu-iotests/tests/mirror-ready-cancel-error
@@ -0,0 +1,143 @@
+#!/usr/bin/env python3
+# group: rw quick
+#
+# Test what happens when errors occur to a mirror job after it has
+# been cancelled in the READY phase
+#
+# Copyright (C) 2021 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+import os
+import iotests
+
+
+image_size = 1 * 1024 * 1024
+source = os.path.join(iotests.test_dir, 'source.img')
+target = os.path.join(iotests.test_dir, 'target.img')
+
+
+class TestMirrorReadyCancelError(iotests.QMPTestCase):
+def setUp(self) -> None:
+assert iotests.qemu_img_create('-f', iotests.imgfmt, source,
+   str(image_size)) == 0
+assert iotests.qemu_img_create('-f', iotests.imgfmt, target,
+   str(image_size)) == 0
+
+self.vm = iotests.VM()
+self.vm.launch()
+
+def tearDown(self) -> None:
+self.vm.shutdown()
+os.remove(source)
+os.remove(target)
+
+def add_blockdevs(self, once: bool) -> None:
+res = self.vm.qmp('blockdev-add',
+  **{'node-name': 'source',
+ 'driver': iotests.imgfmt,
+ 'file': {
+ 'driver': 'file',
+ 'filename': source
+ }})
+self.assert_qmp(res, 'return', {})
+
+# blkdebug notes:
+# Enter state 2 on the first flush, which happens before the
+# job enters the READY state.  The second flush will happen
+# when the job is about to complete, and we want that one to
+# fail.
+res = self.vm.qmp('blockdev-add',
+  **{'node-name': 'target',
+ 'driver': iotests.imgfmt,
+ 'file': {
+ 'driver': 'blkdebug',
+ 'image': {
+ 'driver': 'file',
+ 'filename': target
+ },
+ 'set-state': [{
+ 'event': 'flush_to_disk',
+ 'state': 1,
+ 'new_state': 2
+ }],
+ 'inject-error': [{
+ 'event': 'flush_to_disk',
+ 'once': once,
+ 'immediately': True,
+ 'state': 2
+ }]}})
+self.assert_qmp(res, 'return', {})
+
+def start_mirror(self) -> None:
+res = self.vm.qmp('blockdev-mirror',
+  job_id='mirror',
+  device='source',
+  target='target',
+  filter_node_name='mirror-top',
+  sync='full',
+  on_target_error='stop')
+self.assert_qmp(res, 'return', {})
+
+def cancel_mirror_with_error(self) -> None:
+self.vm.event_wait('BLOCK_JOB_READY')
+
+# Write something so will not leave the job immediately, but
+# flush first (which will fail, thanks to blkdebug)
+res = self.vm.qmp('human-monitor-command',
+  command_line='qemu-io mirror-top "write 0 64k"')
+self.assert_qmp(res, 'return', '')
+
+# Drain status change events
+while self.vm.event_wait('JOB_STATUS_CHANGE', timeout=0.0) is not

[PATCH for-6.2 v3 09/12] mirror: Check job_is_cancelled() earlier

[Max Reitz]

We must check whether the job is force-cancelled early in our main loop,
most importantly before any `continue` statement.  For example, we used
to have `continue`s before our current checking location that are
triggered by `mirror_flush()` failing.  So, if `mirror_flush()` kept
failing, force-cancelling the job would not terminate it.

Jobs can be cancelled while they yield, and once they are
(force-cancelled), they should not generate new I/O requests.
Therefore, we should put the check after the last yield before
mirror_iteration() is invoked.

Buglink: https://gitlab.com/qemu-project/qemu/-/issues/462
Signed-off-by: Max Reitz 
---
 block/mirror.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 024fa2dcea..bf1d50ff1c 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1000,6 +1000,11 @@ static int coroutine_fn mirror_run(Job *job, Error 
**errp)
 
 job_pause_point(&s->common.job);
 
+if (job_is_cancelled(&s->common.job)) {
+ret = 0;
+goto immediate_exit;
+}
+
 cnt = bdrv_get_dirty_count(s->dirty_bitmap);
 /* cnt is the number of dirty bytes remaining and s->bytes_in_flight is
  * the number of bytes currently being processed; together those are
@@ -1078,8 +1083,6 @@ static int coroutine_fn mirror_run(Job *job, Error **errp)
 break;
 }
 
-ret = 0;
-
 if (job_is_ready(&s->common.job) && !should_complete) {
 delay_ns = (s->in_flight == 0 &&
 cnt == 0 ? BLOCK_JOB_SLICE_TIME : 0);
@@ -1087,9 +1090,6 @@ static int coroutine_fn mirror_run(Job *job, Error **errp)
 trace_mirror_before_sleep(s, cnt, job_is_ready(&s->common.job),
   delay_ns);
 job_sleep_ns(&s->common.job, delay_ns);
-if (job_is_cancelled(&s->common.job)) {
-break;
-}
 s->last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
 }
 
-- 
2.31.1

[PATCH for-6.2 v3 11/12] mirror: Do not clear .cancelled

[Max Reitz]

Clearing .cancelled before leaving the main loop when the job has been
soft-cancelled is no longer necessary since job_is_cancelled() only
returns true for jobs that have been force-cancelled.

Therefore, this only makes a differences in places that call
job_cancel_requested().  In block/mirror.c, this is done only before
.cancelled was cleared.

In job.c, there are two callers:
- job_completed_txn_abort() asserts that .cancelled is true, so keeping
  it true will not affect this place.

- job_complete() refuses to let a job complete that has .cancelled set.
  It is correct to refuse to let the user invoke job-complete on mirror
  jobs that have already been soft-cancelled.

With this change, there are no places that reset .cancelled to false and
so we can be sure that .force_cancel can only be true of .cancelled is
true as well.  Assert this in job_is_cancelled().

Signed-off-by: Max Reitz 
---
 block/mirror.c | 2 --
 job.c  | 4 +++-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index af89c1716a..f94aa52fae 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -939,7 +939,6 @@ static int coroutine_fn mirror_run(Job *job, Error **errp)
 while (!job_cancel_requested(&s->common.job) && !s->should_complete) {
 job_yield(&s->common.job);
 }
-s->common.job.cancelled = false;
 goto immediate_exit;
 }
 
@@ -1078,7 +1077,6 @@ static int coroutine_fn mirror_run(Job *job, Error **errp)
  * completion.
  */
 assert(QLIST_EMPTY(&bs->tracked_requests));
-s->common.job.cancelled = false;
 need_drain = false;
 break;
 }
diff --git a/job.c b/job.c
index 2bd3c946a7..2ce6865ab2 100644
--- a/job.c
+++ b/job.c
@@ -217,7 +217,9 @@ const char *job_type_str(const Job *job)
 
 bool job_is_cancelled(Job *job)
 {
-return job->cancelled && job->force_cancel;
+/* force_cancel may be true only if cancelled is true, too */
+assert(job->cancelled || !job->force_cancel);
+return job->force_cancel;
 }
 
 bool job_cancel_requested(Job *job)
-- 
2.31.1

Re: [PATCH for-6.2 v3 01/12] job: Context changes in job_completed_txn_abort()

[Eric Blake]

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 
> ---
>  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 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH for-6.2 v3 04/12] job: Force-cancel jobs in a failed transaction

[Eric Blake]

On Fri, Aug 06, 2021 at 11:38:51AM +0200, Max Reitz wrote:
> When a transaction is aborted, no result matters, and so all jobs within
> should be force-cancelled.
> 
> Signed-off-by: Max Reitz 
> ---
>  job.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

Reviewed-by: Eric Blake 

> 
> diff --git a/job.c b/job.c
> index 3fe23bb77e..24e7c4fcb7 100644
> --- a/job.c
> +++ b/job.c
> @@ -766,7 +766,12 @@ static void job_completed_txn_abort(Job *job)
>  if (other_job != job) {
>  ctx = other_job->aio_context;
>  aio_context_acquire(ctx);
> -job_cancel_async(other_job, false);
> +/*
> + * This is a transaction: If one job failed, no result will 
> matter.
> + * Therefore, pass force=true to terminate all other jobs as 
> quickly
> + * as possible.
> + */
> +job_cancel_async(other_job, true);
>  aio_context_release(ctx);
>  }
>  }
> -- 
> 2.31.1
> 
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH for-6.2 v3 05/12] job: @force parameter for job_cancel_sync{, _all}()

[Eric Blake]

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 
> ---

> +++ 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 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH for-6.2 v3 07/12] job: Add job_cancel_requested()

[Eric Blake]

On Fri, Aug 06, 2021 at 11:38:54AM +0200, Max Reitz wrote:
> Most callers of job_is_cancelled() actually want to know whether the job
> is on its way to immediate termination.  For example, we refuse to pause
> jobs that are cancelled; but this only makes sense for jobs that are
> really actually cancelled.
> 
> A mirror job that is cancelled during READY with force=false should
> absolutely be allowed to pause.  This "cancellation" (which is actually
> a kind of completion) may take an indefinite amount of time, and so
> should behave like any job during normal operation.  For example, with
> on-target-error=stop, the job should stop on write errors.  (In
> contrast, force-cancelled jobs should not get write errors, as they
> should just terminate and not do further I/O.)
> 
> Therefore, redefine job_is_cancelled() to only return true for jobs that
> are force-cancelled (which as of HEAD^ means any job that interprets the
> cancellation request as a request for immediate termination), and add
> job_cancel_requested() as the general variant, which returns true for
> any jobs which have been requested to be cancelled, whether it be
> immediately or after an arbitrarily long completion phase.
> 
> Finally, here is a justification for how different job_is_cancelled()
> invocations are treated by this patch:

Thanks for this list; it's really thorough and helpful.

> 
> Buglink: https://gitlab.com/qemu-project/qemu/-/issues/462
> Signed-off-by: Max Reitz 

Although it is fixing a bug, the bug has been long-standing, so I
agree with your claim that this is 6.2 material.

> ---
>  include/qemu/job.h |  8 +++-
>  block/mirror.c | 10 --
>  job.c  |  9 +++--
>  3 files changed, 18 insertions(+), 9 deletions(-)
> 

> +++ b/job.c
> @@ -216,6 +216,11 @@ const char *job_type_str(const Job *job)
>  }
>  
>  bool job_is_cancelled(Job *job)
> +{
> +return job->cancelled && job->force_cancel;
> +}
> +
> +bool job_cancel_requested(Job *job)
>  {
>  return job->cancelled;
>  }

Works out rather nicely.

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH for-6.2 v3 08/12] mirror: Use job_is_cancelled()

[Eric Blake]

On Fri, Aug 06, 2021 at 11:38:55AM +0200, Max Reitz wrote:
> mirror_drained_poll() returns true whenever the job is cancelled,
> because "we [can] be sure that it won't issue more requests".  However,
> this is only true for force-cancelled jobs, so use job_is_cancelled().
> 
> Signed-off-by: Max Reitz 
> ---
>  block/mirror.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/mirror.c b/block/mirror.c
> index 72e02fa34e..024fa2dcea 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -1177,7 +1177,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 && !s->common.job.cancelled && !s->in_drain) {
> +if (!s->common.job.paused && !job_is_cancelled(&job->job) && 
> !s->in_drain) {
>  return true;
>  }

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH for-6.2 v3 09/12] mirror: Check job_is_cancelled() earlier

[Eric Blake]

On Fri, Aug 06, 2021 at 11:38:56AM +0200, Max Reitz wrote:
> We must check whether the job is force-cancelled early in our main loop,
> most importantly before any `continue` statement.  For example, we used
> to have `continue`s before our current checking location that are
> triggered by `mirror_flush()` failing.  So, if `mirror_flush()` kept
> failing, force-cancelling the job would not terminate it.
> 
> Jobs can be cancelled while they yield, and once they are
> (force-cancelled), they should not generate new I/O requests.
> Therefore, we should put the check after the last yield before
> mirror_iteration() is invoked.
> 
> Buglink: https://gitlab.com/qemu-project/qemu/-/issues/462
> Signed-off-by: Max Reitz 
> ---
>  block/mirror.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH for-6.2 v3 10/12] mirror: Stop active mirroring after force-cancel

[Eric Blake]

On Fri, Aug 06, 2021 at 11:38:57AM +0200, Max Reitz wrote:
> Once the mirror job is force-cancelled (job_is_cancelled() is true), we
> should not generate new I/O requests.  This applies to active mirroring,
> too, so stop it once the job is cancelled.
> 
> (We must still forward all I/O requests to the source, though, of
> course, but those are not really I/O requests generated by the job, so
> this is fine.)
> 
> Signed-off-by: Max Reitz 
> ---
>  block/mirror.c | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH for-6.2 v3 11/12] mirror: Do not clear .cancelled

[Eric Blake]

On Fri, Aug 06, 2021 at 11:38:58AM +0200, Max Reitz wrote:
> Clearing .cancelled before leaving the main loop when the job has been
> soft-cancelled is no longer necessary since job_is_cancelled() only
> returns true for jobs that have been force-cancelled.
> 
> Therefore, this only makes a differences in places that call
> job_cancel_requested().  In block/mirror.c, this is done only before
> .cancelled was cleared.
> 
> In job.c, there are two callers:
> - job_completed_txn_abort() asserts that .cancelled is true, so keeping
>   it true will not affect this place.
> 
> - job_complete() refuses to let a job complete that has .cancelled set.
>   It is correct to refuse to let the user invoke job-complete on mirror
>   jobs that have already been soft-cancelled.
> 
> With this change, there are no places that reset .cancelled to false and
> so we can be sure that .force_cancel can only be true of .cancelled is

s/of/if/

> true as well.  Assert this in job_is_cancelled().
> 
> Signed-off-by: Max Reitz 
> ---
>  block/mirror.c | 2 --
>  job.c  | 4 +++-
>  2 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[PATCH for-6.2 05/12] [automated] Move QOM typedefs and add missing includes

[Eduardo Habkost]

Some typedefs and macros are defined after the type check macros.
This makes it difficult to automatically replace their
definitions with OBJECT_DECLARE_TYPE.

Patch generated using:

 $ ./scripts/codeconverter/converter.py -i --pattern=MoveSymbols \
$(git grep -l '' -- '*.[ch]')

which will:
- split "typdef struct { ... } TypedefName" declarations
- move the typedefs and #defines above the type check macros
- add missing #include "qom/object.h" lines if necessary

Signed-off-by: Eduardo Habkost 
---
Cc: Richard Henderson 
Cc: Paolo Bonzini 
Cc: "Marc-André Lureau" 
Cc: Peter Maydell 
Cc: Andrew Baumann 
Cc: "Philippe Mathieu-Daudé" 
Cc: Thomas Huth 
Cc: "Michael S. Tsirkin" 
Cc: Igor Mammedov 
Cc: Alexander Bulekov 
Cc: Bandan Das 
Cc: Stefan Hajnoczi 
Cc: Huacai Chen 
Cc: Jiaxun Yang 
Cc: Aurelien Jarno 
Cc: Aleksandar Rikalo 
Cc: Havard Skinnemoen 
Cc: Tyrone Ting 
Cc: Pavel Pisa 
Cc: Vikram Garhwal 
Cc: Jason Wang 
Cc: Keith Busch 
Cc: Klaus Jensen 
Cc: Cornelia Huck 
Cc: Halil Pasic 
Cc: Christian Borntraeger 
Cc: David Hildenbrand 
Cc: Gerd Hoffmann 
Cc: Vijai Kumar K 
Cc: Alistair Francis 
Cc: Bin Meng 
Cc: Palmer Dabbelt 
Cc: "Edgar E. Iglesias" 
Cc: Laurent Vivier 
Cc: Corey Minyard 
Cc: "Cédric Le Goater" 
Cc: Andrew Jeffery 
Cc: Joel Stanley 
Cc: Francisco Iglesias 
Cc: David Gibson 
Cc: Greg Kurz 
Cc: Alexey Kardashevskiy 
Cc: "Hervé Poussineau" 
Cc: Bastian Koppelmann 
Cc: Juan Quintela 
Cc: "Dr. David Alan Gilbert" 
Cc: "Daniel P. Berrangé" 
Cc: Eduardo Habkost 
Cc: Stefan Berger 
Cc: Taylor Simpson 
Cc: qemu-de...@nongnu.org
Cc: qemu-...@nongnu.org
Cc: qemu-block@nongnu.org
Cc: qemu-s3...@nongnu.org
Cc: qemu-ri...@nongnu.org
Cc: qemu-...@nongnu.org
---
 hw/nvme/nvme.h  |  6 --
 hw/usb/hcd-uhci.h   |  1 +
 hw/usb/hcd-xhci-pci.h   |  6 --
 hw/usb/hcd-xhci-sysbus.h|  6 --
 hw/usb/u2f.h|  6 --
 include/hw/acpi/acpi_dev_interface.h|  2 +-
 include/hw/adc/npcm7xx_adc.h|  1 +
 include/hw/arm/linux-boot-if.h  |  2 +-
 include/hw/arm/npcm7xx.h| 11 +++
 include/hw/char/shakti_uart.h   |  6 --
 include/hw/core/accel-cpu.h |  1 +
 include/hw/dma/sifive_pdma.h|  1 +
 include/hw/dma/xlnx_csu_dma.h   |  1 +
 include/hw/fw-path-provider.h   |  2 +-
 include/hw/gpio/npcm7xx_gpio.h  |  1 +
 include/hw/hotplug.h|  2 +-
 include/hw/i2c/npcm7xx_smbus.h  |  1 +
 include/hw/intc/intc.h  |  2 +-
 include/hw/intc/m68k_irqc.h |  6 --
 include/hw/intc/sifive_clint.h  |  6 --
 include/hw/ipmi/ipmi.h  |  2 +-
 include/hw/mem/memory-device.h  |  2 +-
 include/hw/mem/npcm7xx_mc.h |  1 +
 include/hw/misc/aspeed_lpc.h|  6 --
 include/hw/misc/bcm2835_cprman.h|  1 +
 include/hw/misc/bcm2835_cprman_internals.h  |  1 +
 include/hw/misc/mchp_pfsoc_dmc.h|  1 +
 include/hw/misc/mchp_pfsoc_ioscb.h  |  1 +
 include/hw/misc/mchp_pfsoc_sysreg.h |  1 +
 include/hw/misc/npcm7xx_clk.h   |  1 +
 include/hw/misc/npcm7xx_gcr.h   |  1 +
 include/hw/misc/npcm7xx_pwm.h   |  1 +
 include/hw/misc/npcm7xx_rng.h   |  1 +
 include/hw/misc/xlnx-versal-xramc.h |  6 --
 include/hw/net/npcm7xx_emc.h|  1 +
 include/hw/net/xlnx-zynqmp-can.h|  6 --
 include/hw/nmi.h|  2 +-
 include/hw/nvram/npcm7xx_otp.h  |  1 +
 include/hw/ppc/spapr_drc.h  | 15 +--
 include/hw/ppc/spapr_xive.h | 11 +++
 include/hw/ppc/vof.h|  1 +
 include/hw/rdma/rdma.h  |  2 +-
 include/hw/riscv/microchip_pfsoc.h  |  1 +
 include/hw/riscv/shakti_c.h | 11 +++
 include/hw/riscv/sifive_e.h |  6 --
 include/hw/riscv/sifive_u.h | 11 +++
 include/hw/rtc/m48t59.h |  2 +-
 include/hw/sd/cadence_sdhci.h   |  1 +
 include/hw/ssi/npcm7xx_fiu.h|  1 +
 include/hw/ssi/sifive_spi.h |  6 --
 include/hw/stream.h |  2 +-
 include/hw/timer/npcm7xx_timer.h|  1 +
 include/hw/tricore/tricore_testdevice.h |  6 --
 include/hw/usb/hcd-dwc3.h   |  6 --
 include/hw/usb/msd.h|  1 +
 include/hw/usb/xlnx-usb-subsystem.h |  6 --
 include/hw/usb/xlnx-versal-usb2-ctrl-regs.h |  6 --
 include/hw/vmstate-if.h |  2 +-
 include/hw/watchdog/sbsa_gwdt.h |  6 --
 include/qom/object_interfaces.h |  2 +-
 include/sysemu/tpm.h  

[PATCH for-6.2 06/12] [automated] Split QOM "typedef struct T { ... } T" declarations

[Eduardo Habkost]

Automatically split struct definition and typedef declaration in
separate declarations, using a codeconverter rule.  The rule will
only touch declarations of structs/typedefs actually used by QOM
types.

This will make automated changes to use OBJECT_DECLARE* macros
easier to implement, because automated removal of typedef lines
will be easier and safer.

Generated using:

  $ ./scripts/codeconverter/converter.py -i \
--pattern=QOMStructTypedefSplit $(git grep -l '' -- '*.[ch]')

Signed-off-by: Eduardo Habkost 
---
Cc: "Marc-André Lureau" 
Cc: Paolo Bonzini 
Cc: Patrick Venture 
Cc: Thomas Huth 
Cc: Keith Busch 
Cc: Klaus Jensen 
Cc: "Michael S. Tsirkin" 
Cc: Cornelia Huck 
Cc: Halil Pasic 
Cc: Christian Borntraeger 
Cc: Richard Henderson 
Cc: David Hildenbrand 
Cc: Gerd Hoffmann 
Cc: Havard Skinnemoen 
Cc: Tyrone Ting 
Cc: Alistair Francis 
Cc: Bin Meng 
Cc: Palmer Dabbelt 
Cc: "Edgar E. Iglesias" 
Cc: Peter Maydell 
Cc: Andrew Baumann 
Cc: "Philippe Mathieu-Daudé" 
Cc: "Cédric Le Goater" 
Cc: David Gibson 
Cc: Greg Kurz 
Cc: Laurent Vivier 
Cc: qemu-de...@nongnu.org
Cc: qemu-block@nongnu.org
Cc: qemu-s3...@nongnu.org
Cc: qemu-...@nongnu.org
Cc: qemu-ri...@nongnu.org
Cc: qemu-...@nongnu.org
---
 hw/nvme/nvme.h  | 10 ++
 hw/usb/hcd-uhci.h   |  5 +++--
 hw/usb/u2f.h|  5 +++--
 include/hw/adc/npcm7xx_adc.h|  5 +++--
 include/hw/arm/npcm7xx.h| 10 ++
 include/hw/core/accel-cpu.h |  5 +++--
 include/hw/dma/sifive_pdma.h|  5 +++--
 include/hw/dma/xlnx_csu_dma.h   |  5 +++--
 include/hw/gpio/npcm7xx_gpio.h  |  5 +++--
 include/hw/i2c/npcm7xx_smbus.h  |  5 +++--
 include/hw/mem/npcm7xx_mc.h |  5 +++--
 include/hw/misc/bcm2835_cprman.h| 20 
 include/hw/misc/mchp_pfsoc_dmc.h| 10 ++
 include/hw/misc/mchp_pfsoc_ioscb.h  |  5 +++--
 include/hw/misc/mchp_pfsoc_sysreg.h |  5 +++--
 include/hw/misc/npcm7xx_clk.h   | 15 +--
 include/hw/misc/npcm7xx_gcr.h   |  5 +++--
 include/hw/misc/npcm7xx_mft.h   |  5 +++--
 include/hw/misc/npcm7xx_rng.h   |  5 +++--
 include/hw/nvram/npcm7xx_otp.h  |  5 +++--
 include/hw/riscv/microchip_pfsoc.h  | 10 ++
 include/hw/riscv/sifive_e.h |  5 +++--
 include/hw/sd/cadence_sdhci.h   |  5 +++--
 include/qemu/accel.h| 10 ++
 chardev/char-parallel.c | 10 ++
 hw/i2c/i2c_mux_pca954x.c|  5 +++--
 hw/m68k/mcf5206.c   |  5 +++--
 hw/misc/sbsa_ec.c   |  5 +++--
 hw/s390x/vhost-user-fs-ccw.c|  5 +++--
 tests/qtest/pnv-xscom-test.c|  5 +++--
 30 files changed, 123 insertions(+), 82 deletions(-)

diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index 5ddcd783055..0840f585d6e 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -118,7 +118,7 @@ typedef struct NvmeNamespaceParams {
 uint32_t zd_extension_size;
 } NvmeNamespaceParams;
 
-typedef struct NvmeNamespace {
+struct NvmeNamespace {
 DeviceState  parent_obj;
 BlockConfblkconf;
 int32_t  bootindex;
@@ -153,7 +153,8 @@ typedef struct NvmeNamespace {
 struct {
 uint32_t err_rec;
 } features;
-} NvmeNamespace;
+};
+typedef struct NvmeNamespace NvmeNamespace;
 
 static inline uint32_t nvme_nsid(NvmeNamespace *ns)
 {
@@ -395,7 +396,7 @@ typedef struct NvmeParams {
 bool legacy_cmb;
 } NvmeParams;
 
-typedef struct NvmeCtrl {
+struct NvmeCtrl {
 PCIDeviceparent_obj;
 MemoryRegion bar0;
 MemoryRegion iomem;
@@ -462,7 +463,8 @@ typedef struct NvmeCtrl {
 };
 uint32_tasync_config;
 } features;
-} NvmeCtrl;
+};
+typedef struct NvmeCtrl NvmeCtrl;
 
 static inline NvmeNamespace *nvme_ns(NvmeCtrl *n, uint32_t nsid)
 {
diff --git a/hw/usb/hcd-uhci.h b/hw/usb/hcd-uhci.h
index 57d0d574644..2597a6d2eb6 100644
--- a/hw/usb/hcd-uhci.h
+++ b/hw/usb/hcd-uhci.h
@@ -43,7 +43,7 @@ typedef struct UHCIPort {
 uint16_t ctrl;
 } UHCIPort;
 
-typedef struct UHCIState {
+struct UHCIState {
 PCIDevice dev;
 MemoryRegion io_bar;
 USBBus bus; /* Note unused when we're a companion controller */
@@ -73,7 +73,8 @@ typedef struct UHCIState {
 char *masterbus;
 uint32_t firstport;
 uint32_t maxframes;
-} UHCIState;
+};
+typedef struct UHCIState UHCIState;
 
 #define TYPE_UHCI "pci-uhci-usb"
 DECLARE_INSTANCE_CHECKER(UHCIState, UHCI, TYPE_UHCI)
diff --git a/hw/usb/u2f.h b/hw/usb/u2f.h
index 705d5c43ce6..7191a23ee1a 100644
--- a/hw/usb/u2f.h
+++ b/hw/usb/u2f.h
@@ -62,7 +62,7 @@ struct U2FKeyClass {
 /*
  * State of the U2F key base device (i.e. hw/u2f.c)
  */
-typedef struct U2FKeyState {
+struct U2FKeyState {
 USBDevice dev;
 USBEndpoint *ep;
 uint8_t idle;
@@ -72,7 +72,8 @@ typedef struct U2FKeyState {
 uint8_t pending_in_start;
 uint8_t pending_in_end;
 uint8_t pending_in_num;
-} U2FKeyState;
+};
+typedef struct U2FKeyState U2FKeySt

[PATCH for-6.2 07/12] [automated] Use DECLARE_*CHECKER* macros when possible

[Eduardo Habkost]

Converting existing QOM types to OBJECT_DECLARE_TYPE is not
always trivial (due to inconsistent type/macro naming schemes),
but at least converting existing manual QOM type checking macros
to use DECLARE_*CHECKER* is a simpler process, and should at
least discourage people from defining new QOM type checker macros
manually.

Generated using:

  $ ./scripts/codeconverter/converter.py -i \
--pattern=TypeCheckMacro $(git grep -l '' -- '*.[ch]')

Signed-off-by: Eduardo Habkost 
---
Cc: "Marc-André Lureau" 
Cc: Paolo Bonzini 
Cc: Patrick Venture 
Cc: Thomas Huth 
Cc: "Michael S. Tsirkin" 
Cc: Igor Mammedov 
Cc: Alexander Bulekov 
Cc: Bandan Das 
Cc: Stefan Hajnoczi 
Cc: Keith Busch 
Cc: Klaus Jensen 
Cc: Cornelia Huck 
Cc: Halil Pasic 
Cc: Christian Borntraeger 
Cc: Richard Henderson 
Cc: David Hildenbrand 
Cc: Gerd Hoffmann 
Cc: Havard Skinnemoen 
Cc: Tyrone Ting 
Cc: Vijai Kumar K 
Cc: Alistair Francis 
Cc: Bin Meng 
Cc: Palmer Dabbelt 
Cc: "Edgar E. Iglesias" 
Cc: Peter Maydell 
Cc: Laurent Vivier 
Cc: "Cédric Le Goater" 
Cc: Andrew Jeffery 
Cc: Joel Stanley 
Cc: Jason Wang 
Cc: Vikram Garhwal 
Cc: Francisco Iglesias 
Cc: David Gibson 
Cc: Greg Kurz 
Cc: Bastian Koppelmann 
Cc: Taylor Simpson 
Cc: qemu-de...@nongnu.org
Cc: qemu-block@nongnu.org
Cc: qemu-s3...@nongnu.org
Cc: qemu-...@nongnu.org
Cc: qemu-ri...@nongnu.org
Cc: qemu-...@nongnu.org
---
 hw/nvme/nvme.h  | 12 ++--
 hw/usb/hcd-xhci-pci.h   |  4 ++--
 hw/usb/hcd-xhci-sysbus.h|  4 ++--
 hw/usb/u2f.h|  8 ++--
 include/hw/adc/npcm7xx_adc.h|  4 ++--
 include/hw/arm/npcm7xx.h| 15 ---
 include/hw/char/shakti_uart.h   |  4 ++--
 include/hw/dma/sifive_pdma.h|  4 ++--
 include/hw/dma/xlnx_csu_dma.h   |  4 ++--
 include/hw/gpio/npcm7xx_gpio.h  |  4 ++--
 include/hw/i2c/npcm7xx_smbus.h  |  4 ++--
 include/hw/intc/m68k_irqc.h |  4 ++--
 include/hw/intc/sifive_clint.h  |  4 ++--
 include/hw/mem/npcm7xx_mc.h |  3 ++-
 include/hw/misc/aspeed_lpc.h|  3 ++-
 include/hw/misc/mchp_pfsoc_dmc.h| 10 --
 include/hw/misc/mchp_pfsoc_ioscb.h  |  4 ++--
 include/hw/misc/mchp_pfsoc_sysreg.h |  5 ++---
 include/hw/misc/npcm7xx_clk.h   |  3 ++-
 include/hw/misc/npcm7xx_gcr.h   |  3 ++-
 include/hw/misc/npcm7xx_mft.h   |  4 ++--
 include/hw/misc/npcm7xx_pwm.h   |  4 ++--
 include/hw/misc/npcm7xx_rng.h   |  3 ++-
 include/hw/misc/xlnx-versal-xramc.h |  4 ++--
 include/hw/net/npcm7xx_emc.h|  4 ++--
 include/hw/net/xlnx-zynqmp-can.h|  4 ++--
 include/hw/nvram/npcm7xx_otp.h  |  3 ++-
 include/hw/ppc/spapr_drc.h  | 13 -
 include/hw/ppc/spapr_xive.h |  7 ++-
 include/hw/riscv/microchip_pfsoc.h  |  9 -
 include/hw/riscv/shakti_c.h |  8 
 include/hw/riscv/sifive_e.h |  8 
 include/hw/riscv/sifive_u.h |  8 
 include/hw/sd/cadence_sdhci.h   |  4 ++--
 include/hw/ssi/npcm7xx_fiu.h|  3 ++-
 include/hw/ssi/sifive_spi.h |  3 ++-
 include/hw/timer/npcm7xx_timer.h|  4 ++--
 include/hw/tricore/tricore_testdevice.h |  4 ++--
 include/hw/usb/hcd-dwc3.h   |  4 ++--
 include/hw/usb/xlnx-usb-subsystem.h |  4 ++--
 include/hw/usb/xlnx-versal-usb2-ctrl-regs.h |  4 ++--
 include/hw/watchdog/sbsa_gwdt.h |  4 ++--
 include/qemu/accel.h|  8 ++--
 target/hexagon/cpu.h|  8 ++--
 chardev/char-parallel.c |  6 ++
 hw/i2c/i2c_mux_pca954x.c|  4 ++--
 hw/m68k/mcf5206.c   |  3 ++-
 hw/mem/sparse-mem.c |  3 ++-
 hw/misc/sbsa_ec.c   |  3 ++-
 hw/s390x/vhost-user-fs-ccw.c|  4 ++--
 hw/sensor/adm1272.c |  3 ++-
 hw/sensor/max34451.c|  3 ++-
 hw/usb/u2f-emulated.c   |  4 ++--
 hw/usb/u2f-passthru.c   |  4 ++--
 54 files changed, 126 insertions(+), 146 deletions(-)

diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index 0840f585d6e..c4c43da5c17 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -43,8 +43,8 @@ typedef struct NvmeBus {
 
 #define TYPE_NVME_SUBSYS "nvme-subsys"
 typedef struct NvmeSubsystem NvmeSubsystem;
-#define NVME_SUBSYS(obj) \
-OBJECT_CHECK(NvmeSubsystem, (obj), TYPE_NVME_SUBSYS)
+DECLARE_INSTANCE_CHECKER(NvmeSubsystem, NVME_SUBSYS,
+ TYPE_NVME_SUBSYS)
 
 struct NvmeSubsystem {
 DeviceState parent_obj;
@@ -83,8 +83,8 @@ static inline NvmeNamespace *nvme_

[PATCH for-6.2 11/12] [automated] Use OBJECT_DECLARE_TYPE when possible

[Eduardo Habkost]

Replace typedefs + DECLARE_OBJ_CHECKERS with equivalent
OBJECT_DECLARE_TYPE macro.

Generated using:

$ ./scripts/codeconverter/converter.py -i \
  --pattern=AddObjectDeclareType $(git grep -l '' -- '*.[ch]')

Signed-off-by: Eduardo Habkost 
---
Cc: John Snow 
Cc: Kevin Wolf 
Cc: Max Reitz 
Cc: Gerd Hoffmann 
Cc: "Daniel P. Berrangé" 
Cc: David Gibson 
Cc: Greg Kurz 
Cc: "Cédric Le Goater" 
Cc: Richard Henderson 
Cc: Paolo Bonzini 
Cc: qemu-block@nongnu.org
Cc: qemu-de...@nongnu.org
Cc: qemu-...@nongnu.org
---
 hw/usb/u2f.h| 6 +-
 include/crypto/tlscreds.h   | 5 +
 include/hw/ppc/spapr_drc.h  | 5 +
 include/hw/ppc/spapr_xive.h | 5 +
 include/qemu/accel.h| 4 +---
 hw/block/fdc-sysbus.c   | 5 +
 6 files changed, 6 insertions(+), 24 deletions(-)

diff --git a/hw/usb/u2f.h b/hw/usb/u2f.h
index 5767ca8fac9..e6e3ead40f1 100644
--- a/hw/usb/u2f.h
+++ b/hw/usb/u2f.h
@@ -32,13 +32,10 @@
 #define U2FHID_PACKET_SIZE 64
 #define U2FHID_PENDING_IN_NUM 32
 
-typedef struct U2FKeyState U2FKeyState;
 typedef struct U2FKeyInfo U2FKeyInfo;
 
 #define TYPE_U2F_KEY "u2f-key"
-typedef struct U2FKeyClass U2FKeyClass;
-DECLARE_OBJ_CHECKERS(U2FKeyState, U2FKeyClass,
- U2F_KEY, TYPE_U2F_KEY)
+OBJECT_DECLARE_TYPE(U2FKeyState, U2FKeyClass, U2F_KEY)
 
 /*
  * Callbacks to be used by the U2F key base device (i.e. hw/u2f.c)
@@ -69,7 +66,6 @@ struct U2FKeyState {
 uint8_t pending_in_end;
 uint8_t pending_in_num;
 };
-typedef struct U2FKeyState U2FKeyState;
 
 /*
  * API to be used by the U2F key device variants (i.e. hw/u2f-*.c)
diff --git a/include/crypto/tlscreds.h b/include/crypto/tlscreds.h
index 2a8a8570109..617979a3986 100644
--- a/include/crypto/tlscreds.h
+++ b/include/crypto/tlscreds.h
@@ -25,10 +25,7 @@
 #include "qom/object.h"
 
 #define TYPE_QCRYPTO_TLS_CREDS "tls-creds"
-typedef struct QCryptoTLSCreds QCryptoTLSCreds;
-typedef struct QCryptoTLSCredsClass QCryptoTLSCredsClass;
-DECLARE_OBJ_CHECKERS(QCryptoTLSCreds, QCryptoTLSCredsClass, QCRYPTO_TLS_CREDS,
- TYPE_QCRYPTO_TLS_CREDS)
+OBJECT_DECLARE_TYPE(QCryptoTLSCreds, QCryptoTLSCredsClass, QCRYPTO_TLS_CREDS)
 
 
 #define QCRYPTO_TLS_CREDS_DH_PARAMS "dh-params.pem"
diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index 540439812f0..ff876fd74ca 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -20,10 +20,7 @@
 #include "qapi/error.h"
 
 #define TYPE_SPAPR_DR_CONNECTOR "spapr-dr-connector"
-typedef struct SpaprDrc SpaprDrc;
-typedef struct SpaprDrcClass SpaprDrcClass;
-DECLARE_OBJ_CHECKERS(SpaprDrc, SpaprDrcClass,
- SPAPR_DR_CONNECTOR, TYPE_SPAPR_DR_CONNECTOR)
+OBJECT_DECLARE_TYPE(SpaprDrc, SpaprDrcClass, SPAPR_DR_CONNECTOR)
 
 #define TYPE_SPAPR_DRC_PHYSICAL "spapr-drc-physical"
 typedef struct SpaprDrcPhysical SpaprDrcPhysical;
diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
index 333095c3fd9..9e7c46c801f 100644
--- a/include/hw/ppc/spapr_xive.h
+++ b/include/hw/ppc/spapr_xive.h
@@ -15,10 +15,7 @@
 #include "qom/object.h"
 
 #define TYPE_SPAPR_XIVE "spapr-xive"
-typedef struct SpaprXive SpaprXive;
-typedef struct SpaprXiveClass SpaprXiveClass;
-DECLARE_OBJ_CHECKERS(SpaprXive, SpaprXiveClass,
- SPAPR_XIVE, TYPE_SPAPR_XIVE)
+OBJECT_DECLARE_TYPE(SpaprXive, SpaprXiveClass, SPAPR_XIVE)
 
 struct SpaprXive {
 XiveRouterparent;
diff --git a/include/qemu/accel.h b/include/qemu/accel.h
index 8dc4ccc39ef..1b7696f9fbc 100644
--- a/include/qemu/accel.h
+++ b/include/qemu/accel.h
@@ -54,15 +54,13 @@ struct AccelClass {
  */
 GPtrArray *compat_props;
 };
-typedef struct AccelClass AccelClass;
 
 #define TYPE_ACCEL_BASE "accel"
 
 #define ACCEL_CLASS_SUFFIX  "-" TYPE_ACCEL_BASE
 #define ACCEL_CLASS_NAME(a) (a ACCEL_CLASS_SUFFIX)
 
-DECLARE_OBJ_CHECKERS(AccelState, AccelClass,
- ACCEL_BASE, TYPE_ACCEL_BASE)
+OBJECT_DECLARE_TYPE(AccelState, AccelClass, ACCEL_BASE)
 
 AccelClass *accel_find(const char *opt_name);
 AccelState *current_accel(void);
diff --git a/hw/block/fdc-sysbus.c b/hw/block/fdc-sysbus.c
index 57fc8773f12..ecce27db34e 100644
--- a/hw/block/fdc-sysbus.c
+++ b/hw/block/fdc-sysbus.c
@@ -33,10 +33,7 @@
 #include "trace.h"
 
 #define TYPE_SYSBUS_FDC "base-sysbus-fdc"
-typedef struct FDCtrlSysBusClass FDCtrlSysBusClass;
-typedef struct FDCtrlSysBus FDCtrlSysBus;
-DECLARE_OBJ_CHECKERS(FDCtrlSysBus, FDCtrlSysBusClass,
- SYSBUS_FDC, TYPE_SYSBUS_FDC)
+OBJECT_DECLARE_TYPE(FDCtrlSysBus, FDCtrlSysBusClass, SYSBUS_FDC)
 
 struct FDCtrlSysBusClass {
 /*< private >*/
-- 
2.31.1

[PATCH for-6.2 12/12] [automated] Use OBJECT_DECLARE_SIMPLE_TYPE when possible

[Eduardo Habkost]

Replace typedef + DECLARE_INSTANCE_CHECKER with
equivalent OBJECT_DECLARE_SIMPLE_TYPE macro.

Generated using:

$ ./scripts/codeconverter/converter.py -i \
  --pattern=AddObjectDeclareSimpleType $(git grep -l '' -- '*.[ch]')

Signed-off-by: Eduardo Habkost 
---
Cc: Thomas Huth 
Cc: Paul Burton 
Cc: Aleksandar Rikalo 
Cc: "Philippe Mathieu-Daudé" 
Cc: Aurelien Jarno 
Cc: Jiaxun Yang 
Cc: Havard Skinnemoen 
Cc: Tyrone Ting 
Cc: Pavel Pisa 
Cc: Vikram Garhwal 
Cc: Jason Wang 
Cc: Keith Busch 
Cc: Klaus Jensen 
Cc: "Michael S. Tsirkin" 
Cc: Richard Henderson 
Cc: David Hildenbrand 
Cc: Cornelia Huck 
Cc: Halil Pasic 
Cc: Christian Borntraeger 
Cc: Gerd Hoffmann 
Cc: Vijai Kumar K 
Cc: Alistair Francis 
Cc: Bin Meng 
Cc: Palmer Dabbelt 
Cc: "Edgar E. Iglesias" 
Cc: Peter Maydell 
Cc: Laurent Vivier 
Cc: "Cédric Le Goater" 
Cc: Andrew Jeffery 
Cc: Joel Stanley 
Cc: Andrew Baumann 
Cc: Francisco Iglesias 
Cc: David Gibson 
Cc: Greg Kurz 
Cc: Bastian Koppelmann 
Cc: qemu-de...@nongnu.org
Cc: qemu-...@nongnu.org
Cc: qemu-block@nongnu.org
Cc: qemu-s3...@nongnu.org
Cc: qemu-ri...@nongnu.org
Cc: qemu-...@nongnu.org
---
 hw/nvme/nvme.h  | 10 +++---
 hw/usb/hcd-xhci-pci.h   |  4 +---
 hw/usb/hcd-xhci-sysbus.h|  4 +---
 include/hw/adc/npcm7xx_adc.h|  4 +---
 include/hw/char/shakti_uart.h   |  4 +---
 include/hw/dma/sifive_pdma.h|  4 +---
 include/hw/dma/xlnx_csu_dma.h   |  4 +---
 include/hw/gpio/sifive_gpio.h   |  4 +---
 include/hw/intc/m68k_irqc.h |  4 +---
 include/hw/intc/sifive_clint.h  |  4 +---
 include/hw/intc/sifive_plic.h   |  4 +---
 include/hw/misc/aspeed_lpc.h|  4 +---
 include/hw/misc/bcm2835_cprman_internals.h  | 12 
 include/hw/misc/led.h   |  3 +--
 include/hw/misc/mchp_pfsoc_dmc.h|  8 ++--
 include/hw/misc/mchp_pfsoc_ioscb.h  |  4 +---
 include/hw/misc/mchp_pfsoc_sysreg.h |  4 +---
 include/hw/misc/npcm7xx_clk.h   |  3 +--
 include/hw/misc/npcm7xx_gcr.h   |  4 +---
 include/hw/misc/npcm7xx_mft.h   |  4 +---
 include/hw/misc/npcm7xx_pwm.h   |  3 +--
 include/hw/misc/sifive_e_prci.h |  4 +---
 include/hw/misc/sifive_test.h   |  4 +---
 include/hw/misc/sifive_u_otp.h  |  4 +---
 include/hw/misc/sifive_u_prci.h |  4 +---
 include/hw/misc/xlnx-versal-xramc.h |  4 +---
 include/hw/net/npcm7xx_emc.h|  4 +---
 include/hw/net/xlnx-zynqmp-can.h|  4 +---
 include/hw/ppc/spapr_drc.h  |  4 +---
 include/hw/register.h   |  3 +--
 include/hw/riscv/microchip_pfsoc.h  |  4 +---
 include/hw/riscv/shakti_c.h |  8 ++--
 include/hw/riscv/sifive_e.h |  4 +---
 include/hw/riscv/sifive_u.h |  4 +---
 include/hw/sd/cadence_sdhci.h   |  4 +---
 include/hw/ssi/sifive_spi.h |  4 +---
 include/hw/timer/npcm7xx_timer.h|  3 +--
 include/hw/tricore/tricore_testdevice.h |  4 +---
 include/hw/usb/hcd-dwc3.h   |  4 +---
 include/hw/usb/xlnx-versal-usb2-ctrl-regs.h |  4 +---
 hw/m68k/mcf5206.c   |  4 +---
 hw/mips/boston.c|  4 +---
 hw/misc/npcm7xx_clk.c   |  9 +++--
 hw/net/can/ctucan_pci.c |  4 +---
 hw/s390x/vhost-user-fs-ccw.c|  4 +---
 hw/sensor/adm1272.c |  4 +---
 hw/sensor/max34451.c|  4 +---
 47 files changed, 56 insertions(+), 154 deletions(-)

diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index c4c43da5c17..a9ee5c4f1de 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -42,9 +42,7 @@ typedef struct NvmeBus {
 } NvmeBus;
 
 #define TYPE_NVME_SUBSYS "nvme-subsys"
-typedef struct NvmeSubsystem NvmeSubsystem;
-DECLARE_INSTANCE_CHECKER(NvmeSubsystem, NVME_SUBSYS,
- TYPE_NVME_SUBSYS)
+OBJECT_DECLARE_SIMPLE_TYPE(NvmeSubsystem, NVME_SUBSYS)
 
 struct NvmeSubsystem {
 DeviceState parent_obj;
@@ -83,8 +81,7 @@ static inline NvmeNamespace *nvme_subsys_ns(NvmeSubsystem 
*subsys,
 }
 
 #define TYPE_NVME_NS "nvme-ns"
-DECLARE_INSTANCE_CHECKER(NvmeNamespace, NVME_NS,
- TYPE_NVME_NS)
+OBJECT_DECLARE_SIMPLE_TYPE(NvmeNamespace, NVME_NS)
 
 typedef struct NvmeZone {
 NvmeZoneDescr   d;
@@ -377,8 +374,7 @@ typedef struct NvmeCQueue {
 } NvmeCQueue;
 
 #define TYPE_NVME "nvme"
-DECLARE_INSTANCE_CHECKER(NvmeCtrl, NVME,
- TYPE_NVME)
+OBJECT_DECLARE_SIMPLE_TYPE(NvmeCtrl, NVME)
 
 typedef struct NvmeParams {
 char *serial;
diff --git a/hw/usb/hcd-xhci-pci.h b/hw/usb/hcd-xhci-pci.h
index d83aad82e04..2ad2eeefb7f 100644
--- a/hw/usb/hcd-xhci-pci.h
+++ b/hw/usb/hcd-xhci-pci