qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC 5/9] block: add block job transactions


From: Max Reitz
Subject: Re: [Qemu-devel] [RFC 5/9] block: add block job transactions
Date: Wed, 24 Jun 2015 20:37:43 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0

On 12.06.2015 12:09, Stefan Hajnoczi wrote:
Sometimes block jobs must execute as a transaction group.  Finishing
jobs wait until all other jobs are ready to complete successfully.
Failure or cancellation of one job cancels the other jobs in the group.

Signed-off-by: Stefan Hajnoczi <address@hidden>
---
  blockjob.c                | 160 ++++++++++++++++++++++++++++++++++++++++++++++
  include/block/block.h     |   1 +
  include/block/block_int.h |   3 +-
  include/block/blockjob.h  |  49 ++++++++++++++
  trace-events              |   4 ++
  5 files changed, 216 insertions(+), 1 deletion(-)

diff --git a/blockjob.c b/blockjob.c
index 2755465..ff622f5 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -399,3 +399,163 @@ void block_job_defer_to_main_loop(BlockJob *job,
qemu_bh_schedule(data->bh);
  }
+
+/* Transactional group of block jobs */
+struct BlockJobTxn {
+    /* Jobs may be in different AioContexts so protect all fields */
+    QemuMutex lock;
+
+    /* Reference count for txn object */
+    unsigned int ref;
+
+    /* Is this txn cancelling its jobs? */
+    bool aborting;
+
+    /* Number of jobs still running */
+    unsigned int jobs_pending;
+
+    /* List of jobs */
+    QLIST_HEAD(, BlockJob) jobs;
+};
+
+BlockJobTxn *block_job_txn_new(void)
+{
+    BlockJobTxn *txn = g_new(BlockJobTxn, 1);
+    qemu_mutex_init(&txn->lock);
+    txn->ref = 1; /* dropped by block_job_txn_begin() */
+    txn->aborting = false;
+    txn->jobs_pending = 0;
+    QLIST_INIT(&txn->jobs);
+    return txn;
+}
+
+static void block_job_txn_unref(BlockJobTxn *txn)
+{
+    qemu_mutex_lock(&txn->lock);
+
+    if (--txn->ref > 0) {
+        qemu_mutex_unlock(&txn->lock);
+        return;
+    }
+
+    qemu_mutex_unlock(&txn->lock);
+    qemu_mutex_destroy(&txn->lock);
+    g_free(txn);
+}
+
+/* The purpose of this is to keep txn alive until all jobs have been added */
+void block_job_txn_begin(BlockJobTxn *txn)
+{
+    block_job_txn_unref(txn);
+}
+
+void block_job_txn_add_job(BlockJobTxn *txn, BlockJob *job)
+{
+    if (!txn) {
+        return;
+    }

Do you plan on making use of this case? I'm asking because while I'm usually in favor of handling everything gracefully as long as it's easy to implement, here I can't think of a case where using NULL with this function makes sense, that is to me it would seem like the caller made some bad mistake. This in turn would mean that dereferencing a NULL pointer or failing an assertion were preferable to hiding that mistake.

Other than this small thing and that it doesn't compile (until patch 7, I presume), looks good.

Max



reply via email to

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