> 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.
Hi Ilya, thx for the review. > > 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. I tested it and it seems to be working fine. > > Some minor comments below. ack, I will fix them in the next revision. Regards, Lorenzo > > 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
