[ovs-dev] [PATCH 1/5] ovsdb-idl: Improve comments.

[Ben Pfaff]

This change documents the IDL state machine, adds other comments,
and fixes a spelling error in a comment.

Signed-off-by: Ben Pfaff <blp at 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
+     * 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
-- 
2.10.2