diff options
Diffstat (limited to 'mgmtd')
| -rw-r--r-- | mgmtd/mgmt_be_adapter.c | 11 | ||||
| -rw-r--r-- | mgmtd/mgmt_be_adapter.h | 6 | ||||
| -rw-r--r-- | mgmtd/mgmt_ds.c | 130 | ||||
| -rw-r--r-- | mgmtd/mgmt_ds.h | 21 | ||||
| -rw-r--r-- | mgmtd/mgmt_fe_adapter.c | 481 | ||||
| -rw-r--r-- | mgmtd/mgmt_history.c | 38 | ||||
| -rw-r--r-- | mgmtd/mgmt_txn.c | 175 | ||||
| -rw-r--r-- | mgmtd/mgmt_txn.h | 21 |
8 files changed, 308 insertions, 575 deletions
diff --git a/mgmtd/mgmt_be_adapter.c b/mgmtd/mgmt_be_adapter.c index 49a307e9c2..512cc49feb 100644 --- a/mgmtd/mgmt_be_adapter.c +++ b/mgmtd/mgmt_be_adapter.c @@ -648,8 +648,7 @@ static void mgmt_be_adapter_process_msg(uint8_t version, uint8_t *data, mgmtd__be_message__free_unpacked(be_msg, NULL); } -static void mgmt_be_iter_and_get_cfg(struct mgmt_ds_ctx *ds_ctx, - const char *xpath, struct lyd_node *node, +static void mgmt_be_iter_and_get_cfg(const char *xpath, struct lyd_node *node, struct nb_node *nb_node, void *ctx) { struct mgmt_be_get_adapter_config_params *parms = ctx; @@ -806,10 +805,10 @@ mgmt_be_get_adapter_by_name(const char *name) } int mgmt_be_get_adapter_config(struct mgmt_be_client_adapter *adapter, - struct mgmt_ds_ctx *ds_ctx, - struct nb_config_cbs **cfg_chgs) + struct nb_config_cbs **cfg_chgs) { struct mgmt_be_get_adapter_config_params parms; + struct nb_config *cfg_root = mgmt_ds_get_nb_config(mm->running_ds); assert(cfg_chgs); @@ -825,8 +824,8 @@ int mgmt_be_get_adapter_config(struct mgmt_be_client_adapter *adapter, parms.cfg_chgs = &adapter->cfg_chgs; parms.seq = 0; - mgmt_ds_iter_data(ds_ctx, "", mgmt_be_iter_and_get_cfg, - (void *)&parms); + mgmt_ds_iter_data(MGMTD_DS_RUNNING, cfg_root, "", + mgmt_be_iter_and_get_cfg, (void *)&parms); } *cfg_chgs = &adapter->cfg_chgs; diff --git a/mgmtd/mgmt_be_adapter.h b/mgmtd/mgmt_be_adapter.h index e1676e63af..ca8f55c457 100644 --- a/mgmtd/mgmt_be_adapter.h +++ b/mgmtd/mgmt_be_adapter.h @@ -110,10 +110,8 @@ extern struct mgmt_be_client_adapter * mgmt_be_get_adapter_by_id(enum mgmt_be_client_id id); /* Fetch backend adapter config. */ -extern int -mgmt_be_get_adapter_config(struct mgmt_be_client_adapter *adapter, - struct mgmt_ds_ctx *ds_ctx, - struct nb_config_cbs **cfg_chgs); +extern int mgmt_be_get_adapter_config(struct mgmt_be_client_adapter *adapter, + struct nb_config_cbs **cfg_chgs); /* Create/destroy a transaction. */ extern int mgmt_be_send_txn_req(struct mgmt_be_client_adapter *adapter, diff --git a/mgmtd/mgmt_ds.c b/mgmtd/mgmt_ds.c index 5a4b00d309..a0e610c7c7 100644 --- a/mgmtd/mgmt_ds.c +++ b/mgmtd/mgmt_ds.c @@ -22,7 +22,9 @@ struct mgmt_ds_ctx { Mgmtd__DatastoreId ds_id; - int lock; /* 0 unlocked, >0 read locked < write locked */ + + bool locked; + uint64_t vty_session_id; /* Owner of the lock or 0 */ bool config_ds; @@ -76,29 +78,20 @@ static int mgmt_ds_dump_in_memory(struct mgmt_ds_ctx *ds_ctx, static int mgmt_ds_replace_dst_with_src_ds(struct mgmt_ds_ctx *src, struct mgmt_ds_ctx *dst) { - struct lyd_node *dst_dnode, *src_dnode; - if (!src || !dst) return -1; MGMTD_DS_DBG("Replacing %s with %s", mgmt_ds_id2name(dst->ds_id), mgmt_ds_id2name(src->ds_id)); - src_dnode = src->config_ds ? src->root.cfg_root->dnode - : dst->root.dnode_root; - dst_dnode = dst->config_ds ? dst->root.cfg_root->dnode - : dst->root.dnode_root; - - if (dst_dnode) - yang_dnode_free(dst_dnode); - - /* Not using nb_config_replace as the oper ds does not contain nb_config - */ - dst_dnode = yang_dnode_dup(src_dnode); - if (dst->config_ds) - dst->root.cfg_root->dnode = dst_dnode; - else - dst->root.dnode_root = dst_dnode; + if (src->config_ds && dst->config_ds) + nb_config_replace(dst->root.cfg_root, src->root.cfg_root, true); + else { + assert(!src->config_ds && !dst->config_ds); + if (dst->root.dnode_root) + yang_dnode_free(dst->root.dnode_root); + dst->root.dnode_root = yang_dnode_dup(src->root.dnode_root); + } if (src->ds_id == MGMTD_DS_CANDIDATE) { /* @@ -108,8 +101,6 @@ static int mgmt_ds_replace_dst_with_src_ds(struct mgmt_ds_ctx *src, nb_config_diff_del_changes(&src->root.cfg_root->cfg_chgs); } - /* TODO: Update the versions if nb_config present */ - return 0; } @@ -117,20 +108,21 @@ static int mgmt_ds_merge_src_with_dst_ds(struct mgmt_ds_ctx *src, struct mgmt_ds_ctx *dst) { int ret; - struct lyd_node **dst_dnode, *src_dnode; if (!src || !dst) return -1; MGMTD_DS_DBG("Merging DS %d with %d", dst->ds_id, src->ds_id); - - src_dnode = src->config_ds ? src->root.cfg_root->dnode - : dst->root.dnode_root; - dst_dnode = dst->config_ds ? &dst->root.cfg_root->dnode - : &dst->root.dnode_root; - ret = lyd_merge_siblings(dst_dnode, src_dnode, 0); + if (src->config_ds && dst->config_ds) + ret = nb_config_merge(dst->root.cfg_root, src->root.cfg_root, + true); + else { + assert(!src->config_ds && !dst->config_ds); + ret = lyd_merge_siblings(&dst->root.dnode_root, + src->root.dnode_root, 0); + } if (ret != 0) { - MGMTD_DS_ERR("lyd_merge() failed with err %d", ret); + MGMTD_DS_ERR("merge failed with err: %d", ret); return ret; } @@ -212,9 +204,11 @@ int mgmt_ds_init(struct mgmt_master *mm) void mgmt_ds_destroy(void) { - /* - * TODO: Free the datastores. - */ + nb_config_free(candidate.root.cfg_root); + candidate.root.cfg_root = NULL; + + yang_dnode_free(oper.root.dnode_root); + oper.root.dnode_root = NULL; } struct mgmt_ds_ctx *mgmt_ds_get_ctx_by_id(struct mgmt_master *mm, @@ -244,40 +238,33 @@ bool mgmt_ds_is_config(struct mgmt_ds_ctx *ds_ctx) return ds_ctx->config_ds; } -int mgmt_ds_read_lock(struct mgmt_ds_ctx *ds_ctx) +bool mgmt_ds_is_locked(struct mgmt_ds_ctx *ds_ctx, uint64_t session_id) { - if (!ds_ctx) - return EINVAL; - if (ds_ctx->lock < 0) - return EBUSY; - ++ds_ctx->lock; - return 0; + assert(ds_ctx); + return (ds_ctx->locked && ds_ctx->vty_session_id == session_id); } -int mgmt_ds_write_lock(struct mgmt_ds_ctx *ds_ctx) +int mgmt_ds_lock(struct mgmt_ds_ctx *ds_ctx, uint64_t session_id) { - if (!ds_ctx) - return EINVAL; - if (ds_ctx->lock != 0) + assert(ds_ctx); + + if (ds_ctx->locked) return EBUSY; - ds_ctx->lock = -1; + + ds_ctx->locked = true; + ds_ctx->vty_session_id = session_id; return 0; } -int mgmt_ds_unlock(struct mgmt_ds_ctx *ds_ctx) +void mgmt_ds_unlock(struct mgmt_ds_ctx *ds_ctx) { - if (!ds_ctx) - return EINVAL; - if (ds_ctx->lock > 0) - --ds_ctx->lock; - else if (ds_ctx->lock < 0) { - assert(ds_ctx->lock == -1); - ds_ctx->lock = 0; - } else { - assert(ds_ctx->lock != 0); - return EINVAL; - } - return 0; + assert(ds_ctx); + if (!ds_ctx->locked) + zlog_warn( + "%s: WARNING: unlock on unlocked in DS:%s last session-id %" PRIu64, + __func__, mgmt_ds_id2name(ds_ctx->ds_id), + ds_ctx->vty_session_id); + ds_ctx->locked = 0; } int mgmt_ds_copy_dss(struct mgmt_ds_ctx *src_ds_ctx, @@ -314,10 +301,9 @@ struct nb_config *mgmt_ds_get_nb_config(struct mgmt_ds_ctx *ds_ctx) } static int mgmt_walk_ds_nodes( - struct mgmt_ds_ctx *ds_ctx, const char *base_xpath, + struct nb_config *root, const char *base_xpath, struct lyd_node *base_dnode, - void (*mgmt_ds_node_iter_fn)(struct mgmt_ds_ctx *ds_ctx, - const char *xpath, struct lyd_node *node, + void (*mgmt_ds_node_iter_fn)(const char *xpath, struct lyd_node *node, struct nb_node *nb_node, void *ctx), void *ctx) { @@ -336,10 +322,7 @@ static int mgmt_walk_ds_nodes( * This function only returns the first node of a possible set * of matches issuing a warning if more than 1 matches */ - base_dnode = yang_dnode_get( - ds_ctx->config_ds ? ds_ctx->root.cfg_root->dnode - : ds_ctx->root.dnode_root, - base_xpath); + base_dnode = yang_dnode_get(root->dnode, base_xpath); if (!base_dnode) return -1; @@ -348,7 +331,7 @@ static int mgmt_walk_ds_nodes( sizeof(xpath))); nbnode = (struct nb_node *)base_dnode->schema->priv; - (*mgmt_ds_node_iter_fn)(ds_ctx, base_xpath, base_dnode, nbnode, ctx); + (*mgmt_ds_node_iter_fn)(base_xpath, base_dnode, nbnode, ctx); /* * If the base_xpath points to a leaf node we can skip the tree walk. @@ -370,7 +353,7 @@ static int mgmt_walk_ds_nodes( MGMTD_DS_DBG(" -- Child xpath: %s", xpath); - ret = mgmt_walk_ds_nodes(ds_ctx, xpath, dnode, + ret = mgmt_walk_ds_nodes(root, xpath, dnode, mgmt_ds_node_iter_fn, ctx); if (ret != 0) break; @@ -459,9 +442,9 @@ int mgmt_ds_load_config_from_file(struct mgmt_ds_ctx *dst, return 0; } -int mgmt_ds_iter_data(struct mgmt_ds_ctx *ds_ctx, const char *base_xpath, - void (*mgmt_ds_node_iter_fn)(struct mgmt_ds_ctx *ds_ctx, - const char *xpath, +int mgmt_ds_iter_data(Mgmtd__DatastoreId ds_id, struct nb_config *root, + const char *base_xpath, + void (*mgmt_ds_node_iter_fn)(const char *xpath, struct lyd_node *node, struct nb_node *nb_node, void *ctx), @@ -472,7 +455,7 @@ int mgmt_ds_iter_data(struct mgmt_ds_ctx *ds_ctx, const char *base_xpath, struct lyd_node *base_dnode = NULL; struct lyd_node *node; - if (!ds_ctx) + if (!root) return -1; strlcpy(xpath, base_xpath, sizeof(xpath)); @@ -484,12 +467,11 @@ int mgmt_ds_iter_data(struct mgmt_ds_ctx *ds_ctx, const char *base_xpath, * Oper-state should be kept in mind though for the prefix walk */ - MGMTD_DS_DBG(" -- START DS walk for DSid: %d", ds_ctx->ds_id); + MGMTD_DS_DBG(" -- START DS walk for DSid: %d", ds_id); /* If the base_xpath is empty then crawl the sibblings */ if (xpath[0] == 0) { - base_dnode = ds_ctx->config_ds ? ds_ctx->root.cfg_root->dnode - : ds_ctx->root.dnode_root; + base_dnode = root->dnode; /* get first top-level sibling */ while (base_dnode->parent) @@ -499,11 +481,11 @@ int mgmt_ds_iter_data(struct mgmt_ds_ctx *ds_ctx, const char *base_xpath, base_dnode = base_dnode->prev; LY_LIST_FOR (base_dnode, node) { - ret = mgmt_walk_ds_nodes(ds_ctx, xpath, node, + ret = mgmt_walk_ds_nodes(root, xpath, node, mgmt_ds_node_iter_fn, ctx); } } else - ret = mgmt_walk_ds_nodes(ds_ctx, xpath, base_dnode, + ret = mgmt_walk_ds_nodes(root, xpath, base_dnode, mgmt_ds_node_iter_fn, ctx); return ret; diff --git a/mgmtd/mgmt_ds.h b/mgmtd/mgmt_ds.h index e5c88742dd..1cf4816027 100644 --- a/mgmtd/mgmt_ds.h +++ b/mgmtd/mgmt_ds.h @@ -179,19 +179,19 @@ extern struct mgmt_ds_ctx *mgmt_ds_get_ctx_by_id(struct mgmt_master *mm, extern bool mgmt_ds_is_config(struct mgmt_ds_ctx *ds_ctx); /* - * Acquire read lock to a ds given a ds_handle + * Check if a given datastore is locked by a given session */ -extern int mgmt_ds_read_lock(struct mgmt_ds_ctx *ds_ctx); +extern bool mgmt_ds_is_locked(struct mgmt_ds_ctx *ds_ctx, uint64_t session_id); /* * Acquire write lock to a ds given a ds_handle */ -extern int mgmt_ds_write_lock(struct mgmt_ds_ctx *ds_ctx); +extern int mgmt_ds_lock(struct mgmt_ds_ctx *ds_ctx, uint64_t session_id); /* * Remove a lock from ds given a ds_handle */ -extern int mgmt_ds_unlock(struct mgmt_ds_ctx *ds_ctx); +extern void mgmt_ds_unlock(struct mgmt_ds_ctx *ds_ctx); /* * Copy from source to destination datastore. @@ -233,8 +233,11 @@ extern int mgmt_ds_delete_data_nodes(struct mgmt_ds_ctx *ds_ctx, /* * Iterate over datastore data. * - * ds_ctx - * Datastore context. + * ds_id + * Datastore ID.. + * + * root + * The root of the tree to iterate over. * * base_xpath * Base YANG xpath from where needs to be iterated. @@ -252,9 +255,9 @@ extern int mgmt_ds_delete_data_nodes(struct mgmt_ds_ctx *ds_ctx, * 0 on success, -1 on failure. */ extern int mgmt_ds_iter_data( - struct mgmt_ds_ctx *ds_ctx, const char *base_xpath, - void (*mgmt_ds_node_iter_fn)(struct mgmt_ds_ctx *ds_ctx, - const char *xpath, struct lyd_node *node, + Mgmtd__DatastoreId ds_id, struct nb_config *root, + const char *base_xpath, + void (*mgmt_ds_node_iter_fn)(const char *xpath, struct lyd_node *node, struct nb_node *nb_node, void *ctx), void *ctx); diff --git a/mgmtd/mgmt_fe_adapter.c b/mgmtd/mgmt_fe_adapter.c index e9cbd444e8..70c08d5cb4 100644 --- a/mgmtd/mgmt_fe_adapter.c +++ b/mgmtd/mgmt_fe_adapter.c @@ -40,9 +40,7 @@ struct mgmt_fe_session_ctx { uint64_t client_id; uint64_t txn_id; uint64_t cfg_txn_id; - uint8_t ds_write_locked[MGMTD_DS_MAX_ID]; - uint8_t ds_read_locked[MGMTD_DS_MAX_ID]; - uint8_t ds_locked_implict[MGMTD_DS_MAX_ID]; + uint8_t ds_locked[MGMTD_DS_MAX_ID]; struct event *proc_cfg_txn_clnp; struct event *proc_show_txn_clnp; @@ -72,8 +70,12 @@ mgmt_fe_session_write_lock_ds(Mgmtd__DatastoreId ds_id, struct mgmt_ds_ctx *ds_ctx, struct mgmt_fe_session_ctx *session) { - if (!session->ds_write_locked[ds_id]) { - if (mgmt_ds_write_lock(ds_ctx) != 0) { + if (session->ds_locked[ds_id]) + zlog_warn("multiple lock taken by session-id: %" PRIu64 + " on DS:%s", + session->session_id, mgmt_ds_id2name(ds_id)); + else { + if (mgmt_ds_lock(ds_ctx, session->session_id)) { MGMTD_FE_ADAPTER_DBG( "Failed to lock the DS:%s for session-id: %" PRIu64 " from %s!", @@ -82,7 +84,7 @@ mgmt_fe_session_write_lock_ds(Mgmtd__DatastoreId ds_id, return -1; } - session->ds_write_locked[ds_id] = true; + session->ds_locked[ds_id] = true; MGMTD_FE_ADAPTER_DBG( "Write-Locked the DS:%s for session-id: %" PRIu64 " from %s", @@ -93,97 +95,32 @@ mgmt_fe_session_write_lock_ds(Mgmtd__DatastoreId ds_id, return 0; } -static int -mgmt_fe_session_read_lock_ds(Mgmtd__DatastoreId ds_id, - struct mgmt_ds_ctx *ds_ctx, - struct mgmt_fe_session_ctx *session) -{ - if (!session->ds_read_locked[ds_id]) { - if (mgmt_ds_read_lock(ds_ctx) != 0) { - MGMTD_FE_ADAPTER_DBG( - "Failed to lock the DS:%s for session-is: %" PRIu64 - " from %s", - mgmt_ds_id2name(ds_id), session->session_id, - session->adapter->name); - return -1; - } - - session->ds_read_locked[ds_id] = true; - MGMTD_FE_ADAPTER_DBG( - "Read-Locked the DS:%s for session-id: %" PRIu64 - " from %s", - mgmt_ds_id2name(ds_id), session->session_id, - session->adapter->name); - } - - return 0; -} - -static int mgmt_fe_session_unlock_ds(Mgmtd__DatastoreId ds_id, - struct mgmt_ds_ctx *ds_ctx, - struct mgmt_fe_session_ctx *session, - bool unlock_write, bool unlock_read) +static void mgmt_fe_session_unlock_ds(Mgmtd__DatastoreId ds_id, + struct mgmt_ds_ctx *ds_ctx, + struct mgmt_fe_session_ctx *session) { - if (unlock_write && session->ds_write_locked[ds_id]) { - session->ds_write_locked[ds_id] = false; - session->ds_locked_implict[ds_id] = false; - if (mgmt_ds_unlock(ds_ctx) != 0) { - MGMTD_FE_ADAPTER_DBG( - "Failed to unlock the DS:%s taken earlier by session-id: %" PRIu64 - " from %s", - mgmt_ds_id2name(ds_id), session->session_id, - session->adapter->name); - return -1; - } + if (!session->ds_locked[ds_id]) + zlog_warn("unlock unlocked by session-id: %" PRIu64 " on DS:%s", + session->session_id, mgmt_ds_id2name(ds_id)); - MGMTD_FE_ADAPTER_DBG( - "Unlocked DS:%s write-locked earlier by session-id: %" PRIu64 - " from %s", - mgmt_ds_id2name(ds_id), session->session_id, - session->adapter->name); - } else if (unlock_read && session->ds_read_locked[ds_id]) { - session->ds_read_locked[ds_id] = false; - session->ds_locked_implict[ds_id] = false; - if (mgmt_ds_unlock(ds_ctx) != 0) { - MGMTD_FE_ADAPTER_DBG( - "Failed to unlock the DS:%s taken earlier by session-id: %" PRIu64 - " from %s", - mgmt_ds_id2name(ds_id), session->session_id, - session->adapter->name); - return -1; - } - - MGMTD_FE_ADAPTER_DBG( - "Unlocked DS:%s read-locked earlier by session-id: %" PRIu64 - " from %s", - mgmt_ds_id2name(ds_id), session->session_id, - session->adapter->name); - } - - return 0; + session->ds_locked[ds_id] = false; + mgmt_ds_unlock(ds_ctx); + MGMTD_FE_ADAPTER_DBG( + "Unlocked DS:%s write-locked earlier by session-id: %" PRIu64 + " from %s", + mgmt_ds_id2name(ds_id), session->session_id, + session->adapter->name); } 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) { - if (session->ds_locked_implict[ds_id]) - mgmt_fe_session_unlock_ds( - ds_id, ds_ctx, session, true, false); - } - } - /* * Destroy the actual transaction created earlier. */ @@ -194,17 +131,6 @@ mgmt_fe_session_cfg_txn_cleanup(struct mgmt_fe_session_ctx *session) static void mgmt_fe_session_show_txn_cleanup(struct mgmt_fe_session_ctx *session) { - Mgmtd__DatastoreId ds_id; - struct mgmt_ds_ctx *ds_ctx; - - 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) { - mgmt_fe_session_unlock_ds(ds_id, ds_ctx, session, - false, true); - } - } - /* * Destroy the transaction created recently. */ @@ -245,25 +171,29 @@ mgmt_fe_session_compute_commit_timers(struct mgmt_commit_stats *cmt_stats) } } -static void mgmt_fe_cleanup_session(struct mgmt_fe_session_ctx **session) +static void mgmt_fe_cleanup_session(struct mgmt_fe_session_ctx **sessionp) { - if ((*session)->adapter) { - mgmt_fe_session_cfg_txn_cleanup((*session)); - mgmt_fe_session_show_txn_cleanup((*session)); - mgmt_fe_session_unlock_ds(MGMTD_DS_CANDIDATE, mm->candidate_ds, - *session, true, true); - mgmt_fe_session_unlock_ds(MGMTD_DS_RUNNING, mm->running_ds, - *session, true, true); - - mgmt_fe_sessions_del(&(*session)->adapter->fe_sessions, - *session); - assert((*session)->adapter->refcount > 1); - mgmt_fe_adapter_unlock(&(*session)->adapter); + Mgmtd__DatastoreId ds_id; + struct mgmt_ds_ctx *ds_ctx; + struct mgmt_fe_session_ctx *session = *sessionp; + + if (session->adapter) { + mgmt_fe_session_cfg_txn_cleanup(session); + mgmt_fe_session_show_txn_cleanup(session); + 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[ds_id]) + mgmt_fe_session_unlock_ds(ds_id, ds_ctx, + session); + } + mgmt_fe_sessions_del(&session->adapter->fe_sessions, session); + assert(session->adapter->refcount > 1); + mgmt_fe_adapter_unlock(&session->adapter); } - hash_release(mgmt_fe_sessions, *session); - XFREE(MTYPE_MGMTD_FE_SESSION, *session); - *session = NULL; + hash_release(mgmt_fe_sessions, session); + XFREE(MTYPE_MGMTD_FE_SESSION, session); + *sessionp = NULL; } static struct mgmt_fe_session_ctx * @@ -389,6 +319,7 @@ static int mgmt_fe_send_lockds_reply(struct mgmt_fe_session_ctx *session, { Mgmtd__FeMessage fe_msg; Mgmtd__FeLockDsReply lockds_reply; + bool scok = session->adapter->conn->is_short_circuit; assert(session->adapter); @@ -406,10 +337,10 @@ static int mgmt_fe_send_lockds_reply(struct mgmt_fe_session_ctx *session, fe_msg.lockds_reply = &lockds_reply; MGMTD_FE_ADAPTER_DBG( - "Sending LOCK_DS_REPLY message to MGMTD Frontend client '%s'", - session->adapter->name); + "Sending LOCK_DS_REPLY message to MGMTD Frontend client '%s' scok: %d", + session->adapter->name, scok); - return mgmt_fe_adapter_send_msg(session->adapter, &fe_msg, false); + return mgmt_fe_adapter_send_msg(session->adapter, &fe_msg, scok); } static int mgmt_fe_send_setcfg_reply(struct mgmt_fe_session_ctx *session, @@ -432,6 +363,7 @@ static int mgmt_fe_send_setcfg_reply(struct mgmt_fe_session_ctx *session, 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; @@ -670,8 +602,7 @@ static int mgmt_fe_adapter_notify_disconnect(struct msg_conn *conn) } /* - * 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) @@ -679,17 +610,16 @@ 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); } } @@ -699,16 +629,12 @@ mgmt_fe_session_handle_lockds_req_msg(struct mgmt_fe_session_ctx *session, { 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; } @@ -731,10 +657,8 @@ mgmt_fe_session_handle_lockds_req_msg(struct mgmt_fe_session_ctx *session, "Lock already taken on DS by another session!"); return -1; } - - session->ds_locked_implict[lockds_req->ds_id] = false; } else { - if (!session->ds_write_locked[lockds_req->ds_id]) { + if (!session->ds_locked[lockds_req->ds_id]) { mgmt_fe_send_lockds_reply( session, lockds_req->ds_id, lockds_req->req_id, lockds_req->lock, false, @@ -742,8 +666,7 @@ mgmt_fe_session_handle_lockds_req_msg(struct mgmt_fe_session_ctx *session, return 0; } - (void)mgmt_fe_session_unlock_ds(lockds_req->ds_id, ds_ctx, - session, true, false); + mgmt_fe_session_unlock_ds(lockds_req->ds_id, ds_ctx, session); } if (mgmt_fe_send_lockds_reply(session, lockds_req->ds_id, @@ -769,79 +692,49 @@ static int 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_write_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 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) { @@ -850,14 +743,15 @@ mgmt_fe_session_handle_setcfg_req_msg(struct mgmt_fe_session_ctx *session, 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); @@ -876,22 +770,7 @@ mgmt_fe_session_handle_setcfg_req_msg(struct mgmt_fe_session_ctx *session, } } - 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, @@ -902,21 +781,11 @@ mgmt_fe_session_handle_setcfg_req_msg(struct mgmt_fe_session_ctx *session, 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_write_locked[setcfg_req->ds_id]) - mgmt_fe_session_unlock_ds(setcfg_req->ds_id, ds_ctx, session, - true, false); + /* delete transaction if we just created it */ + if (txn_created) + mgmt_destroy_txn(&session->cfg_txn_id); + } return 0; } @@ -926,6 +795,7 @@ mgmt_fe_session_handle_getcfg_req_msg(struct mgmt_fe_session_ctx *session, Mgmtd__FeGetConfigReq *getcfg_req) { struct mgmt_ds_ctx *ds_ctx; + struct nb_config *cfg_root = NULL; /* * Get the DS handle. @@ -938,11 +808,7 @@ mgmt_fe_session_handle_getcfg_req_msg(struct mgmt_fe_session_ctx *session, return 0; } - /* - * Next check first if the GETCFG_REQ is for Candidate DS - * or not. Report failure if its not. MGMTD currently only - * supports editing the Candidate DS. - */ + /* GETCFG must be on candidate or running DS */ if (getcfg_req->ds_id != MGMTD_DS_CANDIDATE && getcfg_req->ds_id != MGMTD_DS_RUNNING) { mgmt_fe_send_getcfg_reply( @@ -954,27 +820,6 @@ mgmt_fe_session_handle_getcfg_req_msg(struct mgmt_fe_session_ctx *session, if (session->txn_id == MGMTD_TXN_ID_NONE) { /* - * Try taking read-lock on the requested DS (if not already - * locked). If the DS has already been write-locked by a ongoing - * CONFIG transaction we may allow reading the contents of the - * same DS. - */ - if (!session->ds_read_locked[getcfg_req->ds_id] - && !session->ds_write_locked[getcfg_req->ds_id]) { - if (mgmt_fe_session_read_lock_ds(getcfg_req->ds_id, - ds_ctx, session) - != 0) { - mgmt_fe_send_getcfg_reply( - session, getcfg_req->ds_id, - getcfg_req->req_id, false, NULL, - "Failed to lock the DS! Another session might have locked it!"); - goto mgmt_fe_sess_handle_getcfg_req_failed; - } - - session->ds_locked_implict[getcfg_req->ds_id] = true; - } - - /* * Start a SHOW Transaction (if not started already) */ session->txn_id = mgmt_create_txn(session->session_id, @@ -998,12 +843,16 @@ mgmt_fe_session_handle_getcfg_req_msg(struct mgmt_fe_session_ctx *session, } /* + * Get a copy of the datastore config root, avoids locking. + */ + cfg_root = nb_config_dup(mgmt_ds_get_nb_config(ds_ctx)); + + /* * Create a GETConfig request under the transaction. */ - if (mgmt_txn_send_get_config_req(session->txn_id, getcfg_req->req_id, - getcfg_req->ds_id, ds_ctx, - getcfg_req->data, getcfg_req->n_data) - != 0) { + if (mgmt_txn_send_get_config_req( + session->txn_id, getcfg_req->req_id, getcfg_req->ds_id, + cfg_root, getcfg_req->data, getcfg_req->n_data) != 0) { mgmt_fe_send_getcfg_reply( session, getcfg_req->ds_id, getcfg_req->req_id, false, NULL, "Request processing for GET-CONFIG failed!"); @@ -1014,14 +863,13 @@ mgmt_fe_session_handle_getcfg_req_msg(struct mgmt_fe_session_ctx *session, mgmt_fe_sess_handle_getcfg_req_failed: + if (cfg_root) + nb_config_free(cfg_root); /* * Destroy the transaction created recently. */ if (session->txn_id != MGMTD_TXN_ID_NONE) mgmt_destroy_txn(&session->txn_id); - if (ds_ctx && session->ds_read_locked[getcfg_req->ds_id]) - mgmt_fe_session_unlock_ds(getcfg_req->ds_id, ds_ctx, session, - false, true); return -1; } @@ -1043,28 +891,16 @@ mgmt_fe_session_handle_getdata_req_msg(struct mgmt_fe_session_ctx *session, return 0; } - if (session->txn_id == MGMTD_TXN_ID_NONE) { - /* - * Try taking read-lock on the requested DS (if not already - * locked). If the DS has already been write-locked by a ongoing - * CONFIG transaction we may allow reading the contents of the - * same DS. - */ - if (!session->ds_read_locked[getdata_req->ds_id] - && !session->ds_write_locked[getdata_req->ds_id]) { - if (mgmt_fe_session_read_lock_ds(getdata_req->ds_id, - ds_ctx, session) - != 0) { - mgmt_fe_send_getdata_reply( - session, getdata_req->ds_id, - getdata_req->req_id, false, NULL, - "Failed to lock the DS! Another session might have locked it!"); - goto mgmt_fe_sess_handle_getdata_req_failed; - } - - session->ds_locked_implict[getdata_req->ds_id] = true; - } + /* GETDATA must be on operational DS */ + if (getdata_req->ds_id != MGMTD_DS_OPERATIONAL) { + mgmt_fe_send_getdata_reply( + session, getdata_req->ds_id, getdata_req->req_id, false, + NULL, + "Get-Data on datastore other than Operational DS not permitted!"); + return 0; + } + if (session->txn_id == MGMTD_TXN_ID_NONE) { /* * Start a SHOW Transaction (if not started already) */ @@ -1091,9 +927,8 @@ mgmt_fe_session_handle_getdata_req_msg(struct mgmt_fe_session_ctx *session, * Create a GETData request under the transaction. */ if (mgmt_txn_send_get_data_req(session->txn_id, getdata_req->req_id, - getdata_req->ds_id, ds_ctx, - getdata_req->data, getdata_req->n_data) - != 0) { + getdata_req->ds_id, getdata_req->data, + getdata_req->n_data) != 0) { mgmt_fe_send_getdata_reply( session, getdata_req->ds_id, getdata_req->req_id, false, NULL, "Request processing for GET-CONFIG failed!"); @@ -1110,10 +945,6 @@ mgmt_fe_sess_handle_getdata_req_failed: if (session->txn_id != MGMTD_TXN_ID_NONE) mgmt_destroy_txn(&session->txn_id); - if (ds_ctx && session->ds_read_locked[getdata_req->ds_id]) - mgmt_fe_session_unlock_ds(getdata_req->ds_id, ds_ctx, - session, false, true); - return -1; } @@ -1126,43 +957,30 @@ static int mgmt_fe_session_handle_commit_config_req_msg( 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; } @@ -1187,26 +1005,6 @@ static int mgmt_fe_session_handle_commit_config_req_msg( session->cfg_txn_id, session->session_id); } - - /* - * Try taking write-lock on the destination DS (if not already). - */ - if (!session->ds_write_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 */ @@ -1214,8 +1012,7 @@ static int mgmt_fe_session_handle_commit_config_req_msg( session->cfg_txn_id, commcfg_req->req_id, commcfg_req->src_ds_id, src_ds_ctx, commcfg_req->dst_ds_id, dst_ds_ctx, commcfg_req->validate_only, commcfg_req->abort, - false) - != 0) { + false) != 0) { mgmt_fe_send_commitcfg_reply( session, commcfg_req->src_ds_id, commcfg_req->dst_ds_id, commcfg_req->req_id, MGMTD_INTERNAL_ERROR, @@ -1743,18 +1540,10 @@ void mgmt_fe_adapter_status_write(struct vty *vty, bool detail) session->session_id); vty_out(vty, " DS-Locks:\n"); FOREACH_MGMTD_DS_ID (ds_id) { - if (session->ds_write_locked[ds_id] - || session->ds_read_locked[ds_id]) { + if (session->ds_locked[ds_id]) { locked = true; - vty_out(vty, - " %s\t\t\t%s, %s\n", - mgmt_ds_id2name(ds_id), - session->ds_write_locked[ds_id] - ? "Write" - : "Read", - session->ds_locked_implict[ds_id] - ? "Implicit" - : "Explicit"); + vty_out(vty, " %s\n", + mgmt_ds_id2name(ds_id)); } } if (!locked) diff --git a/mgmtd/mgmt_history.c b/mgmtd/mgmt_history.c index 54eb45fdf4..d4069325ca 100644 --- a/mgmtd/mgmt_history.c +++ b/mgmtd/mgmt_history.c @@ -196,23 +196,21 @@ static int mgmt_history_rollback_to_cmt(struct vty *vty, } 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_write_lock(dst_ds_ctx); + 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)); @@ -223,27 +221,30 @@ static int mgmt_history_rollback_to_cmt(struct vty *vty, 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(); /* + * 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 * is completed. On rollback completion mgmt_history_rollback_complete() * shall be called to resume the rollback command return to VTYSH. @@ -251,6 +252,11 @@ static int mgmt_history_rollback_to_cmt(struct vty *vty, 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) diff --git a/mgmtd/mgmt_txn.c b/mgmtd/mgmt_txn.c index e64cbe1425..de1ffa1a1f 100644 --- a/mgmtd/mgmt_txn.c +++ b/mgmtd/mgmt_txn.c @@ -164,7 +164,7 @@ struct mgmt_get_data_reply { struct mgmt_get_data_req { Mgmtd__DatastoreId ds_id; - struct mgmt_ds_ctx *ds_ctx; + struct nb_config *cfg_root; char *xpaths[MGMTD_MAX_NUM_DATA_REQ_IN_BATCH]; int num_xpaths; @@ -576,6 +576,10 @@ static void mgmt_txn_req_free(struct mgmt_txn_req **txn_req) if ((*txn_req)->req.get_data->reply) XFREE(MTYPE_MGMTD_TXN_GETDATA_REPLY, (*txn_req)->req.get_data->reply); + + if ((*txn_req)->req.get_data->cfg_root) + nb_config_free((*txn_req)->req.get_data->cfg_root); + XFREE(MTYPE_MGMTD_TXN_GETDATA_REQ, (*txn_req)->req.get_data); break; case MGMTD_TXN_PROC_GETDATA: @@ -683,18 +687,18 @@ static void mgmt_txn_process_set_cfg(struct event *thread) assert(mgmt_txn_reqs_count(&txn->set_cfg_reqs) == 1); assert(txn_req->req.set_cfg->dst_ds_ctx); - ret = mgmt_ds_write_lock( - txn_req->req.set_cfg->dst_ds_ctx); - 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; } @@ -703,8 +707,8 @@ static void mgmt_txn_process_set_cfg(struct event *thread) txn_req->req.set_cfg->ds_id, txn_req->req.set_cfg->ds_ctx, txn_req->req.set_cfg->dst_ds_id, - txn_req->req.set_cfg->dst_ds_ctx, false, - false, true); + txn_req->req.set_cfg->dst_ds_ctx, false, false, + true); if (mm->perf_stats_en) gettimeofday(&cmt_stats->last_start, NULL); @@ -746,7 +750,6 @@ static int mgmt_txn_send_commit_cfg_reply(struct mgmt_txn_ctx *txn, enum mgmt_result result, const char *error_if_any) { - int ret = 0; bool success, create_cmt_info_rec; if (!txn->commit_cfg_req) @@ -754,7 +757,12 @@ static int mgmt_txn_send_commit_cfg_reply(struct mgmt_txn_ctx *txn, 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, @@ -770,6 +778,7 @@ static int mgmt_txn_send_commit_cfg_reply(struct mgmt_txn_ctx *txn, } 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, @@ -784,6 +793,7 @@ static int mgmt_txn_send_commit_cfg_reply(struct mgmt_txn_ctx *txn, if (success) { /* Stop the commit-timeout timer */ + /* XXX why only on success? */ EVENT_OFF(txn->comm_cfg_timeout); create_cmt_info_rec = @@ -830,27 +840,18 @@ static int mgmt_txn_send_commit_cfg_reply(struct mgmt_txn_ctx *txn, } if (txn->commit_cfg_req->req.commit_cfg.rollback) { - ret = mgmt_ds_unlock( - txn->commit_cfg_req->req.commit_cfg.dst_ds_ctx); - if (ret != 0) - MGMTD_TXN_ERR( - "Failed to unlock the dst DS during rollback : %s", - strerror(ret)); - + 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) - if (mgmt_ds_unlock( - txn->commit_cfg_req->req.commit_cfg.dst_ds_ctx) - != 0) - MGMTD_TXN_ERR( - "Failed to unlock the dst DS during implicit : %s", - strerror(ret)); - txn->commit_cfg_req->req.commit_cfg.cmt_stats = NULL; mgmt_txn_req_free(&txn->commit_cfg_req); @@ -1724,8 +1725,7 @@ static void mgmt_txn_send_getcfg_reply_data(struct mgmt_txn_req *txn_req, mgmt_reset_get_data_reply_buf(get_req); } -static void mgmt_txn_iter_and_send_get_cfg_reply(struct mgmt_ds_ctx *ds_ctx, - const char *xpath, +static void mgmt_txn_iter_and_send_get_cfg_reply(const char *xpath, struct lyd_node *node, struct nb_node *nb_node, void *ctx) @@ -1770,7 +1770,7 @@ static void mgmt_txn_iter_and_send_get_cfg_reply(struct mgmt_ds_ctx *ds_ctx, static int mgmt_txn_get_config(struct mgmt_txn_ctx *txn, struct mgmt_txn_req *txn_req, - struct mgmt_ds_ctx *ds_ctx) + struct nb_config *root) { int indx; struct mgmt_get_data_req *get_data; @@ -1805,7 +1805,8 @@ static int mgmt_txn_get_config(struct mgmt_txn_ctx *txn, * want to also use an xpath regexp we need to add this * functionality. */ - if (mgmt_ds_iter_data(get_data->ds_ctx, get_data->xpaths[indx], + if (mgmt_ds_iter_data(get_data->ds_id, root, + get_data->xpaths[indx], mgmt_txn_iter_and_send_get_cfg_reply, (void *)txn_req) == -1) { MGMTD_TXN_DBG("Invalid Xpath '%s", @@ -1837,7 +1838,7 @@ static void mgmt_txn_process_get_cfg(struct event *thread) { struct mgmt_txn_ctx *txn; struct mgmt_txn_req *txn_req; - struct mgmt_ds_ctx *ds_ctx; + struct nb_config *cfg_root; int num_processed = 0; bool error; @@ -1852,18 +1853,10 @@ static void mgmt_txn_process_get_cfg(struct event *thread) FOREACH_TXN_REQ_IN_LIST (&txn->get_cfg_reqs, txn_req) { error = false; assert(txn_req->req_event == MGMTD_TXN_PROC_GETCFG); - ds_ctx = txn_req->req.get_data->ds_ctx; - if (!ds_ctx) { - mgmt_fe_send_get_cfg_reply( - txn->session_id, txn->txn_id, - txn_req->req.get_data->ds_id, txn_req->req_id, - MGMTD_INTERNAL_ERROR, NULL, - "No such datastore!"); - error = true; - goto mgmt_txn_process_get_cfg_done; - } + cfg_root = txn_req->req.get_data->cfg_root; + assert(cfg_root); - if (mgmt_txn_get_config(txn, txn_req, ds_ctx) != 0) { + if (mgmt_txn_get_config(txn, txn_req, cfg_root) != 0) { MGMTD_TXN_ERR( "Unable to retrieve config from DS %d txn-id: %" PRIu64 " session-id: %" PRIu64 " req-id: %" PRIu64, @@ -1872,8 +1865,6 @@ static void mgmt_txn_process_get_cfg(struct event *thread) error = true; } - mgmt_txn_process_get_cfg_done: - if (error) { /* * Delete the txn request. @@ -1904,9 +1895,7 @@ static void mgmt_txn_process_get_data(struct event *thread) { struct mgmt_txn_ctx *txn; struct mgmt_txn_req *txn_req; - struct mgmt_ds_ctx *ds_ctx; int num_processed = 0; - bool error; txn = (struct mgmt_txn_ctx *)EVENT_ARG(thread); assert(txn); @@ -1917,54 +1906,23 @@ static void mgmt_txn_process_get_data(struct event *thread) txn->session_id); FOREACH_TXN_REQ_IN_LIST (&txn->get_data_reqs, txn_req) { - error = false; assert(txn_req->req_event == MGMTD_TXN_PROC_GETDATA); - ds_ctx = txn_req->req.get_data->ds_ctx; - if (!ds_ctx) { - mgmt_fe_send_get_data_reply( - txn->session_id, txn->txn_id, - txn_req->req.get_data->ds_id, txn_req->req_id, - MGMTD_INTERNAL_ERROR, NULL, - "No such datastore!"); - error = true; - goto mgmt_txn_process_get_data_done; - } - if (mgmt_ds_is_config(ds_ctx)) { - if (mgmt_txn_get_config(txn, txn_req, ds_ctx) - != 0) { - MGMTD_TXN_ERR( - "Unable to retrieve config from DS %d txn-id %" PRIu64 - " session-id: %" PRIu64 - " req-id: %" PRIu64, - txn_req->req.get_data->ds_id, - txn->txn_id, txn->session_id, - txn_req->req_id); - error = true; - } - } else { - /* - * TODO: Trigger GET procedures for Backend - * For now return back error. - */ - mgmt_fe_send_get_data_reply( - txn->session_id, txn->txn_id, - txn_req->req.get_data->ds_id, txn_req->req_id, - MGMTD_INTERNAL_ERROR, NULL, - "GET-DATA on Oper DS is not supported yet!"); - error = true; - } - - mgmt_txn_process_get_data_done: - - if (error) { - /* - * Delete the txn request. - * Note: The following will remove it from the list - * as well. - */ - mgmt_txn_req_free(&txn_req); - } + /* + * TODO: Trigger GET procedures for Backend + * For now return back error. + */ + mgmt_fe_send_get_data_reply( + txn->session_id, txn->txn_id, + txn_req->req.get_data->ds_id, txn_req->req_id, + MGMTD_INTERNAL_ERROR, NULL, + "GET-DATA on Oper DS is not supported yet!"); + /* + * Delete the txn request. + * Note: The following will remove it from the list + * as well. + */ + mgmt_txn_req_free(&txn_req); /* * Else the transaction would have been already deleted or @@ -2344,12 +2302,12 @@ int mgmt_txn_send_set_config_req(uint64_t txn_id, uint64_t req_id, } int mgmt_txn_send_commit_config_req(uint64_t txn_id, uint64_t req_id, - Mgmtd__DatastoreId src_ds_id, - struct mgmt_ds_ctx *src_ds_ctx, - Mgmtd__DatastoreId dst_ds_id, - struct mgmt_ds_ctx *dst_ds_ctx, - bool validate_only, bool abort, - bool implicit) + Mgmtd__DatastoreId src_ds_id, + struct mgmt_ds_ctx *src_ds_ctx, + Mgmtd__DatastoreId dst_ds_id, + struct mgmt_ds_ctx *dst_ds_ctx, + bool validate_only, bool abort, + bool implicit) { struct mgmt_txn_ctx *txn; struct mgmt_txn_req *txn_req; @@ -2395,9 +2353,8 @@ int mgmt_txn_notify_be_adapter_conn(struct mgmt_be_client_adapter *adapter, memset(&dummy_stats, 0, sizeof(dummy_stats)); if (connect) { /* Get config for this single backend client */ - mgmt_be_get_adapter_config(adapter, mm->running_ds, - &adapter_cfgs); + mgmt_be_get_adapter_config(adapter, &adapter_cfgs); if (!adapter_cfgs || RB_EMPTY(nb_config_cbs, adapter_cfgs)) { SET_FLAG(adapter->flags, MGMTD_BE_ADAPTER_FLAGS_CFG_SYNCED); @@ -2619,10 +2576,10 @@ int mgmt_txn_notify_be_cfg_apply_reply(uint64_t txn_id, bool success, } int mgmt_txn_send_get_config_req(uint64_t txn_id, uint64_t req_id, - Mgmtd__DatastoreId ds_id, - struct mgmt_ds_ctx *ds_ctx, - Mgmtd__YangGetDataReq **data_req, - size_t num_reqs) + Mgmtd__DatastoreId ds_id, + struct nb_config *cfg_root, + Mgmtd__YangGetDataReq **data_req, + size_t num_reqs) { struct mgmt_txn_ctx *txn; struct mgmt_txn_req *txn_req; @@ -2634,7 +2591,7 @@ int mgmt_txn_send_get_config_req(uint64_t txn_id, uint64_t req_id, txn_req = mgmt_txn_req_alloc(txn, req_id, MGMTD_TXN_PROC_GETCFG); txn_req->req.get_data->ds_id = ds_id; - txn_req->req.get_data->ds_ctx = ds_ctx; + txn_req->req.get_data->cfg_root = cfg_root; for (indx = 0; indx < num_reqs && indx < MGMTD_MAX_NUM_DATA_REPLY_IN_BATCH; indx++) { @@ -2650,10 +2607,9 @@ int mgmt_txn_send_get_config_req(uint64_t txn_id, uint64_t req_id, } int mgmt_txn_send_get_data_req(uint64_t txn_id, uint64_t req_id, - Mgmtd__DatastoreId ds_id, - struct mgmt_ds_ctx *ds_ctx, - Mgmtd__YangGetDataReq **data_req, - size_t num_reqs) + Mgmtd__DatastoreId ds_id, + Mgmtd__YangGetDataReq **data_req, + size_t num_reqs) { struct mgmt_txn_ctx *txn; struct mgmt_txn_req *txn_req; @@ -2665,7 +2621,7 @@ int mgmt_txn_send_get_data_req(uint64_t txn_id, uint64_t req_id, txn_req = mgmt_txn_req_alloc(txn, req_id, MGMTD_TXN_PROC_GETDATA); txn_req->req.get_data->ds_id = ds_id; - txn_req->req.get_data->ds_ctx = ds_ctx; + txn_req->req.get_data->cfg_root = NULL; for (indx = 0; indx < num_reqs && indx < MGMTD_MAX_NUM_DATA_REPLY_IN_BATCH; indx++) { @@ -2703,10 +2659,11 @@ int mgmt_txn_rollback_trigger_cfg_apply(struct mgmt_ds_ctx *src_ds_ctx, 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)); diff --git a/mgmtd/mgmt_txn.h b/mgmtd/mgmt_txn.h index 1a9f6d8502..69d75fed07 100644 --- a/mgmtd/mgmt_txn.h +++ b/mgmtd/mgmt_txn.h @@ -169,12 +169,12 @@ extern int mgmt_txn_send_set_config_req(uint64_t txn_id, uint64_t req_id, * 0 on success, -1 on failures. */ extern int mgmt_txn_send_commit_config_req(uint64_t txn_id, uint64_t req_id, - Mgmtd__DatastoreId src_ds_id, - struct mgmt_ds_ctx *dst_ds_ctx, - Mgmtd__DatastoreId dst_ds_id, - struct mgmt_ds_ctx *src_ds_ctx, - bool validate_only, bool abort, - bool implicit); + Mgmtd__DatastoreId src_ds_id, + struct mgmt_ds_ctx *dst_ds_ctx, + Mgmtd__DatastoreId dst_ds_id, + struct mgmt_ds_ctx *src_ds_ctx, + bool validate_only, bool abort, + bool implicit); /* * Send get-config request to be processed later in transaction. @@ -182,10 +182,10 @@ extern int mgmt_txn_send_commit_config_req(uint64_t txn_id, uint64_t req_id, * Similar to set-config request. */ extern int mgmt_txn_send_get_config_req(uint64_t txn_id, uint64_t req_id, - Mgmtd__DatastoreId ds_id, - struct mgmt_ds_ctx *ds_ctx, - Mgmtd__YangGetDataReq **data_req, - size_t num_reqs); + Mgmtd__DatastoreId ds_id, + struct nb_config *cfg_root, + Mgmtd__YangGetDataReq **data_req, + size_t num_reqs); /* * Send get-data request to be processed later in transaction. @@ -194,7 +194,6 @@ extern int mgmt_txn_send_get_config_req(uint64_t txn_id, uint64_t req_id, */ extern int mgmt_txn_send_get_data_req(uint64_t txn_id, uint64_t req_id, Mgmtd__DatastoreId ds_id, - struct mgmt_ds_ctx *ds_ctx, Mgmtd__YangGetDataReq **data_req, size_t num_reqs); |