diff --git a/CHANGELOG.md b/CHANGELOG.md index b5f10c15..f8c222a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,14 +33,17 @@ Expected: April ### New features - +* Add multiple yang support also for obsolete versions + * This means that files and datastores supporting modstate also looks for deleted or updated yang modules. + * A stricter binding which gives error if loading outdated YANG file does not exist. + ### API changes on existing protocol/config features * Native RESTCONF mode * Renamed restconf "evhtp" mode to "native" mode * To configure native mode use: `configure --with-restconf=native`, changed from: `configure --with-restconf=evhtp` * Native mode MUST use libevhtp from https://github.com/clixon/clixon-libevhtp.git instead from criticalstack - +* Stricter yang checks: you cannot do get-config on datastores that have obsolete YANG. * NETCONF Hello message semantics has been made stricter according to RFC 6241 Sec 8.1, for example: * A client MUST send a element. * Each peer MUST send at least the base NETCONF capability, "urn:ietf:params:netconf:base:1.1" (or 1.0 for RFC 4741) @@ -59,6 +62,7 @@ Developers may need to change their code * Removed `cli_debug()`. Use `cli_debug_backend()` or `cli_debug_restconf()` instead. * Removed `yspec_free()` - replace with `ys_free()` +* Added xerr output parameter to `xmldb_get0()` * Removed `endtag` parameter of `clixon_xml_parse_file()` * Restconf authentication callback (ca_auth) signature changed (again) * Minor modification to 5.0 change: userp removed. diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index 0dc24a30..0335a009 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -419,17 +419,24 @@ client_get_config_only(clicon_handle h, cxobj *xret = NULL; cxobj *xnacm = NULL; cxobj **xvec = NULL; + cxobj *xerr = NULL; size_t xlen; + int ret; /* Note xret can be pruned by nacm below (and change name), * so zero-copy cant be used * Also, must use external namespace context here due to stmt */ if (clicon_option_bool(h, "CLICON_VALIDATE_STATE_XML")){ - if (xmldb_get0(h, "running", YB_MODULE, nsc, NULL, 1, &xret, NULL) < 0) { + if (xmldb_get0(h, "running", YB_MODULE, nsc, NULL, 1, &xret, NULL, NULL) < 0) { if (netconf_operation_failed(cbret, "application", "read registry")< 0) goto done; goto ok; } } else{ - if (xmldb_get0(h, "running", YB_MODULE, nsc, xpath, 1, &xret, NULL) < 0) { + if (xmldb_get0(h, "running", YB_MODULE, nsc, xpath, 1, &xret, NULL, NULL) < 0) { if (netconf_operation_failed(cbret, "application", "read registry")< 0) goto done; goto ok; diff --git a/apps/backend/backend_commit.c b/apps/backend/backend_commit.c index 785c46e0..08734eb6 100644 --- a/apps/backend/backend_commit.c +++ b/apps/backend/backend_commit.c @@ -203,8 +203,9 @@ startup_common(clicon_handle h, /* Get the startup datastore WITHOUT binding to YANG, sorting and default setting. * It is done below, later in this function */ - if (xmldb_get0(h, db, YB_NONE, NULL, "/", 0, &xt, msdiff) < 0) + if ((ret = xmldb_get0(h, db, YB_NONE, NULL, "/", 0, &xt, msdiff, NULL)) < 0) goto done; + /* ret should not be 0 */ if ((yspec = clicon_dbspec_yang(h)) == NULL){ clicon_err(OE_YANG, 0, "Yang spec not set"); goto done; @@ -484,7 +485,7 @@ from_validate_common(clicon_handle h, goto done; } /* This is the state we are going to */ - if (xmldb_get0(h, candidate, YB_MODULE, NULL, "/", 0, &td->td_target, NULL) < 0) + if (xmldb_get0(h, candidate, YB_MODULE, NULL, "/", 0, &td->td_target, NULL, NULL) < 0) goto done; /* Clear flags xpath for get */ @@ -492,7 +493,7 @@ from_validate_common(clicon_handle h, (void*)(XML_FLAG_MARK|XML_FLAG_CHANGE)); /* 2. Parse xml trees * This is the state we are going from */ - if (xmldb_get0(h, "running", YB_MODULE, NULL, "/", 0, &td->td_src, NULL) < 0) + if (xmldb_get0(h, "running", YB_MODULE, NULL, "/", 0, &td->td_src, NULL, NULL) < 0) goto done; /* Clear flags xpath for get */ xml_apply0(td->td_src, CX_ELMNT, (xml_applyfn_t*)xml_flag_reset, @@ -884,7 +885,7 @@ from_client_restart_one(clicon_handle h, if ((td = transaction_new()) == NULL) goto done; /* This is the state we are going to */ - if (xmldb_get0(h, "running", YB_MODULE, NULL, "/", 0, &td->td_target, NULL) < 0) + if (xmldb_get0(h, "running", YB_MODULE, NULL, "/", 0, &td->td_target, NULL, NULL) < 0) goto done; if ((ret = xml_yang_validate_all_top(h, td->td_target, &xerr)) < 0) goto done; @@ -894,7 +895,7 @@ from_client_restart_one(clicon_handle h, goto fail; } /* This is the state we are going from */ - if (xmldb_get0(h, db, YB_MODULE, NULL, "/", 0, &td->td_src, NULL) < 0) + if (xmldb_get0(h, db, YB_MODULE, NULL, "/", 0, &td->td_src, NULL, NULL) < 0) goto done; /* 3. Compute differences */ diff --git a/apps/backend/backend_main.c b/apps/backend/backend_main.c index 79744fa2..2510db17 100644 --- a/apps/backend/backend_main.c +++ b/apps/backend/backend_main.c @@ -946,6 +946,8 @@ main(int argc, if (startup_failsafe(h) < 0){ goto done; } + status = STARTUP_OK; + cbuf_reset(cbret); /* cbret contains error info */ } /* Initiate the shared candidate. */ diff --git a/apps/backend/backend_startup.c b/apps/backend/backend_startup.c index b4a82014..9dd44436 100644 --- a/apps/backend/backend_startup.c +++ b/apps/backend/backend_startup.c @@ -86,7 +86,7 @@ db_merge(clicon_handle h, cxobj *xt = NULL; /* Get data as xml from db1 */ - if (xmldb_get0(h, (char*)db1, YB_MODULE, NULL, NULL, 0, &xt, NULL) < 0) + if (xmldb_get0(h, (char*)db1, YB_MODULE, NULL, NULL, 0, &xt, NULL, NULL) < 0) goto done; xml_name_set(xt, NETCONF_INPUT_CONFIG); /* Merge xml into db2. Without commit */ diff --git a/example/main/example_backend.c b/example/main/example_backend.c index 9d7d757d..cad6dc30 100644 --- a/example/main/example_backend.c +++ b/example/main/example_backend.c @@ -439,7 +439,7 @@ example_statedata(clicon_handle h, * Get config according to xpath */ if ((nsc1 = xml_nsctx_init(NULL, "urn:ietf:params:xml:ns:yang:ietf-interfaces")) == NULL) goto done; - if (xmldb_get0(h, "running", YB_MODULE, nsc1, "/interfaces/interface/name", 1, &xt, NULL) < 0) + if (xmldb_get0(h, "running", YB_MODULE, nsc1, "/interfaces/interface/name", 1, &xt, NULL, NULL) < 0) goto done; if (xpath_vec(xt, nsc1, "/interfaces/interface/name", &xvec, &xlen) < 0) goto done; diff --git a/lib/clixon/clixon_datastore.h b/lib/clixon/clixon_datastore.h index 6aaecb78..cb175f17 100644 --- a/lib/clixon/clixon_datastore.h +++ b/lib/clixon/clixon_datastore.h @@ -53,7 +53,7 @@ int xmldb_disconnect(clicon_handle h); int xmldb_get(clicon_handle h, const char *db, cvec *nsc, char *xpath, cxobj **xtop); int xmldb_get0(clicon_handle h, const char *db, yang_bind yb, cvec *nsc, const char *xpath, - int copy, cxobj **xtop, modstate_diff_t *msd); + int copy, cxobj **xtop, modstate_diff_t *msd, cxobj **xerr); int xmldb_get0_clear(clicon_handle h, cxobj *x); int xmldb_get0_free(clicon_handle h, cxobj **xp); int xmldb_put(clicon_handle h, const char *db, enum operation_type op, cxobj *xt, char *username, cbuf *cbret); /* in clixon_datastore_write.[ch] */ diff --git a/lib/clixon/clixon_yang.h b/lib/clixon/clixon_yang.h index 5a882e03..2311473f 100644 --- a/lib/clixon/clixon_yang.h +++ b/lib/clixon/clixon_yang.h @@ -211,10 +211,12 @@ yang_stmt *yspec_new(void); yang_stmt *ys_new(enum rfc_6020 keyw); yang_stmt *ys_prune(yang_stmt *yp, int i); +int ys_free1(yang_stmt *ys, int self); int ys_free(yang_stmt *ys); int ys_cp(yang_stmt *nw, yang_stmt *old); yang_stmt *ys_dup(yang_stmt *old); int yn_insert(yang_stmt *ys_parent, yang_stmt *ys_child); +int yn_insert1(yang_stmt *ys_parent, yang_stmt *ys_child); yang_stmt *yn_each(yang_stmt *yn, yang_stmt *ys); char *yang_key2str(int keyword); int ys_module_by_xml(yang_stmt *ysp, struct xml *xt, yang_stmt **ymodp); diff --git a/lib/clixon/clixon_yang_module.h b/lib/clixon/clixon_yang_module.h index 4d41decc..e0fe4dfa 100644 --- a/lib/clixon/clixon_yang_module.h +++ b/lib/clixon/clixon_yang_module.h @@ -73,6 +73,8 @@ int clixon_module_upgrade(clicon_handle h, cxobj *xt, modstate_diff_t *msd, cbuf yang_stmt *yang_find_module_by_prefix(yang_stmt *ys, char *prefix); yang_stmt *yang_find_module_by_prefix_yspec(yang_stmt *yspec, char *prefix); yang_stmt *yang_find_module_by_namespace(yang_stmt *yspec, char *ns); +yang_stmt *yang_find_module_by_namespace_revision(yang_stmt *yspec, const char *ns, const char *revision); +yang_stmt *yang_find_module_by_name_revision(yang_stmt *yspec, const char *name, const char *revision); yang_stmt *yang_find_module_by_name(yang_stmt *yspec, char *name); #endif /* _CLIXON_YANG_MODULE_H_ */ diff --git a/lib/src/clixon_datastore_read.c b/lib/src/clixon_datastore_read.c index 1932f65c..93ec02a7 100644 --- a/lib/src/clixon_datastore_read.c +++ b/lib/src/clixon_datastore_read.c @@ -76,10 +76,10 @@ #include "clixon_path.h" #include "clixon_netconf_lib.h" #include "clixon_yang_module.h" +#include "clixon_yang_parse_lib.h" #include "clixon_xml_map.h" #include "clixon_xml_io.h" #include "clixon_xml_nsctx.h" - #include "clixon_datastore.h" #include "clixon_datastore_read.h" @@ -307,6 +307,7 @@ text_read_modstate(clicon_handle h, clicon_err(OE_UNIX, errno, "strdup"); goto done; } + continue; } if (strcmp(xml_name(xf), "module")) continue; /* ignore other tags, such as module-set-id */ @@ -420,13 +421,14 @@ disable_nacm_on_empty(cxobj *xt, * @param[out] xp XML tree read from file * @param[out] de If set, return db-element status (eg empty flag) * @param[out] msdiff If set, return modules-state differences + * @param[out] xerr XML error if retval is 0 * @retval -1 General error, check specific clicon_errno, clicon_suberrno * @retval 0 Parse OK but yang assigment not made (or only partial) and xerr set * @retval 1 OK * @note Use of 1 for OK * @note retval 0 is NYI because calling functions cannot handle it yet + * XXX if this code pass tests this code can be rewritten, esp the modstate stuff */ -#undef XMLDB_READFILE_FAIL /* See comment on retval = 0 above */ int xmldb_readfile(clicon_handle h, const char *db, @@ -434,15 +436,30 @@ xmldb_readfile(clicon_handle h, yang_stmt *yspec, cxobj **xp, db_elmnt *de, - modstate_diff_t *msdiff) + modstate_diff_t *msdiff0, + cxobj **xerr) { - int retval = -1; - cxobj *x0 = NULL; - char *dbfile = NULL; - FILE *fp = NULL; - char *format; - int ret; + int retval = -1; + cxobj *x0 = NULL; + char *dbfile = NULL; + FILE *fp = NULL; + char *format; + int ret; + modstate_diff_t *msdiff = NULL; + cxobj *xmsd; /* XML module state diff */ + yang_stmt *ymod; + char *name; + char *ns; /* namespace */ + char *rev; /* revision */ + int needclone; + cxobj *xmodfile = NULL; + cxobj *x; + yang_stmt *yspec1 = NULL; + if (yb != YB_MODULE && yb != YB_NONE){ + clicon_err(OE_XML, EINVAL, "yb is %d but should be module or none", yb); + goto done; + } if (xmldb_db2file(h, db, &dbfile) < 0) goto done; if (dbfile==NULL){ @@ -458,19 +475,16 @@ xmldb_readfile(clicon_handle h, clicon_err(OE_UNIX, errno, "open(%s)", dbfile); goto done; } + /* ret == 0 should not happen with YB_NONE. Binding is done later */ if (strcmp(format, "json")==0){ - if ((ret = clixon_json_parse_file(fp, yb, yspec, &x0, NULL)) < 0) /* XXX: ret == 0*/ + if (clixon_json_parse_file(fp, YB_NONE, yspec, &x0, xerr) < 0) goto done; } else { - if ((ret = clixon_xml_parse_file(fp, yb, yspec, &x0, NULL)) < 0){ + if (clixon_xml_parse_file(fp, YB_NONE, yspec, &x0, xerr) < 0){ goto done; } } -#ifdef XMLDB_READFILE_FAIL /* The functions calling this function cannot handle a failed parse yet */ - if (ret == 0) - goto fail; -#endif /* Always assert a top-level called "config". * To ensure that, deal with two cases: * 1. File is empty -> rename top-level to "config" @@ -491,15 +505,102 @@ xmldb_readfile(clicon_handle h, /* Datastore files may contain module-state defining * which modules are used in the file. + * Strip module-state and return msdiff */ + if (clicon_option_bool(h, "CLICON_XMLDB_MODSTATE")) + if ((msdiff = modstate_diff_new()) == NULL) + goto done; + if ((x = xml_find_type(x0, NULL, "modules-state", CX_ELMNT)) != NULL) + if ((xmodfile = xml_dup(x)) == NULL) + goto done; if (text_read_modstate(h, yspec, x0, msdiff) < 0) goto done; + if (yb == YB_MODULE){ + if (msdiff){ + /* Check if old/deleted yangs not present in the loaded/running yangspec. + * If so, append them to the global yspec + */ + needclone = 0; + xmsd = NULL; + while ((xmsd = xml_child_each(msdiff->md_diff, xmsd, CX_ELMNT)) != NULL) { + if (xml_flag(xmsd, XML_FLAG_CHANGE|XML_FLAG_DEL) == 0) + continue; + needclone++; + /* Extract name, namespace, and revision */ + if ((name = xml_find_body(xmsd, "name")) == NULL) + continue; + if ((ns = xml_find_body(xmsd, "namespace")) == NULL) + continue; + /* Extract revision */ + if ((rev = xml_find_body(xmsd, "revision")) == NULL) + continue; + if ((ymod = yang_find_module_by_namespace_revision(yspec, ns, rev)) == NULL){ + /* Append it */ + if (yang_spec_parse_module(h, name, rev, yspec) < 0){ + /* Special case: file-not-found errors */ + if (clicon_suberrno == ENOENT){ + cbuf *cberr = NULL; + if ((cberr = cbuf_new()) == NULL){ + clicon_err(OE_XML, errno, "cbuf_new"); + goto done; + } + cprintf(cberr, "Internal error: %s", clicon_err_reason); + clicon_err_reset(); + if (netconf_operation_failed_xml(xerr, "application", cbuf_get(cberr))< 0) + goto done; + cbuf_free(cberr); + goto fail; + } + goto done; + } + } + } + /* If we found an obsolete yang module, we need to make a clone yspec with the + * exactly the yang modules found + */ + if (needclone && xmodfile){ + if ((yspec1 = yspec_new()) == NULL) + goto done; + xmsd = NULL; + while ((xmsd = xml_child_each(xmodfile, xmsd, CX_ELMNT)) != NULL) { + if (strcmp(xml_name(xmsd), "module")) + continue; + if ((ns = xml_find_body(xmsd, "namespace")) == NULL) + continue; + if ((rev = xml_find_body(xmsd, "revision")) == NULL) + continue; + if ((ymod = yang_find_module_by_namespace_revision(yspec, ns, rev)) == NULL) + continue; // XXX error? + if (yn_insert1(yspec1, ymod) < 0) + goto done; + } + } + } + /* xml looks like: ... actually YB_MODULE_NEXT + */ + if ((ret = xml_bind_yang(x0, YB_MODULE, yspec1?yspec1:yspec, xerr)) < 0) + goto done; + if (ret == 0) + goto fail; + } if (xp){ *xp = x0; x0 = NULL; } + if (msdiff0){ + *msdiff0 = *msdiff; + free(msdiff); /* Just body */ + msdiff = NULL; + } retval = 1; done: + if (yspec1){ + ys_free1(yspec1, 1); // XXX free childvec + } + if (xmodfile) + xml_free(xmodfile); + if (msdiff) + modstate_diff_free(msdiff); if (fp) fclose(fp); if (dbfile) @@ -507,11 +608,9 @@ xmldb_readfile(clicon_handle h, if (x0) xml_free(x0); return retval; -#ifdef XMLDB_READFILE_FAIL /* The functions calling this function cannot handle a failed parse yet */ fail: retval = 0; goto done; -#endif } /*! Get content of database using xpath. return a set of matching sub-trees @@ -524,7 +623,8 @@ xmldb_readfile(clicon_handle h, * @param[in] nsc External XML namespace context, or NULL * @param[in] xpath String with XPATH syntax. or NULL for all * @param[out] xret Single return XML tree. Free with xml_free() - * @param[out] msdiff If set, return modules-state differences + * @param[out] msdiff If set, return modules-state differences + * @param[out] xerr XML error if retval is 0 * @retval -1 General error, check specific clicon_errno, clicon_suberrno * @retval 0 Parse OK but yang assigment not made (or only partial) and xerr set * @retval 1 OK @@ -538,7 +638,8 @@ xmldb_get_nocache(clicon_handle h, cvec *nsc, const char *xpath, cxobj **xtop, - modstate_diff_t *msdiff) + modstate_diff_t *msdiff, + cxobj **xerr) { int retval = -1; char *dbfile = NULL; @@ -557,9 +658,7 @@ xmldb_get_nocache(clicon_handle h, goto done; } /* xml looks like: ... where "x" is a top-level symbol in a module */ - if ((ret = xmldb_readfile(h, db, - yb==YB_MODULE?YB_MODULE_NEXT:yb, - yspec, &xt, &de0, msdiff)) < 0) + if ((ret = xmldb_readfile(h, db, yb, yspec, &xt, &de0, msdiff, xerr)) < 0) goto done; if (ret == 0) goto fail; @@ -635,7 +734,8 @@ xmldb_get_nocache(clicon_handle h, * @param[in] nsc External XML namespace context, or NULL * @param[in] xpath String with XPATH syntax. or NULL for all * @param[out] xret Single return XML tree. Free with xml_free() - * @param[out] msdiff If set, return modules-state differences + * @param[out] msdiff If set, return modules-state differences + * @param[out] xerr XML error if retval is 0 * @retval -1 General error, check specific clicon_errno, clicon_suberrno * @retval 0 Parse OK but yang assigment not made (or only partial) and xerr set * @retval 1 OK @@ -649,19 +749,21 @@ xmldb_get_cache(clicon_handle h, cvec *nsc, const char *xpath, cxobj **xtop, - modstate_diff_t *msdiff) + modstate_diff_t *msdiff, + cxobj **xerr) + { - int retval = -1; - yang_stmt *yspec; - cxobj *x0t = NULL; /* (cached) top of tree */ - cxobj *x0; - cxobj **xvec = NULL; - size_t xlen; - int i; - db_elmnt *de = NULL; - cxobj *x1t = NULL; - db_elmnt de0 = {0,}; - int ret; + int retval = -1; + yang_stmt *yspec; + cxobj *x0t = NULL; /* (cached) top of tree */ + cxobj *x0; + cxobj **xvec = NULL; + size_t xlen; + int i; + db_elmnt *de = NULL; + cxobj *x1t = NULL; + db_elmnt de0 = {0,}; + int ret; if ((yspec = clicon_dbspec_yang(h)) == NULL){ clicon_err(OE_YANG, ENOENT, "No yang spec"); @@ -671,9 +773,7 @@ xmldb_get_cache(clicon_handle h, if (de == NULL || de->de_xml == NULL){ /* Cache miss, read XML from file */ /* If there is no xml x0 tree (in cache), then read it from file */ /* xml looks like: ... where "x" is a top-level symbol in a module */ - if ((ret = xmldb_readfile(h, db, - yb==YB_MODULE?YB_MODULE_NEXT:yb, - yspec, &x0t, &de0, msdiff)) < 0) + if ((ret = xmldb_readfile(h, db, yb, yspec, &x0t, &de0, msdiff, xerr)) < 0) goto done; if (ret == 0) goto fail; @@ -686,9 +786,12 @@ xmldb_get_cache(clicon_handle h, else x0t = de->de_xml; - if (yb == YB_MODULE && !xml_spec(x0t)) - if (xml_bind_yang(x0t, YB_MODULE, yspec, NULL) < 0) + if (yb == YB_MODULE && !xml_spec(x0t)){ + if ((ret = xml_bind_yang(x0t, YB_MODULE, yspec, xerr)) < 0) goto done; + if (ret == 0) + ; /* XXX */ + } /* Here x0t looks like: ... */ /* Given the xpath, return a vector of matches in xvec @@ -784,7 +887,8 @@ xmldb_get_cache(clicon_handle h, * @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() - * @param[out] msdiff If set, return modules-state differences + * @param[out] msdiff If set, return modules-state differences + * @param[out] xerr XML error if retval is 0 * @retval -1 General error, check specific clicon_errno, clicon_suberrno * @retval 0 Parse OK but yang assigment not made (or only partial) and xerr set * @retval 1 OK @@ -797,7 +901,9 @@ xmldb_get_zerocopy(clicon_handle h, cvec *nsc, const char *xpath, cxobj **xtop, - modstate_diff_t *msdiff) + modstate_diff_t *msdiff, + cxobj **xerr) + { int retval = -1; yang_stmt *yspec; @@ -818,9 +924,7 @@ xmldb_get_zerocopy(clicon_handle h, if (de == NULL || de->de_xml == NULL){ /* Cache miss, read XML from file */ /* If there is no xml x0 tree (in cache), then read it from file */ /* xml looks like: ... where "x" is a top-level symbol in a module */ - if ((ret = xmldb_readfile(h, db, - yb==YB_MODULE?YB_MODULE_NEXT:yb, - yspec, &x0t, &de0, msdiff)) < 0) + if ((ret = xmldb_readfile(h, db, yb, yspec, &x0t, &de0, msdiff, xerr)) < 0) goto done; if (ret == 0) goto fail; @@ -896,7 +1000,7 @@ xmldb_get(clicon_handle h, char *xpath, cxobj **xret) { - return xmldb_get0(h, db, YB_MODULE, nsc, xpath, 1, xret, NULL); + return xmldb_get0(h, db, YB_MODULE, nsc, xpath, 1, xret, NULL, NULL); } /*! Zero-copy variant of get content of database @@ -915,13 +1019,14 @@ xmldb_get(clicon_handle h, * @param[in] copy Force copy. Overrides cache_zerocopy -> cache * @param[out] xret Single return XML tree. Free with xml_free() * @param[out] msdiff If set, return modules-state differences (upgrade code) + * @param[out] xerr XML error if retval is 0 * @retval -1 General error, check specific clicon_errno, clicon_suberrno * @retval 0 Parse OK but yang assigment not made (or only partial) and xerr set * @retval 1 OK * @note Use of 1 for OK * @code * cxobj *xt; - * if (xmldb_get0(h, "running", YB_MODULE, nsc, "/interface[name="eth"]", 0, &xt, NULL) < 0) + * if (xmldb_get0(h, "running", YB_MODULE, nsc, "/interface[name="eth"]", 0, &xt, NULL, NULL) < 0) * err; * ... * xmldb_get0_clear(h, xt); # Clear tree from default values and flags @@ -938,7 +1043,7 @@ xmldb_get(clicon_handle h, * And a db content: * 1 * With the following call: - * xmldb_get0(h, "running", NULL, NULL, "/c[x=0]", 1, &xt, NULL) + * xmldb_get0(h, "running", NULL, NULL, "/c[x=0]", 1, &xt, NULL, NULL) * which result in a miss (there is no c with x=0), but when the returned xt is printed * (the existing tree is discarded), the default (empty) xml tree is: * 0 @@ -951,7 +1056,8 @@ xmldb_get0(clicon_handle h, const char *xpath, int copy, cxobj **xret, - modstate_diff_t *msdiff) + modstate_diff_t *msdiff, + cxobj **xerr) { int retval = -1; @@ -961,7 +1067,7 @@ xmldb_get0(clicon_handle h, * Add default values in copy * Copy deleted by xmldb_free */ - retval = xmldb_get_nocache(h, db, yb, nsc, xpath, xret, msdiff); + retval = xmldb_get_nocache(h, db, yb, nsc, xpath, xret, msdiff, xerr); break; case DATASTORE_CACHE_ZEROCOPY: /* Get cache (file if empty) mark xpath match in original tree @@ -969,7 +1075,7 @@ xmldb_get0(clicon_handle h, * Default values and markings removed in xmldb_clear */ if (!copy){ - retval = xmldb_get_zerocopy(h, db, yb, nsc, xpath, xret, msdiff); + retval = xmldb_get_zerocopy(h, db, yb, nsc, xpath, xret, msdiff, xerr); break; } /* fall through */ @@ -978,7 +1084,7 @@ xmldb_get0(clicon_handle h, * Add default values in copy, return copy * Copy deleted by xmldb_free */ - retval = xmldb_get_cache(h, db, yb, nsc, xpath, xret, msdiff); + retval = xmldb_get_cache(h, db, yb, nsc, xpath, xret, msdiff, xerr); break; } return retval; diff --git a/lib/src/clixon_datastore_read.h b/lib/src/clixon_datastore_read.h index 91dde81d..8ced129c 100644 --- a/lib/src/clixon_datastore_read.h +++ b/lib/src/clixon_datastore_read.h @@ -42,6 +42,6 @@ * Prototypes */ int xmldb_readfile(clicon_handle h, const char *db, yang_bind yb, yang_stmt *yspec, - cxobj **xp, db_elmnt *de, modstate_diff_t *msd); + cxobj **xp, db_elmnt *de, modstate_diff_t *msd, cxobj **xerr); #endif /* _CLIXON_DATASTORE_READ_H */ diff --git a/lib/src/clixon_datastore_write.c b/lib/src/clixon_datastore_write.c index 68468a65..a4425258 100644 --- a/lib/src/clixon_datastore_write.c +++ b/lib/src/clixon_datastore_write.c @@ -910,22 +910,23 @@ xmldb_put(clicon_handle h, char *username, cbuf *cbret) { - int retval = -1; - char *dbfile = NULL; - FILE *f = NULL; - cbuf *cb = NULL; - yang_stmt *yspec; - cxobj *x0 = NULL; - db_elmnt *de = NULL; - int ret; - cxobj *xnacm = NULL; - cxobj *xmodst = NULL; - cxobj *x; - int permit = 0; /* nacm permit all */ - char *format; - cvec *nsc = NULL; /* nacm namespace context */ - int firsttime = 0; - int pretty; + int retval = -1; + char *dbfile = NULL; + FILE *f = NULL; + cbuf *cb = NULL; + yang_stmt *yspec; + cxobj *x0 = NULL; + db_elmnt *de = NULL; + int ret; + cxobj *xnacm = NULL; + cxobj *xmodst = NULL; + cxobj *x; + int permit = 0; /* nacm permit all */ + char *format; + cvec *nsc = NULL; /* nacm namespace context */ + int firsttime = 0; + int pretty; + cxobj *xerr = NULL; if (cbret == NULL){ clicon_err(OE_XML, EINVAL, "cbret is NULL"); @@ -948,7 +949,7 @@ xmldb_put(clicon_handle h, if (x0 == NULL){ firsttime++; /* to avoid leakage on error, see fail from text_modify */ /* xml looks like: ... where "x" is a top-level symbol in a module */ - if ((ret = xmldb_readfile(h, db, YB_MODULE_NEXT, yspec, &x0, de, NULL)) < 0) + if ((ret = xmldb_readfile(h, db, YB_MODULE, yspec, &x0, de, NULL, &xerr)) < 0) goto done; if (ret == 0) goto fail; @@ -1052,6 +1053,8 @@ xmldb_put(clicon_handle h, done: if (f != NULL) fclose(f); + if (xerr) + xml_free(xerr); if (nsc) xml_nsctx_free(nsc); if (dbfile) diff --git a/lib/src/clixon_nacm.c b/lib/src/clixon_nacm.c index 112a1b46..8ae0de56 100644 --- a/lib/src/clixon_nacm.c +++ b/lib/src/clixon_nacm.c @@ -1212,7 +1212,7 @@ nacm_access_pre(clicon_handle h, goto done; } else if (strcmp(mode, "internal")==0){ - if (xmldb_get0(h, "running", YB_MODULE, nsc, "nacm", 1, &xnacm0, NULL) < 0) + if (xmldb_get0(h, "running", YB_MODULE, nsc, "nacm", 1, &xnacm0, NULL, NULL) < 0) goto done; } else{ diff --git a/lib/src/clixon_xml_bind.c b/lib/src/clixon_xml_bind.c index caa00725..35050cc5 100644 --- a/lib/src/clixon_xml_bind.c +++ b/lib/src/clixon_xml_bind.c @@ -355,7 +355,6 @@ xml_bind_yang(cxobj *xt, int retval = -1; cxobj *xc; /* xml child */ int ret; - int failed = 0; /* we continue loop after failure, should we stop at fail?`*/ strip_whitespace(xt); xc = NULL; /* Apply on children */ @@ -363,10 +362,8 @@ xml_bind_yang(cxobj *xt, if ((ret = xml_bind_yang0(xc, yb, yspec, xerr)) < 0) goto done; if (ret == 0) - failed++; + goto fail; } - if (failed) - goto fail; retval = 1; done: return retval; @@ -384,7 +381,6 @@ xml_bind_yang0_opt(cxobj *xt, int retval = -1; cxobj *xc; /* xml child */ int ret; - int failed = 0; /* we continue loop after failure, should we stop at fail?`*/ yang_stmt *yc0 = NULL; cxobj *xc0 = NULL; cxobj *xs; @@ -428,14 +424,12 @@ xml_bind_yang0_opt(cxobj *xt, else if ((ret = xml_bind_yang0_opt(xc, YB_PARENT, NULL, xerr)) < 0) goto done; if (ret == 0) - failed++; + goto fail; xc0 = xc; yc0 = xml_spec(xc); /* cache */ name0 = xml_name(xc); prefix0 = xml_prefix(xc); } - if (failed) - goto fail; ok: retval = 1; done: @@ -466,7 +460,6 @@ xml_bind_yang0(cxobj *xt, int retval = -1; cxobj *xc; /* xml child */ int ret; - int failed = 0; /* we continue loop after failure, should we stop at fail?`*/ switch (yb){ case YB_MODULE: @@ -495,10 +488,8 @@ xml_bind_yang0(cxobj *xt, if ((ret = xml_bind_yang0_opt(xc, YB_PARENT, NULL, xerr)) < 0) goto done; if (ret == 0) - failed++; + goto fail; } - if (failed) - goto fail; ok: retval = 1; done: diff --git a/lib/src/clixon_yang.c b/lib/src/clixon_yang.c index c33a66ac..6e0c0ab3 100644 --- a/lib/src/clixon_yang.c +++ b/lib/src/clixon_yang.c @@ -452,7 +452,7 @@ ys_new(enum rfc_6020 keyw) * @retval -1 Error * @see ys_free */ -static int +int ys_free1(yang_stmt *ys, int self) { @@ -708,6 +708,20 @@ yn_insert(yang_stmt *ys_parent, return 0; } +/*! Variant of yn_insert where parent is not set + */ +int +yn_insert1(yang_stmt *ys_parent, + yang_stmt *ys_child) +{ + int pos = ys_parent->ys_len; + + if (yn_realloc(ys_parent) < 0) + return -1; + ys_parent->ys_stmt[pos] = ys_child; + return 0; +} + /*! Iterate through all yang statements from a yang node * * @param[in] yparent yang statement whose children should be iterated diff --git a/lib/src/clixon_yang_module.c b/lib/src/clixon_yang_module.c index e19a8d1f..0e256fda 100644 --- a/lib/src/clixon_yang_module.c +++ b/lib/src/clixon_yang_module.c @@ -86,6 +86,7 @@ modstate_diff_t * modstate_diff_new(void) { modstate_diff_t *md; + if ((md = malloc(sizeof(modstate_diff_t))) == NULL){ clicon_err(OE_UNIX, errno, "malloc"); return NULL; @@ -582,6 +583,83 @@ yang_find_module_by_namespace(yang_stmt *yspec, return ymod; } +/*! Given a yang spec, a namespace and revision, return yang module + * + * @param[in] yspec A yang specification + * @param[in] ns Namespace + * @param[in] rev Revision + * @retval ymod Yang module statement if found + * @retval NULL not found + * @see yang_find_module_by_namespace + * @note a module may have many revisions, but only the first is significant + */ +yang_stmt * +yang_find_module_by_namespace_revision(yang_stmt *yspec, + const char *ns, + const char *rev) +{ + yang_stmt *ymod = NULL; + yang_stmt *yrev; + char *rev1; + + if (ns == NULL || rev == NULL){ + clicon_err(OE_CFG, EINVAL, "No ns or rev"); + goto done; + } + while ((ymod = yn_each(yspec, ymod)) != NULL) { + if (yang_find(ymod, Y_NAMESPACE, ns) != NULL) + /* Get FIRST revision */ + if ((yrev = yang_find(ymod, Y_REVISION, NULL)) != NULL){ + rev1 = yang_argument_get(yrev); + if (strcmp(rev, rev1) == 0) + break; /* return this ymod */ + } + } + done: + return ymod; +} + +/*! Given a yang spec, name and revision, return yang module + * + * @param[in] yspec A yang specification + * @param[in] name Name + * @param[in] rev Revision + * @retval ymod Yang module statement if found + * @retval NULL not found + * @see yang_find_module_by_namespace + * @note a module may have many revisions, but only the first is significant + */ +yang_stmt * +yang_find_module_by_name_revision(yang_stmt *yspec, + const char *name, + const char *rev) +{ + yang_stmt *ymod = NULL; + yang_stmt *yrev; + char *rev1; + + if (name == NULL){ + clicon_err(OE_CFG, EINVAL, "No ns or rev"); + goto done; + } + while ((ymod = yn_each(yspec, ymod)) != NULL) { + if (yang_keyword_get(ymod) != Y_MODULE) + continue; + if (strcmp(yang_argument_get(ymod), name) != 0) + continue; + if (rev == NULL) + break; /* Matching revision is NULL, match that */ + /* Get FIRST revision */ + if ((yrev = yang_find(ymod, Y_REVISION, NULL)) != NULL){ + rev1 = yang_argument_get(yrev); + if (strcmp(rev, rev1) == 0) + break; /* return this ymod */ + } + } + done: + return ymod; +} + /*! Given a yang spec and a module name, return yang module or submodule * * @param[in] yspec A yang specification diff --git a/lib/src/clixon_yang_parse_lib.c b/lib/src/clixon_yang_parse_lib.c index a37e4dbd..792264bd 100644 --- a/lib/src/clixon_yang_parse_lib.c +++ b/lib/src/clixon_yang_parse_lib.c @@ -1404,7 +1404,7 @@ yang_parse_post(clicon_handle h, */ int yang_spec_parse_module(clicon_handle h, - const char *module, + const char *name, const char *revision, yang_stmt *yspec) { @@ -1416,16 +1416,16 @@ yang_spec_parse_module(clicon_handle h, clicon_err(OE_YANG, EINVAL, "yang spec is NULL"); goto done; } - if (module == NULL){ + if (name == NULL){ clicon_err(OE_YANG, EINVAL, "yang module not set"); goto done; } /* Apply steps 2.. on new modules, ie ones after modmin. */ modmin = yang_len_get(yspec); /* Do not load module if it already exists */ - if (yang_find(yspec, Y_MODULE, module) != NULL) + if (yang_find_module_by_name_revision(yspec, name, revision) != NULL) goto ok; - if (yang_parse_module(h, module, revision, yspec) == NULL) + if (yang_parse_module(h, name, revision, yspec) == NULL) goto done; if (yang_parse_post(h, yspec, modmin) < 0) goto done; diff --git a/test/test_upgrade_failsafe.sh b/test/test_upgrade_failsafe.sh index 86498657..b2b5f04d 100755 --- a/test/test_upgrade_failsafe.sh +++ b/test/test_upgrade_failsafe.sh @@ -98,6 +98,7 @@ cat < $cfg $cfg ietf-netconf:startup /usr/local/share/clixon + $dir $dir /usr/local/var/$APPNAME/$APPNAME.sock /usr/local/lib/example/backend @@ -266,13 +267,12 @@ runtest(){ 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" - wait_backend - else - new "Restart backend as eg follows: -Ff $cfg -s $mode -o \"CLICON_XMLDB_MODSTATE=$modstate\" ($BETIMEOUT s)" -# sleep $BETIMEOUT +# new "Restart backend as eg follows: -Ff $cfg -s $mode -o \"CLICON_XMLDB_MODSTATE=$modstate\" ($BETIMEOUT s)" fi + new "wait backend" + wait_backend + new "Check running db content" expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^$exprun]]>]]>$" @@ -333,14 +333,13 @@ new "4. Load non-compat valid startup" # Just test that a valid db survives start from running (cd $dir; rm -f tmp_db candidate_db running_db startup_db) # remove databases (cd $dir; cp non-compat-valid.xml startup_db) -runtest true startup 'always workother text' 'always workother text' +runtest true startup 'always workother text' #'always workother text' new "5. Load non-compat invalid startup. Enter failsafe, startup invalid." # A test that if a non-valid startup is encountered, validation fails and failsafe is entered (cd $dir; rm -f tmp_db candidate_db running_db startup_db) # remove databases (cd $dir; cp non-compat-invalid.xml startup_db) -runtest true startup 'always work' 'old versionalways workother textbla bla' # sorted -#runtest true startup 'always work' 'old versionbla blaalways workother text' # unsorted +runtest true startup 'always work' # 'old versionalways workother textbla bla' # sorted 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 @@ -353,8 +352,7 @@ runtest true running 'always work' ' 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 true startup 'always work' 'old versionalways workother textbla bla' # sorted -#runtest true startup 'always work' 'old versionbla blaalways workother text' # unsorted +runtest true startup 'always work' # 'old versionalways workother textbla bla' # sorted # This testcase contains an error/exception of the clixon xml parser, and # I cant track down the memory leakage. diff --git a/test/test_upgrade_repair.sh b/test/test_upgrade_repair.sh index 7d8ab875..4d3e6e32 100755 --- a/test/test_upgrade_repair.sh +++ b/test/test_upgrade_repair.sh @@ -59,6 +59,7 @@ cat < $cfg $cfg ietf-netconf:startup /usr/local/share/clixon + $dir $dir /usr/local/var/$APPNAME/$APPNAME.sock /usr/local/lib/example/backend diff --git a/test/test_yang_anydata.sh b/test/test_yang_anydata.sh index c92147e9..16043e41 100755 --- a/test/test_yang_anydata.sh +++ b/test/test_yang_anydata.sh @@ -116,12 +116,11 @@ function testrun() XML="$XMLA$XMLU" else XML="$XMLA" - unknownreply="applicationunknown-elementu1errorFailed to find YANG spec of XML node: u1 with parent: config in namespace: urn:example:unknown" + unknownreply="applicationunknown-elementu3errorFailed to find YANG spec of XML node: u3 with parent: b in namespace: urn:example:unknown" fi else XML="$XMLA" - unknownreply="applicationunknown --elementu1errorFailed to find YANG spec of XML node: u1 with parent: config in namespace: urn:example:unknown" + unknownreply="applicationunknown-elementu3errorFailed to find YANG spec of XML node: u3 with parent: b in namespace: urn:example:unknown" fi if $startup; then # get config from startup @@ -268,16 +267,16 @@ EOF new "test params: -f $cfg" -new "no startup, dont treat unknown as anydata----" +new "1. no startup, dont treat unknown as anydata----" testrun false false -new "startup, dont treat unknown as anydata----" +new "2. startup, dont treat unknown as anydata----" testrun true false -new "no startup, treat unknown as anydata----" +new "3. no startup, treat unknown as anydata----" testrun false true -new "startup, treat unknown as anydata----" +new "4. startup, treat unknown as anydata----" testrun true true # Set by restconf_config