From 4d3c61735cb94864c88e550e09fef6351b38d1c5 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Thu, 28 Feb 2019 16:03:23 +0100 Subject: [PATCH] Ensure non-modstate enabled backends ignore the modstate --- apps/backend/backend_startup.c | 4 + datastore/clixon_xmldb_text.c | 201 +++++++++++++++++++-------------- test/test_upgrade.sh | 60 ++++++---- 3 files changed, 160 insertions(+), 105 deletions(-) diff --git a/apps/backend/backend_startup.c b/apps/backend/backend_startup.c index 9c8df72a..c378d047 100644 --- a/apps/backend/backend_startup.c +++ b/apps/backend/backend_startup.c @@ -318,6 +318,7 @@ startup_failsafe(clicon_handle h) /*! Init modules state of the backend (server). To compare with startup XML * Set the modules state as setopt to the datastore module. + * Only if CLICON_XMLDB_MODSTATE is enabled */ int startup_module_state(clicon_handle h, @@ -326,6 +327,8 @@ startup_module_state(clicon_handle h, int retval = -1; cxobj *x = NULL; + if (!clicon_option_bool(h, "CLICON_XMLDB_MODSTATE")) + goto ok; if (yang_modules_state_get(h, yspec, NULL, 1, &x) < 0) goto done; if (x){ @@ -334,6 +337,7 @@ startup_module_state(clicon_handle h, if (xmldb_setopt(h, "modules_state", (void*)x) < 0) goto done; } + ok: retval = 0; done: return retval; diff --git a/datastore/clixon_xmldb_text.c b/datastore/clixon_xmldb_text.c index 6bd84f1c..91b11f60 100644 --- a/datastore/clixon_xmldb_text.c +++ b/datastore/clixon_xmldb_text.c @@ -90,7 +90,9 @@ struct text_handle { int th_pretty; /* Store xml/json pretty-printed. */ char *th_nacm_mode; cxobj *th_nacm_xtree; - cxobj *th_modst; /* According to RFC7895 for rev mgmnt */ + cxobj *th_modst; /* According to RFC7895 for rev mgmnt + * Should be set only if CLICON_XMLDB_MODSTATE is true + */ }; /* Struct per database in hash */ @@ -552,10 +554,108 @@ xml_copy_marked(cxobj *x0, return retval; } +/*! Read module-state in an XML tree + * + * @param[in] th Datastore text handle + * @param[in] yspec Top-level yang spec + * @param[in] xt XML tree + * @param[out] xmsp If set, return modules-state differences + * + * Read mst (module-state-tree) from xml tree (if any) and compare it with + * the system state mst. + * This can happen: + * 1) There is no modules-state info in the file + * 2) There is module state info in the file + * 3) For each module state m in the file: + * 3a) There is no such module in the system + * 3b) File module-state matches system + * 3c) File module-state does not match system + */ +static int +text_read_modstate(struct text_handle *th, + yang_spec *yspec, + cxobj *xt, + cxobj **xmsp) +{ + int retval = -1; + cxobj *xmodst; + cxobj *xm = NULL; + cxobj *xm2; + cxobj *xs; + char *name; /* module name */ + char *mrev; /* file revision */ + char *srev; /* system revision */ + cxobj *xmsd = NULL; /* Local modules-state diff tree */ + + if ((xmodst = xml_find_type(xt, NULL, "modules-state", CX_ELMNT)) == NULL){ + /* 1) There is no modules-state info in the file */ + } + else if (th->th_modst){ + /* Create a diff tree */ + if (xml_parse_string("", yspec, &xmsd) < 0) + goto done; + if (xml_rootchild(xmsd, 0, &xmsd) < 0) + goto done; + + /* 3) For each module state m in the file */ + while ((xm = xml_child_each(xmodst, xm, CX_ELMNT)) != NULL) { + if (strcmp(xml_name(xm), "module")) + continue; + if ((name = xml_find_body(xm, "name")) == NULL) + continue; + /* 3a) There is no such module in the system */ + if ((xs = xpath_first(th->th_modst, "module[name=\"%s\"]", name)) == NULL){ + // fprintf(stderr, "%s: Module %s: not in system\n", __FUNCTION__, name); + if ((xm2 = xml_dup(xm)) == NULL) + goto done; + if (xml_addsub(xmsd, xm2) < 0) + goto done; + continue; + } + /* These two shouldnt happen since revision is key, just ignore */ + if ((mrev = xml_find_body(xm, "revision")) == NULL) + continue; + if ((srev = xml_find_body(xs, "revision")) == NULL) + continue; + if (strcmp(mrev, srev)==0){ + /* 3b) File module-state matches system */ + // fprintf(stderr, "%s: Module %s: file \"%s\" and system revisions match\n", __FUNCTION__, name, mrev); + } + else{ + /* 3c) File module-state does not match system */ + // fprintf(stderr, "%s: Module %s: file \"%s\" and system \"%s\" revisions do not match\n", __FUNCTION__, name, mrev, srev); + if ((xm2 = xml_dup(xm)) == NULL) + goto done; + if (xml_addsub(xmsd, xm2) < 0) + goto done; + } + } + } + /* The module-state is removed from the input XML tree. This is done + * in all cases, whether CLICON_XMLDB_MODSTATE is on or not. + * Clixon systems with CLICON_XMLDB_MODSTATE disabled ignores it + */ + if (xmodst){ + if (xml_purge(xmodst) < 0) + goto done; + } + if (xmsp){ + *xmsp = xmsd; + xmsd = NULL; + } + retval = 0; + done: + if (xmsd) + xml_free(xmsd); + return retval; +} + /*! Common read function that reads an XML tree from file - * @param[in] db Symbolic database name, eg "candidate", "running" - * @param[out] xp XML tree read from file - * @param[out] xmsp If set, return modules-state differences + * @param[in] th Datastore text handle + * @param[in] db Symbolic database name, eg "candidate", "running" + * @param[in] yspec Top-level yang spec + * @param[out] xp XML tree read from file + * @param[out] xmsp If set, return modules-state differences */ static int text_readfile(struct text_handle *th, @@ -568,7 +668,6 @@ text_readfile(struct text_handle *th, cxobj *x0 = NULL; char *dbfile = NULL; int fd = -1; - cxobj *xmsd = NULL; /* Local modules-state diff tree */ if (text_db2file(th, db, &dbfile) < 0) goto done; @@ -600,86 +699,17 @@ text_readfile(struct text_handle *th, if (singleconfigroot(x0, &x0) < 0) goto done; } - { - cxobj *xmodst; - cxobj *xm = NULL; - cxobj *xm2; - cxobj *xs; - char *name; /* module name */ - char *mrev; /* file revision */ - char *srev; /* system revision */ - /* Read existing if any and compare with systems module revisions: - * This can happen: - * 1) There is no modules-state info in the file - * 2) There is module state info in the file - * 3) For each module state m in the file: - * 3a) There is no such module in the system - * 3b) File module-state matches system - * 3c) File module-state does not match system - */ - if ((xmodst = xml_find_type(x0, NULL, "modules-state", CX_ELMNT)) == NULL){ - /* 1) There is no modules-state info in the file */ - } - else{ - - /* Create a diff tree */ - if (xml_parse_string("", yspec, &xmsd) < 0) - goto done; - if (xml_rootchild(xmsd, 0, &xmsd) < 0) - goto done; - - /* 3) For each module state m in the file */ - while ((xm = xml_child_each(xmodst, xm, CX_ELMNT)) != NULL) { - if (strcmp(xml_name(xm), "module")) - continue; - if ((name = xml_find_body(xm, "name")) == NULL) - continue; - /* 3a) There is no such module in the system */ - if ((xs = xpath_first(th->th_modst, "module[name=\"%s\"]", name)) == NULL){ - // fprintf(stderr, "%s: Module %s: not in system\n", __FUNCTION__, name); - if ((xm2 = xml_dup(xm)) == NULL) - goto done; - if (xml_addsub(xmsd, xm2) < 0) - goto done; - continue; - } - /* These two shouldnt happen since revision is key, just ignore */ - if ((mrev = xml_find_body(xm, "revision")) == NULL) - continue; - if ((srev = xml_find_body(xs, "revision")) == NULL) - continue; - if (strcmp(mrev, srev)==0){ - /* 3b) File module-state matches system */ - // fprintf(stderr, "%s: Module %s: file \"%s\" and system revisions match\n", __FUNCTION__, name, mrev); - } - else{ - /* 3c) File module-state does not match system */ - // fprintf(stderr, "%s: Module %s: file \"%s\" and system \"%s\" revisions do not match\n", __FUNCTION__, name, mrev, srev); - if ((xm2 = xml_dup(xm)) == NULL) - goto done; - if (xml_addsub(xmsd, xm2) < 0) - goto done; - } - } - } - if (xmodst){ - /* For now ignore the modules_state - just remove it*/ - if (xml_purge(xmodst) < 0) - goto done; - } - } + /* From Clixon 3.10,datastore files may contain module-state defining + * which modules are used in the file. + */ + if (text_read_modstate(th, yspec, x0, xmsp) < 0) + goto done; if (xp){ *xp = x0; x0 = NULL; } - if (xmsp){ - *xmsp = xmsd; - xmsd = NULL; - } retval = 0; done: - if (xmsd) - xml_free(xmsd); if (fd != -1) close(fd); if (dbfile) @@ -693,8 +723,8 @@ text_readfile(struct text_handle *th, * The function returns a minimal tree that includes all sub-trees that match * xpath. * This is a clixon datastore plugin of the the xmldb api - * @param[in] h Clicon handle - * @param[in] dbname Name of database to search in (filename including dir path + * @param[in] th Datastore text handle + * @param[in] db Name of database to search in (filename including dir path * @param[in] xpath String with XPATH syntax. or NULL for all * @param[in] config If set only configuration data, else also state * @param[out] xret Single return XML tree. Free with xml_free() @@ -793,8 +823,8 @@ text_get_cache(struct text_handle *th, * The function returns a minimal tree that includes all sub-trees that match * xpath. * This is a clixon datastore plugin of the the xmldb api - * @param[in] h Clicon handle - * @param[in] dbname Name of database to search in (filename including dir path + * @param[in] th Datastore text handle + * @param[in] db Name of database to search in (filename including dir path * @param[in] xpath String with XPATH syntax. or NULL for all * @param[in] config If set only configuration data, else also state * @param[out] xret Single return XML tree. Free with xml_free() @@ -819,7 +849,7 @@ text_get(xmldb_handle xh, } /*! Modify a base tree x0 with x1 with yang spec y according to operation op - * @param[in] th text handle + * @param[in] th Datastore text handle * @param[in] x0 Base xml tree (can be NULL in add scenarios) * @param[in] y0 Yang spec corresponding to xml-node x0. NULL if x0 is NULL * @param[in] x0p Parent of x0 @@ -1104,7 +1134,7 @@ text_modify(struct text_handle *th, } /* text_modify */ /*! Modify a top-level base tree x0 with modification tree x1 - * @param[in] th text handle + * @param[in] th Datastore text handle * @param[in] x0 Base xml tree (can be NULL in add scenarios) * @param[in] x1 xml tree which modifies base * @param[in] yspec Top-level yang spec (if y is NULL) @@ -1406,6 +1436,7 @@ text_put(xmldb_handle xh, goto done; } /* Add module revision info before writing to file) + * Only if CLICON_XMLDB_MODSTATE is set */ if (th->th_modst){ if ((xmodst = xml_dup(th->th_modst)) == NULL) diff --git a/test/test_upgrade.sh b/test/test_upgrade.sh index 9ce19d81..d76cd237 100755 --- a/test/test_upgrade.sh +++ b/test/test_upgrade.sh @@ -248,9 +248,10 @@ EOF # mode is one of: init, none, running, or startup # db is one of: running_db or startup_db runtest(){ - mode=$1 - expect=$2 - startup=$3 + modstate=$1 + mode=$2 + expect=$3 + startup=$4 new "test params: -f $cfg" # Bring your own backend @@ -261,13 +262,13 @@ runtest(){ if [ $? -ne 0 ]; then err fi - new "start backend -s $mode -f $cfg" - start_backend -s $mode -f $cfg + new "start backend -s $mode -f $cfg -o \"CLICON_XMLDB_MODSTATE=$modstate\"" + start_backend -s $mode -f $cfg -o "CLICON_XMLDB_MODSTATE=$modstate" new "waiting" sleep $RCWAIT else - new "Restart backend as eg follows: -Ff $cfg -s $mode ($BETIMEOUT s)" + new "Restart backend as eg follows: -Ff $cfg -s $mode -o \"CLICON_XMLDB_MODSTATE=$modstate\" ($BETIMEOUT s)" sleep $BETIMEOUT fi @@ -292,43 +293,62 @@ runtest(){ # Compatible == all yang modules match # runtest -new "1. Load compatible valid startup (all OK)" +new "1. Run without CLICON_XMLDB_MODSTATE ensure no modstate in datastore" (cd $dir; rm -f tmp_db candidate_db running_db startup_db) # remove databases (cd $dir; cp compat-valid.xml startup_db) -runtest startup 'always workother text' 'always workother text' +runtest false startup 'always workother text' 'always workother text' -new "2. Load compatible running valid running (rest of tests are startup)" +new "Verify no modstate in running" +expect="module" +ret=$(sudo grep $expect $dir/running_db) +if [ -n "$ret" ]; then + err "did not expect $expect" "$ret" +fi + +new "2. Load compatible valid startup (all OK)" +(cd $dir; rm -f tmp_db candidate_db running_db startup_db) # remove databases +(cd $dir; cp compat-valid.xml startup_db) +runtest true startup 'always workother text' 'always workother text' + +new "Verify modstate in running" +expect="module" +ret=$(sudo grep $expect $dir/running_db) +if [ -z "$ret" ]; then + err "Expected $expect" "$ret" +fi + +new "3. Load compatible running valid running (rest of tests are startup)" (cd $dir; rm -f tmp_db candidate_db running_db startup_db) # remove databases (cd $dir; cp compat-valid.xml running_db) -runtest running 'always workother text' 'always workother text' +runtest true running 'always workother text' 'always workother text' -new "3. Load non-compat valid startup" +new "4. Load non-compat valid startup" (cd $dir; rm -f tmp_db candidate_db running_db startup_db) # remove databases (cd $dir; cp non-compat-valid.xml startup_db) -runtest startup 'always workother text' 'always workother text' +runtest true startup 'always workother text' 'always workother text' -new "4. Load non-compat invalid startup. Enter failsafe, startup invalid." +new "5. Load non-compat invalid startup. Enter failsafe, startup invalid." (cd $dir; rm -f tmp_db candidate_db running_db startup_db) # remove databases (cd $dir; cp non-compat-invalid.xml startup_db) -runtest startup 'always work' 'old versionalways workother textbla bla' +runtest true startup 'always work' 'old versionalways workother textbla bla' -new "5. Load non-compat invalid running. Enter failsafe, startup invalid." +new "6. Load non-compat invalid running. Enter failsafe, startup invalid." (cd $dir; rm -f tmp_db candidate_db running_db startup_db) # remove databases (cd $dir; cp non-compat-invalid.xml running_db) -runtest running 'always work' 'old versionalways workother textbla bla' +runtest true running 'always work' 'old versionalways workother textbla bla' -new "6. Load compatible invalid startup." +new "7. Load compatible invalid startup." (cd $dir; rm -f tmp_db candidate_db running_db startup_db) # remove databases (cd $dir; cp compat-invalid.xml startup_db) -runtest startup 'always work' 'old versionalways workother textbla bla' +runtest true startup 'always work' 'old versionalways workother textbla bla' # This testcase contains an error/exception of the clixon xml parser, and # I cant track down the memory leakage. if [ $valgrindtest -ne 2 ]; then -new "7. Load non-compat startup. Syntax fail, enter failsafe, startup invalid" +new "8. Load non-compat startup. Syntax fail, enter failsafe, startup invalid" (cd $dir; rm -f tmp_db candidate_db running_db startup_db) # remove databases (cd $dir; cp compat-err.xml startup_db) -runtest startup 'always work' 'applicationoperation-failederrorread registry' +runtest true startup 'always work' 'applicationoperation-failederrorread registry' fi