mgmtd: normalize argument order to copy(dst, src)

Having just completed a code audit during RCA, the fact that we have 2
different argument orders for the related datastore copying functions
was unnecessary and super confusing.

Fix this code-maintenance/comprehension mistake and move the newer mgmtd
copy routines to use the same arg order as the pre-existing underlying
northbound copy functions (i.e., use `copy(dst, src)`)

Signed-off-by: Christian Hopps <chopps@labn.net>

diff --git a/mgmtd/mgmt_ds.c b/mgmtd/mgmt_ds.c
index dabae4afd1b0274884ce258dcf8b9a1dadcda148..e83980f97bdb623b63f20e0de0bc662b2394c3cd 100644 (file)
--- a/mgmtd/mgmt_ds.c
+++ b/mgmtd/mgmt_ds.c
@@ -74,8 +74,7 @@ static int mgmt_ds_dump_in_memory(struct mgmt_ds_ctx *ds_ctx,
        return 0;
 }
 
-static int mgmt_ds_replace_dst_with_src_ds(struct mgmt_ds_ctx *src,
-                                          struct mgmt_ds_ctx *dst)
+static int ds_copy(struct mgmt_ds_ctx *dst, struct mgmt_ds_ctx *src)
 {
        if (!src || !dst)
                return -1;
@@ -95,8 +94,7 @@ static int mgmt_ds_replace_dst_with_src_ds(struct mgmt_ds_ctx *src,
        return 0;
 }
 
-static int mgmt_ds_merge_src_with_dst_ds(struct mgmt_ds_ctx *src,
-                                        struct mgmt_ds_ctx *dst)
+static int ds_merge(struct mgmt_ds_ctx *dst, struct mgmt_ds_ctx *src)
 {
        int ret;
 
@@ -251,14 +249,13 @@ void mgmt_ds_unlock(struct mgmt_ds_ctx *ds_ctx)
        ds_ctx->locked = 0;
 }
 
-int mgmt_ds_copy_dss(struct mgmt_ds_ctx *src_ds_ctx,
-                    struct mgmt_ds_ctx *dst_ds_ctx, bool updt_cmt_rec)
+int mgmt_ds_copy_dss(struct mgmt_ds_ctx *dst, struct mgmt_ds_ctx *src, bool updt_cmt_rec)
 {
-       if (mgmt_ds_replace_dst_with_src_ds(src_ds_ctx, dst_ds_ctx) != 0)
+       if (ds_copy(dst, src) != 0)
                return -1;
 
-       if (updt_cmt_rec && dst_ds_ctx->ds_id == MGMTD_DS_RUNNING)
-               mgmt_history_new_record(dst_ds_ctx);
+       if (updt_cmt_rec && dst->ds_id == MGMTD_DS_RUNNING)
+               mgmt_history_new_record(dst);
 
        return 0;
 }
@@ -416,9 +413,9 @@ int mgmt_ds_load_config_from_file(struct mgmt_ds_ctx *dst,
        parsed.ds_id = dst->ds_id;
 
        if (merge)
-               mgmt_ds_merge_src_with_dst_ds(&parsed, dst);
+               ds_merge(dst, &parsed);
        else
-               mgmt_ds_replace_dst_with_src_ds(&parsed, dst);
+               ds_copy(dst, &parsed);
 
        nb_config_free(parsed.root.cfg_root);
 

diff --git a/mgmtd/mgmt_ds.h b/mgmtd/mgmt_ds.h
index b8e77e330afeb062cc1c601d20604e369e8af1ec..f7e1d7c5ee4a91ee15f8b0e346404a05a5d416ff 100644 (file)
--- a/mgmtd/mgmt_ds.h
+++ b/mgmtd/mgmt_ds.h
@@ -196,21 +196,19 @@ extern void mgmt_ds_unlock(struct mgmt_ds_ctx *ds_ctx);
 /*
  * Copy from source to destination datastore.
  *
- * src_ds
- *    Source datastore handle (ds to be copied from).
- *
- * dst_ds
+ * dst
  *    Destination datastore handle (ds to be copied to).
  *
+ * src
+ *    Source datastore handle (ds to be copied from).
+ *
  * update_cmd_rec
  *    TRUE if need to update commit record, FALSE otherwise.
  *
  * Returns:
  *    0 on success, -1 on failure.
  */
-extern int mgmt_ds_copy_dss(struct mgmt_ds_ctx *src_ds_ctx,
-                           struct mgmt_ds_ctx *dst_ds_ctx,
-                           bool update_cmt_rec);
+extern int mgmt_ds_copy_dss(struct mgmt_ds_ctx *dst, struct mgmt_ds_ctx *src, bool update_cmt_rec);
 
 /*
  * Fetch northbound configuration for a given datastore context.

diff --git a/mgmtd/mgmt_txn.c b/mgmtd/mgmt_txn.c
index 483dfab8e85e005a1291ec5e596d0e2b81915894..2b4734e971a3e8eba6692742eb7bfdb68992b17c 100644 (file)
--- a/mgmtd/mgmt_txn.c
+++ b/mgmtd/mgmt_txn.c
@@ -764,17 +764,15 @@ static int mgmt_txn_send_commit_cfg_reply(struct mgmt_txn_ctx *txn,
                         !txn->commit_cfg_req->req.commit_cfg.rollback);
 
                /*
-                * Successful commit: Merge Src DS into Dst DS if and only if
+                * Successful commit: Copy Src DS to Dst DS if and only if
                 * this was not a validate-only or abort request.
                 */
                if ((txn->session_id &&
                     !txn->commit_cfg_req->req.commit_cfg.validate_only &&
                     !txn->commit_cfg_req->req.commit_cfg.abort) ||
                    txn->commit_cfg_req->req.commit_cfg.rollback) {
-                       mgmt_ds_copy_dss(txn->commit_cfg_req->req.commit_cfg
-                                                .src_ds_ctx,
-                                        txn->commit_cfg_req->req.commit_cfg
-                                                .dst_ds_ctx,
+                       mgmt_ds_copy_dss(txn->commit_cfg_req->req.commit_cfg.dst_ds_ctx,
+                                        txn->commit_cfg_req->req.commit_cfg.src_ds_ctx,
                                         create_cmt_info_rec);
                }
 
@@ -783,22 +781,18 @@ static int mgmt_txn_send_commit_cfg_reply(struct mgmt_txn_ctx *txn,
                 * request.
                 */
                if (txn->session_id && txn->commit_cfg_req->req.commit_cfg.abort)
-                       mgmt_ds_copy_dss(txn->commit_cfg_req->req.commit_cfg
-                                                .dst_ds_ctx,
-                                        txn->commit_cfg_req->req.commit_cfg
-                                                .src_ds_ctx,
-                                        false);
+                       mgmt_ds_copy_dss(txn->commit_cfg_req->req.commit_cfg.src_ds_ctx,
+                                        txn->commit_cfg_req->req.commit_cfg.dst_ds_ctx, false);
        } else {
                /*
                 * The commit has failied. For implicit commit requests restore
-                * back the contents of the candidate DS.
+                * back the contents of the candidate DS. For non-implicit
+                * commit we want to allow the user to re-commit on the changes
+                * (whether further modified or not).
                 */
                if (txn->commit_cfg_req->req.commit_cfg.implicit)
-                       mgmt_ds_copy_dss(txn->commit_cfg_req->req.commit_cfg
-                                                .dst_ds_ctx,
-                                        txn->commit_cfg_req->req.commit_cfg
-                                                .src_ds_ctx,
-                                        false);
+                       mgmt_ds_copy_dss(txn->commit_cfg_req->req.commit_cfg.src_ds_ctx,
+                                        txn->commit_cfg_req->req.commit_cfg.dst_ds_ctx, false);
        }
 
        if (txn->commit_cfg_req->req.commit_cfg.rollback) {