[ovs-dev] [PATCH] ovsdb-idl: Fix bad logic in ovsdb_idl_txn_commit() state transitions.

Justin Pettit jpettit at nicira.com
Fri Feb 26 21:17:33 PST 2010


Looks good to me.  Thanks for the quick turnaround!

--Justin


On Feb 26, 2010, at 8:36 PM, Ben Pfaff wrote:

> If sending the transaction fails (jsonrpc_session_send() returns 0),
> then we need to transition to TXN_TRY_AGAIN.  (Transitioning to
> TXN_INCOMPLETE is actually a no-op, because at this point in the code
> we are guaranteed to be in that state already.)
> 
> Leaving the transaction in TXN_INCOMPLETE causes a segfault later in
> ovsdb_idl_txn_destroy() when it calls hmap_remove() on the transaction's
> txn_node.
> 
> This bug reveals a hole in the ovsdb_idl_txn state machine: destroying
> a transaction without committing it or aborting it will cause the same
> problem.  This problem is *not* fixed by this patch: it really should be
> handled by adding a new state TXN_UNCOMMITTED that indicates that the
> transaction is not yet committed or aborted.  That's too much for this
> patch, and doesn't really matter for OVS at the moment since none of its
> code paths destroy a transaction without committing or aborting it.
> 
> Bug #2435.
> ---
> lib/ovsdb-idl.c |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
> index bca8224..4826d62 100644
> --- a/lib/ovsdb-idl.c
> +++ b/lib/ovsdb-idl.c
> @@ -1226,7 +1226,7 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
>         hmap_insert(&txn->idl->outstanding_txns, &txn->hmap_node,
>                     json_hash(txn->request_id, 0));
>     } else {
> -        txn->status = TXN_INCOMPLETE;
> +        txn->status = TXN_TRY_AGAIN;
>     }
> 
>     ovsdb_idl_txn_disassemble(txn);
> -- 
> 1.6.6.1
> 
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev_openvswitch.org





More information about the dev mailing list