Re: [Qemu-devel] [PATCH v4 09/14] block: add block job transactions

[Fam Zheng <famz@redhat.com>]

On Mon, 08/03 20:06, Max Reitz wrote:
> 
> s/who/which/

OK.

> 
> >+         */
> >+        return;
> >+    }
> >+    txn->aborting = true;
> >+    /* We are the first failed job. Cancel other jobs. */
> >+    QLIST_FOREACH(other_job, &txn->jobs, txn_list) {
> >+        ctx = bdrv_get_aio_context(other_job->bs);
> >+        aio_context_acquire(ctx);
> >+    }
> >+    QLIST_FOREACH(other_job, &txn->jobs, txn_list) {
> >+        if (other_job == job || other_job->completed) {
> >+            /* Other are "effectively" cancelled by us, so set the status for
> 
> s/Other/Other jobs/?

OK.

> 
> >+             * them; this job, however, may or may not be cancelled, depending
> >+             * on the caller, so leave it. */
> >+            other_job->cancelled = other_job != job;
> 
> If other_job == job, this is not left as it was, but set to false.

Right, I'll use an if block.

> >+/**
> >+ * block_job_txn_add_job:
> >+ * @txn: The transaction (may be NULL)
> >+ * @job: Job to add to the transaction
> >+ *
> >+ * Add @job to the transaction.  The @job must not already be in a transaction.
> >+ * The block job driver must call block_job_txn_prepare_to_complete() before
> >+ * final cleanup and completion.
> 
> I see this function mentioned only here, only in this patch. That doesn't
> seem right to me.

It's stale. Will fix.

Thanks!

Fam