[Qemu-devel] [PATCH v4 04/11] block: re-add BlkTransactionState

[John Snow]

Now that the structure formerly known as BlkTransactionState has been
renamed to something sensible (BlkActionState), re-introduce an actual
BlkTransactionState that actually manages state for the entire Transaction.

In the process, convert the old QSIMPLEQ list of actions into a QTAILQ,
to let us more efficiently delete items in arbitrary order, which will
be more important in the future when some actions will expire at the end
of the transaction, but others may persist until all callbacks triggered
by the transaction are recollected.

Signed-off-by: John Snow <address@hidden>
Reviewed-by: Max Reitz <address@hidden>
---
 blockdev.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 59 insertions(+), 7 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 6df575d..068eccb 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1253,6 +1253,19 @@ typedef struct BlkActionOps {
 } BlkActionOps;
 
 /**
+ * BlkTransactionState:
+ * Object to track the job completion info for jobs launched
+ * by a transaction group.
+ *
+ * @jobs: A reference count that tracks how many jobs still need to complete.
+ * @actions: A list of all Actions in the Transaction.
+ */
+typedef struct BlkTransactionState {
+    int jobs;
+    QTAILQ_HEAD(actions, BlkActionState) actions;
+} BlkTransactionState;
+
+/**
  * BlkActionState:
  * Describes one Action's state within a Transaction.
  *
@@ -1267,9 +1280,45 @@ typedef struct BlkActionOps {
 struct BlkActionState {
     TransactionAction *action;
     const BlkActionOps *ops;
-    QSIMPLEQ_ENTRY(BlkActionState) entry;
+    QTAILQ_ENTRY(BlkActionState) entry;
 };
 
+static BlkTransactionState *new_blk_transaction_state(void)
+{
+    BlkTransactionState *bts = g_new0(BlkTransactionState, 1);
+
+    /* The qmp_transaction function itself can be considered a pending job
+     * that should complete before pending action callbacks are executed,
+     * so increment the jobs remaining refcount to indicate this. */
+    bts->jobs = 1;
+    QTAILQ_INIT(&bts->actions);
+    return bts;
+}
+
+static void destroy_blk_transaction_state(BlkTransactionState *bts)
+{
+    BlkActionState *bas, *bas_next;
+
+    /* The list should in normal cases be empty,
+     * but in case someone really just wants to kibosh the whole deal: */
+    QTAILQ_FOREACH_SAFE(bas, &bts->actions, entry, bas_next) {
+        QTAILQ_REMOVE(&bts->actions, bas, entry);
+        g_free(bas);
+    }
+
+    g_free(bts);
+}
+
+static BlkTransactionState *transaction_job_complete(BlkTransactionState *bts)
+{
+    bts->jobs--;
+    if (bts->jobs == 0) {
+        destroy_blk_transaction_state(bts);
+        return NULL;
+    }
+    return bts;
+}
+
 /* internal snapshot private data */
 typedef struct InternalSnapshotState {
     BlkActionState common;
@@ -1870,10 +1919,10 @@ void qmp_transaction(TransactionActionList *dev_list, 
Error **errp)
 {
     TransactionActionList *dev_entry = dev_list;
     BlkActionState *state, *next;
+    BlkTransactionState *bts;
     Error *local_err = NULL;
 
-    QSIMPLEQ_HEAD(snap_bdrv_states, BlkActionState) snap_bdrv_states;
-    QSIMPLEQ_INIT(&snap_bdrv_states);
+    bts = new_blk_transaction_state();
 
     /* drain all i/o before any operations */
     bdrv_drain_all();
@@ -1894,7 +1943,7 @@ void qmp_transaction(TransactionActionList *dev_list, 
Error **errp)
         state = g_malloc0(ops->instance_size);
         state->ops = ops;
         state->action = dev_info;
-        QSIMPLEQ_INSERT_TAIL(&snap_bdrv_states, state, entry);
+        QTAILQ_INSERT_TAIL(&bts->actions, state, entry);
 
         state->ops->prepare(state, &local_err);
         if (local_err) {
@@ -1903,7 +1952,7 @@ void qmp_transaction(TransactionActionList *dev_list, 
Error **errp)
         }
     }
 
-    QSIMPLEQ_FOREACH(state, &snap_bdrv_states, entry) {
+    QTAILQ_FOREACH(state, &bts->actions, entry) {
         if (state->ops->commit) {
             state->ops->commit(state);
         }
@@ -1914,18 +1963,21 @@ void qmp_transaction(TransactionActionList *dev_list, 
Error **errp)
 
 delete_and_fail:
     /* failure, and it is all-or-none; roll back all operations */
-    QSIMPLEQ_FOREACH(state, &snap_bdrv_states, entry) {
+    QTAILQ_FOREACH(state, &bts->actions, entry) {
         if (state->ops->abort) {
             state->ops->abort(state);
         }
     }
 exit:
-    QSIMPLEQ_FOREACH_SAFE(state, &snap_bdrv_states, entry, next) {
+    QTAILQ_FOREACH_SAFE(state, &bts->actions, entry, next) {
         if (state->ops->clean) {
             state->ops->clean(state);
         }
+        QTAILQ_REMOVE(&bts->actions, state, entry);
         g_free(state);
     }
+
+    transaction_job_complete(bts);
 }
 
 
-- 
2.1.0