diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4f9833d2..95e73eea 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,11 @@
## 3.4.0 (Upcoming)
+* Fixed bug that deletes running on startup if backup started with -m running.
+ The error was that the running (or startup) configuration may fail when
+ clixon backend starts.
+ The fix now makes a copy of running and copies it back on failure.
+
* experimental netconf yang rpc
* datastore/keyvalue/Makefile is left behind on make distclean. Fixed by conditional configure. Thanks renato@netgate.com.
diff --git a/apps/backend/backend_commit.c b/apps/backend/backend_commit.c
index bee83370..289fa5c7 100644
--- a/apps/backend/backend_commit.c
+++ b/apps/backend/backend_commit.c
@@ -130,7 +130,8 @@ generic_validate(yang_spec *yspec,
* @param[in] h Clicon handle
* @param[in] candidate The candidate database. The wanted backend state
* @retval 0 OK
- * @retval -1 Fatal error or netconf error XXX Differentiate
+ * @retval -1 Fatal error or validation fail
+ * @note Need to differentiate between error and validation fail
*/
static int
validate_common(clicon_handle h,
@@ -215,7 +216,10 @@ validate_common(clicon_handle h,
* do something more drastic?
* @param[in] h Clicon handle
* @param[in] candidate A candidate database, not necessarily "candidate"
-*/
+ * @retval 0 OK
+ * @retval -1 Fatal error or validation fail
+ * @note Need to differentiate between error and validation fail
+ */
int
candidate_commit(clicon_handle h,
char *candidate)
@@ -285,7 +289,7 @@ from_client_commit(clicon_handle h,
piddb);
goto ok;
}
- if (candidate_commit(h, "candidate") < 0){
+ if (candidate_commit(h, "candidate") < 0){ /* Assume validation fail, nofatal */
clicon_debug(1, "Commit candidate failed");
cprintf(cbret, ""
"invalid-value"
@@ -295,8 +299,6 @@ from_client_commit(clicon_handle h,
"",
clicon_err_reason);
goto ok;
-
- goto ok;
}
cprintf(cbret, "");
ok:
diff --git a/apps/backend/backend_main.c b/apps/backend/backend_main.c
index 927749e1..0eec1203 100644
--- a/apps/backend/backend_main.c
+++ b/apps/backend/backend_main.c
@@ -164,7 +164,7 @@ static int
db_reset(clicon_handle h,
char *db)
{
- if (xmldb_delete(h, db) != 0 && errno != ENOENT)
+ if (xmldb_exists(h, db) == 1 && xmldb_delete(h, db) != 0 && errno != ENOENT)
return -1;
if (xmldb_create(h, db) < 0)
return -1;
@@ -502,13 +502,22 @@ startup_mode_init(clicon_handle h)
/*! Clixon running startup mode: Commit running db configuration into running
*
- copy reset commit merge
-running----+ |--------------------+-----+------>
- \ / /
-candidate +--------------------+ /
- /
-tmp |-------+-----+---------+
+OK:
+ copy reset commit merge
+running----+ |--------------------+--------+------>
+ \ / /
+candidate +--------------------+ /
+ /
+tmp |-------+-----+------------+---|
reset extra file
+
+COMMIT ERROR:
+ copy reset copy
+running----+ |--------------------+------> EXIT
+ \ /
+candidate +--------------------+
+
+ * @note: if commit fails, copy candidate to running and exit
*/
static int
startup_mode_running(clicon_handle h,
@@ -522,9 +531,6 @@ startup_mode_running(clicon_handle h,
/* Load plugins and call plugin_init() */
if (plugin_initiate(h) != 0)
goto done;
- /* Clear running db */
- if (db_reset(h, "running") < 0)
- goto done;
/* Clear tmp db */
if (db_reset(h, "tmp") < 0)
goto done;
@@ -534,25 +540,53 @@ startup_mode_running(clicon_handle h,
/* Get application extra xml from file */
if (load_extraxml(h, extraxml_file, "tmp") < 0)
goto done;
- /* Commit original running */
- if (candidate_commit(h, "candidate") < 0)
+ /* Clear running db */
+ if (db_reset(h, "running") < 0)
goto done;
+ /* Commit original running. Assume -1 is validate fail */
+ if (candidate_commit(h, "candidate") < 0){
+ /* (1) We cannot differentiate between fatal errors and validation
+ * failures
+ * (2) If fatal error, we should exit
+ * (3) If validation fails we cannot continue. How could we?
+ * (4) Need to restore the running db since we destroyed it above
+ */
+ clicon_log(LOG_NOTICE, "%s: Commit of saved running failed, exiting.", __FUNCTION__);
+ /* Reinstate original */
+ if (xmldb_copy(h, "candidate", "running") < 0)
+ goto done;
+ goto done;
+ }
/* Merge user reset state and extra xml file (no commit) */
if (db_merge(h, "tmp", "running") < 0)
goto done;
retval = 0;
done:
+ if (xmldb_delete(h, "tmp") < 0)
+ goto done;
return retval;
}
/*! Clixon startup startup mode: Commit startup configuration into running state
- reset commit merge
-running |--------------------+-----+------>
- / /
-startup --------------------+ /
- /
-tmp |-------+-----+---------+
+
+
+backup +--------------------|
+ copy / reset commit merge
+running |-+----|--------------------+-----+------>
+ / /
+startup -------------------------+--> /
+ /
+tmp -----|-------+-----+---------+--|
reset extra file
+
+COMMIT ERROR:
+backup +------------------------+--|
+ copy / reset copy \
+running |-+----|--------------------+---+------->EXIT
+ error /
+startup -------------------------+--|
+
+ * @note: if commit fails, copy backup to commit and exit
*/
static int
startup_mode_startup(clicon_handle h,
@@ -560,6 +594,9 @@ startup_mode_startup(clicon_handle h,
{
int retval = -1;
+ /* Stash original running to backup */
+ if (xmldb_copy(h, "running", "backup") < 0)
+ goto done;
/* If startup does not exist, clear it */
if (xmldb_exists(h, "startup") != 1) /* diff */
if (xmldb_create(h, "startup") < 0) /* diff */
@@ -567,9 +604,6 @@ startup_mode_startup(clicon_handle h,
/* Load plugins and call plugin_init() */
if (plugin_initiate(h) != 0)
goto done;
- /* Clear running db */
- if (db_reset(h, "running") < 0)
- goto done;
/* Clear tmp db */
if (db_reset(h, "tmp") < 0)
goto done;
@@ -579,14 +613,29 @@ startup_mode_startup(clicon_handle h,
/* Get application extra xml from file */
if (load_extraxml(h, extraxml_file, "tmp") < 0)
goto done;
- /* Commit startup */
- if (candidate_commit(h, "startup") < 0) /* diff */
+ /* Clear running db */
+ if (db_reset(h, "running") < 0)
goto done;
+ /* Commit startup */
+ if (candidate_commit(h, "startup") < 0){ /* diff */
+ /* We cannot differentiate between fatal errors and validation
+ * failures
+ * In both cases we copy back the original running and quit
+ */
+ clicon_log(LOG_NOTICE, "%s: Commit of startup failed, exiting.", __FUNCTION__);
+ if (xmldb_copy(h, "backup", "running") < 0)
+ goto done;
+ goto done;
+ }
/* Merge user reset state and extra xml file (no commit) */
if (db_merge(h, "tmp", "running") < 0)
goto done;
retval = 0;
done:
+ if (xmldb_delete(h, "backup") < 0)
+ goto done;
+ if (xmldb_delete(h, "tmp") < 0)
+ goto done;
return retval;
}
diff --git a/apps/backend/backend_plugin.h b/apps/backend/backend_plugin.h
index c2d2b8a0..b3b0d57b 100644
--- a/apps/backend/backend_plugin.h
+++ b/apps/backend/backend_plugin.h
@@ -40,10 +40,15 @@
* Types
*/
-/*! Transaction data
+/*! Transaction data describing a system transition from a src to target state
* Clicon internal, presented as void* to app's callback in the 'transaction_data'
* type in clicon_backend_api.h
- * XXX: move to .c file?
+ * The struct contains source and target XML tree (e.g. candidate/running)
+ * But primarily a set of XML tree vectors (dvec, avec, cvec) and associated lengths
+ * These contain the difference between src and target XML, ie "what has changed".
+ * It is up to the validate callbacks to ensure that these changes are OK
+ * It is up to the commit callbacks to enforce these changes in the "state" of
+ *the system.
*/
typedef struct {
uint64_t td_id; /* Transaction id */
diff --git a/datastore/text/clixon_xmldb_text.c b/datastore/text/clixon_xmldb_text.c
index d45dca93..375f5a86 100644
--- a/datastore/text/clixon_xmldb_text.c
+++ b/datastore/text/clixon_xmldb_text.c
@@ -975,7 +975,7 @@ text_put(xmldb_handle xh,
/*! Copy database from db1 to db2
* @param[in] xh XMLDB handle
- * @param[in] from Source database copy
+ * @param[in] from Source database
* @param[in] to Destination database
* @retval -1 Error
* @retval 0 OK
@@ -995,7 +995,6 @@ text_copy(xmldb_handle xh,
/* XXX lock */
if (xmltree_cache){
/* 1. Free xml tree in "to"
-
*/
if ((de = hash_value(th->th_dbs, to, NULL)) != NULL){
if (de->de_xml != NULL){
diff --git a/lib/src/clixon_xml_db.c b/lib/src/clixon_xml_db.c
index c07be468..b66a1d56 100644
--- a/lib/src/clixon_xml_db.c
+++ b/lib/src/clixon_xml_db.c
@@ -432,7 +432,7 @@ xmldb_put(clicon_handle h,
/*! Copy database from db1 to db2
* @param[in] h Clicon handle
- * @param[in] from Source database copy
+ * @param[in] from Source database
* @param[in] to Destination database
* @retval -1 Error
* @retval 0 OK