Thanks, I fixed that typo in two places and I'll push this series soon.
On Wed, Apr 11, 2012 at 11:33:24AM -0700, Ethan Jackson wrote:
> Yay! We've needed this for a while. Looks good.
>
> s/columsn/columns
>
> Ethan
>
> On Tue, Mar 27, 2012 at 17:00, Ben Pfaff <b...@nicira.com> wrote:
> > Signed-off-by: Ben Pfaff <b...@nicira.com>
> > ---
> > lib/ovsdb-idl.c | 126
> > ++++++++++++++++++++++++++++++++++++++++++++++++--
> > lib/ovsdb-idl.h | 51 ++++++++++++++++++++-
> > python/ovs/db/idl.py | 116 ++++++++++++++++++++++++++++++++++++++++++---
> > 3 files changed, 280 insertions(+), 13 deletions(-)
> >
> > diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
> > index 5019c43..81f1548 100644
> > --- a/lib/ovsdb-idl.c
> > +++ b/lib/ovsdb-idl.c
> > @@ -357,8 +357,23 @@ ovsdb_idl_wait(struct ovsdb_idl *idl)
> > jsonrpc_session_recv_wait(idl->session);
> > }
> >
> > -/* Returns a number that represents the state of 'idl'. When 'idl' is
> > updated
> > - * (by ovsdb_idl_run()), the return value changes. */
> > +/* Returns a "sequence number" that represents the state of 'idl'. When
> > + * ovsdb_idl_run() changes the database, the sequence number changes. The
> > + * initial fetch of the entire contents of the remote database is
> > considered to
> > + * be one kind of change. Successfully acquiring a lock, if one has been
> > + * configured with ovsdb_idl_set_lock(), is also considered to be a change.
> > + *
> > + * As long as the sequence number does not change, the client may continue
> > to
> > + * use any data structures it obtains from 'idl'. But when it changes, the
> > + * client must not access any of these data structures again, because they
> > + * could have freed or reused for other purposes.
> > + *
> > + * The sequence number can occasionally change even if the database does
> > not.
> > + * This happens if the connection to the database drops and reconnects,
> > which
> > + * causes the database contents to be reloaded even if they didn't change.
> > (It
> > + * could also happen if the database server sends out a "change" that
> > reflects
> > + * what the IDL already thought was in the database. The database server
> > is
> > + * not supposed to do that, but bugs could in theory cause it to do so.) */
> > unsigned int
> > ovsdb_idl_get_seqno(const struct ovsdb_idl *idl)
> > {
> > @@ -992,6 +1007,7 @@ ovsdb_idl_table_from_class(const struct ovsdb_idl *idl,
> > return &idl->tables[table_class - idl->class->tables];
> > }
> >
> > +/* Called by ovsdb-idlc generated code. */
> > struct ovsdb_idl_row *
> > ovsdb_idl_get_row_arc(struct ovsdb_idl_row *src,
> > struct ovsdb_idl_table_class *dst_table_class,
> > @@ -1036,6 +1052,8 @@ ovsdb_idl_get_row_arc(struct ovsdb_idl_row *src,
> > }
> > }
> >
> > +/* Searches 'tc''s table in 'idl' for a row with UUID 'uuid'. Returns a
> > + * pointer to the row if there is one, otherwise a null pointer. */
> > const struct ovsdb_idl_row *
> > ovsdb_idl_get_row_for_uuid(const struct ovsdb_idl *idl,
> > const struct ovsdb_idl_table_class *tc,
> > @@ -1058,6 +1076,12 @@ next_real_row(struct ovsdb_idl_table *table, struct
> > hmap_node *node)
> > return NULL;
> > }
> >
> > +/* Returns a row in 'table_class''s table in 'idl', or a null pointer if
> > that
> > + * table is empty.
> > + *
> > + * Database tables are internally maintained as hash tables, so adding or
> > + * removing rows while traversing the same table can cause some rows to be
> > + * visited twice or not at apply. */
> > const struct ovsdb_idl_row *
> > ovsdb_idl_first_row(const struct ovsdb_idl *idl,
> > const struct ovsdb_idl_table_class *table_class)
> > @@ -1067,6 +1091,8 @@ ovsdb_idl_first_row(const struct ovsdb_idl *idl,
> > return next_real_row(table, hmap_first(&table->rows));
> > }
> >
> > +/* Returns a row following 'row' within its table, or a null pointer if
> > 'row'
> > + * is the last row in its table. */
> > const struct ovsdb_idl_row *
> > ovsdb_idl_next_row(const struct ovsdb_idl_row *row)
> > {
> > @@ -1143,6 +1169,11 @@ ovsdb_idl_row_is_synthetic(const struct
> > ovsdb_idl_row *row)
> > static void ovsdb_idl_txn_complete(struct ovsdb_idl_txn *txn,
> > enum ovsdb_idl_txn_status);
> >
> > +/* Returns a string representation of 'status'. The caller must not
> > modify or
> > + * free the returned string.
> > + *
> > + * The return value is probably useful only for debug log messages and unit
> > + * tests. */
> > const char *
> > ovsdb_idl_txn_status_to_string(enum ovsdb_idl_txn_status status)
> > {
> > @@ -1167,6 +1198,9 @@ ovsdb_idl_txn_status_to_string(enum
> > ovsdb_idl_txn_status status)
> > return "<unknown>";
> > }
> >
> > +/* Starts a new transaction on 'idl'. A given ovsdb_idl may only have a
> > single
> > + * active transaction at a time. See the large comment in ovsdb-idl.h for
> > + * general information on transactions. */
> > struct ovsdb_idl_txn *
> > ovsdb_idl_txn_create(struct ovsdb_idl *idl)
> > {
> > @@ -1185,7 +1219,6 @@ ovsdb_idl_txn_create(struct ovsdb_idl *idl)
> >
> > txn->inc_table = NULL;
> > txn->inc_column = NULL;
> > - txn->inc_where = NULL;
> >
> > hmap_init(&txn->inserted_rows);
> >
> > @@ -1210,6 +1243,13 @@ ovsdb_idl_txn_add_comment(struct ovsdb_idl_txn *txn,
> > const char *s, ...)
> > va_end(args);
> > }
> >
> > +/* Marks 'txn' as a transaction that will not actually modify the
> > database. In
> > + * almost every way, the transaction is treated like other transactions.
> > It
> > + * must be committed or aborted like other transactions, it will be sent
> > to the
> > + * database server like other transactions, and so on. The only
> > difference is
> > + * that the operations sent to the database server will include, as the
> > last
> > + * step, an "abort" operation, so that any changes made by the transaction
> > will
> > + * not actually take effect. */
> > void
> > ovsdb_idl_txn_set_dry_run(struct ovsdb_idl_txn *txn)
> > {
> > @@ -1242,6 +1282,11 @@ ovsdb_idl_txn_increment(struct ovsdb_idl_txn *txn,
> > txn->inc_row = row->uuid;
> > }
> >
> > +/* Destroys 'txn' and frees all associated memory. If
> > ovsdb_idl_txn_commit()
> > + * has been called for 'txn' but the commit is still incomplete (that is,
> > the
> > + * last call returned TXN_INCOMPLETE) then the transaction may or may not
> > still
> > + * end up committing at the database server, but the client will not be
> > able to
> > + * get any further status information back. */
> > void
> > ovsdb_idl_txn_destroy(struct ovsdb_idl_txn *txn)
> > {
> > @@ -1261,6 +1306,7 @@ ovsdb_idl_txn_destroy(struct ovsdb_idl_txn *txn)
> > free(txn);
> > }
> >
> > +/* Causes poll_block() to wake up if 'txn' has completed committing. */
> > void
> > ovsdb_idl_txn_wait(const struct ovsdb_idl_txn *txn)
> > {
> > @@ -1391,6 +1437,55 @@ ovsdb_idl_txn_disassemble(struct ovsdb_idl_txn *txn)
> > hmap_init(&txn->txn_rows);
> > }
> >
> > +/* Attempts to commit 'txn'. Returns the status of the commit operation,
> > one
> > + * of the following TXN_* constants:
> > + *
> > + * TXN_INCOMPLETE:
> > + *
> > + * The transaction is in progress, but not yet complete. The caller
> > + * should call again later, after calling ovsdb_idl_run() to let the
> > IDL
> > + * do OVSDB protocol processing.
> > + *
> > + * TXN_UNCHANGED:
> > + *
> > + * The transaction is complete. (It didn't actually change the
> > database,
> > + * so the IDL didn't send any request to the database server.)
> > + *
> > + * TXN_ABORTED:
> > + *
> > + * The caller previously called ovsdb_idl_txn_abort().
> > + *
> > + * TXN_SUCCESS:
> > + *
> > + * The transaction was successful. The update made by the
> > transaction
> > + * (and possibly other changes made by other database clients) should
> > + * already be visible in the IDL.
> > + *
> > + * TXN_TRY_AGAIN:
> > + *
> > + * The transaction failed for some transient reason, e.g. because a
> > + * "verify" operation reported an inconsistency or due to a network
> > + * problem. The caller should wait for a change to the database,
> > then
> > + * compose a new transaction, and commit the new transaction.
> > + *
> > + * Use the return value of ovsdb_idl_get_seqno() to wait for a
> > change in
> > + * the database. It is important to use its return value *before*
> > the
> > + * initial call to ovsdb_idl_txn_commit() as the baseline for this
> > + * purpose, because the change that one should wait for can happen
> > after
> > + * the initial call but before the call that returns TXN_TRY_AGAIN,
> > and
> > + * using some other baseline value in that situation could cause an
> > + * indefinite wait if the database rarely changes.
> > + *
> > + * TXN_NOT_LOCKED:
> > + *
> > + * The transaction failed because the IDL has been configured to
> > require
> > + * a database lock (with ovsdb_idl_set_lock()) but didn't get it yet
> > or
> > + * has already lost it.
> > + *
> > + * Committing a transaction rolls back all of the changes that it made to
> > the
> > + * IDL's copy of the database. If the transaction commits successfully,
> > then
> > + * the database server will send an update and, thus, the IDL will be
> > updated
> > + * with the committed changes. */
> > enum ovsdb_idl_txn_status
> > ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
> > {
> > @@ -1596,7 +1691,10 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
> >
> > /* Attempts to commit 'txn', blocking until the commit either succeeds or
> > * fails. Returns the final commit status, which may be any TXN_* value
> > other
> > - * than TXN_INCOMPLETE. */
> > + * than TXN_INCOMPLETE.
> > + *
> > + * This function calls ovsdb_idl_run() on 'txn''s IDL, so it may cause the
> > + * return value of ovsdb_idl_get_seqno() to change. */
> > enum ovsdb_idl_txn_status
> > ovsdb_idl_txn_commit_block(struct ovsdb_idl_txn *txn)
> > {
> > @@ -1612,6 +1710,9 @@ ovsdb_idl_txn_commit_block(struct ovsdb_idl_txn *txn)
> > return status;
> > }
> >
> > +/* Returns the final (incremented) value of the column in 'txn' that was
> > set to
> > + * be incremented by ovsdb_idl_txn_increment(). 'txn' must have committed
> > + * successfully. */
> > int64_t
> > ovsdb_idl_txn_get_increment_new_value(const struct ovsdb_idl_txn *txn)
> > {
> > @@ -1619,6 +1720,12 @@ ovsdb_idl_txn_get_increment_new_value(const struct
> > ovsdb_idl_txn *txn)
> > return txn->inc_new_value;
> > }
> >
> > +/* Aborts 'txn' without sending it to the database server. This is
> > effective
> > + * only if ovsdb_idl_txn_commit() has not yet been called for 'txn'.
> > + * Otherwise, it has no effect.
> > + *
> > + * Aborting a transaction doesn't free its memory. Use
> > + * ovsdb_idl_txn_destroy() to do that. */
> > void
> > ovsdb_idl_txn_abort(struct ovsdb_idl_txn *txn)
> > {
> > @@ -1628,6 +1735,14 @@ ovsdb_idl_txn_abort(struct ovsdb_idl_txn *txn)
> > }
> > }
> >
> > +/* Returns a string that reports the error status for 'txn'. The caller
> > must
> > + * not modify or free the returned string. A call to
> > ovsdb_idl_txn_destroy()
> > + * for 'txn' may free the returned string.
> > + *
> > + * The return value is ordinarily one of the strings that
> > + * ovsdb_idl_txn_status_to_string() would return, but if the transaction
> > failed
> > + * due to an error reported by the database server, the return value is
> > that
> > + * error. */
> > const char *
> > ovsdb_idl_txn_get_error(const struct ovsdb_idl_txn *txn)
> > {
> > @@ -2101,6 +2216,8 @@ ovsdb_idl_txn_process_reply(struct ovsdb_idl *idl,
> > return true;
> > }
> >
> > +/* Returns the transaction currently active for 'row''s IDL. A transaction
> > + * must currently be active. */
> > struct ovsdb_idl_txn *
> > ovsdb_idl_txn_get(const struct ovsdb_idl_row *row)
> > {
> > @@ -2109,6 +2226,7 @@ ovsdb_idl_txn_get(const struct ovsdb_idl_row *row)
> > return txn;
> > }
> >
> > +/* Returns the IDL on which 'txn' acts. */
> > struct ovsdb_idl *
> > ovsdb_idl_txn_get_idl (struct ovsdb_idl_txn *txn)
> > {
> > diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
> > index 34ccf06..b7beccc 100644
> > --- a/lib/ovsdb-idl.h
> > +++ b/lib/ovsdb-idl.h
> > @@ -112,7 +112,56 @@ const struct ovsdb_datum *ovsdb_idl_get(const struct
> > ovsdb_idl_row *,
> >
> > bool ovsdb_idl_row_is_synthetic(const struct ovsdb_idl_row *);
> >
> > -/* Transactions. */
> > +/* Transactions.
> > + *
> > + * A transaction may modify the contents of a database by modifying the
> > values
> > + * of columns, deleting rows, inserting rows, or adding checks that
> > columns in
> > + * the database have not changed ("verify" operations), through
> > + * ovsdb_idl_txn_*() functions. (The OVSDB IDL code generator produces
> > helper
> > + * functions that internally call the ovsdb_idl_txn_*() functions. These
> > are
> > + * likely to be more convenient.)
> > + *
> > + * Reading and writing columsn and inserting and deleting rows are all
> > + * straightforward. The reasons to verify columns are less obvious.
> > + * Verification is the key to maintaining transactional integrity. Because
> > + * OVSDB handles multiple clients, it can happen that between the time that
> > + * OVSDB client A reads a column and writes a new value, OVSDB client B has
> > + * written that column. Client A's write should not ordinarily overwrite
> > + * client B's, especially if the column in question is a "map" column that
> > + * contains several more or less independent data items. If client A adds
> > a
> > + * "verify" operation before it writes the column, then the transaction
> > fails
> > + * in case client B modifies it first. Client A will then see the new
> > value of
> > + * the column and compose a new transaction based on the new contents
> > written
> > + * by client B.
> > + *
> > + * When a transaction is complete, which must be before the next call to
> > + * ovsdb_idl_run() on 'idl', call ovsdb_idl_txn_commit() or
> > + * ovsdb_idl_txn_abort().
> > + *
> > + * The life-cycle of a transaction looks like this:
> > + *
> > + * 1. Create the transaction and record the initial sequence number:
> > + *
> > + * seqno = ovsdb_idl_get_seqno(idl);
> > + * txn = ovsdb_idl_txn_create(idl);
> > + *
> > + * 2. Modify the database with ovsdb_idl_txn_*() functions directly or
> > + * indirectly.
> > + *
> > + * 3. Commit the transaction by calling ovsdb_idl_txn_commit(). The first
> > call
> > + * to this function probably returns TXN_INCOMPLETE. The client must
> > keep
> > + * calling again along as this remains true, calling ovsdb_idl_run() in
> > + * between to let the IDL do protocol processing. (If the client
> > doesn't
> > + * have anything else to do in the meantime, it can use
> > + * ovsdb_idl_txn_commit_block() to avoid having to loop itself.)
> > + *
> > + * 4. If the final status is TXN_TRY_AGAIN, wait for ovsdb_idl_get_seqno()
> > to
> > + * change from the saved 'seqno' (it's possible that it's already
> > changed,
> > + * in which case the client should not wait at all), then start over
> > from
> > + * step 1. Only a call to ovsdb_idl_run() will change the return value
> > of
> > + * ovsdb_idl_get_seqno(). (ovsdb_idl_txn_commit_block() calls
> > + * ovsdb_idl_run().)
> > + */
> >
> > enum ovsdb_idl_txn_status {
> > TXN_UNCOMMITTED, /* Not yet committed or aborted. */
> > diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
> > index f36ab03..4c11499 100644
> > --- a/python/ovs/db/idl.py
> > +++ b/python/ovs/db/idl.py
> > @@ -52,7 +52,13 @@ class Idl:
> > 'rows' map values. Refer to Row for more details.
> >
> > - 'change_seqno': A number that represents the IDL's state. When the
> > IDL
> > - is updated (by Idl.run()), its value changes.
> > + is updated (by Idl.run()), its value changes. The sequence number
> > can
> > + occasionally change even if the database does not. This happens if
> > the
> > + connection to the database drops and reconnects, which causes the
> > + database contents to be reloaded even if they didn't change. (It
> > could
> > + also happen if the database server sends out a "change" that reflects
> > + what the IDL already thought was in the database. The database
> > server is
> > + not supposed to do that, but bugs could in theory cause it to do so.)
> >
> > - 'lock_name': The name of the lock configured with Idl.set_lock(), or
> > None
> > if no lock is configured.
> > @@ -635,6 +641,49 @@ class _InsertedRow(object):
> >
> >
> > class Transaction(object):
> > + """A transaction may modify the contents of a database by modifying the
> > + values of columns, deleting rows, inserting rows, or adding checks that
> > + columns in the database have not changed ("verify" operations), through
> > + Row methods.
> > +
> > + Reading and writing columsn and inserting and deleting rows are all
> > + straightforward. The reasons to verify columns are less obvious.
> > + Verification is the key to maintaining transactional integrity.
> > Because
> > + OVSDB handles multiple clients, it can happen that between the time
> > that
> > + OVSDB client A reads a column and writes a new value, OVSDB client B
> > has
> > + written that column. Client A's write should not ordinarily overwrite
> > + client B's, especially if the column in question is a "map" column that
> > + contains several more or less independent data items. If client A
> > adds a
> > + "verify" operation before it writes the column, then the transaction
> > fails
> > + in case client B modifies it first. Client A will then see the new
> > value
> > + of the column and compose a new transaction based on the new contents
> > + written by client B.
> > +
> > + When a transaction is complete, which must be before the next call to
> > + Idl.run(), call Transaction.commit() or Transaction.abort().
> > +
> > + The life-cycle of a transaction looks like this:
> > +
> > + 1. Create the transaction and record the initial sequence number:
> > +
> > + seqno = idl.change_seqno(idl)
> > + txn = Transaction(idl)
> > +
> > + 2. Modify the database with Row and Transaction methods.
> > +
> > + 3. Commit the transaction by calling Transaction.commit(). The first
> > call
> > + to this function probably returns Transaction.INCOMPLETE. The
> > client
> > + must keep calling again along as this remains true, calling
> > Idl.run() in
> > + between to let the IDL do protocol processing. (If the client
> > doesn't
> > + have anything else to do in the meantime, it can use
> > + Transaction.commit_block() to avoid having to loop itself.)
> > +
> > + 4. If the final status is Transaction.TRY_AGAIN, wait for
> > Idl.change_seqno
> > + to change from the saved 'seqno' (it's possible that it's already
> > + changed, in which case the client should not wait at all), then
> > start
> > + over from step 1. Only a call to Idl.run() will change the return
> > value
> > + of Idl.change_seqno. (Transaction.commit_block() calls
> > Idl.run().)"""
> > +
> > # Status values that Transaction.commit() can return.
> > UNCOMMITTED = "uncommitted" # Not yet committed or aborted.
> > UNCHANGED = "unchanged" # Transaction didn't include any changes.
> > @@ -694,6 +743,8 @@ class Transaction(object):
> > self._comments.append(comment)
> >
> > def wait(self, poller):
> > + """Causes poll_block() to wake up if this transaction has completed
> > + committing."""
> > if self._status not in (Transaction.UNCOMMITTED,
> > Transaction.INCOMPLETE):
> > poller.immediate_wake()
> > @@ -722,16 +773,56 @@ class Transaction(object):
> > self._txn_rows = {}
> >
> > def commit(self):
> > - """Attempts to commit this transaction and returns the status of
> > the
> > - commit operation, one of the constants declared as class
> > attributes.
> > - If the return value is Transaction.INCOMPLETE, then the
> > transaction is
> > - not yet complete and the caller should try calling again later,
> > after
> > - calling Idl.run() to run the Idl.
> > + """Attempts to commit 'txn'. Returns the status of the commit
> > + operation, one of the following constants:
> > +
> > + Transaction.INCOMPLETE:
> > +
> > + The transaction is in progress, but not yet complete. The
> > caller
> > + should call again later, after calling Idl.run() to let the
> > + IDL do OVSDB protocol processing.
> > +
> > + Transaction.UNCHANGED:
> > +
> > + The transaction is complete. (It didn't actually change the
> > + database, so the IDL didn't send any request to the database
> > + server.)
> > +
> > + Transaction.ABORTED:
> > +
> > + The caller previously called Transaction.abort().
> > +
> > + Transaction.SUCCESS:
> > +
> > + The transaction was successful. The update made by the
> > + transaction (and possibly other changes made by other
> > database
> > + clients) should already be visible in the IDL.
> > +
> > + Transaction.TRY_AGAIN:
> > +
> > + The transaction failed for some transient reason, e.g.
> > because a
> > + "verify" operation reported an inconsistency or due to a
> > network
> > + problem. The caller should wait for a change to the
> > database,
> > + then compose a new transaction, and commit the new
> > transaction.
> > +
> > + Use Idl.change_seqno to wait for a change in the database.
> > It is
> > + important to use its value *before* the initial call to
> > + Transaction.commit() as the baseline for this purpose,
> > because
> > + the change that one should wait for can happen after the
> > initial
> > + call but before the call that returns Transaction.TRY_AGAIN,
> > and
> > + using some other baseline value in that situation could
> > cause an
> > + indefinite wait if the database rarely changes.
> > +
> > + Transaction.NOT_LOCKED:
> > +
> > + The transaction failed because the IDL has been configured to
> > + require a database lock (with Idl.set_lock()) but didn't
> > + get it yet or has already lost it.
> >
> > Committing a transaction rolls back all of the changes that it made
> > to
> > - the Idl's copy of the database. If the transaction commits
> > + the IDL's copy of the database. If the transaction commits
> > successfully, then the database server will send an update and,
> > thus,
> > - the Idl will be updated with the committed changes."""
> > + the IDL will be updated with the committed changes."""
> > # The status can only change if we're the active transaction.
> > # (Otherwise, our status will change only in Idl.run().)
> > if self != self.idl.txn:
> > @@ -849,6 +940,12 @@ class Transaction(object):
> > return self._status
> >
> > def commit_block(self):
> > + """Attempts to commit this transaction, blocking until the commit
> > + either succeeds or fails. Returns the final commit status, which
> > may
> > + be any Transaction.* value other than Transaction.INCOMPLETE.
> > +
> > + This function calls Idl.run() on this transaction'ss IDL, so it may
> > + cause Idl.change_seqno to change."""
> > while True:
> > status = self.commit()
> > if status != Transaction.INCOMPLETE:
> > @@ -862,6 +959,9 @@ class Transaction(object):
> > poller.block()
> >
> > def get_increment_new_value(self):
> > + """Returns the final (incremented) value of the column in this
> > + transaction that was set to be incremented by Row.increment. This
> > + transaction must have committed successfully."""
> > assert self._status == Transaction.SUCCESS
> > return self._inc_new_value
> >
> > --
> > 1.7.2.5
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
