diff --git a/lib/command.c b/lib/command.c index 04f2bd95a0..32e877280d 100644 --- a/lib/command.c +++ b/lib/command.c @@ -1709,6 +1709,8 @@ static int vty_write_config(struct vty *vty) if (host.noconfig) return CMD_SUCCESS; + nb_cli_show_config_prepare(running_config, false); + if (vty->type == VTY_TERM) { vty_out(vty, "\nCurrent configuration:\n"); vty_out(vty, "!\n"); diff --git a/lib/libfrr.c b/lib/libfrr.c index d4aa1f899a..8ef32eaa8a 100644 --- a/lib/libfrr.c +++ b/lib/libfrr.c @@ -853,22 +853,34 @@ static void frr_daemonize(void) */ static int frr_config_read_in(struct thread *t) { - if (!vty_read_config(NULL, di->config_file, config_default) && - di->backup_config_file) { + if (!vty_read_config(vty_shared_candidate_config, di->config_file, + config_default) + && di->backup_config_file) { char *orig = XSTRDUP(MTYPE_TMP, host_config_get()); zlog_info("Attempting to read backup config file: %s specified", di->backup_config_file); - vty_read_config(NULL, di->backup_config_file, config_default); + vty_read_config(vty_shared_candidate_config, + di->backup_config_file, config_default); host_config_set(orig); XFREE(MTYPE_TMP, orig); } /* - * Update the shared candidate after reading the startup configuration. + * Automatically commit the candidate configuration after + * reading the configuration file. */ - nb_config_replace(vty_shared_candidate_config, running_config, true); + if (frr_get_cli_mode() == FRR_CLI_TRANSACTIONAL) { + int ret; + + ret = nb_candidate_commit(vty_shared_candidate_config, + NB_CLIENT_CLI, NULL, true, + "Read configuration file", NULL); + if (ret != NB_OK && ret != NB_ERR_NO_CHANGES) + zlog_err("%s: failed to read configuration file.", + __func__); + } return 0; } diff --git a/lib/northbound.c b/lib/northbound.c index 1b332fb1e8..debd463624 100644 --- a/lib/northbound.c +++ b/lib/northbound.c @@ -350,7 +350,22 @@ static inline int nb_config_cb_compare(const struct nb_config_cb *a, /* * Preserve the order of the configuration changes as told by libyang. */ - return a->seq - b->seq; + if (a->seq < b->seq) + return -1; + if (a->seq > b->seq) + return 1; + + /* + * All 'apply_finish' callbacks have their sequence number set to zero. + * In this case, compare them using their dnode pointers (the order + * doesn't matter for callbacks that have the same priority). + */ + if (a->dnode < b->dnode) + return -1; + if (a->dnode > b->dnode) + return 1; + + return 0; } RB_GENERATE(nb_config_cbs, nb_config_cb, entry, nb_config_cb_compare); @@ -366,7 +381,6 @@ static void nb_config_diff_add_change(struct nb_config_cbs *changes, change->cb.seq = *seq; *seq = *seq + 1; change->cb.nb_node = dnode->schema->priv; - yang_dnode_get_path(dnode, change->cb.xpath, sizeof(change->cb.xpath)); change->cb.dnode = dnode; RB_INSERT(nb_config_cbs, changes, &change->cb); @@ -516,17 +530,6 @@ int nb_candidate_edit(struct nb_config *candidate, __func__); return NB_ERR; } - - /* - * If a new node was created, call lyd_validate() only to create - * default child nodes. - */ - if (dnode) { - lyd_schema_sort(dnode, 0); - lyd_validate(&dnode, - LYD_OPT_CONFIG | LYD_OPT_WHENAUTODEL, - ly_native_ctx); - } break; case NB_OP_DESTROY: dnode = yang_dnode_get(candidate->dnode, xpath_edit); @@ -806,7 +809,7 @@ static int nb_callback_configuration(const enum nb_event event, struct nb_config_change *change) { enum nb_operation operation = change->cb.operation; - const char *xpath = change->cb.xpath; + char xpath[XPATH_MAXLEN]; const struct nb_node *nb_node = change->cb.nb_node; const struct lyd_node *dnode = change->cb.dnode; union nb_resource *resource; @@ -818,6 +821,7 @@ static int nb_callback_configuration(const enum nb_event event, if (dnode && !yang_snode_is_typeless_data(dnode->schema)) value = yang_dnode_get_string(dnode, NULL); + yang_dnode_get_path(dnode, xpath, sizeof(xpath)); nb_log_callback(event, operation, xpath, value); } @@ -840,6 +844,7 @@ static int nb_callback_configuration(const enum nb_event event, ret = (*nb_node->cbs.move)(event, dnode); break; default: + yang_dnode_get_path(dnode, xpath, sizeof(xpath)); flog_err(EC_LIB_DEVELOPMENT, "%s: unknown operation (%u) [xpath %s]", __func__, operation, xpath); @@ -850,6 +855,8 @@ static int nb_callback_configuration(const enum nb_event event, int priority; enum lib_log_refs ref; + yang_dnode_get_path(dnode, xpath, sizeof(xpath)); + switch (event) { case NB_EV_VALIDATE: priority = LOG_WARNING; @@ -1020,14 +1027,12 @@ static int nb_transaction_process(enum nb_event event, } static struct nb_config_cb * -nb_apply_finish_cb_new(struct nb_config_cbs *cbs, const char *xpath, - const struct nb_node *nb_node, +nb_apply_finish_cb_new(struct nb_config_cbs *cbs, const struct nb_node *nb_node, const struct lyd_node *dnode) { struct nb_config_cb *cb; cb = XCALLOC(MTYPE_TMP, sizeof(*cb)); - strlcpy(cb->xpath, xpath, sizeof(cb->xpath)); cb->nb_node = nb_node; cb->dnode = dnode; RB_INSERT(nb_config_cbs, cbs, cb); @@ -1036,13 +1041,15 @@ nb_apply_finish_cb_new(struct nb_config_cbs *cbs, const char *xpath, } static struct nb_config_cb * -nb_apply_finish_cb_find(struct nb_config_cbs *cbs, const char *xpath, - const struct nb_node *nb_node) +nb_apply_finish_cb_find(struct nb_config_cbs *cbs, + const struct nb_node *nb_node, + const struct lyd_node *dnode) { struct nb_config_cb s; - strlcpy(s.xpath, xpath, sizeof(s.xpath)); + s.seq = 0; s.nb_node = nb_node; + s.dnode = dnode; return RB_FIND(nb_config_cbs, cbs, &s); } @@ -1051,6 +1058,7 @@ static void nb_transaction_apply_finish(struct nb_transaction *transaction) { struct nb_config_cbs cbs; struct nb_config_cb *cb; + char xpath[XPATH_MAXLEN]; /* Initialize tree of 'apply_finish' callbacks. */ RB_INIT(nb_config_cbs, &cbs); @@ -1067,8 +1075,6 @@ static void nb_transaction_apply_finish(struct nb_transaction *transaction) * be called though). */ if (change->cb.operation == NB_OP_DESTROY) { - char xpath[XPATH_MAXLEN]; - dnode = dnode->parent; if (!dnode) break; @@ -1084,7 +1090,6 @@ static void nb_transaction_apply_finish(struct nb_transaction *transaction) xpath); } while (dnode) { - char xpath[XPATH_MAXLEN]; struct nb_node *nb_node; nb_node = dnode->schema->priv; @@ -1095,11 +1100,10 @@ static void nb_transaction_apply_finish(struct nb_transaction *transaction) * Don't call the callback more than once for the same * data node. */ - yang_dnode_get_path(dnode, xpath, sizeof(xpath)); - if (nb_apply_finish_cb_find(&cbs, xpath, nb_node)) + if (nb_apply_finish_cb_find(&cbs, nb_node, dnode)) goto next; - nb_apply_finish_cb_new(&cbs, xpath, nb_node, dnode); + nb_apply_finish_cb_new(&cbs, nb_node, dnode); next: dnode = dnode->parent; @@ -1108,9 +1112,11 @@ static void nb_transaction_apply_finish(struct nb_transaction *transaction) /* Call the 'apply_finish' callbacks, sorted by their priorities. */ RB_FOREACH (cb, nb_config_cbs, &cbs) { - if (DEBUG_MODE_CHECK(&nb_dbg_cbs_config, DEBUG_MODE_ALL)) - nb_log_callback(NB_EV_APPLY, NB_OP_APPLY_FINISH, - cb->xpath, NULL); + if (DEBUG_MODE_CHECK(&nb_dbg_cbs_config, DEBUG_MODE_ALL)) { + yang_dnode_get_path(cb->dnode, xpath, sizeof(xpath)); + nb_log_callback(NB_EV_APPLY, NB_OP_APPLY_FINISH, xpath, + NULL); + } (*cb->nb_node->cbs.apply_finish)(cb->dnode); } diff --git a/lib/northbound.h b/lib/northbound.h index fbd7771db7..f52fcc90cf 100644 --- a/lib/northbound.h +++ b/lib/northbound.h @@ -458,7 +458,6 @@ struct nb_config_cb { RB_ENTRY(nb_config_cb) entry; enum nb_operation operation; uint32_t seq; - char xpath[XPATH_MAXLEN]; const struct nb_node *nb_node; const struct lyd_node *dnode; }; diff --git a/lib/northbound_cli.c b/lib/northbound_cli.c index a15fe3d1c9..07e8c9dff9 100644 --- a/lib/northbound_cli.c +++ b/lib/northbound_cli.c @@ -84,20 +84,12 @@ void nb_cli_enqueue_change(struct vty *vty, const char *xpath, int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...) { - struct nb_config *candidate_transitory; char xpath_base[XPATH_MAXLEN] = {}; bool error = false; int ret; VTY_CHECK_XPATH; - /* - * Create a copy of the candidate configuration. For consistency, we - * need to ensure that either all changes made by the command are - * accepted or none are. - */ - candidate_transitory = nb_config_dup(vty->candidate_config); - /* Parse the base XPath format string. */ if (xpath_base_fmt) { va_list ap; @@ -137,7 +129,7 @@ int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...) flog_warn(EC_LIB_YANG_UNKNOWN_DATA_PATH, "%s: unknown data path: %s", __func__, xpath); error = true; - break; + continue; } /* If the value is not set, get the default if it exists. */ @@ -149,7 +141,7 @@ int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...) * Ignore "not found" errors when editing the candidate * configuration. */ - ret = nb_candidate_edit(candidate_transitory, nb_node, + ret = nb_candidate_edit(vty->candidate_config, nb_node, change->operation, xpath, NULL, data); yang_data_free(data); if (ret != NB_OK && ret != NB_ERR_NOT_FOUND) { @@ -159,29 +151,20 @@ int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...) __func__, nb_operation_name(change->operation), xpath); error = true; - break; + continue; } } if (error) { - nb_config_free(candidate_transitory); - - switch (frr_get_cli_mode()) { - case FRR_CLI_CLASSIC: - vty_out(vty, "%% Configuration failed.\n\n"); - break; - case FRR_CLI_TRANSACTIONAL: - vty_out(vty, - "%% Failed to edit candidate configuration.\n\n"); - break; - } + /* + * Failure to edit the candidate configuration should never + * happen in practice, unless there's a bug in the code. When + * that happens, log the error but otherwise ignore it. + */ + vty_out(vty, "%% Failed to edit configuration.\n\n"); vty_show_libyang_errors(vty, ly_native_ctx); - - return CMD_WARNING_CONFIG_FAILED; } - nb_config_replace(vty->candidate_config, candidate_transitory, false); - /* Do an implicit "commit" when using the classic CLI mode. */ if (frr_get_cli_mode() == FRR_CLI_CLASSIC) { ret = nb_candidate_commit(vty->candidate_config, NB_CLIENT_CLI, @@ -438,6 +421,23 @@ static struct lyd_node *ly_iter_next_up(const struct lyd_node *elem) return elem->parent; } +/* Prepare the configuration for display. */ +void nb_cli_show_config_prepare(struct nb_config *config, bool with_defaults) +{ + lyd_schema_sort(config->dnode, 1); + + /* + * When "with-defaults" is used, call lyd_validate() only to create + * default child nodes, ignoring any possible validation error. This + * doesn't need to be done when displaying the running configuration + * since it's always fully validated. + */ + if (with_defaults && config != running_config) + (void)lyd_validate(&config->dnode, + LYD_OPT_CONFIG | LYD_OPT_WHENAUTODEL, + ly_native_ctx); +} + void nb_cli_show_dnode_cmds(struct vty *vty, struct lyd_node *root, bool with_defaults) { @@ -530,6 +530,8 @@ static int nb_cli_show_config(struct vty *vty, struct nb_config *config, struct yang_translator *translator, bool with_defaults) { + nb_cli_show_config_prepare(config, with_defaults); + switch (format) { case NB_CFG_FMT_CMDS: nb_cli_show_config_cmds(vty, config, with_defaults); diff --git a/lib/northbound_cli.h b/lib/northbound_cli.h index 209239ca63..b2d8c8f035 100644 --- a/lib/northbound_cli.h +++ b/lib/northbound_cli.h @@ -109,6 +109,8 @@ extern void nb_cli_show_dnode_cmds(struct vty *vty, struct lyd_node *dnode, bool show_defaults); /* Prototypes of internal functions. */ +extern void nb_cli_show_config_prepare(struct nb_config *config, + bool with_defaults); extern void nb_cli_confirmed_commit_clean(struct vty *vty); extern int nb_cli_confirmed_commit_rollback(struct vty *vty); extern void nb_cli_install_default(int node); diff --git a/lib/vty.c b/lib/vty.c index 502d2c9d04..c08e5e151a 100644 --- a/lib/vty.c +++ b/lib/vty.c @@ -2341,8 +2341,7 @@ static void vty_read_file(struct nb_config *config, FILE *confp) * Automatically commit the candidate configuration after * reading the configuration file. */ - if (config == NULL && vty->candidate_config - && frr_get_cli_mode() == FRR_CLI_TRANSACTIONAL) { + if (config == NULL) { ret = nb_candidate_commit(vty->candidate_config, NB_CLIENT_CLI, vty, true, "Read configuration file", NULL); diff --git a/lib/vty.h b/lib/vty.h index c7352efbd3..a0b8e8afa1 100644 --- a/lib/vty.h +++ b/lib/vty.h @@ -254,7 +254,8 @@ static inline void vty_push_context(struct vty *vty, int node, uint64_t id) #define VTY_CHECK_XPATH \ do { \ - if (vty->xpath_index > 0 \ + if (vty->type != VTY_FILE && !vty->private_config \ + && vty->xpath_index > 0 \ && !yang_dnode_exists(vty->candidate_config->dnode, \ VTY_CURR_XPATH)) { \ vty_out(vty, \