Enable the -Wmissing-noreturn warning, and resolve warnings
for gcc and clang. Add a FRR_NORETURN macro and use that for
the new changes.
Signed-off-by: Mark Stapp <mjs@cisco.com>
Also, only message interested backend clients about a path removal when last
supporting session is being removed from a given notification path.
Signed-off-by: Christian Hopps <chopps@labn.net>
Say you have 2 mgmtd frontend sessions (2 vtysh's) the first one is long
running and is actively changing the global candidate datastore (DS),
the second one starts and exits, this code would then copy running
back over the candidate, blowing away any changes made by the first
session.
(the long running session could technically be any user)
Instead we need to trust the various cleanup code that already exits.
For example in the commit_cfg_reply on success candidate is copied to
running, and on failure *for implicit commit* running is copied back to
candidate clearing the change. This leaves the non-implicit
configuration changes in this case we actually want candidate to keep
it's changes in transactional cases, in the other case of pending commit
during a file read the code restores candidate (if needed) on exit from
"config terminal", with this call stack:
vty_config_node_exit()
nb_cli_pending_commit_check()
nb_cli_classic_commit()
nb_candidate_commit_prepare() [fail] -> copy running -> candidate
nb_candidate_commit_apply() -> copy candidate -> running
fixes#18541
Signed-off-by: Christian Hopps <chopps@labn.net>
- Previously we sent selectors to all backends when a replace was
done, improve this to only send them to backends that provide
the selected state.
Signed-off-by: Christian Hopps <chopps@labn.net>
Also an empty (thus non-replace) notify selectors message shouldn't
clear the selectors, it should just do nothing.
Signed-off-by: Christian Hopps <chopps@labn.net>
The previous idea of using config xpath registrations for actions b/c
the config is the subject of the action is sub-optimal. It splits
handling of YANG "actions" (Action and RPCs) between 2 different
registartion maps for the same category of functionality.
Signed-off-by: Christian Hopps <chopps@labn.net>
This operation basically implements support for RESTCONF operations. It
receives an xpath and a data tree in JSON/XML format, instead of a list
of (xpath, value) tuples as required by the current protobuf interface.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
When FRR starts, after mgmtd is initialized, backend clients connect to
it and request their config. To supply the config, mgmtd creates a
configuration transaction. At the same time, `vtysh -b` tries to read
the startup config and configure mgmtd, which also creates a
configuration transaction. If these two actions happen at the exact same
time, there's a conflict between them, because only a single
configuration translaction is allowed. Because of that, vtysh fails and
the config is completely ignored.
When starting the config reading, vtysh locks candidate and running
datastores in mgmtd. This commit adds locking of running datastore when
initializing the backend client. It allows to retry locking on the vtysh
side and read the config only when the lock is aquired instead of
failing.
This change also prevents running datastore from being changed during
initialization of backend clients. This could lead to a desynchronized
state between mgmtd and backends.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
We don't need to create an actual tree to print an empty tree, libyang
handles NULL just fine. The actual problem is that `yang_dnode_new`
creates a tree by validating it, and the validation creates all implicit
default nodes. Therefore, when called with "with-default" flags, instead
of getting an empty tree, we get a tree with all top-level default set.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
There are places, where we can receive an existing commit transaction.
If we don't check that the request already exists, it gets overwritten
and we start having problems with transaction refcounters. Forbid having
multiple configuration sessions simultaneously.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Like in RESTCONF GET request and NETCONF get-data request, make it
possible to request state-only, config-only, or all data.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Currently it's the same as get-tree request for the backend, but it is
going to be expanded in the following commits.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Allow user to specify full YANG compatible XPath 1.0 predicates. This
allows for trimming results of generic queries using functions and other
non-key predicates from XPath 1.0
Signed-off-by: Christian Hopps <chopps@labn.net>
Adds the guts of the get-tree functionality that is called by or calls
the FE and BE code for get-tree processing.
Signed-off-by: Christian Hopps <chopps@labn.net>
The lock/unlocks are being done short-circuit so they are never pending;
however, the handling of the unlock notification was always resuming the command
if pending was set. In all cases pending is set for another command. For example
implicit commit locks then when notified its done unlocks which was clearing the
set-config pending flag and resuming that command incorrectly.
Signed-off-by: Christian Hopps <chopps@labn.net>
Move away from things like "lock if not locked" type code, require the
user has locked prior to geting to that point.
For now we warn if we are taking a lock we already had; however, this
should really be a failure point.
New requirements:
SETCFG -
not implicit commit - requires user has locked candidate DS and they
must unlock after
implicit commit - requires user has locked candidate and running DS
both locks will be unlocked on reply to the SETCFG
COMMITCFG -
requires user has locked candidate and running DS and they must unlock
after
rollback - this code now get both locks and then does an unlock and
early return thing on the adapter side. It needs to be un-special
cased in follow up work that would also include tests for this
functionality.
Signed-off-by: Christian Hopps <chopps@labn.net>
- log names of datastores not numbers
- improve logging for mgmt_msg_read
- Rather than use a bool, instead store the pending const string name of
the command being run that has postponed the CLI. This adds some nice
information to the logging when enabled.
Signed-off-by: Christian Hopps <chopps@labn.net>
