LGTM. Just two nits. > -----Original Message----- > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev- > boun...@openvswitch.org] On Behalf Of Ben Pfaff > Sent: Friday, December 8, 2017 11:24 PM > To: d...@openvswitch.org > Cc: Ben Pfaff <b...@ovn.org> > Subject: [ovs-dev] [PATCH 1/5] ovsdb-idl: Improve comments. > > This change documents the IDL state machine, adds other comments, and > fixes a spelling error in a comment. > > Signed-off-by: Ben Pfaff <b...@ovn.org> > --- > lib/ovsdb-idl.c | 57 > ++++++++++++++++++++++++++++++++++++++++++++++----------- > 1 file changed, 46 insertions(+), 11 deletions(-) > > diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index > 26a19fe8d5e6..7e3abdee8e62 100644 > --- a/lib/ovsdb-idl.c > +++ b/lib/ovsdb-idl.c > @@ -79,29 +79,63 @@ struct ovsdb_idl_arc { > struct ovsdb_idl_row *dst; /* Destination row. */ }; > > +/* Connection state machine. > + * > + * When a JSON-RPC session connects, the IDL sends a "get_schema" > +request and > + * transitions to IDL_S_SCHEMA_REQUESTED. If the session drops and > +reconnects, > + * the IDL starts over again in the same way. */ > enum ovsdb_idl_state { > + /* Waits for "get_schema" reply, then sends a "monitor_cond" request > whose > + * details are informed by the schema and transitions to > + * IDL_S_MONITOR_COND_REQUESTED. */ > IDL_S_SCHEMA_REQUESTED, > - IDL_S_MONITOR_REQUESTED, > - IDL_S_MONITORING, > + > + /* Waits for "monitor_cond" reply: > + * > + * - If the reply indicates success, replaces the IDL contents by the > + * data carried in the reply and transitions to > IDL_S_MONITORING_COND. > + * > + * - If the reply indicates failure because the database is too old to > + * support monitor_cond, sends a "monitor" request and transitions > to > + * IDl_S_MONITOR_REQUESTED. */ > IDL_S_MONITOR_COND_REQUESTED, > + > + /* Waits for "monitor" reply, then replaces the IDL contents by the data > + * carried in the reply and transitions to IDL_S_MONITORING. */ > + IDL_S_MONITOR_REQUESTED, > + > + /* Terminal states that process "update2" (IDL_S_MONITORING_COND) > or > + * "update" (IDL_S_MONITORING) notifications. */ > IDL_S_MONITORING_COND, > + IDL_S_MONITORING, > + > + /* Terminal error state that indicates that nothing useful can be done. > + * The most likely reason is that the database server doesn't actually > have [Alin Serdean] doesn't have, maybe? > + * the desired database. We maintain the session with the database > server > + * anyway. If it starts serving the database that we want, then it will > + * kill the session and we will automatically reconnect and try > + again. */ > IDL_S_NO_SCHEMA > }; > > struct ovsdb_idl { > + /* Data. */ > const struct ovsdb_idl_class *class_; > - struct jsonrpc_session *session; > - struct uuid uuid; > struct shash table_by_name; /* Contains "struct ovsdb_idl_table *"s.*/ > struct ovsdb_idl_table *tables; /* Array of ->class_->n_tables elements. > */ > unsigned int change_seqno; > - bool verify_write_only; > > - /* Session state. */ > - unsigned int state_seqno; > - enum ovsdb_idl_state state; > - struct json *request_id; > - struct json *schema; > + /* Session state. > + * > + *'state_seqno' is a snapshot of the session's sequence number as > returned > + * jsonrpc_session_get_seqno(session), so if it differs from the value > that > + * function currently returns then the session has reconnected and the > + * state machine must restart. */ > + struct jsonrpc_session *session; /* Connection to the server. */ > + enum ovsdb_idl_state state; /* Current session state. */ > + unsigned int state_seqno; /* See above. */ > + struct json *request_id; /* JSON ID for request awaiting reply. > */ > + struct json *schema; /* Temporary copy of database schema. */ > + struct uuid uuid; /* Identifier for monitor. */ > > /* Database locking. */ > char *lock_name; /* Name of lock we need, NULL if none. */ > @@ -112,6 +146,7 @@ struct ovsdb_idl { > /* Transaction support. */ > struct ovsdb_idl_txn *txn; > struct hmap outstanding_txns; > + bool verify_write_only; > > /* Conditional monitoring. */ > bool cond_changed; > @@ -1118,7 +1153,7 @@ ovsdb_idl_condition_clone(struct > ovsdb_idl_condition *dst, > * arranges to send the new condition to the database server. > * > * Return the next conditional update sequence number. When this > - * value and ovsdb_idl_get_condition_seqno() matchs, the 'idl' > + * value and ovsdb_idl_get_condition_seqno() matches, the 'idl' > * contains rows that match the 'condition'. > */ > unsigned int > -- s/. *\//. *\//
Acked-by: Alin Gabriel Serdean <aserd...@ovn.org> _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
