/* Configuration from terminal */
DEFUN (config_terminal,
config_terminal_cmd,
- "configure [terminal]",
+ "configure [terminal [file-lock]]",
"Configuration from vty interface\n"
+ "Configuration with locked datastores\n"
"Configuration terminal\n")
{
- return vty_config_enter(vty, false, false);
+ return vty_config_enter(vty, false, false, argc == 3);
}
/* Enable command */
required DatastoreId ds_id = 2;
required uint64 req_id = 3;
required bool success = 4;
- optional string error_if_any = 5;
+ required bool implicit_commit = 5;
+ optional string error_if_any = 6;
}
message FeCommitConfigReq {
session->user_ctx, fe_msg->setcfg_reply->req_id,
fe_msg->setcfg_reply->success,
fe_msg->setcfg_reply->ds_id,
+ fe_msg->setcfg_reply->implicit_commit,
fe_msg->setcfg_reply->error_if_any);
break;
case MGMTD__FE_MESSAGE__MESSAGE_COMMCFG_REPLY:
uintptr_t session_id,
uintptr_t user_session_client,
uint64_t req_id, bool success,
- Mgmtd__DatastoreId ds_id,
+ Mgmtd__DatastoreId ds_id, bool implcit_commit,
char *errmsg_if_any);
void (*commit_config_notify)(struct mgmt_fe_client *client,
"Configuration from vty interface\n"
"Configure exclusively from this terminal\n")
{
- return vty_config_enter(vty, true, true);
+ return vty_config_enter(vty, true, true, false);
}
/* Configure using a private candidate configuration. */
"Configuration from vty interface\n"
"Configure using a private candidate configuration\n")
{
- return vty_config_enter(vty, true, false);
+ return vty_config_enter(vty, true, false, false);
}
DEFPY (config_commit,
if (!mgmt_client_id_next)
mgmt_client_id_next++;
new->mgmt_client_id = mgmt_client_id_next++;
- if (mgmt_fe_create_client_session(
- mgmt_fe_client, new->mgmt_client_id,
- (uintptr_t) new) != MGMTD_SUCCESS)
- zlog_err(
- "Failed to open a MGMTD Frontend session for VTY session %p!!",
- new);
+ new->mgmt_session_id = 0;
+ mgmt_fe_create_client_session(
+ mgmt_fe_client, new->mgmt_client_id, (uintptr_t) new);
+ /* we short-circuit create the session so it must be set now */
+ assertf(new->mgmt_session_id != 0,
+ "Failed to create client session for VTY");
}
return new;
vty->candidate_config = vty_shared_candidate_config;
+ vty_mgmt_lock_candidate_inline(vty);
+ vty_mgmt_lock_running_inline(vty);
+
for (index = 0; index < array_size(mgmt_daemons); index++) {
snprintf(path, sizeof(path), "%s/%s.conf", frr_sysconfdir,
mgmt_daemons[index]);
fclose(confp);
}
+ /* Conditionally unlock as the config file may have "exit"d early which
+ * would then have unlocked things.
+ */
+ if (vty->mgmt_locked_running_ds)
+ vty_mgmt_unlock_running_inline(vty);
+ if (vty->mgmt_locked_candidate_ds)
+ vty_mgmt_unlock_candidate_inline(vty);
+
vty->pending_allowed = false;
if (!count)
return true;
}
-int vty_config_enter(struct vty *vty, bool private_config, bool exclusive)
+int vty_config_enter(struct vty *vty, bool private_config, bool exclusive,
+ bool file_lock)
{
- if (exclusive && nb_running_lock(NB_CLIENT_CLI, vty)) {
+ if (exclusive && !vty_mgmt_fe_enabled() &&
+ nb_running_lock(NB_CLIENT_CLI, vty)) {
vty_out(vty, "%% Configuration is locked by other client\n");
return CMD_WARNING;
}
* message. For user interactive mode we are doing implicit commits
* those will obtain the lock (or not) when they try and commit.
*/
- if (vty_mgmt_fe_enabled() && vty->pending_allowed && !private_config) {
- /*
- * lock using short-circuit, we set the locked boolean to true
- * here so that it can be flipped to false by our locked_notify
- * handler during the synchronous call.
- */
- vty->mgmt_locked_candidate_ds = true;
- if (vty_mgmt_send_lockds_req(vty, MGMTD_DS_CANDIDATE, true,
- true) ||
- !vty->mgmt_locked_candidate_ds) {
+ if (file_lock && vty_mgmt_fe_enabled() && !private_config) {
+ if (vty_mgmt_lock_candidate_inline(vty)) {
vty_out(vty,
"%% Can't enter config; candidate datastore locked by another session\n");
return CMD_WARNING_CONFIG_FAILED;
}
+ if (vty_mgmt_lock_running_inline(vty)) {
+ vty_out(vty,
+ "%% Can't enter config; running datastore locked by another session\n");
+ vty_mgmt_unlock_candidate_inline(vty);
+ return CMD_WARNING_CONFIG_FAILED;
+ }
+ assert(vty->mgmt_locked_candidate_ds);
+ assert(vty->mgmt_locked_running_ds);
}
vty->node = CONFIG_NODE;
int vty_config_node_exit(struct vty *vty)
{
- int ret;
-
vty->xpath_index = 0;
- if (vty->mgmt_locked_candidate_ds) {
- assert(vty->type != VTY_FILE);
- /* use short-circuit call to immediately unlock */
- ret = vty_mgmt_send_lockds_req(vty, MGMTD_DS_CANDIDATE, false,
- true);
- assert(!ret);
- vty->mgmt_locked_candidate_ds = false;
- }
+ /* TODO: could we check for un-commited changes here? */
+
+ if (vty->mgmt_locked_running_ds)
+ vty_mgmt_unlock_running_inline(vty);
+
+ if (vty->mgmt_locked_candidate_ds)
+ vty_mgmt_unlock_candidate_inline(vty);
/* Perform any pending commits. */
(void)nb_cli_pending_commit_check(vty);
static void vty_mgmt_set_config_result_notified(
struct mgmt_fe_client *client, uintptr_t usr_data, uint64_t client_id,
uintptr_t session_id, uintptr_t session_ctx, uint64_t req_id,
- bool success, Mgmtd__DatastoreId ds_id, char *errmsg_if_any)
+ bool success, Mgmtd__DatastoreId ds_id, bool implicit_commit,
+ char *errmsg_if_any)
{
struct vty *vty;
client_id, req_id);
}
+ if (implicit_commit) {
+ /* In this case the changes have been applied, we are done */
+ vty_mgmt_unlock_candidate_inline(vty);
+ vty_mgmt_unlock_running_inline(vty);
+ }
+
vty_mgmt_resume_response(vty, success);
}
Mgmtd__YangCfgDataReq cfg_req[VTY_MAXCFGCHANGES];
Mgmtd__YangCfgDataReq *cfgreq[VTY_MAXCFGCHANGES] = {0};
size_t indx;
- int cnt;
if (vty->type == VTY_FILE) {
/*
return 0;
}
+ /* If we are FE client and we have a vty then we have a session */
+ assert(mgmt_fe_client && vty->mgmt_client_id && vty->mgmt_session_id);
- if (mgmt_fe_client && vty->mgmt_client_id && !vty->mgmt_session_id) {
- /*
- * We are connected to mgmtd but we do not yet have an
- * established session. this means we need to send any changes
- * made during this "down-time" to all backend clients when this
- * FE client finishes coming up.
- */
- MGMTD_FE_CLIENT_DBG("skipping as no session exists");
+ if (!vty->num_cfg_changes)
return 0;
+
+ /* grab the candidate and running lock prior to sending implicit commit
+ * command
+ */
+ if (implicit_commit) {
+ if (vty_mgmt_lock_candidate_inline(vty)) {
+ vty_out(vty,
+ "%% command failed, could not lock candidate DS\n");
+ return -1;
+ } else if (vty_mgmt_lock_running_inline(vty)) {
+ vty_out(vty,
+ "%% command failed, could not lock running DS\n");
+ return -1;
+ }
}
- if (mgmt_fe_client && vty->mgmt_session_id) {
- cnt = 0;
- for (indx = 0; indx < vty->num_cfg_changes; indx++) {
- mgmt_yang_data_init(&cfg_data[cnt]);
-
- if (vty->cfg_changes[indx].value) {
- mgmt_yang_data_value_init(&value[cnt]);
- value[cnt].encoded_str_val =
- (char *)vty->cfg_changes[indx].value;
- value[cnt].value_case =
- MGMTD__YANG_DATA_VALUE__VALUE_ENCODED_STR_VAL;
- cfg_data[cnt].value = &value[cnt];
- }
+ for (indx = 0; indx < vty->num_cfg_changes; indx++) {
+ mgmt_yang_data_init(&cfg_data[indx]);
- cfg_data[cnt].xpath = vty->cfg_changes[indx].xpath;
+ if (vty->cfg_changes[indx].value) {
+ mgmt_yang_data_value_init(&value[indx]);
+ value[indx].encoded_str_val =
+ (char *)vty->cfg_changes[indx].value;
+ value[indx].value_case =
+ MGMTD__YANG_DATA_VALUE__VALUE_ENCODED_STR_VAL;
+ cfg_data[indx].value = &value[indx];
+ }
- mgmt_yang_cfg_data_req_init(&cfg_req[cnt]);
- cfg_req[cnt].data = &cfg_data[cnt];
- switch (vty->cfg_changes[indx].operation) {
- case NB_OP_DESTROY:
- cfg_req[cnt].req_type =
- MGMTD__CFG_DATA_REQ_TYPE__DELETE_DATA;
- break;
+ cfg_data[indx].xpath = vty->cfg_changes[indx].xpath;
- case NB_OP_CREATE:
- case NB_OP_MODIFY:
- case NB_OP_MOVE:
- case NB_OP_PRE_VALIDATE:
- case NB_OP_APPLY_FINISH:
- cfg_req[cnt].req_type =
- MGMTD__CFG_DATA_REQ_TYPE__SET_DATA;
- break;
- case NB_OP_GET_ELEM:
- case NB_OP_GET_NEXT:
- case NB_OP_GET_KEYS:
- case NB_OP_LOOKUP_ENTRY:
- case NB_OP_RPC:
- assert(!"Invalid type of operation");
- break;
- default:
- assert(!"non-enum value, invalid");
- }
+ mgmt_yang_cfg_data_req_init(&cfg_req[indx]);
+ cfg_req[indx].data = &cfg_data[indx];
+ switch (vty->cfg_changes[indx].operation) {
+ case NB_OP_DESTROY:
+ cfg_req[indx].req_type =
+ MGMTD__CFG_DATA_REQ_TYPE__DELETE_DATA;
+ break;
- cfgreq[cnt] = &cfg_req[cnt];
- cnt++;
+ case NB_OP_CREATE:
+ case NB_OP_MODIFY:
+ case NB_OP_MOVE:
+ case NB_OP_PRE_VALIDATE:
+ case NB_OP_APPLY_FINISH:
+ cfg_req[indx].req_type =
+ MGMTD__CFG_DATA_REQ_TYPE__SET_DATA;
+ break;
+ case NB_OP_GET_ELEM:
+ case NB_OP_GET_NEXT:
+ case NB_OP_GET_KEYS:
+ case NB_OP_LOOKUP_ENTRY:
+ case NB_OP_RPC:
+ default:
+ assertf(false,
+ "Invalid operation type for send config: %d",
+ vty->cfg_changes[indx].operation);
+ /*NOTREACHED*/
+ abort();
}
- vty->mgmt_req_id++;
- if (cnt && mgmt_fe_send_setcfg_req(
- mgmt_fe_client, vty->mgmt_session_id,
- vty->mgmt_req_id, MGMTD_DS_CANDIDATE, cfgreq,
- cnt, implicit_commit,
- MGMTD_DS_RUNNING) != MGMTD_SUCCESS) {
- zlog_err("Failed to send %d Config Xpaths to MGMTD!!",
- (int)indx);
- vty_out(vty, "Failed to send SETCFG-REQ to MGMTD!\n");
- return -1;
- }
+ cfgreq[indx] = &cfg_req[indx];
+ }
+ if (!indx)
+ return 0;
- vty->mgmt_req_pending_cmd = "MESSAGE_SETCFG_REQ";
+ vty->mgmt_req_id++;
+ if (mgmt_fe_send_setcfg_req(mgmt_fe_client, vty->mgmt_session_id,
+ vty->mgmt_req_id, MGMTD_DS_CANDIDATE,
+ cfgreq, indx, implicit_commit,
+ MGMTD_DS_RUNNING)) {
+ zlog_err("Failed to send %zu config xpaths to mgmtd", indx);
+ vty_out(vty, "%% Failed to send commands to mgmtd\n");
+ return -1;
}
+ vty->mgmt_req_pending_cmd = "MESSAGE_SETCFG_REQ";
+
return 0;
}
extern char *vty_get_cwd(void);
extern void vty_update_xpath(const char *oldpath, const char *newpath);
extern int vty_config_enter(struct vty *vty, bool private_config,
- bool exclusive);
+ bool exclusive, bool file_lock);
extern void vty_config_exit(struct vty *);
extern int vty_config_node_exit(struct vty *);
extern int vty_shell(struct vty *);
uint64_t txn_id;
uint64_t cfg_txn_id;
uint8_t ds_locked[MGMTD_DS_MAX_ID];
- uint8_t ds_locked_implict[MGMTD_DS_MAX_ID];
struct event *proc_cfg_txn_clnp;
struct event *proc_show_txn_clnp;
session->session_id, mgmt_ds_id2name(ds_id));
session->ds_locked[ds_id] = false;
- session->ds_locked_implict[ds_id] = false;
mgmt_ds_unlock(ds_ctx);
MGMTD_FE_ADAPTER_DBG(
"Unlocked DS:%s write-locked earlier by session-id: %" PRIu64
static void
mgmt_fe_session_cfg_txn_cleanup(struct mgmt_fe_session_ctx *session)
{
- Mgmtd__DatastoreId ds_id;
- struct mgmt_ds_ctx *ds_ctx;
-
/*
* Ensure any uncommitted changes in Candidate DS
* is discarded.
*/
mgmt_ds_copy_dss(mm->running_ds, mm->candidate_ds, false);
- for (ds_id = 0; ds_id < MGMTD_DS_MAX_ID; ds_id++) {
- ds_ctx = mgmt_ds_get_ctx_by_id(mm, ds_id);
- if (ds_ctx && session->ds_locked_implict[ds_id])
- mgmt_fe_session_unlock_ds(ds_id, ds_ctx, session);
- }
-
/*
* Destroy the actual transaction created earlier.
*/
setcfg_reply.ds_id = ds_id;
setcfg_reply.req_id = req_id;
setcfg_reply.success = success;
+ setcfg_reply.implicit_commit = implicit_commit;
if (error_if_any)
setcfg_reply.error_if_any = (char *)error_if_any;
}
/*
- * XXX chopps: get rid of this, we should have deleted sessions when there was a
- * disconnect
+ * Purge any old connections that share the same client name with `adapter`
*/
static void
mgmt_fe_adapter_cleanup_old_conn(struct mgmt_fe_client_adapter *adapter)
struct mgmt_fe_client_adapter *old;
FOREACH_ADAPTER_IN_LIST (old) {
- if (old != adapter &&
- !strncmp(adapter->name, old->name, sizeof(adapter->name))) {
- /*
- * We have a Zombie lingering around
- */
- MGMTD_FE_ADAPTER_DBG(
- "Client '%s' (FD:%d) seems to have reconnected. Removing old connection (FD:%d)!",
- adapter->name, adapter->conn->fd,
- old->conn->fd);
- msg_conn_disconnect(old->conn, false);
- }
+ if (old == adapter)
+ continue;
+ if (strncmp(adapter->name, old->name, sizeof(adapter->name)))
+ continue;
+
+ MGMTD_FE_ADAPTER_DBG(
+ "Client '%s' (FD:%d) seems to have reconnected. Removing old connection (FD:%d)",
+ adapter->name, adapter->conn->fd,
+ old->conn->fd);
+ msg_conn_disconnect(old->conn, false);
}
}
{
struct mgmt_ds_ctx *ds_ctx;
- /*
- * Next check first if the SETCFG_REQ is for Candidate DS
- * or not. Report failure if its not. MGMTD currently only
- * supports editing the Candidate DS.
- */
- if (lockds_req->ds_id != MGMTD_DS_CANDIDATE) {
+ if (lockds_req->ds_id != MGMTD_DS_CANDIDATE &&
+ lockds_req->ds_id != MGMTD_DS_RUNNING) {
mgmt_fe_send_lockds_reply(
session, lockds_req->ds_id, lockds_req->req_id,
lockds_req->lock, false,
- "Lock/Unlock on datastores other than Candidate DS not permitted!");
+ "Lock/Unlock on DS other than candidate or running DS not supported");
return -1;
}
"Lock already taken on DS by another session!");
return -1;
}
-
- session->ds_locked_implict[lockds_req->ds_id] = false;
} else {
if (!session->ds_locked[lockds_req->ds_id]) {
mgmt_fe_send_lockds_reply(
mgmt_fe_session_handle_setcfg_req_msg(struct mgmt_fe_session_ctx *session,
Mgmtd__FeSetConfigReq *setcfg_req)
{
- uint64_t cfg_session_id;
- struct mgmt_ds_ctx *ds_ctx, *dst_ds_ctx;
+ struct mgmt_ds_ctx *ds_ctx, *dst_ds_ctx = NULL;
+ bool txn_created = false;
if (mm->perf_stats_en)
gettimeofday(&session->adapter->setcfg_stats.last_start, NULL);
- /*
- * Next check first if the SETCFG_REQ is for Candidate DS
- * or not. Report failure if its not. MGMTD currently only
- * supports editing the Candidate DS.
- */
+ /* MGMTD currently only supports editing the candidate DS. */
if (setcfg_req->ds_id != MGMTD_DS_CANDIDATE) {
mgmt_fe_send_setcfg_reply(
session, setcfg_req->ds_id, setcfg_req->req_id, false,
- "Set-Config on datastores other than Candidate DS not permitted!",
+ "Set-Config on datastores other than Candidate DS not supported",
setcfg_req->implicit_commit);
return 0;
}
-
- /*
- * Get the DS handle.
- */
ds_ctx = mgmt_ds_get_ctx_by_id(mm, setcfg_req->ds_id);
- if (!ds_ctx) {
+ assert(ds_ctx);
+
+ /* MGMTD currently only supports targetting the running DS. */
+ if (setcfg_req->implicit_commit &&
+ setcfg_req->commit_ds_id != MGMTD_DS_RUNNING) {
mgmt_fe_send_setcfg_reply(
session, setcfg_req->ds_id, setcfg_req->req_id, false,
- "No such DS exists!", setcfg_req->implicit_commit);
+ "Implicit commit on datastores other than running DS not supported",
+ setcfg_req->implicit_commit);
+ return 0;
+ }
+ dst_ds_ctx = mgmt_ds_get_ctx_by_id(mm, setcfg_req->commit_ds_id);
+ assert(dst_ds_ctx);
+
+ /* User should have write lock to change the DS */
+ if (!session->ds_locked[setcfg_req->ds_id]) {
+ mgmt_fe_send_setcfg_reply(session, setcfg_req->ds_id,
+ setcfg_req->req_id, false,
+ "Candidate DS is not locked",
+ setcfg_req->implicit_commit);
return 0;
}
if (session->cfg_txn_id == MGMTD_TXN_ID_NONE) {
- /*
- * Check first if the current session can run a CONFIG
- * transaction or not. Report failure if a CONFIG transaction
- * from another session is already in progress.
- */
- cfg_session_id = mgmt_config_txn_in_progress();
- if (cfg_session_id != MGMTD_SESSION_ID_NONE) {
- assert(cfg_session_id != session->session_id);
- mgmt_fe_send_setcfg_reply(
- session, setcfg_req->ds_id, setcfg_req->req_id,
- false,
- "Configuration already in-progress through a different user session!",
- setcfg_req->implicit_commit);
- goto mgmt_fe_sess_handle_setcfg_req_failed;
- }
-
+ /* as we have the lock no-one else should have a config txn */
+ assert(mgmt_config_txn_in_progress() == MGMTD_SESSION_ID_NONE);
- /*
- * Try taking write-lock on the requested DS (if not already).
- */
- if (!session->ds_locked[setcfg_req->ds_id]) {
- MGMTD_FE_ADAPTER_ERR(
- "SETCFG_REQ on session-id: %" PRIu64
- " without obtaining lock",
- session->session_id);
- if (mgmt_fe_session_write_lock_ds(setcfg_req->ds_id,
- ds_ctx, session)
- != 0) {
- mgmt_fe_send_setcfg_reply(
- session, setcfg_req->ds_id,
- setcfg_req->req_id, false,
- "Failed to lock the target DS",
- setcfg_req->implicit_commit);
- goto mgmt_fe_sess_handle_setcfg_req_failed;
- }
-
- session->ds_locked_implict[setcfg_req->ds_id] = true;
- }
-
- /*
- * Start a CONFIG Transaction (if not started already)
- */
+ /* Start a CONFIG Transaction (if not started already) */
session->cfg_txn_id = mgmt_create_txn(session->session_id,
MGMTD_TXN_TYPE_CONFIG);
if (session->cfg_txn_id == MGMTD_SESSION_ID_NONE) {
false,
"Failed to create a Configuration session!",
setcfg_req->implicit_commit);
- goto mgmt_fe_sess_handle_setcfg_req_failed;
+ return 0;
}
+ txn_created = true;
MGMTD_FE_ADAPTER_DBG("Created new Config txn-id: %" PRIu64
" for session-id %" PRIu64,
session->cfg_txn_id, session->session_id);
} else {
- MGMTD_FE_ADAPTER_ERR("Config txn-id: %" PRIu64
+ MGMTD_FE_ADAPTER_DBG("Config txn-id: %" PRIu64
" for session-id: %" PRIu64
" already created",
session->cfg_txn_id, session->session_id);
}
}
- dst_ds_ctx = 0;
- if (setcfg_req->implicit_commit) {
- dst_ds_ctx =
- mgmt_ds_get_ctx_by_id(mm, setcfg_req->commit_ds_id);
- if (!dst_ds_ctx) {
- mgmt_fe_send_setcfg_reply(
- session, setcfg_req->ds_id, setcfg_req->req_id,
- false, "No such commit DS exists!",
- setcfg_req->implicit_commit);
- return 0;
- }
- }
-
- /*
- * Create the SETConfig request under the transaction.
- */
+ /* Create the SETConfig request under the transaction. */
if (mgmt_txn_send_set_config_req(
session->cfg_txn_id, setcfg_req->req_id, setcfg_req->ds_id,
ds_ctx, setcfg_req->data, setcfg_req->n_data,
session, setcfg_req->ds_id, setcfg_req->req_id, false,
"Request processing for SET-CONFIG failed!",
setcfg_req->implicit_commit);
- goto mgmt_fe_sess_handle_setcfg_req_failed;
- }
-
- return 0;
-
-mgmt_fe_sess_handle_setcfg_req_failed:
- /*
- * Delete transaction created recently.
- */
- if (session->cfg_txn_id != MGMTD_TXN_ID_NONE)
- mgmt_destroy_txn(&session->cfg_txn_id);
- if (ds_ctx && session->ds_locked[setcfg_req->ds_id])
- mgmt_fe_session_unlock_ds(setcfg_req->ds_id, ds_ctx, session);
+ /* delete transaction if we just created it */
+ if (txn_created)
+ mgmt_destroy_txn(&session->cfg_txn_id);
+ }
return 0;
}
" for session-id: %" PRIu64,
session->txn_id, session->session_id);
} else {
- /* XXX chopps: Why would we already have a TXN here? */
MGMTD_FE_ADAPTER_DBG("Show txn-id: %" PRIu64
" for session-id: %" PRIu64
" already created",
if (mm->perf_stats_en)
gettimeofday(&session->adapter->cmt_stats.last_start, NULL);
session->adapter->cmt_stats.commit_cnt++;
- /*
- * Get the source DS handle.
- */
- src_ds_ctx = mgmt_ds_get_ctx_by_id(mm, commcfg_req->src_ds_id);
- if (!src_ds_ctx) {
- mgmt_fe_send_commitcfg_reply(
- session, commcfg_req->src_ds_id, commcfg_req->dst_ds_id,
- commcfg_req->req_id, MGMTD_INTERNAL_ERROR,
- commcfg_req->validate_only,
- "No such source DS exists!");
- return 0;
- }
- /*
- * Get the destination DS handle.
- */
- dst_ds_ctx = mgmt_ds_get_ctx_by_id(mm, commcfg_req->dst_ds_id);
- if (!dst_ds_ctx) {
+ /* Validate source and dest DS */
+ if (commcfg_req->src_ds_id != MGMTD_DS_CANDIDATE ||
+ commcfg_req->dst_ds_id != MGMTD_DS_RUNNING) {
mgmt_fe_send_commitcfg_reply(
session, commcfg_req->src_ds_id, commcfg_req->dst_ds_id,
commcfg_req->req_id, MGMTD_INTERNAL_ERROR,
commcfg_req->validate_only,
- "No such destination DS exists!");
+ "Source/Dest for commit must be candidate/running DS");
return 0;
}
+ src_ds_ctx = mgmt_ds_get_ctx_by_id(mm, commcfg_req->src_ds_id);
+ assert(src_ds_ctx);
+ dst_ds_ctx = mgmt_ds_get_ctx_by_id(mm, commcfg_req->dst_ds_id);
+ assert(dst_ds_ctx);
- /*
- * Next check first if the COMMCFG_REQ is for running DS
- * or not. Report failure if its not. MGMTD currently only
- * supports editing the Candidate DS.
- */
- if (commcfg_req->dst_ds_id != MGMTD_DS_RUNNING) {
+ /* User should have lock on both source and dest DS */
+ if (!session->ds_locked[commcfg_req->dst_ds_id] ||
+ !session->ds_locked[commcfg_req->src_ds_id]) {
mgmt_fe_send_commitcfg_reply(
session, commcfg_req->src_ds_id, commcfg_req->dst_ds_id,
- commcfg_req->req_id, MGMTD_INTERNAL_ERROR,
+ commcfg_req->req_id, MGMTD_DS_LOCK_FAILED,
commcfg_req->validate_only,
- "Set-Config on datastores other than Running DS not permitted!");
+ "Commit requires lock on candidate and/or running DS");
return 0;
}
session->cfg_txn_id, session->session_id);
}
-
- /*
- * Try taking write-lock on the destination DS (if not already).
- */
- if (!session->ds_locked[commcfg_req->dst_ds_id]) {
- if (mgmt_fe_session_write_lock_ds(commcfg_req->dst_ds_id,
- dst_ds_ctx, session)
- != 0) {
- mgmt_fe_send_commitcfg_reply(
- session, commcfg_req->src_ds_id,
- commcfg_req->dst_ds_id, commcfg_req->req_id,
- MGMTD_DS_LOCK_FAILED,
- commcfg_req->validate_only,
- "Failed to lock the destination DS!");
- return 0;
- }
-
- session->ds_locked_implict[commcfg_req->dst_ds_id] = true;
- }
-
/*
* Create COMMITConfig request under the transaction
*/
FOREACH_MGMTD_DS_ID (ds_id) {
if (session->ds_locked[ds_id]) {
locked = true;
- vty_out(vty, " %s\t\t\t%s\n",
- mgmt_ds_id2name(ds_id),
- session->ds_locked_implict
- [ds_id]
- ? "Implicit"
- : "Explicit");
+ vty_out(vty, " %s\n",
+ mgmt_ds_id2name(ds_id));
}
}
if (!locked)
}
src_ds_ctx = mgmt_ds_get_ctx_by_id(mm, MGMTD_DS_CANDIDATE);
- if (!src_ds_ctx) {
- vty_out(vty, "ERROR: Couldnot access Candidate datastore!\n");
- return -1;
- }
-
- /*
- * Note: Write lock on src_ds is not required. This is already
- * taken in 'conf te'.
- */
dst_ds_ctx = mgmt_ds_get_ctx_by_id(mm, MGMTD_DS_RUNNING);
- if (!dst_ds_ctx) {
- vty_out(vty, "ERROR: Couldnot access Running datastore!\n");
+ assert(src_ds_ctx);
+ assert(dst_ds_ctx);
+
+ ret = mgmt_ds_lock(src_ds_ctx, vty->mgmt_session_id);
+ if (ret != 0) {
+ vty_out(vty,
+ "Failed to lock the DS %u for rollback Reason: %s!\n",
+ MGMTD_DS_RUNNING, strerror(ret));
return -1;
}
ret = mgmt_ds_lock(dst_ds_ctx, vty->mgmt_session_id);
if (ret != 0) {
+ mgmt_ds_unlock(src_ds_ctx);
vty_out(vty,
"Failed to lock the DS %u for rollback Reason: %s!\n",
MGMTD_DS_RUNNING, strerror(ret));
ret = mgmt_ds_load_config_from_file(
src_ds_ctx, cmt_info->cmt_json_file, false);
if (ret != 0) {
- mgmt_ds_unlock(dst_ds_ctx);
vty_out(vty,
"Error with parsing the file with error code %d\n",
ret);
- return ret;
+ goto failed_unlock;
}
}
/* Internally trigger a commit-request. */
ret = mgmt_txn_rollback_trigger_cfg_apply(src_ds_ctx, dst_ds_ctx);
if (ret != 0) {
- mgmt_ds_unlock(dst_ds_ctx);
vty_out(vty,
"Error with creating commit apply txn with error code %d\n",
ret);
- return ret;
+ goto failed_unlock;
}
mgmt_history_dump_cmt_record_index();
- /* XXX chopps when does this get unlocked? */
+ /*
+ * TODO: Cleanup: the generic TXN code currently checks for rollback
+ * and does the unlock when it completes.
+ */
/*
* Block the rollback command from returning till the rollback
vty->mgmt_req_pending_cmd = "ROLLBACK";
rollback_vty = vty;
return 0;
+
+failed_unlock:
+ mgmt_ds_unlock(src_ds_ctx);
+ mgmt_ds_unlock(dst_ds_ctx);
+ return ret;
}
void mgmt_history_rollback_complete(bool success)
assert(mgmt_txn_reqs_count(&txn->set_cfg_reqs) == 1);
assert(txn_req->req.set_cfg->dst_ds_ctx);
- ret = mgmt_ds_lock(txn_req->req.set_cfg->dst_ds_ctx,
- txn->session_id);
- if (ret != 0) {
+ /* We expect the user to have locked the DST DS */
+ if (!mgmt_ds_is_locked(txn_req->req.set_cfg->dst_ds_ctx,
+ txn->session_id)) {
MGMTD_TXN_ERR(
- "Failed to lock DS %u txn-id: %" PRIu64
+ "DS %u not locked for implicit commit txn-id: %" PRIu64
" session-id: %" PRIu64 " err: %s",
txn_req->req.set_cfg->dst_ds_id,
txn->txn_id, txn->session_id,
strerror(ret));
mgmt_txn_send_commit_cfg_reply(
txn, MGMTD_DS_LOCK_FAILED,
- "Lock running DS before implicit commit failed!");
+ "running DS not locked for implicit commit");
goto mgmt_txn_process_set_cfg_done;
}
success = (result == MGMTD_SUCCESS || result == MGMTD_NO_CFG_CHANGES);
+ /* TODO: these replies should not be send if it's a rollback
+ * b/c right now that is special cased.. that special casing should be
+ * removed; however...
+ */
if (!txn->commit_cfg_req->req.commit_cfg.implicit && txn->session_id
+ && !txn->commit_cfg_req->req.commit_cfg.rollback
&& mgmt_fe_send_commit_cfg_reply(
txn->session_id, txn->txn_id,
txn->commit_cfg_req->req.commit_cfg.src_ds_id,
}
if (txn->commit_cfg_req->req.commit_cfg.implicit && txn->session_id
+ && !txn->commit_cfg_req->req.commit_cfg.rollback
&& mgmt_fe_send_set_cfg_reply(
txn->session_id, txn->txn_id,
txn->commit_cfg_req->req.commit_cfg.src_ds_id,
if (success) {
/* Stop the commit-timeout timer */
+ /* XXX why only on success? */
EVENT_OFF(txn->comm_cfg_timeout);
create_cmt_info_rec =
}
if (txn->commit_cfg_req->req.commit_cfg.rollback) {
+ mgmt_ds_unlock(txn->commit_cfg_req->req.commit_cfg.src_ds_ctx);
mgmt_ds_unlock(txn->commit_cfg_req->req.commit_cfg.dst_ds_ctx);
-
/*
* Resume processing the rollback command.
+ *
+ * TODO: there's no good reason to special case rollback, the
+ * rollback boolean should be passed back to the FE client and it
+ * can do the right thing.
*/
mgmt_history_rollback_complete(success);
}
- if (txn->commit_cfg_req->req.commit_cfg.implicit)
- mgmt_ds_unlock(txn->commit_cfg_req->req.commit_cfg.dst_ds_ctx);
-
txn->commit_cfg_req->req.commit_cfg.cmt_stats = NULL;
mgmt_txn_req_free(&txn->commit_cfg_req);
struct mgmt_ds_ctx *dst_ds_ctx)
{
static struct nb_config_cbs changes;
+ static struct mgmt_commit_stats dummy_stats;
+
struct nb_config_cbs *cfg_chgs = NULL;
struct mgmt_txn_ctx *txn;
struct mgmt_txn_req *txn_req;
- static struct mgmt_commit_stats dummy_stats;
memset(&changes, 0, sizeof(changes));
memset(&dummy_stats, 0, sizeof(dummy_stats));
// Add a route
vty = vty_new();
vty->type = vty::VTY_TERM;
- vty_config_enter(vty, true, false);
+ vty_config_enter(vty, true, false, false);
auto ret = cmd_execute(vty, "ip route 11.0.0.0/8 Null0", NULL, 0);
assert(!ret);
}
DEFUNSH(VTYSH_REALLYALL, vtysh_config_terminal, vtysh_config_terminal_cmd,
- "configure [terminal]",
+ "configure [terminal [file-lock]]",
"Configuration from vty interface\n"
+ "Configuration with locked datastores\n"
"Configuration terminal\n")
{
vty->node = CONFIG_NODE;
if (vty->node == CONFIG_NODE) {
/* resync in case one of the daemons is somewhere else */
vtysh_execute("end");
- vtysh_execute("configure");
+ vtysh_execute("configure terminal file-lock");
}
return CMD_SUCCESS;
}
vty->node = CONFIG_NODE;
vtysh_execute_no_pager("enable");
- vtysh_execute_no_pager("configure terminal");
+ vtysh_execute_no_pager("configure terminal file-lock");
if (!dry_run)
vtysh_execute_no_pager("XFRR_start_configuration");
projects / matthieu / frr.git / commitdiff