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

[Ben Pfaff]

You are welcome.  I pushed this out.

On Fri, Feb 26, 2010 at 09:17:33PM -0800, Justin Pettit wrote:
> 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
>