[ovs-dev] [PATCH] ovsdb-idl: fix index row setting with references.

Ben Pfaff blp at ovn.org
Thu Oct 26 21:28:35 UTC 2017


On Sat, Oct 14, 2017 at 11:53:23PM -0700, Han Zhou wrote:
> IDL index should be able to be used without having to be in a
> transaction. However, current implementation leads to crash if
> a reference type column is being set in an index row for querying
> purpose when it is not in a transaction. It is because of the
> uninitialized arcs and unnecessary updates of the arcs. This patch
> fixes it by telling the column parsing function whether it is for
> index row or not, so that when parsing index row, the arcs are not
> updated. A new test case is added to cover this scenario.
> 
> Signed-off-by: Han Zhou <zhouhan at gmail.com>

Thank you for the bug fix.  I agree that this fix should go in.

I see at least one place where "update" is misspelled "udpate".

Did you consider whether there is a way to distinguish an index row from
other rows, without adding a new member?  It might be possible to simply
consider the UUID, since an index row has UUID zero and other rows do
not.  If this is possible, then I think I'd prefer that approach.

The following incremental fixes some style violations identified by
"checkpatch".

--8<--------------------------cut here-------------------------->8--

diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index 3636a0e09e0e..451172cdcc34 100644
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -2683,8 +2683,8 @@ do_idl_compound_index_with_ref(struct ovs_cmdl_context *ctx)
 
     struct ovsdb_idl_index *index;
     index = ovsdb_idl_create_index(idl, &idltest_table_simple3, "uref");
-    ovsdb_idl_index_add_column(index, &idltest_simple3_col_uref, OVSDB_INDEX_ASC,
-                               NULL);
+    ovsdb_idl_index_add_column(index, &idltest_simple3_col_uref,
+                               OVSDB_INDEX_ASC, NULL);
 
     ovsdb_idl_get_initial_snapshot(idl);
 
@@ -2706,7 +2706,7 @@ do_idl_compound_index_with_ref(struct ovs_cmdl_context *ctx)
     printf("%03d: After add to other table + set of strong ref\n", step++);
     dump_simple3(idl, myRow, step++);
 
-    myRow2 = (struct idltest_simple4*)idltest_simple4_first(idl);
+    myRow2 = (struct idltest_simple4 *) idltest_simple4_first(idl);
     printf("%03d: check simple4: %s\n", step++,
            myRow2 ? "not empty" : "empty");
 
@@ -2714,7 +2714,7 @@ do_idl_compound_index_with_ref(struct ovs_cmdl_context *ctx)
 
     struct idltest_simple3 *equal;
     equal = idltest_simple3_index_init_row(idl, &idltest_table_simple3);
-    myRow2 = (struct idltest_simple4*)idltest_simple4_first(idl);
+    myRow2 = (struct idltest_simple4 *) idltest_simple4_first(idl);
     idltest_simple3_index_set_uref(equal, &myRow2, 1);
     printf("%03d: Query using index with reference\n", step++);
     IDLTEST_SIMPLE3_FOR_EACH_EQUAL (myRow, &cursor, equal) {
@@ -2732,7 +2732,7 @@ do_idl_compound_index_with_ref(struct ovs_cmdl_context *ctx)
     printf("%03d: After delete\n", step++);
     dump_simple3(idl, myRow, step++);
 
-    myRow2 = (struct idltest_simple4*)idltest_simple4_first(idl);
+    myRow2 = (struct idltest_simple4 *) idltest_simple4_first(idl);
     printf("%03d: check simple4: %s\n", step++,
            myRow2 ? "not empty" : "empty");
 


More information about the dev mailing list