]> git.puffer.fish Git - matthieu/frr.git/commitdiff
mgmtd: remove bogus "hedge" code which corrupted active candidate DS
authorChristian Hopps <chopps@labn.net>
Tue, 8 Apr 2025 05:15:53 +0000 (05:15 +0000)
committerChristian Hopps <chopps@labn.net>
Tue, 8 Apr 2025 05:39:18 +0000 (05:39 +0000)
Say you have 2 mgmtd frontend sessions (2 vtysh's) the first one is long
running and is actively changing the global candidate datastore (DS),
the second one starts and exits, this code would then copy running
back over the candidate, blowing away any changes made by the first
session.

(the long running session could technically be any user)

Instead we need to trust the various cleanup code that already exits.
For example in the commit_cfg_reply on success candidate is copied to
running, and on failure *for implicit commit* running is copied back to
candidate clearing the change. This leaves the non-implicit
configuration changes in this case we actually want candidate to keep
it's changes in transactional cases, in the other case of pending commit
during a file read the code restores candidate (if needed) on exit from
"config terminal", with this call stack:

 vty_config_node_exit()
   nb_cli_pending_commit_check()
     nb_cli_classic_commit()
       nb_candidate_commit_prepare() [fail] -> copy running -> candidate
       nb_candidate_commit_apply() -> copy candidate -> running

fixes #18541

Signed-off-by: Christian Hopps <chopps@labn.net>
mgmtd/mgmt_fe_adapter.c

index 8d591988037669c93c93a9cb305b694c176d1c47..d077e08679cb90683593d5cb59a2078593fbc859 100644 (file)
@@ -216,12 +216,6 @@ static void mgmt_fe_session_unlock_ds(Mgmtd__DatastoreId ds_id,
 static void
 mgmt_fe_session_cfg_txn_cleanup(struct mgmt_fe_session_ctx *session)
 {
-       /*
-        * Ensure any uncommitted changes in Candidate DS
-        * is discarded.
-        */
-       mgmt_ds_copy_dss(mm->running_ds, mm->candidate_ds, false);
-
        /*
         * Destroy the actual transaction created earlier.
         */