Merge pull request #16839 from LabNConsulting/chopps/edit-reply

Add `created` and `changed` booleans to `edit-reply` message

lib/mgmt_msg_native.h

@@ -383,11 +383,18 @@ _Static_assert(sizeof(struct mgmt_msg_edit) ==
 /**
  * struct mgmt_msg_edit_reply - frontend edit reply.
  *
- * @data: the xpath of the data node that was created.
+ * @changed: If true then changes in datastore resulted.
+ * @created: If true then object was newly created (non-existing before)
+ * @data: @vsplit values, second value may be zero len.
+ * @data: [0] the xpath of the data node that was created.
+ * @data: [1] Possible structured data to pass back to client (e.g., non-"error"
+ *        yang modeled error data).
  */
 struct mgmt_msg_edit_reply {
 	struct mgmt_msg_header;
-	uint8_t resv2[8];
+	uint8_t changed;
+	uint8_t created;
+	uint8_t resv2[6];
 
 	alignas(8) char data[];
 };

lib/northbound.c

@@ -178,7 +178,7 @@ struct nb_node *nb_node_find(const char *path)
 
 struct nb_node **nb_nodes_find(const char *xpath)
 {
-	struct lysc_node **snodes = NULL;
+	const struct lysc_node **snodes = NULL;
 	struct nb_node **nb_nodes = NULL;
 	bool simple;
 	LY_ERR err;
@@ -816,8 +816,9 @@ int nb_candidate_edit(struct nb_config *candidate, const struct nb_node *nb_node
 static int nb_candidate_edit_tree_add(struct nb_config *candidate,
 				      enum nb_operation operation,
 				      LYD_FORMAT format, const char *xpath,
-				      const char *data, char *xpath_created,
+				      const char *data, bool *created,
-				      char *errmsg, size_t errmsg_len)
+				      char *xpath_created, char *errmsg,
+				      size_t errmsg_len)
 {
 	struct lyd_node *tree = NULL;
 	struct lyd_node *parent = NULL;
@@ -897,10 +898,18 @@ static int nb_candidate_edit_tree_add(struct nb_config *candidate,
 	}
 
 	/* check if the node already exists in candidate */
-	if (operation == NB_OP_CREATE_EXCL || operation == NB_OP_REPLACE) {
+	if (operation == NB_OP_CREATE || operation == NB_OP_MODIFY)
+		existing = yang_dnode_get(candidate->dnode, xpath_created);
+	else if (operation == NB_OP_CREATE_EXCL || operation == NB_OP_REPLACE) {
 		existing = yang_dnode_get(candidate->dnode, xpath_created);
 
 		/* if the existing node is implicit default, ignore */
+		/* Q: Is this correct for CREATE_EXCL which is supposed to error
+		 * if the resouurce already exists? This is used by RESTCONF
+		 * when processing the POST command, for example. RFC8040
+		 * doesn't say POST fails if resource exists "unless it was a
+		 * default".
+		 */
 		if (existing && (existing->flags & LYD_DEFAULT))
 			existing = NULL;
 
@@ -908,7 +917,7 @@ static int nb_candidate_edit_tree_add(struct nb_config *candidate,
 			if (operation == NB_OP_CREATE_EXCL) {
 				snprintf(errmsg, errmsg_len,
 					 "Data already exists");
-				ret = NB_ERR;
+				ret = NB_ERR_EXISTS;
 				goto done;
 			}
 
@@ -930,7 +939,7 @@ static int nb_candidate_edit_tree_add(struct nb_config *candidate,
 				 LYD_MERGE_DESTRUCT | LYD_MERGE_WITH_FLAGS);
 	if (err) {
 		/* if replace failed, restore the original node */
-		if (existing) {
+		if (existing && operation == NB_OP_REPLACE) {
 			if (root) {
 				/* Restoring the whole config. */
 				candidate->dnode = existing;
@@ -954,6 +963,8 @@ static int nb_candidate_edit_tree_add(struct nb_config *candidate,
 		ret = NB_ERR;
 		goto done;
 	} else {
+		if (!existing)
+			*created = true;
 		/*
 		 * Free existing node after replace.
 		 * We're using `lyd_free_siblings` here to free the whole
@@ -961,7 +972,7 @@ static int nb_candidate_edit_tree_add(struct nb_config *candidate,
 		 * siblings if it wasn't root, because the existing node
 		 * was unlinked from the tree.
 		 */
-		if (existing)
+		if (existing && operation == NB_OP_REPLACE)
 			lyd_free_siblings(existing);
 
 		tree = NULL; /* LYD_MERGE_DESTRUCT deleted the tree */
@@ -995,7 +1006,7 @@ static int nb_candidate_edit_tree_del(struct nb_config *candidate,
 	if (!dnode || (dnode->flags & LYD_DEFAULT)) {
 		if (operation == NB_OP_DELETE) {
 			snprintf(errmsg, errmsg_len, "Data missing");
-			return NB_ERR;
+			return NB_ERR_NOT_FOUND;
 		} else
 			return NB_OK;
 	}
@@ -1011,7 +1022,7 @@ static int nb_candidate_edit_tree_del(struct nb_config *candidate,
 
 int nb_candidate_edit_tree(struct nb_config *candidate,
 			   enum nb_operation operation, LYD_FORMAT format,
-			   const char *xpath, const char *data,
+			   const char *xpath, const char *data, bool *created,
 			   char *xpath_created, char *errmsg, size_t errmsg_len)
 {
 	int ret = NB_ERR;
@@ -1022,8 +1033,9 @@ int nb_candidate_edit_tree(struct nb_config *candidate,
 	case NB_OP_MODIFY:
 	case NB_OP_REPLACE:
 		ret = nb_candidate_edit_tree_add(candidate, operation, format,
-						 xpath, data, xpath_created,
+						 xpath, data, created,
-						 errmsg, errmsg_len);
+						 xpath_created, errmsg,
+						 errmsg_len);
 		break;
 	case NB_OP_DESTROY:
 	case NB_OP_DELETE:
@@ -2605,6 +2617,8 @@ const char *nb_err_name(enum nb_error error)
 		return "no changes";
 	case NB_ERR_NOT_FOUND:
 		return "element not found";
+	case NB_ERR_EXISTS:
+		return "element already exists";
 	case NB_ERR_LOCKED:
 		return "resource is locked";
 	case NB_ERR_VALIDATION:

lib/northbound.h

@@ -678,6 +678,7 @@ enum nb_error {
 	NB_ERR,
 	NB_ERR_NO_CHANGES,
 	NB_ERR_NOT_FOUND,
+	NB_ERR_EXISTS,
 	NB_ERR_LOCKED,
 	NB_ERR_VALIDATION,
 	NB_ERR_RESOURCE,
@@ -1015,6 +1016,9 @@ extern int nb_candidate_edit(struct nb_config *candidate,
  * data
  *    New data tree for the node.
  *
+ * created
+ *    OUT param set accordingly if a node was created or just updated
+ *
  * xpath_created
  *    XPath of the created node if operation is "create".
  *
@@ -1029,9 +1033,9 @@ extern int nb_candidate_edit(struct nb_config *candidate,
  *    - NB_ERR for other errors.
  */
 extern int nb_candidate_edit_tree(struct nb_config *candidate,
-				  enum nb_operation operation,
+				  enum nb_operation operation, LYD_FORMAT format,
-				  LYD_FORMAT format, const char *xpath,
+				  const char *xpath, const char *data,
-				  const char *data, char *xpath_created,
+				  bool *created, char *xpath_created,
 				  char *errmsg, size_t errmsg_len);
 
 /*

lib/yang.c

@@ -286,7 +286,7 @@ void yang_snode_get_path(const struct lysc_node *snode,
 }
 
 LY_ERR yang_resolve_snode_xpath(struct ly_ctx *ly_ctx, const char *xpath,
-				struct lysc_node ***snodes, bool *simple)
+				const struct lysc_node ***snodes, bool *simple)
 {
 	struct lysc_node *snode;
 	struct ly_set *set;

lib/yang.h

@@ -827,7 +827,8 @@ extern int yang_xpath_pop_node(char *xpath);
  * Return: a libyang error or LY_SUCCESS.
  */
 extern LY_ERR yang_resolve_snode_xpath(struct ly_ctx *ly_ctx, const char *xpath,
-				       struct lysc_node ***snodes, bool *simple);
+				       const struct lysc_node ***snodes,
+				       bool *simple);
 
 /*
  * Libyang future functions

mgmtd/mgmt_fe_adapter.c

@@ -1164,7 +1164,9 @@ done:
 }
 
 static int fe_adapter_send_edit_reply(struct mgmt_fe_session_ctx *session,
-				      uint64_t req_id, const char *xpath)
+				      uint64_t req_id, bool changed,
+				      bool created, const char *xpath,
+				      const char *data)
 {
 	struct mgmt_msg_edit_reply *msg;
 	int ret;
@@ -1173,14 +1175,19 @@ static int fe_adapter_send_edit_reply(struct mgmt_fe_session_ctx *session,
 					MTYPE_MSG_NATIVE_EDIT_REPLY);
 	msg->refer_id = session->session_id;
 	msg->req_id = req_id;
+	msg->changed = changed;
+	msg->created = created;
 	msg->code = MGMT_MSG_CODE_EDIT_REPLY;
 
 	mgmt_msg_native_xpath_encode(msg, xpath);
 
+	if (data)
+		mgmt_msg_native_append(msg, data, strlen(data) + 1);
+
 	__dbg("Sending edit-reply from adapter %s to session-id %" PRIu64
-	      " req-id %" PRIu64 " len %u",
+	      " req-id %" PRIu64 " changed %u created %u len %u",
-	      session->adapter->name, session->session_id, req_id,
+	      session->adapter->name, session->session_id, req_id, changed,
-	      mgmt_msg_native_get_msg_len(msg));
+	      created, mgmt_msg_native_get_msg_len(msg));
 
 	ret = fe_adapter_send_native_msg(session->adapter, msg,
 					 mgmt_msg_native_get_msg_len(msg),
@@ -1282,8 +1289,7 @@ static void fe_adapter_handle_get_data(struct mgmt_fe_session_ctx *session,
 				       void *__msg, size_t msg_len)
 {
 	struct mgmt_msg_get_data *msg = __msg;
-	struct lysc_node **snodes = NULL;
+	const struct lysc_node **snodes = NULL;
-	char *xpath_resolved = NULL;
 	uint64_t req_id = msg->req_id;
 	Mgmtd__DatastoreId ds_id;
 	uint64_t clients;
@@ -1395,7 +1401,6 @@ static void fe_adapter_handle_get_data(struct mgmt_fe_session_ctx *session,
 	}
 done:
 	darr_free(snodes);
-	darr_free(xpath_resolved);
 }
 
 static void fe_adapter_handle_edit(struct mgmt_fe_session_ctx *session,
@@ -1408,7 +1413,12 @@ static void fe_adapter_handle_edit(struct mgmt_fe_session_ctx *session,
 	bool lock, commit;
 	int ret;
 
-	if (msg->datastore != MGMT_MSG_DATASTORE_CANDIDATE) {
+	lock = CHECK_FLAG(msg->flags, EDIT_FLAG_IMPLICIT_LOCK);
+	commit = CHECK_FLAG(msg->flags, EDIT_FLAG_IMPLICIT_COMMIT);
+
+	if (lock && commit && msg->datastore == MGMT_MSG_DATASTORE_RUNNING)
+		;
+	else if (msg->datastore != MGMT_MSG_DATASTORE_CANDIDATE) {
 		fe_adapter_send_error(session, msg->req_id, false, -EINVAL,
 				      "Unsupported datastore");
 		return;
@@ -1429,9 +1439,6 @@ static void fe_adapter_handle_edit(struct mgmt_fe_session_ctx *session,
 	rds_ctx = mgmt_ds_get_ctx_by_id(mm, rds_id);
 	assert(rds_ctx);
 
-	lock = CHECK_FLAG(msg->flags, EDIT_FLAG_IMPLICIT_LOCK);
-	commit = CHECK_FLAG(msg->flags, EDIT_FLAG_IMPLICIT_COMMIT);
-
 	if (lock) {
 		if (mgmt_fe_session_write_lock_ds(ds_id, ds_ctx, session)) {
 			fe_adapter_send_error(session, msg->req_id, false,
@@ -1977,8 +1984,8 @@ int mgmt_fe_adapter_send_rpc_reply(uint64_t session_id, uint64_t txn_id,
 
 int mgmt_fe_adapter_send_edit_reply(uint64_t session_id, uint64_t txn_id,
 				    uint64_t req_id, bool unlock, bool commit,
-				    const char *xpath, int16_t error,
+				    bool created, const char *xpath,
-				    const char *errstr)
+				    int16_t error, const char *errstr)
 {
 	struct mgmt_fe_session_ctx *session;
 	Mgmtd__DatastoreId ds_id, rds_id;
@@ -2009,11 +2016,12 @@ int mgmt_fe_adapter_send_edit_reply(uint64_t session_id, uint64_t txn_id,
 		}
 	}
 
-	if (error)
+	if (error != 0 && error != -EALREADY)
 		ret = fe_adapter_send_error(session, req_id, false, error, "%s",
 					    errstr);
 	else
-		ret = fe_adapter_send_edit_reply(session, req_id, xpath);
+		ret = fe_adapter_send_edit_reply(session, req_id, created,
+						 !error, xpath, errstr);
 
 	if (session->cfg_txn_id != MGMTD_TXN_ID_NONE && !commit)
 		mgmt_destroy_txn(&session->cfg_txn_id);

mgmtd/mgmt_fe_adapter.h

@@ -193,14 +193,16 @@ extern int mgmt_fe_adapter_send_rpc_reply(uint64_t session_id, uint64_t txn_id,
  *     req_id: the req id for the edit message
  *     unlock: implicit-lock flag was set in the request
  *     commit: implicit-commit flag was set in the request
- *     xpath: the xpath of the data node that was created
+ *     created: true if the node was just created
- *     error: the error code, zero for successful request
+ *     xpath: the xpath of the data node that was created/updated
+ *     error: >0 LY_ERR, < 0 -errno
  *     errstr: the error string, if error is non-zero
  */
 extern int mgmt_fe_adapter_send_edit_reply(uint64_t session_id, uint64_t txn_id,
 					   uint64_t req_id, bool unlock,
-					   bool commit, const char *xpath,
+					   bool commit, bool created,
-					   int16_t error, const char *errstr);
+					   const char *xpath, int16_t error,
+					   const char *errstr);
 
 /**
  * Send an error back to the FE client using native messaging.
@@ -210,7 +212,7 @@ extern int mgmt_fe_adapter_send_edit_reply(uint64_t session_id, uint64_t txn_id,
  * Args:
  *	txn_id: the txn_id this error pertains to.
  *	short_circuit_ok: True if OK to short-circuit the call.
- *	error: An integer error value.
+ *      error: >0 LY_ERR, < 0 -errno
  *	errfmt: An error format string (i.e., printfrr)
  *      ...: args for use by the `errfmt` format string.
  *

mgmtd/mgmt_txn.c

@@ -94,6 +94,7 @@ DECLARE_LIST(mgmt_txn_batches, struct mgmt_txn_be_cfg_batch, list_linkage);
 
 struct mgmt_edit_req {
 	char xpath_created[XPATH_MAXLEN];
+	bool created;
 	bool unlock;
 };
 
@@ -741,6 +742,8 @@ static int mgmt_txn_send_commit_cfg_reply(struct mgmt_txn_ctx *txn,
 					    txn->commit_cfg_req->req.commit_cfg
 						    .edit->unlock,
 					    true,
+					    txn->commit_cfg_req->req.commit_cfg
+						    .edit->created,
 					    txn->commit_cfg_req->req.commit_cfg
 						    .edit->xpath_created,
 					    success ? 0 : -1,
@@ -1335,7 +1338,8 @@ static int txn_get_tree_data_done(struct mgmt_txn_ctx *txn,
 			  " req_id %" PRIu64 " to requested type %u",
 			  txn->txn_id, req_id, get_tree->result_type);
 
-		(void)mgmt_fe_adapter_txn_error(txn->txn_id, req_id, false, ret,
+		(void)mgmt_fe_adapter_txn_error(txn->txn_id, req_id, false,
+						errno_from_nb_error(ret),
 						"Error converting results of GETTREE");
 	}
 
@@ -1351,7 +1355,7 @@ static int txn_rpc_done(struct mgmt_txn_ctx *txn, struct mgmt_txn_req *txn_req)
 	EVENT_OFF(txn->rpc_timeout);
 
 	if (rpc->errstr)
-		mgmt_fe_adapter_txn_error(txn->txn_id, req_id, false, -1,
+		mgmt_fe_adapter_txn_error(txn->txn_id, req_id, false, -EINVAL,
 					  rpc->errstr);
 	else if (mgmt_fe_adapter_send_rpc_reply(txn->session_id, txn->txn_id,
 						req_id, rpc->result_type,
@@ -1360,7 +1364,8 @@ static int txn_rpc_done(struct mgmt_txn_ctx *txn, struct mgmt_txn_req *txn_req)
 			  " req_id %" PRIu64 " to requested type %u",
 			  txn->txn_id, req_id, rpc->result_type);
 
-		(void)mgmt_fe_adapter_txn_error(txn->txn_id, req_id, false, -1,
+		(void)mgmt_fe_adapter_txn_error(txn->txn_id, req_id, false,
+						-EINVAL,
 						"Error converting results of RPC");
 	}
 
@@ -2564,8 +2569,8 @@ int mgmt_txn_send_edit(uint64_t txn_id, uint64_t req_id,
 	assert(nb_config);
 
 	ret = nb_candidate_edit_tree(nb_config, operation, request_type, xpath,
-				     data, edit->xpath_created, errstr,
+				     data, &edit->created, edit->xpath_created,
-				     sizeof(errstr));
+				     errstr, sizeof(errstr));
 	if (ret)
 		goto reply;
 
@@ -2579,8 +2584,9 @@ int mgmt_txn_send_edit(uint64_t txn_id, uint64_t req_id,
 	}
 reply:
 	mgmt_fe_adapter_send_edit_reply(txn->session_id, txn->txn_id, req_id,
-					unlock, commit, edit->xpath_created,
+					unlock, commit, edit->created,
-					ret ? -1 : 0, errstr);
+					edit->xpath_created,
+					errno_from_nb_error(ret), errstr);
 
 	XFREE(MTYPE_MGMTD_TXN_REQ, edit);
 

mgmtd/mgmt_txn.h

@@ -69,6 +69,34 @@ static inline const char *mgmt_txn_type2str(enum mgmt_txn_type type)
 	return "Unknown";
 }
 
+
+static inline int16_t errno_from_nb_error(enum nb_error ret)
+{
+	switch (ret) {
+	case NB_OK:
+		return 0;
+	case NB_ERR_NO_CHANGES:
+		return -EALREADY;
+	case NB_ERR_NOT_FOUND:
+		return -ENOENT;
+	case NB_ERR_EXISTS:
+		return -EEXIST;
+	case NB_ERR_LOCKED:
+		return -EWOULDBLOCK;
+	case NB_ERR_VALIDATION:
+		return -EINVAL;
+	case NB_ERR_RESOURCE:
+		return -ENOMEM;
+	case NB_ERR:
+	case NB_ERR_INCONSISTENCY:
+		return -EINVAL;
+	case NB_YIELD:
+	default:
+		return -EINVAL;
+	}
+}
+
+
 /* Initialise transaction module. */
 extern int mgmt_txn_init(struct mgmt_master *cm, struct event_loop *tm);