On 6/5/26 10:57 AM, Lorenzo Bianconi wrote: > Filter outgoing monitor requests in the IDL layer based on the > db-schema received from the server, skipping tables and columns not > available in the schema. > To support this, store the user-provided monitor condition in pre-JSON > format in ovsdb_idl_table when ovsdb_idl_set_condition() is called, > allowing ovsdb_idl_compose_monitor_request() to build monitor > conditional requests that only include tables and columns present in > the db-schema. > > Reported-at: https://issues.redhat.com/browse/FDP-3114 > Signed-off-by: Lorenzo Bianconi <[email protected]> > --- > Changes in v5: > - Add self-test for last_id management in raft mode. > - Use a pointer for req_cond instead of a struct. > - Fix typos. > - Do not reset server_schema_received in ovsdb_idl_run() > - Move hmap_is_empty() after in_server_schema check in > ovsdb_idl_set_condition__(). > - Add for coverage tests. > Changes in v4: > - Filter monitor requests in ovsdb_idl_set_condition__() > - Cosmetics > - Move ovsdb_cs_db_sync_condition() in ovsdb_cs_send_monitor_request() to > properly update ack_cond json struct > - Store original user monitor request in req_cond in ovsdb_idl_set_condition() > - Add more uni-tests > - Add more comments > Changes in v3: > - Fix seqno reporting when the filtered condition is empty. > Changes in v2: > - Add missing unit-test. > - squash with patch 'ovsdb: Add ovsdb_cs_clear_condition routine to remove > stable ovsdb_cs_db_table entries.'. > - fix the error condition reported by Ilya. > - Remove unnecessary ovsdb_cs_db_sync_condition() in > ovsdb_cs_send_monitor_request(). > - cosmetics. > --- > lib/ovsdb-cs.c | 50 +++++++--- > lib/ovsdb-cs.h | 2 + > lib/ovsdb-idl-provider.h | 3 + > lib/ovsdb-idl.c | 111 ++++++++++++++++++++- > tests/ovsdb-idl.at | 152 ++++++++++++++++++++++++++++- > tests/test-ovsdb.c | 205 ++++++++++++++++++++++++++++++++++++++- > 6 files changed, 504 insertions(+), 19 deletions(-)
Hi, Lorenzo. Thanks for the update. And sorry for delay, it took a lot of time for me to figure out the CI failure. The failure is real. While the code itself is correct, it does change a behavior around the condition synchronization and the results are visible in the tests. In short, before this change ovsdb_cs_db_sync_condition() is called twice - on disconnect and then on connect. With this patch the sync is only called when sending monitor requests, which happens once per reconnection. The synchronization was warning in two stages, first demoting the requsted condition back to new and then promoting this new condition all the way to acked. With this patch the second part didn't happen, so the test first sends the old acked conditions and only then updates them to the current 'new' conditions with a seporate cond_change request. While this is correct, it's less efficient. I posted a change to make the synchronization idempotent, so a single call is enough to achieve the efficient final state. That shoudld be applied before this patch. Link: https://patchwork.ozlabs.org/project/openvswitch/patch/[email protected]/ Lorenzo, Dumitru, please take a look at this patch. Some minor comments below. Best regards, Ilya Maximets. > > diff --git a/lib/ovsdb-cs.c b/lib/ovsdb-cs.c > index 3920c2e5c..253fe4fe6 100644 > --- a/lib/ovsdb-cs.c > +++ b/lib/ovsdb-cs.c > @@ -412,12 +412,6 @@ ovsdb_cs_retry_at(struct ovsdb_cs *cs, const char *where) > static void > ovsdb_cs_restart_fsm(struct ovsdb_cs *cs) > { > - /* Resync data DB table conditions to avoid missing updates due to > - * conditions that were in flight or changed locally while the connection > - * was down. > - */ > - ovsdb_cs_db_sync_condition(&cs->data); > - > ovsdb_cs_send_schema_request(cs, &cs->server); > ovsdb_cs_transition(cs, CS_S_SERVER_SCHEMA_REQUESTED); > cs->data.monitor_version = 0; > @@ -912,17 +906,47 @@ ovsdb_cs_db_get_table(struct ovsdb_cs_db *db, const > char *table) > return t; > } > > +static void > +ovsdb_cs_db_destroy_table(struct ovsdb_cs_db_table *table, > + struct ovsdb_cs_db *db) > +{ > + json_destroy(table->ack_cond); > + json_destroy(table->req_cond); > + json_destroy(table->new_cond); > + hmap_remove(&db->tables, &table->hmap_node); > + free(table->name); > + free(table); > +} > + > +/* Destroy a given ovsdb_cs_db_table according to the table name. */ > +void > +ovsdb_cs_clear_condition(struct ovsdb_cs *cs, const char *table) > +{ > + uint32_t hash = hash_string(table, 0); > + struct ovsdb_cs_db *db = &cs->data; > + > + struct ovsdb_cs_db_table *t; > + HMAP_FOR_EACH_WITH_HASH (t, hmap_node, hash, &db->tables) { > + if (!strcmp(t->name, table)) { > + ovsdb_cs_db_destroy_table(t, db); > + db->last_id = UUID_ZERO; > + return; > + } > + } > +} > + > +void > +ovsdb_cs_reset_last_id(struct ovsdb_cs *cs) > +{ > + cs->data.last_id = UUID_ZERO; > +} > + > static void > ovsdb_cs_db_destroy_tables(struct ovsdb_cs_db *db) > { > struct ovsdb_cs_db_table *table; > HMAP_FOR_EACH_SAFE (table, hmap_node, &db->tables) { > - json_destroy(table->ack_cond); > - json_destroy(table->req_cond); > - json_destroy(table->new_cond); > - hmap_remove(&db->tables, &table->hmap_node); > - free(table->name); > - free(table); > + ovsdb_cs_db_destroy_table(table, db); > } > hmap_destroy(&db->tables); > } > @@ -1511,6 +1535,8 @@ ovsdb_cs_send_monitor_request(struct ovsdb_cs *cs, > struct ovsdb_cs_db *db, > /* XXX handle failure */ > ovs_assert(mrs->type == JSON_OBJECT); > > + ovsdb_cs_db_sync_condition(db); > + > if (version > 1) { > struct ovsdb_cs_db_table *table; > HMAP_FOR_EACH (table, hmap_node, &db->tables) { > diff --git a/lib/ovsdb-cs.h b/lib/ovsdb-cs.h > index bcc3dcd71..98c921cd8 100644 > --- a/lib/ovsdb-cs.h > +++ b/lib/ovsdb-cs.h > @@ -144,6 +144,8 @@ void ovsdb_cs_set_probe_interval(const struct ovsdb_cs *, > int probe_interval); > unsigned int ovsdb_cs_set_condition(struct ovsdb_cs *, const char *table, > const struct json *condition); > unsigned int ovsdb_cs_get_condition_seqno(const struct ovsdb_cs *); > +void ovsdb_cs_clear_condition(struct ovsdb_cs *, const char *table); > +void ovsdb_cs_reset_last_id(struct ovsdb_cs *); > > /* Database change awareness. */ > void ovsdb_cs_set_db_change_aware(struct ovsdb_cs *, bool > set_db_change_aware); > diff --git a/lib/ovsdb-idl-provider.h b/lib/ovsdb-idl-provider.h > index 6cf32fb50..2e0ab3003 100644 > --- a/lib/ovsdb-idl-provider.h > +++ b/lib/ovsdb-idl-provider.h > @@ -130,6 +130,9 @@ struct ovsdb_idl_table { > * or not. */ > struct ovs_list indexes; /* Contains "struct ovsdb_idl_index"s */ > struct ovs_list track_list; /* Tracked rows (ovsdb_idl_row.track_node). > */ > + > + struct ovsdb_idl_condition *req_cond; /* User requested monitor > + * condition. */ > }; > > struct ovsdb_idl_class { > diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c > index fe90deda7..8d85a23f1 100644 > --- a/lib/ovsdb-idl.c > +++ b/lib/ovsdb-idl.c > @@ -99,6 +99,8 @@ struct ovsdb_idl { > struct ovs_list rows_to_reparse; /* Stores rows that might need to be > * re-parsed due to insertion of a > * referenced row. */ > + bool server_schema_received; /* Set to true when the IDL has received > + * the DB schema from the server. */ > }; > > static struct ovsdb_cs_ops ovsdb_idl_cs_ops; > @@ -205,6 +207,17 @@ static void ovsdb_idl_add_to_indexes(const struct > ovsdb_idl_row *); > static void ovsdb_idl_remove_from_indexes(const struct ovsdb_idl_row *); > static int ovsdb_idl_try_commit_loop_txn(struct ovsdb_idl_loop *loop, > bool *may_need_wakeup); > +static void ovsdb_idl_condition_clone(struct ovsdb_idl_condition *dest, > + const struct ovsdb_idl_condition *); > +static void ovsdb_idl_create_req_condition( > + struct ovsdb_idl *, > + const struct ovsdb_idl_table_class *, > + const struct ovsdb_idl_condition *); > +static void ovsdb_idl_destroy_req_condition(struct ovsdb_idl_table *); > +static unsigned int ovsdb_idl_set_condition__( > + struct ovsdb_idl *, > + const struct ovsdb_idl_table_class *, > + const struct ovsdb_idl_condition *); > > static void add_tracked_change_for_references(struct ovsdb_idl_row *); > > @@ -266,6 +279,7 @@ ovsdb_idl_create_unconnected(const struct ovsdb_idl_class > *class, > .txn = NULL, > .outstanding_txns = HMAP_INITIALIZER(&idl->outstanding_txns), > .verify_write_only = false, > + .server_schema_received = false, > .deleted_untracked_rows > = OVS_LIST_INITIALIZER(&idl->deleted_untracked_rows), > .rows_to_reparse > @@ -298,6 +312,7 @@ ovsdb_idl_create_unconnected(const struct ovsdb_idl_class > *class, > = table->change_seqno[OVSDB_IDL_CHANGE_DELETE] = 0; > table->idl = idl; > table->in_server_schema = false; > + table->req_cond = NULL; > shash_init(&table->schema_columns); > } > > @@ -372,6 +387,7 @@ ovsdb_idl_destroy(struct ovsdb_idl *idl) > > ovsdb_idl_schema_columns_clear(&table->schema_columns); > shash_destroy(&table->schema_columns); > + ovsdb_idl_destroy_req_condition(table); > > hmap_destroy(&table->rows); > free(table->modes); > @@ -788,6 +804,7 @@ ovsdb_idl_compose_monitor_request(const struct json > *schema_json, void *idl_) > struct shash *schema = ovsdb_cs_parse_schema(schema_json); > struct json *monitor_requests = json_object_create(); > > + idl->server_schema_received = true; > for (size_t i = 0; i < idl->class_->n_tables; i++) { > struct ovsdb_idl_table *table = &idl->tables[i]; > const struct ovsdb_idl_table_class *tc = table->class_; > @@ -842,8 +859,12 @@ ovsdb_idl_compose_monitor_request(const struct json > *schema_json, void *idl_) > idl->class_->database, table->class_->name); > json_destroy(columns); > table->in_server_schema = false; > + ovsdb_cs_clear_condition(idl->cs, table->class_->name); > continue; > } else if (schema && table_schema) { > + if (!table->in_server_schema) { > + ovsdb_cs_reset_last_id(idl->cs); > + } > table->in_server_schema = true; > } > > @@ -852,6 +873,14 @@ ovsdb_idl_compose_monitor_request(const struct json > *schema_json, void *idl_) > json_object_put(monitor_requests, tc->name, > json_array_create_1(monitor_request)); > } > + > + if (!table->in_server_schema) { > + ovsdb_cs_clear_condition(idl->cs, table->class_->name); > + } else if (table->req_cond) { > + /* Update the monitor condition request according to the > + * db schema. */ > + ovsdb_idl_set_condition__(idl, tc, table->req_cond); > + } > } > ovsdb_cs_free_schema(schema); > > @@ -1156,6 +1185,41 @@ ovsdb_idl_condition_add_clause__(struct > ovsdb_idl_condition *condition, > hmap_insert(&condition->clauses, &clause->hmap_node, hash); > } > > +static void > +ovsdb_idl_destroy_req_condition(struct ovsdb_idl_table *table) > +{ > + ovsdb_idl_condition_destroy(table->req_cond); > + free(table->req_cond); > + table->req_cond = NULL; > +} > + > +static void > +ovsdb_idl_condition_clone(struct ovsdb_idl_condition *dest, > + const struct ovsdb_idl_condition *source) > +{ > + ovsdb_idl_condition_init(dest); > + > + struct ovsdb_idl_clause *clause; > + HMAP_FOR_EACH (clause, hmap_node, &source->clauses) { > + ovsdb_idl_condition_add_clause__(dest, clause, > clause->hmap_node.hash); > + } > + dest->is_true = source->is_true; > +} > + > +static void > +ovsdb_idl_create_req_condition(struct ovsdb_idl *idl, > + const struct ovsdb_idl_table_class *tc, > + const struct ovsdb_idl_condition *condition) > +{ > + struct ovsdb_idl_table *table = shash_find_data(&idl->table_by_name, > + tc->name); > + if (table) { > + ovsdb_idl_destroy_req_condition(table); > + table->req_cond = xzalloc(sizeof *table->req_cond); > + ovsdb_idl_condition_clone(table->req_cond, condition); > + } > +} > + > /* Adds a clause to the condition for replicating the table with class 'tc' > in > * 'idl'. > * > @@ -1234,6 +1298,47 @@ ovsdb_idl_condition_to_json(const struct > ovsdb_idl_condition *cnd) > return json_array_create(clauses, n); > } > > +static unsigned int > +ovsdb_idl_set_condition__(struct ovsdb_idl *idl, > + const struct ovsdb_idl_table_class *tc, > + const struct ovsdb_idl_condition *condition) > +{ > + struct ovsdb_idl_condition filter_cond = > + OVSDB_IDL_CONDITION_INIT(&filter_cond); > + struct json *cond_json; > + unsigned int seqno; > + > + if (!idl->server_schema_received) { > + goto out; > + } > + > + struct ovsdb_idl_table *t = ovsdb_idl_table_from_class(idl, tc); > + if (!t || !t->in_server_schema) { > + return ovsdb_idl_get_condition_seqno(idl); > + } > + > + if (hmap_is_empty(&condition->clauses)) { > + goto out; > + } > + > + struct ovsdb_idl_clause *clause; > + HMAP_FOR_EACH (clause, hmap_node, &condition->clauses) { > + if (ovsdb_idl_server_has_column(idl, clause->column)) { > + ovsdb_idl_condition_add_clause__(&filter_cond, clause, > + clause->hmap_node.hash); > + } > + } > + condition = &filter_cond; > +out: > + cond_json = ovsdb_idl_condition_to_json(condition); > + seqno = ovsdb_cs_set_condition(idl->cs, tc->name, cond_json); > + json_destroy(cond_json); > + > + ovsdb_idl_condition_destroy(&filter_cond); > + > + return seqno; > +} > + > /* Sets the replication condition for 'tc' in 'idl' to 'condition' and > * arranges to send the new condition to the database server. > * > @@ -1245,10 +1350,8 @@ ovsdb_idl_set_condition(struct ovsdb_idl *idl, > const struct ovsdb_idl_table_class *tc, > const struct ovsdb_idl_condition *condition) > { > - struct json *cond_json = ovsdb_idl_condition_to_json(condition); > - unsigned int seqno = ovsdb_cs_set_condition(idl->cs, tc->name, > cond_json); > - json_destroy(cond_json); > - return seqno; > + ovsdb_idl_create_req_condition(idl, tc, condition); > + return ovsdb_idl_set_condition__(idl, tc, condition); > } > > /* Turns off OVSDB_IDL_ALERT and OVSDB_IDL_TRACK for 'column' in 'idl'. > diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at > index 15d0e4764..c15e78671 100644 > --- a/tests/ovsdb-idl.at > +++ b/tests/ovsdb-idl.at > @@ -2696,7 +2696,7 @@ dnl idltest2.ovsschema and outputs the presence of > tables and columns. > dnl And then it connectes to the server with the schema idltest.ovsschema > dnl and does the same. > AT_CHECK([test-ovsdb '-vPATTERN:console:test-ovsdb|%c|%m' -vjsonrpc -t10 dnl > - idl-table-column-check unix:socket unix:socket2], [0], [dnl > + idl-table-column-check unix:socket unix:socket2 > unix:socket], [0], [dnl > unix:socket remote has table simple > unix:socket remote has table link1 > unix:socket remote doesn't have table link2 > @@ -2715,11 +2715,161 @@ unix:socket2 remote has col l2 in table link1, type: > set of up to 1 uuids > unix:socket2 remote has col i in table link1, type: integer > unix:socket2 remote has col id in table simple7, type: string > --- remote unix:socket2 done --- > +unix:socket remote has table simple > +unix:socket remote has table link1 > +unix:socket remote doesn't have table link2 > +unix:socket remote doesn't have table simple5 > +unix:socket remote doesn't have col irefmap in table simple5 > +unix:socket remote doesn't have col l2 in table link1 > +unix:socket remote has col i in table link1, type: integer > +unix:socket remote doesn't have col id in table simple7 > +--- remote unix:socket done --- > ], [stderr]) > > +# Check we do not have any errors related to conditional monitoring. > +AT_CHECK([grep -q "received error, error={\"details\":\"No column l2 in > table link1.\"" stderr], [1]) > +AT_CHECK([grep -q "received error, error={\"details\":\"no table named > link2\"" stderr], [1]) You wrapped these lines below, but not here. Also, it may be worth to not be so specific and e.g. just grep for 'receive error'. Also, it's better to avoid -q on negative searches, if it produces any output, it will fail, which is the same behavior, but we'll see that output clearly in the test log, if it is not silenced. > + > +# Check the monitor_cond_since request has properly formatted conditions > connecting to the idltest2 server. Maybe wrap this one as well. > +AT_CHECK([grep monitor_cond_since stderr | dnl In general, new tests should use '\' for command spliting and dnl for comments. Modern autotest prints '\'-split lines correctly and they look better than ones with dnl in the middle. > + grep -qF > '"l2","==",@<:@"uuid","12345678-dd3f-4616-ab6a-83a490bb0991"@:>@']) > +AT_CHECK([grep monitor_cond_since stderr | grep -qF '"i","==",1']) > + > OVSDB_SERVER_SHUTDOWN > AT_CLEANUP > > +AT_SETUP([idl set_condition after connection]) > +AT_KEYWORDS([ovsdb server idl set_condition post connect]) > +OVSDB_START_IDLTEST([], ["$abs_srcdir/idltest2.ovsschema"]) > + > +dnl This test connects to a server with the limited idltest2 schema, > +dnl waits for the initial data, and only then calls set_condition(). > +dnl This exercises the code path where conditions are set after a > +dnl successful connection (server_schema_received is already true). > +dnl > +dnl Two conditions are set: > +dnl - link1.l2 == <uuid>: link1 is in idltest2 but l2 column is not, > +dnl so the clause must be filtered out. > +dnl - link2.i == 1: link2 table is not in idltest2 at all, so the > +dnl condition must be silently dropped. > +AT_CHECK([test-ovsdb '-vPATTERN:console:test-ovsdb|%c|%m' dnl > + -vjsonrpc -t10 dnl > + idl-set-condition-post-connect dnl '\' here and in all commands below. > + unix:socket], [0], [dnl > +link1 set_condition: seqno=1 > +link2 set_condition: seqno=0 > +current condition seqno: 0 > +condition acked: seqno=1 > +], [stderr]) > + > +# No errors about missing columns/tables in monitor_cond_change. > +AT_CHECK([grep -q dnl > + "received error, error={\"details\":\"No column l2 in table link1.\"" dnl > + stderr], [1]) > +AT_CHECK([grep -q dnl > + "received error, error={\"details\":\"no table named link2\"" dnl > + stderr], [1]) Same here: may be worth to just use a simple grep for 'receive error' and without -q. Unless we're expecting some other errors. > + > +# The monitor_cond_change must NOT contain the l2 clause > +# (column absent from idltest2 schema). > +AT_CHECK([grep monitor_cond_change stderr | dnl > + grep -qF dnl > + > '"l2","==",@<:@"uuid","12345678-dd3f-4616-ab6a-83a490bb0991"@:>@'], dnl > + [1]) > + > +# The monitor_cond_change must contain the i clause > +# (column present in idltest2 schema). > +AT_CHECK([grep monitor_cond_change stderr | dnl > + grep -qF '"i","==",1']) This command is short enough, no need to split. > + > +# The monitor_cond_change must NOT contain link2 table > +# (table absent from idltest2 schema). > +AT_CHECK([grep monitor_cond_change stderr | dnl > + grep -qF '"link2"'], [1]) Same here. > + > +OVSDB_SERVER_SHUTDOWN > +AT_CLEANUP > + > +AT_SETUP([idl reconnect does not reset last_id for missing tables]) > +AT_KEYWORDS([ovsdb server idl reconnect last_id cluster]) > + > +dnl Use single-node raft clusters so that the servers maintain > +dnl transaction history and monitor_cond_since with a non-zero > +dnl last_id returns found=true with incremental updates. > +dnl > +dnl Server 1 (socket): limited schema (idltest2) - lacks link2 table > +dnl and l2 column in link1. > +dnl Server 2 (socket2): full schema (idltest) - has all tables/columns. > +AT_CHECK([ovsdb-tool create-cluster s1.db dnl > + $abs_srcdir/idltest2.ovsschema unix:s1.raft]) > +schema_name=$(ovsdb-tool schema-name $abs_srcdir/idltest2.ovsschema) > +on_exit 'kill $(cat s*.pid)' > +AT_CHECK([ovsdb-server -vraft -vconsole:warn -vfile:dbg --detach dnl > + --no-chdir --log-file=s1.log --pidfile=s1.pid dnl > + --unixctl=s1 --remote=punix:socket s1.db]) > +OVS_WAIT_UNTIL([ovs-appctl -t $(pwd)/s1 cluster/status dnl > + ${schema_name} | grep -q 'Status: cluster member']) > + > +AT_CHECK([ovsdb-tool create-cluster s2.db dnl > + $abs_srcdir/idltest.ovsschema unix:s2.raft]) > +AT_CHECK([ovsdb-server -vraft -vconsole:warn -vfile:dbg --detach dnl > + --no-chdir --log-file=s2.log --pidfile=s2.pid dnl > + --unixctl=s2 --remote=punix:socket2 s2.db]) > +OVS_WAIT_UNTIL([ovs-appctl -t $(pwd)/s2 cluster/status dnl > + ${schema_name} | grep -q 'Status: cluster member']) > + > +dnl This test sets a condition on link2 (not in idltest2 schema) before > +dnl connecting, then: > +dnl 1. Connects to the limited-schema server (socket) and gets data. > +dnl 2. Reconnects to the same server - last_id must be preserved > +dnl (non-zero) because the CS entry for link2 was already destroyed > +dnl on the first connection, so ovsdb_cs_clear_condition() is a > +dnl no-op. > +dnl 3. Connects to the full-schema server (socket2), simulating a > +dnl schema upgrade. The client must reset last_id to zero because > +dnl previously missing tables/columns are now available and the > +dnl old transaction history does not cover them. > +AT_CHECK([test-ovsdb '-vPATTERN:console:test-ovsdb|%c|%m' dnl > + -vjsonrpc -t10 dnl > + idl-reconnect-last-id dnl > + unix:socket unix:socket2], [0], [dnl > +done > +], [stderr]) > + > +# There should be three data DB monitor_cond_since requests. > +AT_CHECK([grep monitor_cond_since stderr | grep 'send request' | dnl > + grep -c '"idltest"'], [0], [3 > +]) > + > +# First data DB request (initial connect to limited schema): > +# must use UUID_ZERO as last_id (no prior state). > +AT_CHECK([grep monitor_cond_since stderr | grep 'send request' | dnl > + grep '"idltest"' | sed -n '1p' | dnl > + grep -qF '"00000000-0000-0000-0000-000000000000"']) > + > +# Second data DB request (reconnect to same limited schema): > +# must NOT use UUID_ZERO, proving last_id was preserved. > +AT_CHECK([grep monitor_cond_since stderr | grep 'send request' | dnl > + grep '"idltest"' | sed -n '2p' | dnl > + grep -qF '"00000000-0000-0000-0000-000000000000"'], [1]) > + > +# Third data DB request (connect to full schema / upgrade): > +# must use UUID_ZERO because previously missing tables are now > +# present and the old transaction history does not cover them. > +AT_CHECK([grep monitor_cond_since stderr | grep 'send request' | dnl > + grep '"idltest"' | sed -n '3p' | dnl > + grep -qF '"00000000-0000-0000-0000-000000000000"']) > + > +# No errors about missing columns/tables. > +AT_CHECK([grep -q dnl > + "received error, error={\"details\":\"No column l2 in table link1.\"" dnl > + stderr], [1]) > +AT_CHECK([grep -q dnl > + "received error, error={\"details\":\"no table named link2\"" dnl > + stderr], [1]) Ditto. Also, we should stop the database servers here. OVS_APP_EXIT_AND_WAIT_BY_TARGET can probbaly be used. > + > +AT_CLEANUP > + > dnl This test checks that inserting and deleting the source of a reference > dnl doesn't remove the reference in the (deleted) source tracked record. > OVSDB_CHECK_IDL_TRACK([track, insert and delete, refs to link1], > diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c > index bef01486f..8853fede3 100644 > --- a/tests/test-ovsdb.c > +++ b/tests/test-ovsdb.c > @@ -3577,6 +3577,19 @@ do_idl_table_column_check(struct ovs_cmdl_context *ctx) > ovsdb_idl_omit(idl, &idltest_link1_col_i); > ovsdb_idl_omit(idl, &idltest_simple7_col_id); > ovsdb_idl_set_leader_only(idl, false); > + > + struct ovsdb_idl_condition cond_link1 = > + OVSDB_IDL_CONDITION_INIT(&cond_link1); > + struct uuid uuid; An empty line might be good here. > + uuid_from_string(&uuid, "12345678-dd3f-4616-ab6a-83a490bb0991"); > + idltest_link1_add_clause_l2(&cond_link1, OVSDB_F_EQ, &uuid); > + idltest_link1_set_condition(idl, &cond_link1); > + > + struct ovsdb_idl_condition cond_link2 = > + OVSDB_IDL_CONDITION_INIT(&cond_link2); And here. > + idltest_link2_add_clause_i(&cond_link2, OVSDB_F_EQ, 1); > + idltest_link2_set_condition(idl, &cond_link2); > + > struct stream *stream; > > error = stream_open_block(jsonrpc_stream_open(ctx->argv[1], &stream, > @@ -3586,7 +3599,7 @@ do_idl_table_column_check(struct ovs_cmdl_context *ctx) > } > rpc = jsonrpc_open(stream); > > - for (int r = 1; r <= 2; r++) { > + for (int r = 1; r <= 3; r++) { > ovsdb_idl_set_remote(idl, ctx->argv[r], true); > ovsdb_idl_force_reconnect(idl); > > @@ -3658,6 +3671,190 @@ do_idl_table_column_check(struct ovs_cmdl_context > *ctx) > printf("--- remote %s done ---\n", ctx->argv[r]); > } > > + ovsdb_idl_condition_destroy(&cond_link1); > + ovsdb_idl_condition_destroy(&cond_link2); > + > + jsonrpc_close(rpc); > + ovsdb_idl_destroy(idl); > +} > + > +static void > +do_idl_set_condition_post_connect(struct ovs_cmdl_context *ctx) > +{ > + struct jsonrpc *rpc; > + struct ovsdb_idl *idl; > + unsigned int seqno = 0; > + int error; > + > + idl = ovsdb_idl_create(ctx->argv[1], &idltest_idl_class, true, true); > + ovsdb_idl_omit(idl, &idltest_link1_col_i); > + ovsdb_idl_omit(idl, &idltest_simple7_col_id); > + ovsdb_idl_set_leader_only(idl, false); > + > + struct stream *stream; > + > + error = stream_open_block(jsonrpc_stream_open(ctx->argv[1], &stream, > + DSCP_DEFAULT), -1, &stream); > + if (error) { > + ovs_fatal(error, "failed to connect to \"%s\"", ctx->argv[1]); > + } > + rpc = jsonrpc_open(stream); > + > + ovsdb_idl_set_remote(idl, ctx->argv[1], true); > + > + /* Wait for initial data to arrive. */ > + for (;;) { > + ovsdb_idl_run(idl); > + ovsdb_idl_check_consistency(idl); > + if (ovsdb_idl_get_seqno(idl) != seqno) { > + break; > + } > + jsonrpc_run(rpc); > + > + ovsdb_idl_wait(idl); > + jsonrpc_wait(rpc); > + poll_block(); > + } > + > + /* Set conditions after the schema has been received. > + * For link1, set two clauses: l2 (not in idltest2 schema, will be > + * filtered) and i (in idltest2 schema, will pass through). */ > + struct ovsdb_idl_condition cond_link1 = > + OVSDB_IDL_CONDITION_INIT(&cond_link1); > + struct uuid uuid; And here. > + uuid_from_string(&uuid, "12345678-dd3f-4616-ab6a-83a490bb0991"); > + idltest_link1_add_clause_l2(&cond_link1, OVSDB_F_EQ, &uuid); > + idltest_link1_add_clause_i(&cond_link1, OVSDB_F_EQ, 1); > + unsigned int link1_seqno = Maybe define the variable above and just assing here. > + idltest_link1_set_condition(idl, &cond_link1); > + > + struct ovsdb_idl_condition cond_link2 = > + OVSDB_IDL_CONDITION_INIT(&cond_link2); > + idltest_link2_add_clause_i(&cond_link2, OVSDB_F_EQ, 1); > + unsigned int link2_seqno = > + idltest_link2_set_condition(idl, &cond_link2); Same for this block. > + > + printf("link1 set_condition: seqno=%u\n", link1_seqno); > + printf("link2 set_condition: seqno=%u\n", link2_seqno); > + printf("current condition seqno: %u\n", > + ovsdb_idl_get_condition_seqno(idl)); > + > + /* Wait for condition acknowledgement. */ > + for (;;) { > + ovsdb_idl_run(idl); > + ovsdb_idl_check_consistency(idl); > + if (ovsdb_idl_get_condition_seqno(idl) == link1_seqno) { > + break; > + } > + jsonrpc_run(rpc); > + > + ovsdb_idl_wait(idl); > + jsonrpc_wait(rpc); > + poll_block(); > + } > + > + printf("condition acked: seqno=%u\n", > + ovsdb_idl_get_condition_seqno(idl)); > + > + ovsdb_idl_condition_destroy(&cond_link1); > + ovsdb_idl_condition_destroy(&cond_link2); > + > + jsonrpc_close(rpc); > + ovsdb_idl_destroy(idl); > +} > + > +static void > +do_idl_reconnect_last_id(struct ovs_cmdl_context *ctx) > +{ > + struct jsonrpc *rpc; > + struct ovsdb_idl *idl; > + unsigned int seqno = 0; > + int error; > + > + idl = ovsdb_idl_create(ctx->argv[1], &idltest_idl_class, true, true); > + ovsdb_idl_set_leader_only(idl, false); > + > + /* Set a condition on link2 before connecting. This creates a CS > + * table entry that will be destroyed on the first connection (link2 > + * is not in idltest2). On reconnect, the entry is already gone so > + * ovsdb_cs_clear_condition() must be a no-op and must NOT reset > + * last_id. > + * > + * When later connecting to a server with the full schema (all > + * tables/columns present), the client must reset last_id to zero > + * to force a full re-download that covers the newly available > + * tables. */ > + struct ovsdb_idl_condition cond_link2 = > + OVSDB_IDL_CONDITION_INIT(&cond_link2); > + idltest_link2_add_clause_i(&cond_link2, OVSDB_F_EQ, 1); > + idltest_link2_set_condition(idl, &cond_link2); > + > + struct stream *stream; > + > + error = stream_open_block(jsonrpc_stream_open(ctx->argv[1], &stream, > + DSCP_DEFAULT), -1, &stream); > + if (error) { > + ovs_fatal(error, "failed to connect to \"%s\"", ctx->argv[1]); > + } > + rpc = jsonrpc_open(stream); > + > + /* Step 1: connect to the limited-schema server. */ > + ovsdb_idl_set_remote(idl, ctx->argv[1], true); > + > + for (;;) { > + ovsdb_idl_run(idl); > + ovsdb_idl_check_consistency(idl); > + if (ovsdb_idl_get_seqno(idl) != seqno) { > + break; > + } > + jsonrpc_run(rpc); > + > + ovsdb_idl_wait(idl); > + jsonrpc_wait(rpc); > + poll_block(); > + } This loop is repeated 4 times in this patch. Maybe factor out into a function? > + > + seqno = ovsdb_idl_get_seqno(idl); > + > + /* Step 2: reconnect to the same limited-schema server. */ > + ovsdb_idl_force_reconnect(idl); > + > + for (;;) { > + ovsdb_idl_run(idl); > + ovsdb_idl_check_consistency(idl); > + if (ovsdb_idl_get_seqno(idl) != seqno) { > + break; > + } > + jsonrpc_run(rpc); > + > + ovsdb_idl_wait(idl); > + jsonrpc_wait(rpc); > + poll_block(); > + } > + > + seqno = ovsdb_idl_get_seqno(idl); > + > + /* Step 3: connect to the full-schema server (upgrade scenario). */ > + ovsdb_idl_set_remote(idl, ctx->argv[2], true); > + ovsdb_idl_force_reconnect(idl); > + > + for (;;) { > + ovsdb_idl_run(idl); > + ovsdb_idl_check_consistency(idl); > + if (ovsdb_idl_get_seqno(idl) != seqno) { > + break; > + } > + jsonrpc_run(rpc); > + > + ovsdb_idl_wait(idl); > + jsonrpc_wait(rpc); > + poll_block(); > + } > + > + printf("done\n"); > + > + ovsdb_idl_condition_destroy(&cond_link2); > + > jsonrpc_close(rpc); > ovsdb_idl_destroy(idl); > } > @@ -3700,8 +3897,12 @@ static struct ovs_cmdl_command all_commands[] = { > do_idl_partial_update_map_column, OVS_RO }, > { "idl-partial-update-set-column", NULL, 1, INT_MAX, > do_idl_partial_update_set_column, OVS_RO }, > - { "idl-table-column-check", NULL, 2, 2, > + { "idl-table-column-check", NULL, 3, 3, > do_idl_table_column_check, OVS_RO }, > + { "idl-set-condition-post-connect", NULL, 1, 1, > + do_idl_set_condition_post_connect, OVS_RO }, > + { "idl-reconnect-last-id", NULL, 2, 2, > + do_idl_reconnect_last_id, OVS_RO }, > { "help", NULL, 0, INT_MAX, do_help, OVS_RO }, > { NULL, NULL, 0, 0, NULL, OVS_RO }, > }; _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
