lib, mgmtd: respect base xpath in mgmtd

`nb_cli_apply_changes` can be called with base xpath which should be
prepended to xpaths of every change in a transaction. This base xpath is
respected by regular northbound CLI but not by mgmtd. This commit fixes
the problem.

Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>

diff --git a/lib/mgmt_be_client.c b/lib/mgmt_be_client.c
index 762ace13617958ccc947d852c99c4a350c2579cc..a7adcc455d242592b8eae75383a436ee72d0ed85 100644 (file)
--- a/lib/mgmt_be_client.c
+++ b/lib/mgmt_be_client.c
@@ -426,8 +426,7 @@ static int mgmt_be_txn_cfg_prepare(struct mgmt_be_txn_ctx *txn)
                                client_ctx->candidate_config,
                                txn_req->req.set_cfg.cfg_changes,
                                (size_t)txn_req->req.set_cfg.num_cfg_changes,
-                               NULL, NULL, 0, err_buf, sizeof(err_buf),
-                               &error);
+                               NULL, err_buf, sizeof(err_buf), &error);
                        if (error) {
                                err_buf[sizeof(err_buf) - 1] = 0;
                                MGMTD_BE_CLIENT_ERR(

diff --git a/lib/northbound.c b/lib/northbound.c
index 080f3d002eca0558f5c146696349588dcc532604..b0380e6e7e31c518da32435a56ee637627491a26 100644 (file)
--- a/lib/northbound.c
+++ b/lib/northbound.c
@@ -763,8 +763,8 @@ static bool nb_is_operation_allowed(struct nb_node *nb_node,
 
 void nb_candidate_edit_config_changes(
        struct nb_config *candidate_config, struct nb_cfg_change cfg_changes[],
-       size_t num_cfg_changes, const char *xpath_base, const char *curr_xpath,
-       int xpath_index, char *err_buf, int err_bufsize, bool *error)
+       size_t num_cfg_changes, const char *xpath_base, char *err_buf,
+       int err_bufsize, bool *error)
 {
        if (error)
                *error = false;
@@ -776,25 +776,18 @@ void nb_candidate_edit_config_changes(
        for (size_t i = 0; i < num_cfg_changes; i++) {
                struct nb_cfg_change *change = &cfg_changes[i];
                struct nb_node *nb_node;
+               char *change_xpath = change->xpath;
                char xpath[XPATH_MAXLEN];
                struct yang_data *data;
                int ret;
 
-               /* Handle relative XPaths. */
                memset(xpath, 0, sizeof(xpath));
-               if (xpath_index > 0 &&
-                   (xpath_base[0] == '.' || change->xpath[0] == '.'))
-                       strlcpy(xpath, curr_xpath, sizeof(xpath));
-               if (xpath_base[0]) {
-                       if (xpath_base[0] == '.')
-                               strlcat(xpath, xpath_base + 1, sizeof(xpath));
-                       else
-                               strlcat(xpath, xpath_base, sizeof(xpath));
+               /* If change xpath is relative, prepend base xpath. */
+               if (change_xpath[0] == '.') {
+                       strlcpy(xpath, xpath_base, sizeof(xpath));
+                       change_xpath++; /* skip '.' */
                }
-               if (change->xpath[0] == '.')
-                       strlcat(xpath, change->xpath + 1, sizeof(xpath));
-               else
-                       strlcpy(xpath, change->xpath, sizeof(xpath));
+               strlcat(xpath, change_xpath, sizeof(xpath));
 
                /* Find the northbound node associated to the data path. */
                nb_node = nb_node_find(xpath);

diff --git a/lib/northbound.h b/lib/northbound.h
index 5cf6e85b4bd36806e495d4712f1ed81aadd76e25..9c0b4d16c35535de54973f8c98092375536f64f5 100644 (file)
--- a/lib/northbound.h
+++ b/lib/northbound.h
@@ -916,12 +916,6 @@ extern bool nb_candidate_needs_update(const struct nb_config *candidate);
  * xpath_base
  *    Base xpath for config.
  *
- * curr_xpath
- *    Current xpath for config.
- *
- * xpath_index
- *    Index of xpath being processed.
- *
  * err_buf
  *    Buffer to store human-readable error message in case of error.
  *
@@ -933,8 +927,8 @@ extern bool nb_candidate_needs_update(const struct nb_config *candidate);
  */
 extern void nb_candidate_edit_config_changes(
        struct nb_config *candidate_config, struct nb_cfg_change cfg_changes[],
-       size_t num_cfg_changes, const char *xpath_base, const char *curr_xpath,
-       int xpath_index, char *err_buf, int err_bufsize, bool *error);
+       size_t num_cfg_changes, const char *xpath_base, char *err_buf,
+       int err_bufsize, bool *error);
 
 /*
  * Delete candidate configuration changes.

diff --git a/lib/northbound_cli.c b/lib/northbound_cli.c
index 8003679ed57bfb48faee0d66bcc9814630b35b85..f2415d3383ca918b16f5076e5ef101032688d84f 100644 (file)
--- a/lib/northbound_cli.c
+++ b/lib/northbound_cli.c
@@ -147,8 +147,7 @@ static int nb_cli_apply_changes_internal(struct vty *vty,
 
        nb_candidate_edit_config_changes(
                vty->candidate_config, vty->cfg_changes, vty->num_cfg_changes,
-               xpath_base, VTY_CURR_XPATH, vty->xpath_index, buf, sizeof(buf),
-               &error);
+               xpath_base, buf, sizeof(buf), &error);
        if (error) {
                /*
                 * Failure to edit the candidate configuration should never
@@ -180,8 +179,26 @@ static int nb_cli_apply_changes_internal(struct vty *vty,
        return CMD_SUCCESS;
 }
 
+static void create_xpath_base_abs(struct vty *vty, char *xpath_base_abs,
+                                 size_t xpath_base_abs_size,
+                                 const char *xpath_base)
+{
+       memset(xpath_base_abs, 0, xpath_base_abs_size);
+
+       if (xpath_base[0] == 0)
+               xpath_base = ".";
+
+       /* If base xpath is relative, prepend current vty xpath. */
+       if (vty->xpath_index > 0 && xpath_base[0] == '.') {
+               strlcpy(xpath_base_abs, VTY_CURR_XPATH, xpath_base_abs_size);
+               xpath_base++; /* skip '.' */
+       }
+       strlcat(xpath_base_abs, xpath_base, xpath_base_abs_size);
+}
+
 int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...)
 {
+       char xpath_base_abs[XPATH_MAXLEN] = {};
        char xpath_base[XPATH_MAXLEN] = {};
        bool implicit_commit;
        int ret;
@@ -195,6 +212,9 @@ int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...)
                va_end(ap);
        }
 
+       create_xpath_base_abs(vty, xpath_base_abs, sizeof(xpath_base_abs),
+                             xpath_base);
+
        if (vty_mgmt_should_process_cli_apply_changes(vty)) {
                VTY_CHECK_XPATH;
 
@@ -202,18 +222,20 @@ int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...)
                        return CMD_SUCCESS;
 
                implicit_commit = vty_needs_implicit_commit(vty);
-               ret = vty_mgmt_send_config_data(vty, implicit_commit);
+               ret = vty_mgmt_send_config_data(vty, xpath_base_abs,
+                                               implicit_commit);
                if (ret >= 0 && !implicit_commit)
                        vty->mgmt_num_pending_setcfg++;
                return ret;
        }
 
-       return nb_cli_apply_changes_internal(vty, xpath_base, false);
+       return nb_cli_apply_changes_internal(vty, xpath_base_abs, false);
 }
 
 int nb_cli_apply_changes_clear_pending(struct vty *vty,
                                       const char *xpath_base_fmt, ...)
 {
+       char xpath_base_abs[XPATH_MAXLEN] = {};
        char xpath_base[XPATH_MAXLEN] = {};
        bool implicit_commit;
        int ret;
@@ -227,6 +249,9 @@ int nb_cli_apply_changes_clear_pending(struct vty *vty,
                va_end(ap);
        }
 
+       create_xpath_base_abs(vty, xpath_base_abs, sizeof(xpath_base_abs),
+                             xpath_base);
+
        if (vty_mgmt_should_process_cli_apply_changes(vty)) {
                VTY_CHECK_XPATH;
                /*
@@ -238,13 +263,14 @@ int nb_cli_apply_changes_clear_pending(struct vty *vty,
                 * conversions to mgmtd require full proper implementations.
                 */
                implicit_commit = vty_needs_implicit_commit(vty);
-               ret = vty_mgmt_send_config_data(vty, implicit_commit);
+               ret = vty_mgmt_send_config_data(vty, xpath_base_abs,
+                                               implicit_commit);
                if (ret >= 0 && !implicit_commit)
                        vty->mgmt_num_pending_setcfg++;
                return ret;
        }
 
-       return nb_cli_apply_changes_internal(vty, xpath_base, true);
+       return nb_cli_apply_changes_internal(vty, xpath_base_abs, true);
 }
 
 int nb_cli_rpc(struct vty *vty, const char *xpath, struct list *input,

diff --git a/lib/vty.c b/lib/vty.c
index f395fd3ea1d8f63518ec6981f54317bead4b57a9..2cfe34f211dcc689ccde078b0a95603750c5acff 100644 (file)
--- a/lib/vty.c
+++ b/lib/vty.c
@@ -3718,12 +3718,15 @@ int vty_mgmt_send_lockds_req(struct vty *vty, Mgmtd__DatastoreId ds_id,
        return 0;
 }
 
-int vty_mgmt_send_config_data(struct vty *vty, bool implicit_commit)
+int vty_mgmt_send_config_data(struct vty *vty, const char *xpath_base,
+                             bool implicit_commit)
 {
        Mgmtd__YangDataValue value[VTY_MAXCFGCHANGES];
        Mgmtd__YangData cfg_data[VTY_MAXCFGCHANGES];
        Mgmtd__YangCfgDataReq cfg_req[VTY_MAXCFGCHANGES];
        Mgmtd__YangCfgDataReq *cfgreq[VTY_MAXCFGCHANGES] = {0};
+       char xpath[VTY_MAXCFGCHANGES][XPATH_MAXLEN];
+       char *change_xpath;
        size_t indx;
 
        if (vty->type == VTY_FILE) {
@@ -3759,6 +3762,9 @@ int vty_mgmt_send_config_data(struct vty *vty, bool implicit_commit)
                }
        }
 
+       if (xpath_base == NULL)
+               xpath_base = "";
+
        for (indx = 0; indx < vty->num_cfg_changes; indx++) {
                mgmt_yang_data_init(&cfg_data[indx]);
 
@@ -3771,7 +3777,17 @@ int vty_mgmt_send_config_data(struct vty *vty, bool implicit_commit)
                        cfg_data[indx].value = &value[indx];
                }
 
-               cfg_data[indx].xpath = vty->cfg_changes[indx].xpath;
+               change_xpath = vty->cfg_changes[indx].xpath;
+
+               memset(xpath[indx], 0, sizeof(xpath[indx]));
+               /* If change xpath is relative, prepend base xpath. */
+               if (change_xpath[0] == '.') {
+                       strlcpy(xpath[indx], xpath_base, sizeof(xpath[indx]));
+                       change_xpath++; /* skip '.' */
+               }
+               strlcat(xpath[indx], change_xpath, sizeof(xpath[indx]));
+
+               cfg_data[indx].xpath = xpath[indx];
 
                mgmt_yang_cfg_data_req_init(&cfg_req[indx]);
                cfg_req[indx].data = &cfg_data[indx];

diff --git a/lib/vty.h b/lib/vty.h
index 346e44910a01deb0fcf3fd2b6f580d24be2e57ad..1a431fa16a4374f85c4ec631548570fc60f47a31 100644 (file)
--- a/lib/vty.h
+++ b/lib/vty.h
@@ -412,7 +412,8 @@ extern bool vty_mgmt_fe_enabled(void);
 extern bool vty_mgmt_should_process_cli_apply_changes(struct vty *vty);
 
 extern bool mgmt_vty_read_configs(void);
-extern int vty_mgmt_send_config_data(struct vty *vty, bool implicit_commit);
+extern int vty_mgmt_send_config_data(struct vty *vty, const char *xpath_base,
+                                    bool implicit_commit);
 extern int vty_mgmt_send_commit_config(struct vty *vty, bool validate_only,
                                       bool abort);
 extern int vty_mgmt_send_get_req(struct vty *vty, bool is_config,

diff --git a/mgmtd/mgmt_txn.c b/mgmtd/mgmt_txn.c
index 765def7baceadaed60367c86b7b03ae57b0054a1..1af0f2749afa44c75299f8b3b92adbcc61b2b92e 100644 (file)
--- a/mgmtd/mgmt_txn.c
+++ b/mgmtd/mgmt_txn.c
@@ -664,8 +664,8 @@ static void mgmt_txn_process_set_cfg(struct event *thread)
                                                 txn_req->req.set_cfg->cfg_changes,
                                                 (size_t)txn_req->req.set_cfg
                                                         ->num_cfg_changes,
-                                                NULL, NULL, 0, err_buf,
-                                                sizeof(err_buf), &error);
+                                                NULL, err_buf, sizeof(err_buf),
+                                                &error);
                if (error) {
                        mgmt_fe_send_set_cfg_reply(txn->session_id, txn->txn_id,
                                                   txn_req->req.set_cfg->ds_id,

diff --git a/mgmtd/mgmt_vty.c b/mgmtd/mgmt_vty.c
index e7d69667103b704f2ddd85d880c9df5582a37c86..54288dfe9c896c024bd09862ee91ed0d414a28a4 100644 (file)
--- a/mgmtd/mgmt_vty.c
+++ b/mgmtd/mgmt_vty.c
@@ -157,7 +157,7 @@ DEFPY(mgmt_set_config_data, mgmt_set_config_data_cmd,
        vty->cfg_changes[0].operation = NB_OP_CREATE;
        vty->num_cfg_changes = 1;
 
-       vty_mgmt_send_config_data(vty, false);
+       vty_mgmt_send_config_data(vty, NULL, false);
        return CMD_SUCCESS;
 }
 
@@ -174,7 +174,7 @@ DEFPY(mgmt_delete_config_data, mgmt_delete_config_data_cmd,
        vty->cfg_changes[0].operation = NB_OP_DESTROY;
        vty->num_cfg_changes = 1;
 
-       vty_mgmt_send_config_data(vty, false);
+       vty_mgmt_send_config_data(vty, NULL, false);
        return CMD_SUCCESS;
 }