[ovs-dev] [PATCH] ovsdb-idl: Properly handle conditional monitor update error

[Andy Zhou]

When generating conditional monitoring update request, current code
failed to update idl's 'request-id'.  This bug causes the reply
message of the update request, regardless an ACK or a NACK, be
logged as an unexpected message at the debug level and ignored by
the core idl logic.

In addition, the idl should not generate another conditional
monitoring update request when there is an outstanding request.
So that the requests and their reply are properly serialized.

Signed-off-by: Andy Zhou <azhou at ovn.org>
---
 lib/ovsdb-idl.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 8ce228d..fa659a8 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -454,8 +454,11 @@ ovsdb_idl_run(struct ovsdb_idl *idl)
                 idl->schema = NULL;
                 break;
 
-            case IDL_S_MONITORING:
             case IDL_S_MONITORING_COND:
+                /* Conditional monitor clauses were updated. */
+                break;
+
+            case IDL_S_MONITORING:
             case IDL_S_NO_SCHEMA:
             default:
                 OVS_NOT_REACHED();
@@ -494,14 +497,22 @@ ovsdb_idl_run(struct ovsdb_idl *idl)
                 idl->state = IDL_S_MONITOR_REQUESTED;
             }
         } else if (msg->type == JSONRPC_ERROR
+                   && idl->state == IDL_S_MONITORING_COND
+                   && idl->request_id
+                   && json_equal(idl->request_id, msg->id)) {
+            json_destroy(idl->request_id);
+            idl->request_id = NULL;
+            VLOG_ERR("%s: conditional monitor update failed",
+                     jsonrpc_session_get_name(idl->session));
+        } else if (msg->type == JSONRPC_ERROR
                    && idl->state == IDL_S_SCHEMA_REQUESTED
                    && idl->request_id
                    && json_equal(idl->request_id, msg->id)) {
-                json_destroy(idl->request_id);
-                idl->request_id = NULL;
-                VLOG_ERR("%s: requested schema not found",
-                         jsonrpc_session_get_name(idl->session));
-                idl->state = IDL_S_NO_SCHEMA;
+            json_destroy(idl->request_id);
+            idl->request_id = NULL;
+            VLOG_ERR("%s: requested schema not found",
+                     jsonrpc_session_get_name(idl->session));
+            idl->state = IDL_S_NO_SCHEMA;
         } else if ((msg->type == JSONRPC_ERROR
                     || msg->type == JSONRPC_REPLY)
                    && ovsdb_idl_txn_process_reply(idl, msg)) {
@@ -1021,8 +1032,11 @@ ovsdb_idl_send_cond_change(struct ovsdb_idl *idl)
     struct json *params, *json_uuid;
     struct jsonrpc_msg *request;
 
+    /* When 'idl-request_id' is not NULL, there is an outstanding
+     * conditional monitoring update request that we have not heard
+     * from the server yet. Don't generate another request in this case.  */
     if (!idl->cond_changed || !jsonrpc_session_is_connected(idl->session) ||
-        idl->state != IDL_S_MONITORING_COND) {
+        idl->state != IDL_S_MONITORING_COND || idl->request_id) {
         return;
     }
 
@@ -1058,7 +1072,8 @@ ovsdb_idl_send_cond_change(struct ovsdb_idl *idl)
         params = json_array_create_3(json_uuid, json_string_create(uuid),
                                      monitor_cond_change_requests);
 
-        request = jsonrpc_create_request("monitor_cond_change", params, NULL);
+        request = jsonrpc_create_request("monitor_cond_change", params,
+                                         &idl->request_id);
         jsonrpc_session_send(idl->session, request);
     }
     idl->cond_changed = false;
-- 
2.7.4