From 2096903b45d28594c04b47b592a5b7e62b5e1ccc Mon Sep 17 00:00:00 2001
From: Ben Pfaff <blp@nicira.com>
Date: Mon, 20 Jun 2011 16:17:44 -0700
Subject: [PATCH] ovsdb-idl: Plug hole in state machine.

The state machine didn't have a proper state for "not yet committed or
aborted", which meant that destroying an ovsdb_idl_txn without committing
or aborting it caused a segfault.  This fixes the problem by adding a new
state TXN_UNCOMMITTED to the state machine.

This is related to commit 79554078d "ovsdb-idl: Fix bad logic in
ovsdb_idl_txn_commit() state transitions", which fixed a related bug.

Bug #2438.
---
 lib/ovsdb-idl.c       |  9 ++++++---
 lib/ovsdb-idl.h       |  1 +
 tests/ovsdb-idl.at    | 28 ++++++++++++++++++++++++++++
 tests/test-ovsdb.c    |  7 +++++++
 utilities/ovs-vsctl.c |  1 +
 5 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index f4daaed16..51a62dd5f 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -1127,6 +1127,8 @@ const char *
 ovsdb_idl_txn_status_to_string(enum ovsdb_idl_txn_status status)
 {
     switch (status) {
+    case TXN_UNCOMMITTED:
+        return "uncommitted";
     case TXN_UNCHANGED:
         return "unchanged";
     case TXN_INCOMPLETE:
@@ -1153,7 +1155,7 @@ ovsdb_idl_txn_create(struct ovsdb_idl *idl)
     txn->request_id = NULL;
     txn->idl = idl;
     hmap_init(&txn->txn_rows);
-    txn->status = TXN_INCOMPLETE;
+    txn->status = TXN_UNCOMMITTED;
     txn->error = NULL;
     txn->dry_run = false;
     ds_init(&txn->comment);
@@ -1226,7 +1228,7 @@ ovsdb_idl_txn_destroy(struct ovsdb_idl_txn *txn)
 void
 ovsdb_idl_txn_wait(const struct ovsdb_idl_txn *txn)
 {
-    if (txn->status != TXN_INCOMPLETE) {
+    if (txn->status != TXN_UNCOMMITTED && txn->status != TXN_INCOMPLETE) {
         poll_immediate_wake();
     }
 }
@@ -1532,6 +1534,7 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
                        "transact", operations, &txn->request_id))) {
         hmap_insert(&txn->idl->outstanding_txns, &txn->hmap_node,
                     json_hash(txn->request_id, 0));
+        txn->status = TXN_INCOMPLETE;
     } else {
         txn->status = TXN_TRY_AGAIN;
     }
@@ -1569,7 +1572,7 @@ void
 ovsdb_idl_txn_abort(struct ovsdb_idl_txn *txn)
 {
     ovsdb_idl_txn_disassemble(txn);
-    if (txn->status == TXN_INCOMPLETE) {
+    if (txn->status == TXN_UNCOMMITTED || txn->status == TXN_INCOMPLETE) {
         txn->status = TXN_ABORTED;
     }
 }
diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
index d11fb0e02..ac61e1abf 100644
--- a/lib/ovsdb-idl.h
+++ b/lib/ovsdb-idl.h
@@ -111,6 +111,7 @@ bool ovsdb_idl_row_is_synthetic(const struct ovsdb_idl_row *);
 /* Transactions. */
 
 enum ovsdb_idl_txn_status {
+    TXN_UNCOMMITTED,            /* Not yet committed or aborted. */
     TXN_UNCHANGED,              /* Transaction didn't include any changes. */
     TXN_INCOMPLETE,             /* Commit in progress, please wait. */
     TXN_ABORTED,                /* ovsdb_idl_txn_abort() called. */
diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index 5956f72db..f9c8286f7 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -221,6 +221,34 @@ OVSDB_CHECK_IDL([simple idl, increment operation],
 003: done
 ]])
 
+OVSDB_CHECK_IDL([simple idl, aborting],
+  [['["idltest",
+      {"op": "insert",
+       "table": "simple",
+       "row": {}}]']],
+  [['set 0 r 2.0, abort' \
+'+set 0 b 1']],
+  [[000: i=0 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1>
+001: commit, status=aborted
+002: commit, status=success
+003: i=0 r=0 b=true s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1>
+004: done
+]])
+
+OVSDB_CHECK_IDL([simple idl, destroy without commit or abort],
+  [['["idltest",
+      {"op": "insert",
+       "table": "simple",
+       "row": {}}]']],
+  [['set 0 r 2.0, destroy' \
+'+set 0 b 1']],
+  [[000: i=0 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1>
+001: destroy
+002: commit, status=success
+003: i=0 r=0 b=true s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1>
+004: done
+]])
+
 OVSDB_CHECK_IDL([self-linking idl, consistent ops],
   [],
   [['["idltest",
diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index 5594b40f0..e8d87b6c5 100644
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -1828,6 +1828,13 @@ idl_set(struct ovsdb_idl *idl, char *commands, int step)
             }
             ovsdb_idl_txn_increment(txn, arg1, arg2, NULL);
             increment = true;
+        } else if (!strcmp(name, "abort")) {
+            ovsdb_idl_txn_abort(txn);
+            break;
+        } else if (!strcmp(name, "destroy")) {
+            printf("%03d: destroy\n", step);
+            ovsdb_idl_txn_destroy(txn);
+            return;
         } else {
             ovs_fatal(0, "unknown command %s", name);
         }
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index df20f063f..c6fc8a47e 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -3642,6 +3642,7 @@ do_vsctl(const char *args, struct vsctl_command *commands, size_t n_commands,
     txn = the_idl_txn = NULL;
 
     switch (status) {
+    case TXN_UNCOMMITTED:
     case TXN_INCOMPLETE:
         NOT_REACHED();
 
-- 
2.43.0