mgmtd: fix commit request overwrite

There are places, where we can receive an existing commit transaction.
If we don't check that the request already exists, it gets overwritten
and we start having problems with transaction refcounters. Forbid having
multiple configuration sessions simultaneously.

Signed-off-by: Igor Ryzhov <[email protected]>

mgmtd/mgmt_be_adapter.c

@@ -607,28 +607,17 @@ static void mgmt_be_adapter_conn_init(struct event *thread)
 	adapter = (struct mgmt_be_client_adapter *)EVENT_ARG(thread);
 	assert(adapter && adapter->conn->fd >= 0);
 
-	/*
-	 * Check first if the current session can run a CONFIG
-	 * transaction or not. Reschedule if a CONFIG transaction
-	 * from another session is already in progress.
-	 */
-	if (mgmt_config_txn_in_progress() != MGMTD_SESSION_ID_NONE) {
-		zlog_err("XXX txn in progress, retry init");
-		mgmt_be_adapter_sched_init_event(adapter);
-		return;
-	}
-
 	/*
 	 * Notify TXN module to create a CONFIG transaction and
 	 * download the CONFIGs identified for this new client.
 	 * If the TXN module fails to initiate the CONFIG transaction
-	 * disconnect from the client forcing a reconnect later.
-	 * That should also take care of destroying the adapter.
+	 * retry a bit later. It only fails if there's an existing config
+	 * transaction in progress.
 	 */
 	if (mgmt_txn_notify_be_adapter_conn(adapter, true) != 0) {
-		zlog_err("XXX notify be adapter conn fail");
-		msg_conn_disconnect(adapter->conn, false);
-		adapter = NULL;
+		zlog_err("XXX txn in progress, retry init");
+		mgmt_be_adapter_sched_init_event(adapter);
+		return;
 	}
 }
 

mgmtd/mgmt_fe_adapter.c

@@ -724,7 +724,7 @@ mgmt_fe_session_handle_setcfg_req_msg(struct mgmt_fe_session_ctx *session,
 
 	if (session->cfg_txn_id == MGMTD_TXN_ID_NONE) {
 		/* as we have the lock no-one else should have a config txn */
-		assert(mgmt_config_txn_in_progress() == MGMTD_SESSION_ID_NONE);
+		assert(!mgmt_config_txn_in_progress());
 
 		/* Start a CONFIG Transaction (if not started already) */
 		session->cfg_txn_id = mgmt_create_txn(session->session_id,
@@ -890,6 +890,9 @@ static int mgmt_fe_session_handle_commit_config_req_msg(
 	}
 
 	if (session->cfg_txn_id == MGMTD_TXN_ID_NONE) {
+		/* as we have the lock no-one else should have a config txn */
+		assert(!mgmt_config_txn_in_progress());
+
 		/*
 		 * Start a CONFIG Transaction (if not started already)
 		 */

mgmtd/mgmt_txn.c

@@ -1723,15 +1723,9 @@ static struct mgmt_txn_ctx *mgmt_txn_create_new(uint64_t session_id,
 {
 	struct mgmt_txn_ctx *txn = NULL;
 
-	/*
-	 * For 'CONFIG' transaction check if one is already created
-	 * or not. TODO: figure out what code counts on this and fix it.
-	 */
-	if (type == MGMTD_TXN_TYPE_CONFIG && mgmt_txn_mm->cfg_txn) {
-		if (mgmt_config_txn_in_progress() == session_id)
-			txn = mgmt_txn_mm->cfg_txn;
-		goto mgmt_create_txn_done;
-	}
+	/* Do not allow multiple config transactions */
+	if (type == MGMTD_TXN_TYPE_CONFIG && mgmt_config_txn_in_progress())
+		return NULL;
 
 	txn = mgmt_fe_find_txn_by_session_id(mgmt_txn_mm, session_id, type);
 	if (!txn) {
@@ -1761,7 +1755,6 @@ static struct mgmt_txn_ctx *mgmt_txn_create_new(uint64_t session_id,
 		MGMTD_TXN_LOCK(txn);
 	}
 
-mgmt_create_txn_done:
 	return txn;
 }
 
@@ -1949,12 +1942,12 @@ void mgmt_txn_destroy(void)
 	mgmt_txn_hash_destroy();
 }
 
-uint64_t mgmt_config_txn_in_progress(void)
+bool mgmt_config_txn_in_progress(void)
 {
 	if (mgmt_txn_mm && mgmt_txn_mm->cfg_txn)
-		return mgmt_txn_mm->cfg_txn->session_id;
+		return true;
 
-	return MGMTD_SESSION_ID_NONE;
+	return false;
 }
 
 uint64_t mgmt_create_txn(uint64_t session_id, enum mgmt_txn_type type)

mgmtd/mgmt_txn.h

@@ -71,12 +71,12 @@ extern int mgmt_txn_init(struct mgmt_master *cm, struct event_loop *tm);
 extern void mgmt_txn_destroy(void);
 
 /*
- * Check if transaction is in progress.
+ * Check if configuration transaction is in progress.
  *
  * Returns:
- *    session ID if in-progress, MGMTD_SESSION_ID_NONE otherwise.
+ *    true if in-progress, false otherwise.
  */
-extern uint64_t mgmt_config_txn_in_progress(void);
+extern bool mgmt_config_txn_in_progress(void);
 
 /**
  * Get the session ID associated with the given ``txn-id``.