From 04b4ede097c94f04cc1d14ce90ee82e35a63d670 Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Wed, 14 Jun 2023 09:32:16 -0400 Subject: mgmtd: simplify locking, removing read locks Signed-off-by: Christian Hopps --- mgmtd/mgmt_ds.c | 78 +++++++++++++++++++++++++-------------------------------- 1 file changed, 34 insertions(+), 44 deletions(-) (limited to 'mgmtd/mgmt_ds.c') diff --git a/mgmtd/mgmt_ds.c b/mgmtd/mgmt_ds.c index 5a4b00d309..027e306141 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; @@ -244,40 +246,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 +309,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 +330,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 +339,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 +361,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 +450,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 +463,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 +475,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 +489,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; -- cgit v1.2.3