diff options
| author | Jafar Al-Gharaibeh <jafar@atcorp.com> | 2023-06-25 10:22:34 -0500 | 
|---|---|---|
| committer | GitHub <noreply@github.com> | 2023-06-25 10:22:34 -0500 | 
| commit | 64ef7364c6c4ef8a0db7f47fdd9de4e451763702 (patch) | |
| tree | 13878218d483ef79438e03ce6375969878b6ea02 | |
| parent | 0ea4297e3c3e55d3894820f9359215b6957fa213 (diff) | |
| parent | 6dccdb9b0d1b3bbf04196f712ebccf9a8f39e47e (diff) | |
Merge pull request #13846 from LabNConsulting/chopps/backport-13795frr-9.0-rc
backport of #13795
| -rw-r--r-- | lib/command.c | 5 | ||||
| -rw-r--r-- | lib/mgmt.proto | 3 | ||||
| -rw-r--r-- | lib/mgmt_fe_client.c | 5 | ||||
| -rw-r--r-- | lib/mgmt_fe_client.h | 5 | ||||
| -rw-r--r-- | lib/mgmt_msg.c | 12 | ||||
| -rw-r--r-- | lib/mgmt_msg.h | 2 | ||||
| -rw-r--r-- | lib/northbound_cli.c | 4 | ||||
| -rw-r--r-- | lib/vty.c | 333 | ||||
| -rw-r--r-- | lib/vty.h | 5 | ||||
| -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 | ||||
| -rw-r--r-- | tests/lib/test_grpc.cpp | 2 | ||||
| -rw-r--r-- | vtysh/vtysh.c | 5 | ||||
| -rw-r--r-- | vtysh/vtysh_config.c | 2 | 
20 files changed, 530 insertions, 736 deletions
diff --git a/lib/command.c b/lib/command.c index 0995637219..8025ab534f 100644 --- a/lib/command.c +++ b/lib/command.c @@ -1333,11 +1333,12 @@ int config_from_file(struct vty *vty, FILE *fp, unsigned int *line_num)  /* 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 */ diff --git a/lib/mgmt.proto b/lib/mgmt.proto index 8a11ff0fa5..ac44eefd9e 100644 --- a/lib/mgmt.proto +++ b/lib/mgmt.proto @@ -243,7 +243,8 @@ message FeSetConfigReply {    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 { diff --git a/lib/mgmt_fe_client.c b/lib/mgmt_fe_client.c index be7263f21b..45d57175d6 100644 --- a/lib/mgmt_fe_client.c +++ b/lib/mgmt_fe_client.c @@ -164,7 +164,7 @@ static int mgmt_fe_send_session_req(struct mgmt_fe_client *client,  int mgmt_fe_send_lockds_req(struct mgmt_fe_client *client, uint64_t session_id,  			    uint64_t req_id, Mgmtd__DatastoreId ds_id, -			    bool lock) +			    bool lock, bool scok)  {  	(void)req_id;  	Mgmtd__FeMessage fe_msg; @@ -185,7 +185,7 @@ int mgmt_fe_send_lockds_req(struct mgmt_fe_client *client, uint64_t session_id,  		lock ? "" : "UN", dsid2name(ds_id), session_id); -	return mgmt_fe_client_send_msg(client, &fe_msg, false); +	return mgmt_fe_client_send_msg(client, &fe_msg, scok);  }  int mgmt_fe_send_setcfg_req(struct mgmt_fe_client *client, uint64_t session_id, @@ -411,6 +411,7 @@ static int mgmt_fe_client_handle_msg(struct mgmt_fe_client *client,  				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: diff --git a/lib/mgmt_fe_client.h b/lib/mgmt_fe_client.h index b0ac44bb3e..532fee4397 100644 --- a/lib/mgmt_fe_client.h +++ b/lib/mgmt_fe_client.h @@ -91,7 +91,7 @@ struct mgmt_fe_client_cbs {  				  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, @@ -219,7 +219,8 @@ mgmt_fe_destroy_client_session(struct mgmt_fe_client *client,   */  extern int mgmt_fe_send_lockds_req(struct mgmt_fe_client *client,  				   uint64_t session_id, uint64_t req_id, -				   Mgmtd__DatastoreId ds_id, bool lock_ds); +				   Mgmtd__DatastoreId ds_id, bool lock_ds, +				   bool scok);  /*   * Send SET_CONFIG_REQ to MGMTD for one or more config data(s). diff --git a/lib/mgmt_msg.c b/lib/mgmt_msg.c index ba69c20aba..ee5c1008bd 100644 --- a/lib/mgmt_msg.c +++ b/lib/mgmt_msg.c @@ -548,20 +548,26 @@ int msg_conn_send_msg(struct msg_conn *conn, uint8_t version, void *msg,  	if (conn->remote_conn && short_circuit_ok) {  		uint8_t *buf = msg;  		size_t n = mlen; +		bool old;  		if (packf) {  			buf = XMALLOC(MTYPE_TMP, mlen);  			n = packf(msg, buf);  		} +		++conn->short_circuit_depth;  		MGMT_MSG_DBG(dbgtag, "SC send: depth %u msg: %p", -			     ++conn->short_circuit_depth, msg); +			     conn->short_circuit_depth, msg); +		old = conn->remote_conn->is_short_circuit; +		conn->remote_conn->is_short_circuit = true;  		conn->remote_conn->handle_msg(version, buf, n,  					      conn->remote_conn); +		conn->remote_conn->is_short_circuit = old; +		--conn->short_circuit_depth;  		MGMT_MSG_DBG(dbgtag, "SC return from depth: %u msg: %p", -			     conn->short_circuit_depth--, msg); +			     conn->short_circuit_depth, msg);  		if (packf)  			XFREE(MTYPE_TMP, buf); @@ -661,12 +667,10 @@ static bool msg_client_connect_short_circuit(struct msg_client *client)  	set_nonblocking(sockets[0]);  	setsockopt_so_sendbuf(sockets[0], client->conn.mstate.max_write_buf);  	setsockopt_so_recvbuf(sockets[0], client->conn.mstate.max_read_buf); -	client->conn.is_short_circuit = true;  	/* server side */  	memset(&su, 0, sizeof(union sockunion));  	server_conn = server->create(sockets[1], &su); -	server_conn->is_short_circuit = true;  	client->conn.remote_conn = server_conn;  	server_conn->remote_conn = &client->conn; diff --git a/lib/mgmt_msg.h b/lib/mgmt_msg.h index 9fdcb9ecd3..dd7ae59f91 100644 --- a/lib/mgmt_msg.h +++ b/lib/mgmt_msg.h @@ -98,8 +98,8 @@ struct msg_conn {  			   struct msg_conn *conn);  	void *user;  	uint short_circuit_depth; +	bool is_short_circuit;	/* true when the message being handled is SC */  	bool is_client; -	bool is_short_circuit;  	bool debug;  }; diff --git a/lib/northbound_cli.c b/lib/northbound_cli.c index 9d6ec66689..8003679ed5 100644 --- a/lib/northbound_cli.c +++ b/lib/northbound_cli.c @@ -763,7 +763,7 @@ DEFUN (config_exclusive,         "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. */ @@ -773,7 +773,7 @@ DEFUN (config_private,         "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, @@ -70,7 +70,6 @@ struct nb_config *vty_mgmt_candidate_config;  static struct mgmt_fe_client *mgmt_fe_client;  static bool mgmt_fe_connected; -static bool mgmt_candidate_ds_wr_locked;  static uint64_t mgmt_client_id_next;  static uint64_t mgmt_last_req_id = UINT64_MAX; @@ -129,6 +128,35 @@ char const *const mgmt_daemons[] = {  };  uint mgmt_daemons_count = array_size(mgmt_daemons); + +static int vty_mgmt_lock_candidate_inline(struct vty *vty) +{ +	assert(!vty->mgmt_locked_candidate_ds); +	(void)vty_mgmt_send_lockds_req(vty, MGMTD_DS_CANDIDATE, true, true); +	return vty->mgmt_locked_candidate_ds ? 0 : -1; +} + +static int vty_mgmt_unlock_candidate_inline(struct vty *vty) +{ +	assert(vty->mgmt_locked_candidate_ds); +	(void)vty_mgmt_send_lockds_req(vty, MGMTD_DS_CANDIDATE, false, true); +	return vty->mgmt_locked_candidate_ds ? -1 : 0; +} + +static int vty_mgmt_lock_running_inline(struct vty *vty) +{ +	assert(!vty->mgmt_locked_running_ds); +	(void)vty_mgmt_send_lockds_req(vty, MGMTD_DS_RUNNING, true, true); +	return vty->mgmt_locked_running_ds ? 0 : -1; +} + +static int vty_mgmt_unlock_running_inline(struct vty *vty) +{ +	assert(vty->mgmt_locked_running_ds); +	(void)vty_mgmt_send_lockds_req(vty, MGMTD_DS_RUNNING, false, true); +	return vty->mgmt_locked_running_ds ? -1 : 0; +} +  void vty_mgmt_resume_response(struct vty *vty, bool success)  {  	uint8_t header[4] = {0, 0, 0, 0}; @@ -1648,12 +1676,12 @@ struct vty *vty_new(void)  		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; @@ -2202,10 +2230,11 @@ bool mgmt_vty_read_configs(void)  	vty->node = CONFIG_NODE;  	vty->config = true;  	vty->pending_allowed = true; +  	vty->candidate_config = vty_shared_candidate_config; -	vty->mgmt_locked_candidate_ds = true; -	mgmt_candidate_ds_wr_locked = true; +	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, @@ -2250,10 +2279,15 @@ bool mgmt_vty_read_configs(void)  		fclose(confp);  	} -	vty->pending_allowed = false; +	/* 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->mgmt_locked_candidate_ds = false; -	mgmt_candidate_ds_wr_locked = false; +	vty->pending_allowed = false;  	if (!count)  		vty_close(vty); @@ -2367,7 +2401,7 @@ static void vtysh_read(struct event *thread)  				 */  				if (vty->mgmt_req_pending_cmd) {  					MGMTD_FE_CLIENT_DBG( -						"postpone CLI cmd response pending mgmtd %s on vty session-id %" PRIu64, +						"postpone CLI response pending mgmtd %s on vty session-id %" PRIu64,  						vty->mgmt_req_pending_cmd,  						vty->mgmt_session_id);  					return; @@ -2453,6 +2487,9 @@ void vty_close(struct vty *vty)  		MGMTD_FE_CLIENT_ERR(  			"vty closed, uncommitted config will be lost."); +	/* Drop out of configure / transaction if needed. */ +	vty_config_exit(vty); +  	if (mgmt_fe_client && vty->mgmt_session_id) {  		MGMTD_FE_CLIENT_DBG("closing vty session");  		mgmt_fe_destroy_client_session(mgmt_fe_client, @@ -2460,9 +2497,6 @@ void vty_close(struct vty *vty)  		vty->mgmt_session_id = 0;  	} -	/* Drop out of configure / transaction if needed. */ -	vty_config_exit(vty); -  	/* Cancel threads.*/  	EVENT_OFF(vty->t_read);  	EVENT_OFF(vty->t_write); @@ -2821,28 +2855,35 @@ bool vty_read_config(struct nb_config *config, const char *config_file,  	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;  	} -	if (vty_mgmt_fe_enabled()) { -		if (!mgmt_candidate_ds_wr_locked) { -			if (vty_mgmt_send_lockds_req(vty, MGMTD_DS_CANDIDATE, -						     true) != 0) { -				vty_out(vty, "Not able to lock candidate DS\n"); -				return CMD_WARNING; -			} -		} else { +	/* +	 * We only need to do a lock when reading a config file as we will be +	 * sending a batch of setcfg changes followed by a single commit +	 * message. For user interactive mode we are doing implicit commits +	 * those will obtain the lock (or not) when they try and commit. +	 */ +	if (file_lock && vty_mgmt_fe_enabled() && !private_config) { +		if (vty_mgmt_lock_candidate_inline(vty)) {  			vty_out(vty, -				"Candidate DS already locked by different session\n"); -			return CMD_WARNING; +				"%% Can't enter config; candidate datastore locked by another session\n"); +			return CMD_WARNING_CONFIG_FAILED;  		} - -		vty->mgmt_locked_candidate_ds = true; -		mgmt_candidate_ds_wr_locked = true; +		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; @@ -2855,23 +2896,24 @@ int vty_config_enter(struct vty *vty, bool private_config, bool exclusive)  		vty->candidate_config_base = nb_config_dup(running_config);  		vty_out(vty,  			"Warning: uncommitted changes will be discarded on exit.\n\n"); -	} else { -		/* -		 * NOTE: On the MGMTD daemon we point the VTY candidate DS to -		 * the global MGMTD candidate DS. Else we point to the VTY -		 * Shared Candidate Config. -		 */ -		vty->candidate_config = vty_mgmt_candidate_config -						? vty_mgmt_candidate_config -						: vty_shared_candidate_config; -		if (frr_get_cli_mode() == FRR_CLI_TRANSACTIONAL) -			vty->candidate_config_base = -				nb_config_dup(running_config); +		return CMD_SUCCESS;  	} +	/* +	 * NOTE: On the MGMTD daemon we point the VTY candidate DS to +	 * the global MGMTD candidate DS. Else we point to the VTY +	 * Shared Candidate Config. +	 */ +	vty->candidate_config = vty_mgmt_candidate_config +					? vty_mgmt_candidate_config +					: vty_shared_candidate_config; +	if (frr_get_cli_mode() == FRR_CLI_TRANSACTIONAL) +		vty->candidate_config_base = nb_config_dup(running_config); +  	return CMD_SUCCESS;  } +  void vty_config_exit(struct vty *vty)  {  	enum node_type node = vty->node; @@ -2896,21 +2938,13 @@ int vty_config_node_exit(struct vty *vty)  {  	vty->xpath_index = 0; -	/* -	 * If we are not reading config file and we are mgmtd FE and we are -	 * locked then unlock. -	 */ -	if (vty->type != VTY_FILE && vty_mgmt_fe_enabled() && -	    mgmt_candidate_ds_wr_locked && vty->mgmt_locked_candidate_ds) { -		if (vty_mgmt_send_lockds_req(vty, MGMTD_DS_CANDIDATE, false) != -		    0) { -			vty_out(vty, "Not able to unlock candidate DS\n"); -			return CMD_WARNING; -		} +	/* TODO: could we check for un-commited changes here? */ -		vty->mgmt_locked_candidate_ds = false; -		mgmt_candidate_ds_wr_locked = false; -	} +	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); @@ -3493,23 +3527,28 @@ static void vty_mgmt_ds_lock_notified(struct mgmt_fe_client *client,  	vty = (struct vty *)session_ctx; -	if (!success) { -		zlog_err("%socking for DS %u failed, Err: '%s'", -			 lock_ds ? "L" : "Unl", ds_id, errmsg_if_any); -		vty_out(vty, "ERROR: %socking for DS %u failed, Err: '%s'\n", -			lock_ds ? "L" : "Unl", ds_id, errmsg_if_any); -	} else { +	assert(ds_id == MGMTD_DS_CANDIDATE || ds_id == MGMTD_DS_RUNNING); +	if (!success) +		zlog_err("%socking for DS %u failed, Err: '%s' vty %p", +			 lock_ds ? "L" : "Unl", ds_id, errmsg_if_any, vty); +	else {  		MGMTD_FE_CLIENT_DBG("%socked DS %u successfully",  				    lock_ds ? "L" : "Unl", ds_id); +		if (ds_id == MGMTD_DS_CANDIDATE) +			vty->mgmt_locked_candidate_ds = lock_ds; +		else +			vty->mgmt_locked_running_ds = lock_ds;  	} -	vty_mgmt_resume_response(vty, success); +	if (vty->mgmt_req_pending_cmd) +		vty_mgmt_resume_response(vty, success);  }  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; @@ -3527,6 +3566,12 @@ static void vty_mgmt_set_config_result_notified(  				    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);  } @@ -3632,23 +3677,24 @@ bool vty_mgmt_should_process_cli_apply_changes(struct vty *vty)  }  int vty_mgmt_send_lockds_req(struct vty *vty, Mgmtd__DatastoreId ds_id, -			     bool lock) +			     bool lock, bool scok)  { -	if (mgmt_fe_client && vty->mgmt_session_id) { -		vty->mgmt_req_id++; -		if (mgmt_fe_send_lockds_req(mgmt_fe_client, -					    vty->mgmt_session_id, -					    vty->mgmt_req_id, ds_id, lock)) { -			zlog_err("Failed sending %sLOCK-DS-REQ req-id %" PRIu64, -				 lock ? "" : "UN", vty->mgmt_req_id); -			vty_out(vty, "Failed to send %sLOCK-DS-REQ to MGMTD!\n", -				lock ? "" : "UN"); -			return -1; -		} +	assert(mgmt_fe_client); +	assert(vty->mgmt_session_id); -		vty->mgmt_req_pending_cmd = "MESSAGE_LOCKDS_REQ"; +	vty->mgmt_req_id++; +	if (mgmt_fe_send_lockds_req(mgmt_fe_client, vty->mgmt_session_id, +				    vty->mgmt_req_id, ds_id, lock, scok)) { +		zlog_err("Failed sending %sLOCK-DS-REQ req-id %" PRIu64, +			 lock ? "" : "UN", vty->mgmt_req_id); +		vty_out(vty, "Failed to send %sLOCK-DS-REQ to MGMTD!\n", +			lock ? "" : "UN"); +		return -1;  	} +	if (!scok) +		vty->mgmt_req_pending_cmd = "MESSAGE_LOCKDS_REQ"; +  	return 0;  } @@ -3659,7 +3705,6 @@ int vty_mgmt_send_config_data(struct vty *vty, bool implicit_commit)  	Mgmtd__YangCfgDataReq cfg_req[VTY_MAXCFGCHANGES];  	Mgmtd__YangCfgDataReq *cfgreq[VTY_MAXCFGCHANGES] = {0};  	size_t indx; -	int cnt;  	if (vty->type == VTY_FILE) {  		/* @@ -3667,86 +3712,92 @@ int vty_mgmt_send_config_data(struct vty *vty, bool implicit_commit)  		 * changes until we are done reading the file and have modified  		 * the local candidate DS.  		 */ -		assert(vty->mgmt_locked_candidate_ds);  		/* no-one else should be sending data right now */  		assert(!vty->mgmt_num_pending_setcfg);  		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;  } @@ -230,6 +230,7 @@ struct vty {  	 * vty user. */  	const char *mgmt_req_pending_cmd;  	bool mgmt_locked_candidate_ds; +	bool mgmt_locked_running_ds;  };  static inline void vty_push_context(struct vty *vty, int node, uint64_t id) @@ -390,7 +391,7 @@ extern void vty_close(struct vty *);  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 *); @@ -416,7 +417,7 @@ extern int vty_mgmt_send_get_config(struct vty *vty,  extern int vty_mgmt_send_get_data(struct vty *vty, Mgmtd__DatastoreId datastore,  				  const char **xpath_list, int num_req);  extern int vty_mgmt_send_lockds_req(struct vty *vty, Mgmtd__DatastoreId ds_id, -				    bool lock); +				    bool lock, bool scok);  extern void vty_mgmt_resume_response(struct vty *vty, bool success);  static inline bool vty_needs_implicit_commit(struct vty *vty) 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); diff --git a/tests/lib/test_grpc.cpp b/tests/lib/test_grpc.cpp index fd30f04346..87530d41d8 100644 --- a/tests/lib/test_grpc.cpp +++ b/tests/lib/test_grpc.cpp @@ -114,7 +114,7 @@ static void static_startup(void)  	// 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); diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c index c94b47fef5..ee52a98adb 100644 --- a/vtysh/vtysh.c +++ b/vtysh/vtysh.c @@ -2333,8 +2333,9 @@ DEFUNSH(VTYSH_REALLYALL, vtysh_disable, vtysh_disable_cmd, "disable",  }  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; @@ -2355,7 +2356,7 @@ static int vtysh_exit(struct vty *vty)  	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;  } diff --git a/vtysh/vtysh_config.c b/vtysh/vtysh_config.c index 2949faa427..a5f790bbc6 100644 --- a/vtysh/vtysh_config.c +++ b/vtysh/vtysh_config.c @@ -607,7 +607,7 @@ static int vtysh_read_file(FILE *confp, bool dry_run)  	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");  | 
