[ovs-dev] [ovsdb monitor fix v2 1/3] ovsdb: Fix one off error in tracking monitor changes

Andy Zhou azhou at ovn.org
Tue Feb 23 00:28:12 UTC 2016


dbmon's changes should be stored with the next transaction number,
rather than the current transaction number.  This bug causes the
changes of a transaction stored in a monitor to be unnoticed by
the jsonrpc connections that is responsible for flush the monitor
content.

However, the bug was not noticed until it was exposed by a later
optimization patch: "avoid unnecessary call to ovsdb_monitor_get_update()."
The lack of optimization means that the update is still generated
when 'unflushed' equals to n_transactions + 1, which should have
indicated the monitor has been flushed already.

Signed-off-by: Andy Zhou <azhou at ovn.org>

---
v1->v2:
     *Explain the bug in more detail in the commit message.
     *roll back the n_transactions if possible.
---
 ovsdb/monitor.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
index 5ae9cdb..0fe2bd6 100644
--- a/ovsdb/monitor.c
+++ b/ovsdb/monitor.c
@@ -1121,11 +1121,24 @@ ovsdb_monitor_commit(struct ovsdb_replica *replica,
     struct ovsdb_monitor_aux aux;
 
     ovsdb_monitor_init_aux(&aux, m);
+    /* Update ovsdb_monitor's transaction number for
+     * each transaction, before calling ovsdb_monitor_change_cb().  */
+    m->n_transactions++;
     ovsdb_txn_for_each_change(txn, ovsdb_monitor_change_cb, &aux);
 
-    if (aux.efficacy == OVSDB_CHANGES_REQUIRE_EXTERNAL_UPDATE) {
+    switch(aux.efficacy) {
+    case OVSDB_CHANGES_NO_EFFECT:
+        /* The transaction is ignored by the monitor.
+         * Roll back the 'n_transactions' as if the transaction
+         * has never happened. */
+        m->n_transactions--;
+        break;
+    case OVSDB_CHANGES_REQUIRE_INTERNAL_UPDATE:
+        /* Nothing.  */
+        break;
+    case  OVSDB_CHANGES_REQUIRE_EXTERNAL_UPDATE:
         ovsdb_monitor_json_cache_flush(m);
-        m->n_transactions++;
+        break;
     }
 
     return NULL;
-- 
1.9.1




More information about the dev mailing list