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 <stefa...@redhat.com>
---
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