mgmtd: simplify locking, removing read locks

Signed-off-by: Christian Hopps <chopps@labn.net>

1 files changed, 34 insertions, 44 deletions

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;