From 6384fb89f19c16b5028e453a57a4263d87ed11c2 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Thu, 23 Jan 2020 22:53:19 +0100 Subject: [PATCH 01/19] Fixed: Yang xpath statements containing prefixes stopped working due to namespace context updates --- CHANGELOG.md | 6 ++++++ configure | 4 ++-- configure.ac | 4 ++-- lib/src/clixon_validate.c | 11 ++++++++++- 4 files changed, 20 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ce3d157..c16e1803 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Clixon Changelog +## 4.3.1 (Expected: January 2020) + +### Corrected Bugs +* Fixed: Yang `must` xpath statements containing prefixes stopped working due to namespace context updates + ## 4.3.0 (1 January 2020) There were several issues with multiple namespaces with augmented yangs in 4.2 that have been fixed in 4.3. Some other highlights include: several issues with XPaths including "canonical namespace context" support, a reorganization of the YANG files shipped with the release, and a wildchar in the CLICON_MODE variable. @@ -12,6 +17,7 @@ There were several issues with multiple namespaces with augmented yangs in 4.2 t * Optional yang files can be installed in a separate dir with `--with-opt-yang-installdir=DIR` (renamed from `with-std-yang-installdir`) * C-API * Changed `clicon_rpc_generate_error(msg, xerr)` to `clicon_rpc_generate_error(xerr, msg, arg)` + * If you pass NULL as arg it produces the same message as before. * Added namespace-context parameter `nsc` to `xpath_first` and `xpath_vec`, (`xpath_vec_nsc` and xpath_first_nsc` are removed). * Added clicon_handle as parameter to all `clicon_connect_` functions to get better error message diff --git a/configure b/configure index c7b411dd..81ee776e 100755 --- a/configure +++ b/configure @@ -2173,8 +2173,8 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu CLIXON_VERSION_MAJOR="4" CLIXON_VERSION_MINOR="3" -CLIXON_VERSION_PATCH="0" -CLIXON_VERSION="\"${CLIXON_VERSION_MAJOR}.${CLIXON_VERSION_MINOR}.${CLIXON_VERSION_PATCH}\"" +CLIXON_VERSION_PATCH="1" +CLIXON_VERSION="\"${CLIXON_VERSION_MAJOR}.${CLIXON_VERSION_MINOR}.${CLIXON_VERSION_PATCH}.PRE\"" # Check CLIgen if test "$prefix" = "NONE"; then diff --git a/configure.ac b/configure.ac index 7b5fbc70..4dcc37e6 100644 --- a/configure.ac +++ b/configure.ac @@ -44,8 +44,8 @@ AC_INIT(lib/clixon/clixon.h.in) CLIXON_VERSION_MAJOR="4" CLIXON_VERSION_MINOR="3" -CLIXON_VERSION_PATCH="0" -CLIXON_VERSION="\"${CLIXON_VERSION_MAJOR}.${CLIXON_VERSION_MINOR}.${CLIXON_VERSION_PATCH}\"" +CLIXON_VERSION_PATCH="1" +CLIXON_VERSION="\"${CLIXON_VERSION_MAJOR}.${CLIXON_VERSION_MINOR}.${CLIXON_VERSION_PATCH}.PRE\"" # Check CLIgen if test "$prefix" = "NONE"; then diff --git a/lib/src/clixon_validate.c b/lib/src/clixon_validate.c index 5d09e7bd..93af49be 100644 --- a/lib/src/clixon_validate.c +++ b/lib/src/clixon_validate.c @@ -1064,6 +1064,7 @@ xml_yang_validate_all(clicon_handle h, cxobj *x; char *namespace = NULL; cbuf *cb = NULL; + cvec *nsc = NULL; /* if not given by argument (overide) use default link and !Node has a config sub-statement and it is false */ @@ -1124,7 +1125,9 @@ xml_yang_validate_all(clicon_handle h, if (yang_keyword_get(yc) != Y_MUST) continue; xpath = yang_argument_get(yc); /* "must" has xpath argument */ - if ((nr = xpath_vec_bool(xt, NULL, "%s", xpath)) < 0) + if (xml_nsctx_yang(yc, &nsc) < 0) + goto done; + if ((nr = xpath_vec_bool(xt, nsc, "%s", xpath)) < 0) goto done; if (!nr){ ye = yang_find(yc, Y_ERROR_MESSAGE, NULL); @@ -1133,6 +1136,10 @@ xml_yang_validate_all(clicon_handle h, goto done; goto fail; } + if (nsc){ + xml_nsctx_free(nsc); + nsc = NULL; + } } /* "when" sub-node RFC 7950 Sec 7.21.5. Can only be one. */ if ((yc = yang_find(ys, Y_WHEN, NULL)) != NULL){ @@ -1165,6 +1172,8 @@ xml_yang_validate_all(clicon_handle h, ok: retval = 1; done: + if (nsc) + xml_nsctx_free(nsc); if (cb) cbuf_free(cb); return retval; From 64f73771d93ee712cab31effac4059929c985135 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Sun, 26 Jan 2020 22:16:12 +0100 Subject: [PATCH 02/19] Fixed: Leafref validation did not cover case of when the "path" statement is declared within a typedef, only if it was declared in the data part directly under leaf. --- CHANGELOG.md | 4 +++- lib/src/clixon_validate.c | 21 ++++++++++++++++----- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c16e1803..06639c54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,8 +3,10 @@ ## 4.3.1 (Expected: January 2020) ### Corrected Bugs +* Fixed: Leafref validation did not cover case of when the "path" statement is declared within a typedef, only if it was declared in the data part directly under leaf. * Fixed: Yang `must` xpath statements containing prefixes stopped working due to namespace context updates - + + ## 4.3.0 (1 January 2020) There were several issues with multiple namespaces with augmented yangs in 4.2 that have been fixed in 4.3. Some other highlights include: several issues with XPaths including "canonical namespace context" support, a reorganization of the YANG files shipped with the release, and a wildchar in the CLICON_MODE variable. diff --git a/lib/src/clixon_validate.c b/lib/src/clixon_validate.c index 93af49be..cf0b8e8b 100644 --- a/lib/src/clixon_validate.c +++ b/lib/src/clixon_validate.c @@ -87,6 +87,7 @@ /*! Validate xml node of type leafref, ensure the value is one of that path's reference * @param[in] xt XML leaf node of type leafref + * @param[in] ys Yang spec of leaf * @param[in] ytype Yang type statement belonging to the XML node * @param[out] xret Error XML tree. Free with xml_free after use * @retval 1 Validation OK @@ -103,11 +104,13 @@ */ static int validate_leafref(cxobj *xt, + yang_stmt *ys, yang_stmt *ytype, cxobj **xret) { int retval = -1; yang_stmt *ypath; + yang_stmt *yp; cxobj **xvec = NULL; cxobj *x; int i; @@ -115,6 +118,7 @@ validate_leafref(cxobj *xt, char *leafrefbody; char *leafbody; cvec *nsc = NULL; + char *path; if ((leafrefbody = xml_body(xt)) == NULL) goto ok; @@ -123,10 +127,17 @@ validate_leafref(cxobj *xt, goto done; goto fail; } - /* XXX see comment above regarding typeref or not */ - if (xml_nsctx_yang(ytype, &nsc) < 0) - goto done; - if (xpath_vec(xt, nsc, "%s", &xvec, &xlen, yang_argument_get(ypath)) < 0) + /* See comment^: If path is defined in typedef or not */ + if ((yp = yang_parent_get(ytype)) != NULL && + yang_keyword_get(yp) == Y_TYPEDEF){ + if (xml_nsctx_yang(ys, &nsc) < 0) + goto done; + } + else + if (xml_nsctx_yang(ytype, &nsc) < 0) + goto done; + path = yang_argument_get(ypath); + if (xpath_vec(xt, nsc, "%s", &xvec, &xlen, path) < 0) goto done; for (i = 0; i < xlen; i++) { x = xvec[i]; @@ -1103,7 +1114,7 @@ xml_yang_validate_all(clicon_handle h, if (yang_type_get(ys, NULL, &yc, NULL, NULL, NULL, NULL, NULL) < 0) goto done; if (strcmp(yang_argument_get(yc), "leafref") == 0){ - if ((ret = validate_leafref(xt, yc, xret)) < 0) + if ((ret = validate_leafref(xt, ys, yc, xret)) < 0) goto done; if (ret == 0) goto fail; From f5209b1fab3f6f7482d7d621a930e966ef024ce5 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Mon, 27 Jan 2020 22:11:35 +0100 Subject: [PATCH 03/19] * Fixed: Validation of user state data led to wrong validation, if state relied on config data, eg leafref/must/when etc. * Fixed: No revision in yang module led to errors in validation of state data --- CHANGELOG.md | 3 ++- apps/backend/backend_plugin.c | 24 ++++++++++++++---------- lib/src/clixon_yang_type.c | 3 ++- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06639c54..bbba6c80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,10 +3,11 @@ ## 4.3.1 (Expected: January 2020) ### Corrected Bugs +* Fixed: Validation of user state data led to wrong validation, if state relied on config data, eg leafref/must/when etc. +* Fixed: No revision in yang module led to errors in validation of state data * Fixed: Leafref validation did not cover case of when the "path" statement is declared within a typedef, only if it was declared in the data part directly under leaf. * Fixed: Yang `must` xpath statements containing prefixes stopped working due to namespace context updates - ## 4.3.0 (1 January 2020) There were several issues with multiple namespaces with augmented yangs in 4.2 that have been fixed in 4.3. Some other highlights include: several issues with XPaths including "canonical namespace context" support, a reorganization of the YANG files shipped with the release, and a wildchar in the CLICON_MODE variable. diff --git a/apps/backend/backend_plugin.c b/apps/backend/backend_plugin.c index 1dde1a65..19da9990 100644 --- a/apps/backend/backend_plugin.c +++ b/apps/backend/backend_plugin.c @@ -116,6 +116,7 @@ clixon_plugin_statedata(clicon_handle h, clixon_plugin *cp = NULL; plgstatedata_t *fn; /* Plugin statedata fn */ cbuf *cberr = NULL; + int i = 0; while ((cp = clixon_plugin_each(h, cp)) != NULL) { if ((fn = cp->cp_api.ca_statedata) == NULL) @@ -124,14 +125,25 @@ clixon_plugin_statedata(clicon_handle h, goto done; if (fn(h, nsc, xpath, x) < 0) goto fail; /* Dont quit here on user callbacks */ + i++; /* indicates that new state data is added */ if (xml_apply(x, CX_ELMNT, xml_spec_populate, yspec) < 0) goto done; + if ((ret = netconf_trymerge(x, yspec, xret)) < 0) + goto done; + if (ret == 0) + goto fail; + if (x){ + xml_free(x); + x = NULL; + } + } + if (i){ /* Check XML from state callback by validating it. return internal * error with error cause */ - if ((ret = xml_yang_validate_all_top(h, x, &xerr)) < 0) + if ((ret = xml_yang_validate_all_top(h, *xret, &xerr)) < 0) goto done; - if (ret > 0 && (ret = xml_yang_validate_add(h, x, &xerr)) < 0) + if (ret > 0 && (ret = xml_yang_validate_add(h, *xret, &xerr)) < 0) goto done; if (ret == 0){ cxobj *xe; @@ -164,14 +176,6 @@ clixon_plugin_statedata(clicon_handle h, cbuf_free(ccc); } #endif - if ((ret = netconf_trymerge(x, yspec, xret)) < 0) - goto done; - if (ret == 0) - goto fail; - if (x){ - xml_free(x); - x = NULL; - } } retval = 1; done: diff --git a/lib/src/clixon_yang_type.c b/lib/src/clixon_yang_type.c index 9016cdc3..0cd50346 100644 --- a/lib/src/clixon_yang_type.c +++ b/lib/src/clixon_yang_type.c @@ -1104,7 +1104,8 @@ ys_cv_validate(clicon_handle h, /* Note restype can be NULL here for example with unresolved hardcoded uuid */ if (restype && strcmp(restype, "union") == 0){ assert(cvtype == CGV_REST); - val = cv_string_get(cv); + if ((val = cv_string_get(cv)) == NULL) + val = ""; if ((retval2 = ys_cv_validate_union(h, ys, reason, yrestype, origtype, val)) < 0) goto done; retval = retval2; /* invalid (0) with latest reason or valid 1 */ From 20d28b479691a142b83600914e67facb5e0558bb Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Wed, 29 Jan 2020 21:56:50 +0100 Subject: [PATCH 04/19] Compile option `VALIDATE_STATE_XML` introduced in `include/custom.h` to control whether code for state data validation is compiled or not. --- CHANGELOG.md | 1 + apps/backend/backend_client.c | 56 ++++++++++++++++++++++++++++++++++- apps/backend/backend_plugin.c | 54 ++++++--------------------------- include/clixon_custom.h | 8 +++++ lib/src/clixon_xpath_eval.c | 4 +-- 5 files changed, 75 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bbba6c80..44268286 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## 4.3.1 (Expected: January 2020) ### Corrected Bugs +* Compile option `VALIDATE_STATE_XML` introduced in `include/custom.h` to control whether code for state data validation is compiled or not. * Fixed: Validation of user state data led to wrong validation, if state relied on config data, eg leafref/must/when etc. * Fixed: No revision in yang module led to errors in validation of state data * Fixed: Leafref validation did not cover case of when the "path" statement is declared within a typedef, only if it was declared in the data part directly under leaf. diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index 943e9510..100eb162 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -252,6 +252,7 @@ client_get_streams(clicon_handle h, * @param[in] h Clicon handle * @param[in] xpath Xpath selection, not used but may be to filter early * @param[in] nsc XML Namespace context for xpath + * @param[in] content config/state or both * @param[in,out] xret Existing XML tree, merge x into this * @retval -1 Error (fatal) * @retval 0 Statedata callback failed (clicon_err called) @@ -261,6 +262,7 @@ static int client_statedata(clicon_handle h, char *xpath, cvec *nsc, + netconf_content content, cxobj **xret) { int retval = -1; @@ -271,6 +273,9 @@ client_statedata(clicon_handle h, yang_stmt *ymod; int ret; char *namespace; +#ifdef VALIDATE_STATE_XML + cxobj *xerr = NULL; +#endif if ((yspec = clicon_dbspec_yang(h)) == NULL){ clicon_err(OE_YANG, ENOENT, "No yang spec"); @@ -320,6 +325,51 @@ client_statedata(clicon_handle h, goto done; if (ret == 0) goto fail; + +#ifdef VALIDATE_STATE_XML + /* Check XML from state callback by validating it. return internal + * error with error cause + * Only if config has been read, disable validation on state-only + */ + if (content != CONTENT_NONCONFIG){ + if ((ret = xml_yang_validate_all_top(h, *xret, &xerr)) < 0) + goto done; + if (ret > 0 && (ret = xml_yang_validate_add(h, *xret, &xerr)) < 0) + goto done; + if (ret == 0){ + cxobj *xe; + cxobj *xb; + +#if 1 + if (debug){ + cbuf *ccc=cbuf_new(); + if (clicon_xml2cbuf(ccc, *xret, 0, 0, -1) < 0) + goto done; + clicon_debug(1, "%s FAIL: %s", __FUNCTION__, cbuf_get(ccc)); + cbuf_free(ccc); + } +#endif + if ((xe = xpath_first(xerr, NULL, "//error-tag")) != NULL && + (xb = xml_body_get(xe))){ + if (xml_value_set(xb, "operation-failed") < 0) + goto done; + } + if ((xe = xpath_first(xerr, NULL, "//error-message")) != NULL && + (xb = xml_body_get(xe))){ + if (xml_value_append(xb, " Internal error, state callback returned invalid XML") < 0) + goto done; + } + if (*xret){ + xml_free(*xret); + *xret = NULL; + } + *xret = xerr; + xerr = NULL; + goto fail; + } + } +#endif /* VALIDATE_STATE_XML */ + /* Code complex to filter out anything that is outside of xpath * Actually this is a safety catch, should really be done in plugins * and modules_state functions. @@ -344,6 +394,10 @@ client_statedata(clicon_handle h, retval = 1; /* OK */ done: clicon_debug(1, "%s %d", __FUNCTION__, retval); +#ifdef VALIDATE_STATE_XML + if (xerr) + xml_free(xerr); +#endif if (xvec) free(xvec); return retval; @@ -955,7 +1009,7 @@ from_client_get(clicon_handle h, if (content != CONTENT_CONFIG){ /* Get state data from plugins as defined by plugin_statedata(), if any */ clicon_err_reset(); - if ((ret = client_statedata(h, xpath, nsc, &xret)) < 0) + if ((ret = client_statedata(h, xpath, nsc, content, &xret)) < 0) goto done; if (ret == 0){ /* Error from callback (error in xret) */ if (clicon_xml2cbuf(cbret, xret, 0, 0, -1) < 0) diff --git a/apps/backend/backend_plugin.c b/apps/backend/backend_plugin.c index 19da9990..9e5c398b 100644 --- a/apps/backend/backend_plugin.c +++ b/apps/backend/backend_plugin.c @@ -111,12 +111,10 @@ clixon_plugin_statedata(clicon_handle h, { int retval = -1; int ret; - cxobj *xerr = NULL; cxobj *x = NULL; clixon_plugin *cp = NULL; plgstatedata_t *fn; /* Plugin statedata fn */ cbuf *cberr = NULL; - int i = 0; while ((cp = clixon_plugin_each(h, cp)) != NULL) { if ((fn = cp->cp_api.ca_statedata) == NULL) @@ -125,7 +123,15 @@ clixon_plugin_statedata(clicon_handle h, goto done; if (fn(h, nsc, xpath, x) < 0) goto fail; /* Dont quit here on user callbacks */ - i++; /* indicates that new state data is added */ +#if 1 + if (debug){ + cbuf *ccc=cbuf_new(); + if (clicon_xml2cbuf(ccc, x, 0, 0, -1) < 0) + goto done; + clicon_debug(1, "%s STATE: %s", __FUNCTION__, cbuf_get(ccc)); + cbuf_free(ccc); + } +#endif if (xml_apply(x, CX_ELMNT, xml_spec_populate, yspec) < 0) goto done; if ((ret = netconf_trymerge(x, yspec, xret)) < 0) @@ -137,54 +143,12 @@ clixon_plugin_statedata(clicon_handle h, x = NULL; } } - if (i){ - /* Check XML from state callback by validating it. return internal - * error with error cause - */ - if ((ret = xml_yang_validate_all_top(h, *xret, &xerr)) < 0) - goto done; - if (ret > 0 && (ret = xml_yang_validate_add(h, *xret, &xerr)) < 0) - goto done; - if (ret == 0){ - cxobj *xe; - cxobj *xb; - - if ((xe = xpath_first(xerr, NULL, "//error-tag")) != NULL && - (xb = xml_body_get(xe))){ - if (xml_value_set(xb, "operation-failed") < 0) - goto done; - } - if ((xe = xpath_first(xerr, NULL, "//error-message")) != NULL && - (xb = xml_body_get(xe))){ - if (xml_value_append(xb, " Internal error, state callback returned invalid XML") < 0) - goto done; - } - if (*xret){ - xml_free(*xret); - *xret = NULL; - } - *xret = xerr; - xerr = NULL; - goto fail; - } -#if 1 - if (debug){ - cbuf *ccc=cbuf_new(); - if (clicon_xml2cbuf(ccc, x, 0, 0, -1) < 0) - goto done; - clicon_debug(1, "%s MERGE: %s", __FUNCTION__, cbuf_get(ccc)); - cbuf_free(ccc); - } -#endif - } retval = 1; done: if (cberr) cbuf_free(cberr); if (x) xml_free(x); - if (xerr) - xml_free(xerr); return retval; fail: retval = 0; diff --git a/include/clixon_custom.h b/include/clixon_custom.h index 2da8699c..34ebbe85 100644 --- a/include/clixon_custom.h +++ b/include/clixon_custom.h @@ -64,3 +64,11 @@ * binary search. This only works if "y" has proper yang binding and is sorted by system */ #undef XPATH_LIST_OPTIMIZE + +/*! Validate user state callback content + * Use may register state callbacks using ca_statedata callback + * When this option is set, the XML returned from the callback is validated after merging with the running + * db. If it fails, an internal error is returned to the originating user. + * If the option is not set, the XML returned by the user is not validated. + */ +#define VALIDATE_STATE_XML diff --git a/lib/src/clixon_xpath_eval.c b/lib/src/clixon_xpath_eval.c index 60ee0172..dd3c0965 100644 --- a/lib/src/clixon_xpath_eval.c +++ b/lib/src/clixon_xpath_eval.c @@ -932,7 +932,7 @@ xp_eval(xp_ctx *xc, xp_ctx *xr2 = NULL; int use_xr0 = 0; /* In 2nd child use transitively result of 1st child */ - if (debug) + if (debug > 1) ctx_print(stderr, xc, xpath_tree_int2str(xs->xs_type)); /* Pre-actions before check first child c0 */ @@ -1088,7 +1088,7 @@ xp_eval(xp_ctx *xc, xr0 = NULL; } ok: - if (debug) + if (debug > 1) ctx_print(stderr, *xrp, xpath_tree_int2str(xs->xs_type)); retval = 0; done: From b7f1d72c283de437672cf46b4f95f222e14447b3 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Fri, 31 Jan 2020 16:13:50 +0100 Subject: [PATCH 05/19] read whole running config when reading state data --- apps/backend/backend_client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index 100eb162..238a5341 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -1000,7 +1000,7 @@ from_client_get(clicon_handle h, * metrged with state data, so zero-copy cant be used * Also, must use external namespace context here due to Date: Sat, 1 Feb 2020 20:16:04 +0100 Subject: [PATCH 06/19] Restructure of get state/config code to enable all combinations of filtering config/state for both and --- CHANGELOG.md | 2 +- apps/backend/backend_client.c | 386 +++++++++++++++++++--------------- 2 files changed, 223 insertions(+), 165 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44268286..0e621e4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Clixon Changelog -## 4.3.1 (Expected: January 2020) +## 4.3.1 (Expected: February 2020) ### Corrected Bugs * Compile option `VALIDATE_STATE_XML` introduced in `include/custom.h` to control whether code for state data validation is compiled or not. diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index 238a5341..59605028 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -266,17 +266,11 @@ client_statedata(clicon_handle h, cxobj **xret) { int retval = -1; - cxobj **xvec = NULL; - size_t xlen; - int i; yang_stmt *yspec; yang_stmt *ymod; int ret; char *namespace; -#ifdef VALIDATE_STATE_XML - cxobj *xerr = NULL; -#endif - + if ((yspec = clicon_dbspec_yang(h)) == NULL){ clicon_err(OE_YANG, ENOENT, "No yang spec"); goto done; @@ -325,81 +319,9 @@ client_statedata(clicon_handle h, goto done; if (ret == 0) goto fail; - -#ifdef VALIDATE_STATE_XML - /* Check XML from state callback by validating it. return internal - * error with error cause - * Only if config has been read, disable validation on state-only - */ - if (content != CONTENT_NONCONFIG){ - if ((ret = xml_yang_validate_all_top(h, *xret, &xerr)) < 0) - goto done; - if (ret > 0 && (ret = xml_yang_validate_add(h, *xret, &xerr)) < 0) - goto done; - if (ret == 0){ - cxobj *xe; - cxobj *xb; - -#if 1 - if (debug){ - cbuf *ccc=cbuf_new(); - if (clicon_xml2cbuf(ccc, *xret, 0, 0, -1) < 0) - goto done; - clicon_debug(1, "%s FAIL: %s", __FUNCTION__, cbuf_get(ccc)); - cbuf_free(ccc); - } -#endif - if ((xe = xpath_first(xerr, NULL, "//error-tag")) != NULL && - (xb = xml_body_get(xe))){ - if (xml_value_set(xb, "operation-failed") < 0) - goto done; - } - if ((xe = xpath_first(xerr, NULL, "//error-message")) != NULL && - (xb = xml_body_get(xe))){ - if (xml_value_append(xb, " Internal error, state callback returned invalid XML") < 0) - goto done; - } - if (*xret){ - xml_free(*xret); - *xret = NULL; - } - *xret = xerr; - xerr = NULL; - goto fail; - } - } -#endif /* VALIDATE_STATE_XML */ - - /* Code complex to filter out anything that is outside of xpath - * Actually this is a safety catch, should really be done in plugins - * and modules_state functions. - */ - if (xpath_vec(*xret, nsc, "%s", &xvec, &xlen, xpath?xpath:"/") < 0) - goto done; - /* If vectors are specified then mark the nodes found and - * then filter out everything else, - * otherwise return complete tree. - */ - if (xvec != NULL){ - for (i=0; i + * Function reused from both from_client_get() and from_client_get_config + * @param[in] yspec + * @param[in] db + * @param[in] xpath + * @param[in] username + * @param[in] content + * @param[in] depth * @param[out] cbret Return xml tree, eg ..., - * The set of namespace declarations are those in scope on the - * element. - */ - else - if (xml_nsctx_node(xfilter, &nsc) < 0) - goto done; - if (xpath2canonical(xpath0, nsc, yspec, &xpath, &nsc1) < 0) - goto done; - if (nsc) - xml_nsctx_free(nsc); - nsc = nsc1; - } /* 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 0?depth+1:depth) < 0) goto done; } cprintf(cbret, ""); ok: retval = 0; done: - if (xpath) - free(xpath); - if (xnacm) - xml_free(xnacm); if (xvec) free(xvec); + if (xnacm) + xml_free(xnacm); + if (xret) + xml_free(xret); + return retval; +} + +/*! Retrieve all or part of a specified configuration. + * + * @param[in] h Clicon handle + * @param[in] xe Request: + * @param[out] cbret Return xml tree, eg ..., + * The set of namespace declarations are those in scope on the + * element. + */ + else + if (xml_nsctx_node(xfilter, &nsc) < 0) + goto done; + if (xpath2canonical(xpath0, nsc, yspec, &xpath, &nsc1) < 0) + goto done; + if (nsc) + xml_nsctx_free(nsc); + nsc = nsc1; + } + /* Clixon extensions: depth */ + if ((attr = xml_find_value(xe, "depth")) != NULL){ + char *reason = NULL; + if ((ret = parse_int32(attr, &depth, &reason)) < 0){ + clicon_err(OE_XML, errno, "parse_int32"); + goto done; + } + if (ret == 0){ + if (netconf_bad_attribute(cbret, "application", + "depth", "Unrecognized value of depth attribute") < 0) + goto done; + goto ok; + } + } + if ((ret = client_config_only(h, nsc, yspec, db, xpath, username, -1, cbret)) < 0) + goto done; + ok: + retval = 0; + done: + if (xpath) + free(xpath); if (nsc) xml_nsctx_free(nsc); if (cbx) cbuf_free(cbx); - if (xret) - xml_free(xret); return retval; } @@ -547,7 +520,6 @@ from_client_edit_config(clicon_handle h, cxobj *xc; cxobj *x; enum operation_type operation = OP_MERGE; - int non_config = 0; yang_stmt *yspec; cbuf *cbx = NULL; /* Assist cbuf */ @@ -953,6 +925,12 @@ from_client_get(clicon_handle h, netconf_content content = CONTENT_ALL; int32_t depth = -1; /* Nr of levels to print, -1 is all, 0 is none */ yang_stmt *yspec; + int i; +#ifdef VALIDATE_STATE_XML + cxobj *xerr = NULL; + cxobj *xr; + cxobj *xb; +#endif username = clicon_username_get(h); if ((yspec = clicon_dbspec_yang(h)) == NULL){ @@ -977,10 +955,10 @@ from_client_get(clicon_handle h, xml_nsctx_free(nsc); nsc = nsc1; } - /* Clixon extensions: depth and content */ + /* Clixon extensions: content */ if ((attr = xml_find_value(xe, "content")) != NULL) content = netconf_content_str2int(attr); - + /* Clixon extensions: depth */ if ((attr = xml_find_value(xe, "depth")) != NULL){ char *reason = NULL; if ((ret = parse_int32(attr, &depth, &reason)) < 0){ @@ -994,29 +972,105 @@ from_client_get(clicon_handle h, goto ok; } } - if (content != CONTENT_NONCONFIG){ - /* Get config - * Note xret can be pruned by nacm below and change name and - * metrged with state data, so zero-copy cant be used - * Also, must use external namespace context here due to 0 && (ret = xml_yang_validate_add(h, xret, &xerr)) < 0) + goto done; + if (ret == 0){ +#if 1 + if (debug){ + cbuf *ccc=cbuf_new(); + if (clicon_xml2cbuf(ccc, xret, 0, 0, -1) < 0) + goto done; + clicon_debug(1, "%s FAIL: %s", __FUNCTION__, cbuf_get(ccc)); + cbuf_free(ccc); + } +#endif + if ((xr = xpath_first(xerr, NULL, "//error-tag")) != NULL && + (xb = xml_body_get(xr))){ + if (xml_value_set(xb, "operation-failed") < 0) + goto done; + } + if ((xr = xpath_first(xerr, NULL, "//error-message")) != NULL && + (xb = xml_body_get(xr))){ + if (xml_value_append(xb, " Internal error, state callback returned invalid XML") < 0) + goto done; + } + if (clicon_xml2cbuf(cbret, xerr, 0, 0, -1) < 0) + goto done; + goto ok; + } +#endif /* VALIDATE_STATE_XML */ + if (content == CONTENT_NONCONFIG){ /* state only, all config should be removed now */ + /* Keep state data only, remove everything that is not config. Note that state data + * may be a sub-part in a config tree, we need to traverse to find all + */ + if (xml_apply(xret, CX_ELMNT, xml_non_config_data, NULL) < 0) + goto done; + if (xml_tree_prune_flagged_sub(xret, XML_FLAG_MARK, 1, NULL) < 0) + goto done; + if (xml_apply(xret, CX_ELMNT, (xml_applyfn_t*)xml_flag_reset, (void*)XML_FLAG_MARK) < 0) + goto done; + } + /* Code complex to filter out anything that is outside of xpath + * Actually this is a safety catch, should really be done in plugins + * and modules_state functions. + */ + if (xpath_vec(xret, nsc, "%s", &xvec, &xlen, xpath?xpath:"/") < 0) + goto done; + /* If vectors are specified then mark the nodes found and + * then filter out everything else, + * otherwise return complete tree. + */ + if (xvec != NULL){ + for (i=0; i Date: Sun, 2 Feb 2020 08:50:44 +0100 Subject: [PATCH 07/19] 4.3.1 release --- CHANGELOG.md | 4 +++- configure | 2 +- configure.ac | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e621e4a..ddcc3088 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ # Clixon Changelog -## 4.3.1 (Expected: February 2020) +## 4.3.1 (2 February 2020) + +Patch release based on testing by Dave Cornejo, Netgate ### Corrected Bugs * Compile option `VALIDATE_STATE_XML` introduced in `include/custom.h` to control whether code for state data validation is compiled or not. diff --git a/configure b/configure index 81ee776e..96f910f7 100755 --- a/configure +++ b/configure @@ -2174,7 +2174,7 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu CLIXON_VERSION_MAJOR="4" CLIXON_VERSION_MINOR="3" CLIXON_VERSION_PATCH="1" -CLIXON_VERSION="\"${CLIXON_VERSION_MAJOR}.${CLIXON_VERSION_MINOR}.${CLIXON_VERSION_PATCH}.PRE\"" +CLIXON_VERSION="\"${CLIXON_VERSION_MAJOR}.${CLIXON_VERSION_MINOR}.${CLIXON_VERSION_PATCH}\"" # Check CLIgen if test "$prefix" = "NONE"; then diff --git a/configure.ac b/configure.ac index 4dcc37e6..42dacdd7 100644 --- a/configure.ac +++ b/configure.ac @@ -45,7 +45,7 @@ AC_INIT(lib/clixon/clixon.h.in) CLIXON_VERSION_MAJOR="4" CLIXON_VERSION_MINOR="3" CLIXON_VERSION_PATCH="1" -CLIXON_VERSION="\"${CLIXON_VERSION_MAJOR}.${CLIXON_VERSION_MINOR}.${CLIXON_VERSION_PATCH}.PRE\"" +CLIXON_VERSION="\"${CLIXON_VERSION_MAJOR}.${CLIXON_VERSION_MINOR}.${CLIXON_VERSION_PATCH}\"" # Check CLIgen if test "$prefix" = "NONE"; then From 3d5c2cc6783c94fc066981e67877feaac6d15909 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Wed, 5 Feb 2020 16:09:26 +0100 Subject: [PATCH 08/19] * Session-id CLI functionality delayed: "lazy evaluation" * From a cli perspective this is a revert to 4.1 behaviour, where the cli does not immediately exit on start if the backend is not running, but with the new session-id function --- CHANGELOG.md | 4 + apps/backend/backend_client.c | 6 +- apps/cli/cli_main.c | 12 +-- configure | 4 +- configure.ac | 4 +- lib/clixon/clixon_data.h | 3 +- lib/src/clixon_data.c | 20 +++-- lib/src/clixon_proto_client.c | 133 ++++++++++++++++++++++++++-------- test/test_sock.sh | 4 +- 9 files changed, 133 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ddcc3088..4dd90f3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ Patch release based on testing by Dave Cornejo, Netgate +### Minor changes +* Session-id CLI functionality delayed: "lazy evaluation" + * From a cli perspective this is a revert to 4.1 behaviour, where the cli does not immediately exit on start if the backend is not running, but with the new session-id function + ### Corrected Bugs * Compile option `VALIDATE_STATE_XML` introduced in `include/custom.h` to control whether code for state data validation is compiled or not. * Fixed: Validation of user state data led to wrong validation, if state relied on config data, eg leafref/must/when etc. diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index 59605028..b2ddf5f3 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -1426,11 +1426,15 @@ from_client_hello(clicon_handle h, int retval = -1; uint32_t id; - id = clicon_session_id_get(h); + if (clicon_session_id_get(h, &id) < 0){ + clicon_err(OE_DAEMON, ENOENT, "session_id not set"); + goto done; + } id++; clicon_session_id_set(h, id); cprintf(cbret, "%u", id); retval = 0; + done: return retval; } diff --git a/apps/cli/cli_main.c b/apps/cli/cli_main.c index 9f0510b3..9cd860c0 100644 --- a/apps/cli/cli_main.c +++ b/apps/cli/cli_main.c @@ -286,7 +286,6 @@ main(int argc, char **argv) char *str; int tabmode; char *dir; - uint32_t id = 0; cvec *nsctx_global = NULL; /* Global namespace context */ /* Defaults */ @@ -605,17 +604,8 @@ main(int argc, char **argv) } /* Go into event-loop unless -1 command-line */ - if (!once){ - /* Send hello request to backend to get session-id back - * This is done once at the beginning of the session and then this is - * used by the client, even though new TCP sessions are created for - * each message sent to the backend. - */ - if (clicon_hello_req(h, &id) < 0) - goto done; - clicon_session_id_set(h, id); + if (!once) retval = cli_interactive(h); - } else retval = 0; done: diff --git a/configure b/configure index 96f910f7..67a83c0b 100755 --- a/configure +++ b/configure @@ -2173,8 +2173,8 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu CLIXON_VERSION_MAJOR="4" CLIXON_VERSION_MINOR="3" -CLIXON_VERSION_PATCH="1" -CLIXON_VERSION="\"${CLIXON_VERSION_MAJOR}.${CLIXON_VERSION_MINOR}.${CLIXON_VERSION_PATCH}\"" +CLIXON_VERSION_PATCH="2" +CLIXON_VERSION="\"${CLIXON_VERSION_MAJOR}.${CLIXON_VERSION_MINOR}.${CLIXON_VERSION_PATCH}.PRE\"" # Check CLIgen if test "$prefix" = "NONE"; then diff --git a/configure.ac b/configure.ac index 42dacdd7..b7d0ef04 100644 --- a/configure.ac +++ b/configure.ac @@ -44,8 +44,8 @@ AC_INIT(lib/clixon/clixon.h.in) CLIXON_VERSION_MAJOR="4" CLIXON_VERSION_MINOR="3" -CLIXON_VERSION_PATCH="1" -CLIXON_VERSION="\"${CLIXON_VERSION_MAJOR}.${CLIXON_VERSION_MINOR}.${CLIXON_VERSION_PATCH}\"" +CLIXON_VERSION_PATCH="2" +CLIXON_VERSION="\"${CLIXON_VERSION_MAJOR}.${CLIXON_VERSION_MINOR}.${CLIXON_VERSION_PATCH}.PRE\"" # Check CLIgen if test "$prefix" = "NONE"; then diff --git a/lib/clixon/clixon_data.h b/lib/clixon/clixon_data.h index a541beac..43d7b934 100644 --- a/lib/clixon/clixon_data.h +++ b/lib/clixon/clixon_data.h @@ -98,7 +98,8 @@ int clicon_xml_changelog_set(clicon_handle h, cxobj *xchlog); int clicon_argv_get(clicon_handle h, int *argc, char ***argv); int clicon_argv_set(clicon_handle h, char *argv0, int argc, char **argv); +/*! Set and get (client/backend) session id */ int clicon_session_id_set(clicon_handle h, uint32_t id); -uint32_t clicon_session_id_get(clicon_handle h); +int clicon_session_id_get(clicon_handle h, uint32_t *id); #endif /* _CLIXON_DATA_H_ */ diff --git a/lib/src/clixon_data.c b/lib/src/clixon_data.c index 2e11bd2b..6ddc3d5e 100644 --- a/lib/src/clixon_data.c +++ b/lib/src/clixon_data.c @@ -579,19 +579,24 @@ clicon_db_elmnt_set(clicon_handle h, } /*! Get session id - * @param[in] h Clicon handle - * @retval id Session identifier - * Two uses: the backend assigns session-id for clients. + * @param[in] h Clicon handle + * @param[out] sid Session identifier + * @retval 0 OK + * @retval -1 Session id not set + * Session-ids survive TCP sessions that are created for each message sent to the backend. + * The backend assigns session-id for clients: backend assigns, clients get it from backend. */ -uint32_t -clicon_session_id_get(clicon_handle h) +int +clicon_session_id_get(clicon_handle h, + uint32_t *id) { clicon_hash_t *cdat = clicon_data(h); void *p; if ((p = clicon_hash_value(cdat, "session-id", NULL)) == NULL) - return 0; - return *(uint32_t*)p; + return -1; + *id = *(uint32_t*)p; + return 0; } /*! Set session id @@ -599,6 +604,7 @@ clicon_session_id_get(clicon_handle h) * @param[in] id Session id * @retval 0 OK * @retval -1 Error + * Session-ids survive TCP sessions that are created for each message sent to the backend. */ int clicon_session_id_set(clicon_handle h, diff --git a/lib/src/clixon_proto_client.c b/lib/src/clixon_proto_client.c index 85d60d16..bdd87f0f 100644 --- a/lib/src/clixon_proto_client.c +++ b/lib/src/clixon_proto_client.c @@ -151,6 +151,34 @@ clicon_rpc_msg(clicon_handle h, return retval; } +/*! Check if there is a valid (cached) session-id. If not, send a hello request to backend + * Session-ids survive TCP sessions that are created for each message sent to the backend. + * Clients use two approaches, either: + * (1) Once at the beginning of the session. Netconf and restconf does this + * (2) First usage, ie "lazy" evaluation when first needed + * @param[in] h clicon handle + * @param[out] session_id Session id + * @retval 0 OK and session_id set + * @retval -1 Error + */ +static int +session_id_check(clicon_handle h, + uint32_t *session_id) +{ + int retval = -1; + uint32_t id; + + if (clicon_session_id_get(h, &id) < 0){ /* Not set yet */ + if (clicon_hello_req(h, &id) < 0) + goto done; + clicon_session_id_set(h, id); + } + retval = 0; + *session_id = id; + done: + return retval; +} + /*! Generic xml netconf clicon rpc * Want to go over to use netconf directly between client and server,... * @param[in] h clicon handle @@ -172,9 +200,12 @@ clicon_rpc_netconf(clicon_handle h, int *sp) { int retval = -1; + uint32_t session_id; struct clicon_msg *msg = NULL; - if ((msg = clicon_msg_encode(clicon_session_id_get(h), "%s", xmlstr)) < 0) + if (session_id_check(h, &session_id) < 0) + goto done; + if ((msg = clicon_msg_encode(session_id, "%s", xmlstr)) < 0) goto done; if (clicon_rpc_msg(h, msg, xret, sp) < 0) goto done; @@ -305,7 +336,10 @@ clicon_rpc_get_config(clicon_handle h, cxobj *xd; cg_var *cv = NULL; char *prefix; + uint32_t session_id; + if (session_id_check(h, &session_id) < 0) + goto done; if ((cb = cbuf_new()) == NULL) goto done; cprintf(cb, ""); } cprintf(cb, ""); - if ((msg = clicon_msg_encode(clicon_session_id_get(h), - "%s", cbuf_get(cb))) == NULL) + if ((msg = clicon_msg_encode(session_id, "%s", cbuf_get(cb))) == NULL) goto done; if (clicon_rpc_msg(h, msg, &xret, NULL) < 0) goto done; @@ -383,7 +416,10 @@ clicon_rpc_edit_config(clicon_handle h, cxobj *xret = NULL; cxobj *xerr; char *username; - + uint32_t session_id; + + if (session_id_check(h, &session_id) < 0) + goto done; if ((cb = cbuf_new()) == NULL) goto done; cprintf(cb, ""); - if ((msg = clicon_msg_encode(clicon_session_id_get(h), - "%s", cbuf_get(cb))) == NULL) + if ((msg = clicon_msg_encode(session_id, "%s", cbuf_get(cb))) == NULL) goto done; if (clicon_rpc_msg(h, msg, &xret, NULL) < 0) goto done; @@ -439,10 +474,13 @@ clicon_rpc_copy_config(clicon_handle h, cxobj *xret = NULL; cxobj *xerr; char *username; - + uint32_t session_id; + + if (session_id_check(h, &session_id) < 0) + goto done; username = clicon_username_get(h); - if ((msg = clicon_msg_encode(clicon_session_id_get(h), - "<%s/><%s/>", + if ((msg = clicon_msg_encode(session_id, + "<%s/><%s/>", username?username:"", db1, db2)) == NULL) goto done; @@ -480,9 +518,12 @@ clicon_rpc_delete_config(clicon_handle h, cxobj *xret = NULL; cxobj *xerr; char *username; - + uint32_t session_id; + + if (session_id_check(h, &session_id) < 0) + goto done; username = clicon_username_get(h); - if ((msg = clicon_msg_encode(clicon_session_id_get(h), + if ((msg = clicon_msg_encode(session_id, "<%s/>none", username?username:"", db)) == NULL) goto done; @@ -516,9 +557,12 @@ clicon_rpc_lock(clicon_handle h, cxobj *xret = NULL; cxobj *xerr; char *username; - + uint32_t session_id; + + if (session_id_check(h, &session_id) < 0) + goto done; username = clicon_username_get(h); - if ((msg = clicon_msg_encode(clicon_session_id_get(h), + if ((msg = clicon_msg_encode(session_id, "<%s/>", username?username:"", db)) == NULL) goto done; @@ -552,9 +596,12 @@ clicon_rpc_unlock(clicon_handle h, cxobj *xret = NULL; cxobj *xerr; char *username; - + uint32_t session_id; + + if (session_id_check(h, &session_id) < 0) + goto done; username = clicon_username_get(h); - if ((msg = clicon_msg_encode(clicon_session_id_get(h), + if ((msg = clicon_msg_encode(session_id, "<%s/>", username?username:"", db)) == NULL) goto done; if (clicon_rpc_msg(h, msg, &xret, NULL) < 0) @@ -621,7 +668,10 @@ clicon_rpc_get(clicon_handle h, char *username; cg_var *cv = NULL; char *prefix; - + uint32_t session_id; + + if (session_id_check(h, &session_id) < 0) + goto done; if ((cb = cbuf_new()) == NULL) goto done; cprintf(cb, ""); } cprintf(cb, ""); - if ((msg = clicon_msg_encode(clicon_session_id_get(h), + if ((msg = clicon_msg_encode(session_id, "%s", cbuf_get(cb))) == NULL) goto done; if (clicon_rpc_msg(h, msg, &xret, NULL) < 0) @@ -692,9 +742,12 @@ clicon_rpc_close_session(clicon_handle h) cxobj *xret = NULL; cxobj *xerr; char *username; - + uint32_t session_id; + + if (session_id_check(h, &session_id) < 0) + goto done; username = clicon_username_get(h); - if ((msg = clicon_msg_encode(clicon_session_id_get(h), + if ((msg = clicon_msg_encode(session_id, "", username?username:"")) == NULL) goto done; @@ -728,9 +781,12 @@ clicon_rpc_kill_session(clicon_handle h, cxobj *xret = NULL; cxobj *xerr; char *username; - + uint32_t my_session_id; /* Not the one to kill */ + + if (session_id_check(h, &my_session_id) < 0) + goto done; username = clicon_username_get(h); - if ((msg = clicon_msg_encode(clicon_session_id_get(h), + if ((msg = clicon_msg_encode(my_session_id, "%u", username?username:"", session_id)) == NULL) goto done; @@ -764,9 +820,12 @@ clicon_rpc_validate(clicon_handle h, cxobj *xret = NULL; cxobj *xerr; char *username; - + uint32_t session_id; + + if (session_id_check(h, &session_id) < 0) + goto done; username = clicon_username_get(h); - if ((msg = clicon_msg_encode(clicon_session_id_get(h), + if ((msg = clicon_msg_encode(session_id, "<%s/>", username?username:"", db)) == NULL) goto done; if (clicon_rpc_msg(h, msg, &xret, NULL) < 0) @@ -797,9 +856,12 @@ clicon_rpc_commit(clicon_handle h) cxobj *xret = NULL; cxobj *xerr; char *username; - + uint32_t session_id; + + if (session_id_check(h, &session_id) < 0) + goto done; username = clicon_username_get(h); - if ((msg = clicon_msg_encode(clicon_session_id_get(h), + if ((msg = clicon_msg_encode(session_id, "", username?username:"")) == NULL) goto done; if (clicon_rpc_msg(h, msg, &xret, NULL) < 0) @@ -830,9 +892,12 @@ clicon_rpc_discard_changes(clicon_handle h) cxobj *xret = NULL; cxobj *xerr; char *username; - + uint32_t session_id; + + if (session_id_check(h, &session_id) < 0) + goto done; username = clicon_username_get(h); - if ((msg = clicon_msg_encode(clicon_session_id_get(h), + if ((msg = clicon_msg_encode(session_id, "", username?username:"")) == NULL) goto done; if (clicon_rpc_msg(h, msg, &xret, NULL) < 0) @@ -871,9 +936,12 @@ clicon_rpc_create_subscription(clicon_handle h, cxobj *xret = NULL; cxobj *xerr; char *username; - + uint32_t session_id; + + if (session_id_check(h, &session_id) < 0) + goto done; username = clicon_username_get(h); - if ((msg = clicon_msg_encode(clicon_session_id_get(h), + if ((msg = clicon_msg_encode(session_id, "" "%s" "" @@ -911,9 +979,12 @@ clicon_rpc_debug(clicon_handle h, cxobj *xret = NULL; cxobj *xerr; char *username; - + uint32_t session_id; + + if (session_id_check(h, &session_id) < 0) + goto done; username = clicon_username_get(h); - if ((msg = clicon_msg_encode(clicon_session_id_get(h), + if ((msg = clicon_msg_encode(session_id, "%d", username?username:"", level)) == NULL) goto done; if (clicon_rpc_msg(h, msg, &xret, NULL) < 0) diff --git a/test/test_sock.sh b/test/test_sock.sh index ca702790..ceeef78c 100755 --- a/test/test_sock.sh +++ b/test/test_sock.sh @@ -64,10 +64,10 @@ EOF expectfn "$clixon_cli -1f $cfg show version" 0 "$version." new "hello session-id 2" - expecteof "$clixon_util_socket -a $family -s $sock -D $DBG" 0 "" "2" + expecteof "$clixon_util_socket -a $family -s $sock -D $DBG" 0 "" "3" new "hello session-id 2" - expecteof "$clixon_util_socket -a $family -s $sock -D $DBG" 0 "" "3" + expecteof "$clixon_util_socket -a $family -s $sock -D $DBG" 0 "" "4" if [ $BE -ne 0 ]; then new "Kill backend" From a2b1674708b221b9f8861668d26b7f7b52de8176 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Wed, 5 Feb 2020 19:49:23 +0100 Subject: [PATCH 09/19] XML namespace merge bug fixed. Example: two xmlns attributes could both survive a merge whereas one should replace the other. --- CHANGELOG.md | 1 + lib/src/clixon_xml_map.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4dd90f3c..8e94a58b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Patch release based on testing by Dave Cornejo, Netgate * From a cli perspective this is a revert to 4.1 behaviour, where the cli does not immediately exit on start if the backend is not running, but with the new session-id function ### Corrected Bugs +* XML namespace merge bug fixed. Example: two xmlns attributes could both survive a merge whereas one should replace the other. * Compile option `VALIDATE_STATE_XML` introduced in `include/custom.h` to control whether code for state data validation is compiled or not. * Fixed: Validation of user state data led to wrong validation, if state relied on config data, eg leafref/must/when etc. * Fixed: No revision in yang module led to errors in validation of state data diff --git a/lib/src/clixon_xml_map.c b/lib/src/clixon_xml_map.c index 0be80b45..2d822638 100644 --- a/lib/src/clixon_xml_map.c +++ b/lib/src/clixon_xml_map.c @@ -1216,7 +1216,7 @@ check_namespaces(cxobj *x0, /* source */ } else{ /* No, namespace does not exist in x1 _parent_ * Check if it is exists in x1 itself */ - if (nscache_get_prefix(x1, namespace, &pexist) == 1){ + if (xml2prefix(x1, namespace, &pexist) == 1){ /* Yes it exists, but is it equal? */ if ((pexist == NULL && prefix0 == NULL) || (pexist && prefix0 && From 61e03690caf737d115d097e6688768078d617e71 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Thu, 6 Feb 2020 18:02:14 +0100 Subject: [PATCH 10/19] Fixed: Pretty-printed XML using prefixes not parsed correctly. * eg ` ` could lead to errors, wheras (` `) works fine. --- CHANGELOG.md | 5 ++++- lib/src/clixon_xml_parse.y | 15 +++++++++++---- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e94a58b..30851305 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,11 +4,14 @@ Patch release based on testing by Dave Cornejo, Netgate -### Minor changes +### API changes on existing features (you may need to change your code) * Session-id CLI functionality delayed: "lazy evaluation" + * C-api: Changed `clicon_session_id_get(clicon_handle h, uint32_t *id)` * From a cli perspective this is a revert to 4.1 behaviour, where the cli does not immediately exit on start if the backend is not running, but with the new session-id function ### Corrected Bugs +* Fixed: Pretty-printed XML using prefixes not parsed correctly. + * eg ` ` could lead to errors, wheras (` `) works fine. * XML namespace merge bug fixed. Example: two xmlns attributes could both survive a merge whereas one should replace the other. * Compile option `VALIDATE_STATE_XML` introduced in `include/custom.h` to control whether code for state data validation is compiled or not. * Fixed: Validation of user state data led to wrong validation, if state relied on config data, eg leafref/must/when etc. diff --git a/lib/src/clixon_xml_parse.y b/lib/src/clixon_xml_parse.y index 3a1e9419..0c08d811 100644 --- a/lib/src/clixon_xml_parse.y +++ b/lib/src/clixon_xml_parse.y @@ -335,10 +335,17 @@ xml_parse_bslash2(struct xml_parse_yacc_arg *ya, while ((xc = xml_child_each(x, xc, CX_ELMNT)) != NULL) break; if (xc != NULL){ /* at least one element */ - xc = NULL; - while ((xc = xml_child_each(x, xc, CX_BODY)) != NULL) { - xml_value_set(xc, ""); /* XXX remove */ - } + int i; + for (i=0; i Date: Mon, 10 Feb 2020 20:23:48 +0100 Subject: [PATCH 11/19] Fixed: Enabling modstate (CLICON_XMLDB_MODSTATE), changing a revision on a yang, and restarting made the backend daemon exit at start (thanks Matt). * Also: ensure to load `ietf-yang-library.yang ` if CLICON_XMLDB_MODSTATE is set --- CHANGELOG.md | 14 ++++++++++---- apps/backend/backend_main.c | 4 ++++ lib/src/clixon_plugin.c | 2 +- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30851305..48d42933 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,17 +1,23 @@ # Clixon Changelog -## 4.3.1 (2 February 2020) - -Patch release based on testing by Dave Cornejo, Netgate +## 4.3.2 (Upcoming) ### API changes on existing features (you may need to change your code) * Session-id CLI functionality delayed: "lazy evaluation" * C-api: Changed `clicon_session_id_get(clicon_handle h, uint32_t *id)` * From a cli perspective this is a revert to 4.1 behaviour, where the cli does not immediately exit on start if the backend is not running, but with the new session-id function - + ### Corrected Bugs +* Fixed: Enabling modstate (CLICON_XMLDB_MODSTATE), changing a revision on a yang, and restarting made the backend daemon exit at start (thanks Matt). + * Also: ensure to load `ietf-yang-library.yang ` if CLICON_XMLDB_MODSTATE is set * Fixed: Pretty-printed XML using prefixes not parsed correctly. * eg ` ` could lead to errors, wheras (` `) works fine. + +## 4.3.1 (2 February 2020) + +Patch release based on testing by Dave Cornejo, Netgate + +### Corrected Bugs * XML namespace merge bug fixed. Example: two xmlns attributes could both survive a merge whereas one should replace the other. * Compile option `VALIDATE_STATE_XML` introduced in `include/custom.h` to control whether code for state data validation is compiled or not. * Fixed: Validation of user state data led to wrong validation, if state relied on config data, eg leafref/must/when etc. diff --git a/apps/backend/backend_main.c b/apps/backend/backend_main.c index 42820090..15f69092 100644 --- a/apps/backend/backend_main.c +++ b/apps/backend/backend_main.c @@ -745,6 +745,10 @@ main(int argc, if (clicon_option_bool(h, "CLICON_STREAM_DISCOVERY_RFC5277") && yang_spec_parse_module(h, "clixon-rfc5277", NULL, yspec)< 0) goto done; + /* Load yang YANG module state */ + if (clicon_option_bool(h, "CLICON_XMLDB_MODSTATE") && + yang_spec_parse_module(h, "ietf-yang-library", NULL, yspec)< 0) + goto done; /* Here all modules are loaded * Compute and set canonical namespace context */ diff --git a/lib/src/clixon_plugin.c b/lib/src/clixon_plugin.c index 3541d0a1..8bfdefd8 100644 --- a/lib/src/clixon_plugin.c +++ b/lib/src/clixon_plugin.c @@ -716,7 +716,7 @@ upgrade_callback_call(clicon_handle h, int ret; if (upgrade_cb_list == NULL) - return 0; + return 1; uc = upgrade_cb_list; do { /* For matching an upgrade callback: From 3748eefb8eeb31513576a9b740695401f114d786 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Tue, 11 Feb 2020 20:17:15 +0100 Subject: [PATCH 12/19] datastore repair test branch --- apps/backend/backend_commit.c | 16 ++--- example/main/example_backend.c | 95 +++++++++++++++++++++++++- lib/clixon/clixon_plugin.h | 17 ++++- lib/clixon/clixon_xml_map.h | 1 + lib/src/clixon_plugin.c | 27 ++++++++ lib/src/clixon_xml_map.c | 2 +- test/test_datastore_repair.sh | 118 +++++++++++++++++++++++++++++++++ 7 files changed, 265 insertions(+), 11 deletions(-) create mode 100755 test/test_datastore_repair.sh diff --git a/apps/backend/backend_commit.c b/apps/backend/backend_commit.c index 890be1d5..f42a08c5 100644 --- a/apps/backend/backend_commit.c +++ b/apps/backend/backend_commit.c @@ -197,14 +197,14 @@ startup_common(clicon_handle h, xt = NULL; goto ok; } - if (msd){ - /* Here xt is old syntax */ - if ((ret = clixon_module_upgrade(h, xt, msd, cbret)) < 0) - goto done; - if (ret == 0) - goto fail; - /* Here xt is new syntax */ - } + if (clixon_plugin_xmldb_repair(h, db, xt) < 0) + goto done; + /* Here xt is old syntax */ + if ((ret = clixon_module_upgrade(h, xt, msd, cbret)) < 0) + goto done; + if (ret == 0) + goto fail; + if ((yspec = clicon_dbspec_yang(h)) == NULL){ clicon_err(OE_YANG, 0, "Yang spec not set"); goto done; diff --git a/example/main/example_backend.c b/example/main/example_backend.c index b8db088d..440d2d92 100644 --- a/example/main/example_backend.c +++ b/example/main/example_backend.c @@ -173,6 +173,98 @@ main_abort(clicon_handle h, return 0; } +struct map_str2str{ + char *ms_path; + char *ms_ns; +}; +static const struct map_str2str path_namespace_map[] = { + {"/a:x/a:y/a:z/a:w", "urn:example:b"}, + {"/a:x/a:y/a:z", "urn:example:b"}, + {NULL, NULL} +}; + +static int +main_repair_one(cxobj *xt, + char *mypath, + char *mynamespace, + cvec *nsc) +{ + int retval = -1; + cxobj **xvec = NULL; + size_t xlen; + char *myprefix = NULL; + int i; + cxobj *x; + char *namespace; + char *pexist = NULL; /* Existing prefix */ + + if (xpath_vec(xt, nsc, "%s", &xvec, &xlen, mypath) < 0) + goto done; + if (xml_nsctx_get_prefix(nsc, mynamespace, &myprefix) == 0){ + clicon_err(OE_XML, ENOENT, "Namespace %s not found in canonical namespace map", + mynamespace); + goto done; + } + for (i=0; ims_path; ms++) + if (main_repair_one(xt, ms->ms_path, ms->ms_ns, nsc) < 0) + goto done; + ok: + retval = 0; + done: + if (nsc) + cvec_free(nsc); + return retval; +} + /*! Routing example notification timer handler. Here is where the periodic action is */ static int @@ -684,7 +776,8 @@ static clixon_plugin_api api = { .ca_trans_commit=main_commit, /* trans commit */ .ca_trans_revert=main_revert, /* trans revert */ .ca_trans_end=main_end, /* trans end */ - .ca_trans_abort=main_abort /* trans abort */ + .ca_trans_abort=main_abort, /* trans abort */ + .ca_xmldb_repair=main_repair /* xmldb repair. */ }; /*! Backend plugin initialization diff --git a/lib/clixon/clixon_plugin.h b/lib/clixon/clixon_plugin.h index 2b8445c1..092f8d8b 100644 --- a/lib/clixon/clixon_plugin.h +++ b/lib/clixon/clixon_plugin.h @@ -159,6 +159,16 @@ typedef int (trans_cb_t)(clicon_handle h, transaction_data td); */ typedef char *(cli_prompthook_t)(clicon_handle, char *mode); +/*! Datastore repair callback + * Gets called on startup after initial XML parsing, but before upgrading and before validation + * @param[in] h Clicon handle + * @param[in] db Name of datastore, eg "running" + * @param[in] xt XML tree. Repair this "in place" + * @retval -1 Error + * @retval 0 OK + */ +typedef int (xmldb_repair_t)(clicon_handle h, char *db, cxobj *xt); + /*! Startup status for use in startup-callback * Note that for STARTUP_ERR and _INVALID, running runs in failsafe mode * and startup contains the erroneous or invalid database. @@ -206,8 +216,9 @@ struct clixon_plugin_api{ trans_cb_t *cb_trans_revert; /* Transaction revert */ trans_cb_t *cb_trans_end; /* Transaction completed */ trans_cb_t *cb_trans_abort; /* Transaction aborted */ - } cau_backend; + xmldb_repair_t *cb_xmldb_repair; /* Repair datastore on load */ + } cau_backend; } u; }; /* Access fields */ @@ -224,6 +235,8 @@ struct clixon_plugin_api{ #define ca_trans_revert u.cau_backend.cb_trans_revert #define ca_trans_end u.cau_backend.cb_trans_end #define ca_trans_abort u.cau_backend.cb_trans_abort +#define ca_xmldb_repair u.cau_backend.cb_xmldb_repair + /* * Macros @@ -271,6 +284,8 @@ int clixon_plugin_auth(clicon_handle h, void *arg); int clixon_plugin_extension(clicon_handle h, yang_stmt *yext, yang_stmt *ys); +int clixon_plugin_xmldb_repair(clicon_handle h, char *db, cxobj *xt); + /* rpc callback API */ int rpc_callback_register(clicon_handle h, clicon_rpc_cb cb, void *arg, char *namespace, char *name); int rpc_callback_delete_all(clicon_handle h); diff --git a/lib/clixon/clixon_xml_map.h b/lib/clixon/clixon_xml_map.h index 9a5107a2..00731904 100644 --- a/lib/clixon/clixon_xml_map.h +++ b/lib/clixon/clixon_xml_map.h @@ -56,6 +56,7 @@ int xml_diff(yang_stmt *yspec, cxobj *x0, cxobj *x1, cxobj ***changed_x0, cxobj ***changed_x1, size_t *changedlen); int xml_tree_prune_flagged_sub(cxobj *xt, int flag, int test, int *upmark); int xml_tree_prune_flagged(cxobj *xt, int flag, int test); +int add_namespace(cxobj *x1, cxobj *x1p, char *prefix1, char *namespace); int xml_default(cxobj *x, void *arg); int xml_sanity(cxobj *x, void *arg); int xml_non_config_data(cxobj *xt, void *arg); diff --git a/lib/src/clixon_plugin.c b/lib/src/clixon_plugin.c index 8bfdefd8..d28230f7 100644 --- a/lib/src/clixon_plugin.c +++ b/lib/src/clixon_plugin.c @@ -467,6 +467,33 @@ clixon_plugin_extension(clicon_handle h, } return retval; } +/*! Call plugin module repair in all plugins + * + * Repair datastore on load before or as an alternative to the upgrading mechanism + * @param[in] h Clicon handle + * Call plugin start functions (if defined) + * @note Start functions used to have argc/argv. Use clicon_argv_get() instead + */ +int +clixon_plugin_xmldb_repair(clicon_handle h, + char *db, + cxobj *xt) +{ + clixon_plugin *cp; + int i; + xmldb_repair_t *repairfn; + + for (i = 0; i < _clixon_nplugins; i++) { + cp = &_clixon_plugins[i]; + if ((repairfn = cp->cp_api.ca_xmldb_repair) == NULL) + continue; + if (repairfn(h, db, xt) < 0) { + clicon_debug(1, "%s() failed", __FUNCTION__); + return -1; + } + } + return 0; +} /*-------------------------------------------------------------------- * RPC callbacks for both client/frontend and backend plugins. diff --git a/lib/src/clixon_xml_map.c b/lib/src/clixon_xml_map.c index 2d822638..52565c3c 100644 --- a/lib/src/clixon_xml_map.c +++ b/lib/src/clixon_xml_map.c @@ -708,7 +708,7 @@ xml_tree_prune_flagged(cxobj *xt, /*! Add prefix:namespace pair to xml node, set cache, prefix, etc */ -static int +int add_namespace(cxobj *x1, /* target */ cxobj *x1p, char *prefix1, diff --git a/test/test_datastore_repair.sh b/test/test_datastore_repair.sh new file mode 100755 index 00000000..800fb8a7 --- /dev/null +++ b/test/test_datastore_repair.sh @@ -0,0 +1,118 @@ +#!/usr/bin/env bash +# Keep a vector of {xpath, namespace} pairs, match xpath in parsed XML and check if symbols belong to namespace +# If not, set that namespace. +# This is a primitive upgrade mechanism if th emore general upgrade mechanism can not be used + +# Magic line must be first in script (see README.md) +s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi + +APPNAME=example + +cfg=$dir/conf_yang.xml +fyangA=$dir/A.yang +fyangB=$dir/B.yang + +# Create configuration +cat < $cfg + + $cfg + ietf-netconf:startup + /usr/local/share/clixon + $dir + /usr/local/var/$APPNAME/$APPNAME.sock + /usr/local/lib/example/backend + /usr/local/var/$APPNAME/$APPNAME.pidfile + $dir + true + /usr/local/lib/$APPNAME/clispec + /usr/local/lib/$APPNAME/cli + $APPNAME + +EOF + +# Yang module A (base) +cat < $fyangA +module A{ + prefix a; + revision 2020-02-11; + namespace "urn:example:a"; + container x { + container y { + } + } +} +EOF + +# Yang module B (augments A) +cat < $fyangB +module B{ + prefix b; + revision 2020-02-11; + namespace "urn:example:b"; + import A { + prefix "a"; + } + augment "/a:x/ngrt:y" { + container z { + leaf w { + type string; + } + } + } +} +EOF + +# permission kludges +sudo touch $dir/startup_db +sudo chmod 666 $dir/startup_db +# Create startup db of "old" db with incorrect augment namespace tagging +cat < $dir/startup_db + + + + + foo + + + + +EOF + +# This is how it should look after repair, using prefixes +AFTER=$(cat <foo +EOF +) + +# Or using default: +# foo + +new "test params: -f $cfg" +# Bring your own backend +if [ $BE -ne 0 ]; then + # kill old backend (if any) + new "kill old backend" + sudo clixon_backend -zf $cfg + if [ $? -ne 0 ]; then + err + fi + new "start backend -s startup -f $cfg" + start_backend -s startup -f $cfg + + new "waiting" + wait_backend +fi + +new "netconf get config" +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^$AFTER]]>]]>$" + +new "Kill backend" +# Check if premature kill +pid=$(pgrep -u root -f clixon_backend) +if [ -z "$pid" ]; then + err "backend already dead" +fi +# kill backend +stop_backend -f $cfg + +#rm -rf $dir From b6812793f9ea9df87087bf72ca51c3b00117081f Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Wed, 12 Feb 2020 16:40:52 +0100 Subject: [PATCH 13/19] General-purpose upgrade --- apps/backend/backend_commit.c | 6 +- example/main/example_backend.c | 202 +++++++++++++++++--------------- lib/clixon/clixon_plugin.h | 20 ++-- lib/clixon/clixon_string.h | 10 +- lib/clixon/clixon_xml_map.h | 2 +- lib/clixon/clixon_yang_module.h | 6 +- lib/src/clixon_data.c | 1 + lib/src/clixon_datastore.c | 3 +- lib/src/clixon_datastore_read.c | 10 +- lib/src/clixon_plugin.c | 24 ++-- lib/src/clixon_proto_client.c | 1 + lib/src/clixon_xml_map.c | 46 +++++++- lib/src/clixon_xpath.c | 4 +- lib/src/clixon_xpath_eval.c | 13 +- lib/src/clixon_yang.c | 1 + lib/src/clixon_yang_module.c | 4 +- lib/src/clixon_yang_type.c | 1 + test/test_datastore_repair.sh | 97 ++++++++------- 18 files changed, 286 insertions(+), 165 deletions(-) diff --git a/apps/backend/backend_commit.c b/apps/backend/backend_commit.c index f42a08c5..1f2ea859 100644 --- a/apps/backend/backend_commit.c +++ b/apps/backend/backend_commit.c @@ -197,9 +197,11 @@ startup_common(clicon_handle h, xt = NULL; goto ok; } - if (clixon_plugin_xmldb_repair(h, db, xt) < 0) - goto done; /* Here xt is old syntax */ + /* General purpose datastore upgrade */ + if (clixon_plugin_datastore_upgrade(h, db, xt, msd) < 0) + goto done; + /* Module-specific upgrade callbacks */ if ((ret = clixon_module_upgrade(h, xt, msd, cbret)) < 0) goto done; if (ret == 0) diff --git a/example/main/example_backend.c b/example/main/example_backend.c index 440d2d92..b2a83b94 100644 --- a/example/main/example_backend.c +++ b/example/main/example_backend.c @@ -173,98 +173,6 @@ main_abort(clicon_handle h, return 0; } -struct map_str2str{ - char *ms_path; - char *ms_ns; -}; -static const struct map_str2str path_namespace_map[] = { - {"/a:x/a:y/a:z/a:w", "urn:example:b"}, - {"/a:x/a:y/a:z", "urn:example:b"}, - {NULL, NULL} -}; - -static int -main_repair_one(cxobj *xt, - char *mypath, - char *mynamespace, - cvec *nsc) -{ - int retval = -1; - cxobj **xvec = NULL; - size_t xlen; - char *myprefix = NULL; - int i; - cxobj *x; - char *namespace; - char *pexist = NULL; /* Existing prefix */ - - if (xpath_vec(xt, nsc, "%s", &xvec, &xlen, mypath) < 0) - goto done; - if (xml_nsctx_get_prefix(nsc, mynamespace, &myprefix) == 0){ - clicon_err(OE_XML, ENOENT, "Namespace %s not found in canonical namespace map", - mynamespace); - goto done; - } - for (i=0; ims_path; ms++) - if (main_repair_one(xt, ms->ms_path, ms->ms_ns, nsc) < 0) - goto done; - ok: - retval = 0; - done: - if (nsc) - cvec_free(nsc); - return retval; -} - /*! Routing example notification timer handler. Here is where the periodic action is */ static int @@ -502,6 +410,112 @@ example_extension(clicon_handle h, return retval; } +/* Here follows code for general-purpose datastore upgrade + * Nodes affected are identified by paths. + * In this example nodes' namespaces are changed, or they are removed altogether + */ +/* Rename the namespaces of these paths. + * That is, paths (on the left) should get namespaces (to the right) + */ +static const map_str2str namespace_map[] = { + {"/a:x/a:y/a:z/descendant-or-self::node()", "urn:example:b"}, + {NULL, NULL} +}; + +/* Remove these paths */ +static const char *remove_map[] = { + "/a:remove_me", + NULL +}; + +/*! General-purpose datastore upgrade callback called once on startup + * + * Gets called on startup after initial XML parsing, but before module-specific upgrades + * and before validation. + * @param[in] h Clicon handle + * @param[in] db Name of datastore, eg "running", "startup" or "tmp" + * @param[in] xt XML tree. Upgrade this "in place" + * @param[in] msd Info on datastore module-state, if any + * @retval -1 Error + * @retval 0 OK + */ +int +example_upgrade(clicon_handle h, + char *db, + cxobj *xt, + modstate_diff_t *msd) +{ + int retval = -1; + cvec *nsc = NULL; /* Canonical namespace */ + yang_stmt *yspec; yang_stmt *yspec; + const struct map_str2str *ms; /* map iterator */ + cxobj **xvec = NULL; /* vector of result nodes */ + size_t xlen; + int i; + const char **pp; + + if (strcmp(db, "startup") != 0) /* skip other than startup datastore */ + goto ok; + if (msd->md_status) /* skip if there is proper module-state in datastore */ + goto ok; + yspec = clicon_dbspec_yang(h); /* Get all yangs */ + /* Get canonical namespaces for using "normalized" prefixes */ + if (xml_nsctx_yangspec(yspec, &nsc) < 0) + goto done; + /* 1. Remove paths */ + for (pp = remove_map; *pp; ++pp){ + /* Find all nodes matching n */ + if (xpath_vec(xt, nsc, "%s", &xvec, &xlen, *pp) < 0) + goto done; + /* Remove them */ + /* Loop through all nodes matching mypath and change theoir namespace */ + for (i=0; ims_s0; ms++){ + char *mypath; + char *mynamespace; + char *myprefix = NULL; + + mypath = ms->ms_s0; + mynamespace = ms->ms_s1; + if (xml_nsctx_get_prefix(nsc, mynamespace, &myprefix) == 0){ + clicon_err(OE_XML, ENOENT, "Namespace %s not found in canonical namespace map", + mynamespace); + goto done; + } + /* Find all nodes matching mypath */ + if (xpath_vec(xt, nsc, "%s", &xvec, &xlen, mypath) < 0) + goto done; + /* Loop through all nodes matching mypath and change theoir namespace */ + for (i=0; imd_status = 1; /* Create diff trees */ if (xml_parse_string("", yspec, &msd->md_del) < 0) goto done; @@ -258,6 +259,13 @@ text_read_modstate(clicon_handle h, /* 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-set-id") == 0){ + if (xml_body(xm) && (msd->md_set_id = strdup(xml_body(xm))) == NULL){ + clicon_err(OE_UNIX, errno, "strdup"); + goto done; + } + } + continue; /* ignore other tags, such as module-set-id */ if (strcmp(xml_name(xm), "module")) continue; /* ignore other tags, such as module-set-id */ if ((name = xml_find_body(xm, "name")) == NULL) diff --git a/lib/src/clixon_plugin.c b/lib/src/clixon_plugin.c index d28230f7..f997702e 100644 --- a/lib/src/clixon_plugin.c +++ b/lib/src/clixon_plugin.c @@ -59,6 +59,7 @@ #include "clixon_handle.h" #include "clixon_yang.h" #include "clixon_xml.h" +#include "clixon_yang_module.h" #include "clixon_plugin.h" /* List of plugins XXX @@ -467,27 +468,34 @@ clixon_plugin_extension(clicon_handle h, } return retval; } -/*! Call plugin module repair in all plugins +/*! Call plugingeneral-purpose datastore upgrade in all plugins * - * Repair datastore on load before or as an alternative to the upgrading mechanism + * @param[in] h Clicon handle + * @param[in] db Name of datastore, eg "running", "startup" or "tmp" + * @param[in] xt XML tree. Upgrade this "in place" + * @param[in] msd Module-state diff, info on datastore module-state + * @retval -1 Error + * @retval 0 OK + * Upgrade datastore on load before or as an alternative to module-specific upgrading mechanism * @param[in] h Clicon handle * Call plugin start functions (if defined) * @note Start functions used to have argc/argv. Use clicon_argv_get() instead */ int -clixon_plugin_xmldb_repair(clicon_handle h, - char *db, - cxobj *xt) +clixon_plugin_datastore_upgrade(clicon_handle h, + char *db, + cxobj *xt, + modstate_diff_t *msd) { clixon_plugin *cp; int i; - xmldb_repair_t *repairfn; + datastore_upgrade_t *repairfn; for (i = 0; i < _clixon_nplugins; i++) { cp = &_clixon_plugins[i]; - if ((repairfn = cp->cp_api.ca_xmldb_repair) == NULL) + if ((repairfn = cp->cp_api.ca_datastore_upgrade) == NULL) continue; - if (repairfn(h, db, xt) < 0) { + if (repairfn(h, db, xt, msd) < 0) { clicon_debug(1, "%s() failed", __FUNCTION__); return -1; } diff --git a/lib/src/clixon_proto_client.c b/lib/src/clixon_proto_client.c index bdd87f0f..552e5048 100644 --- a/lib/src/clixon_proto_client.c +++ b/lib/src/clixon_proto_client.c @@ -64,6 +64,7 @@ #include "clixon_xml.h" #include "clixon_options.h" #include "clixon_data.h" +#include "clixon_yang_module.h" #include "clixon_plugin.h" #include "clixon_string.h" #include "clixon_xpath_ctx.h" diff --git a/lib/src/clixon_xml_map.c b/lib/src/clixon_xml_map.c index 52565c3c..6cb697b2 100644 --- a/lib/src/clixon_xml_map.c +++ b/lib/src/clixon_xml_map.c @@ -64,6 +64,7 @@ #include "clixon_yang.h" #include "clixon_xml.h" #include "clixon_options.h" +#include "clixon_yang_module.h" #include "clixon_plugin.h" #include "clixon_xml_nsctx.h" #include "clixon_xpath_ctx.h" @@ -708,7 +709,7 @@ xml_tree_prune_flagged(cxobj *xt, /*! Add prefix:namespace pair to xml node, set cache, prefix, etc */ -int +static int add_namespace(cxobj *x1, /* target */ cxobj *x1p, char *prefix1, @@ -746,6 +747,49 @@ add_namespace(cxobj *x1, /* target */ return retval; } +/*! Change namespace of XML node + * + * @param[in] x XML node + * @param[in] namespace Change to this namespace (if it does not already belong to it) + * @param[in] prefix If change, use this namespace + * @param 0 OK + * @param -1 Error + */ +int +xml_namespace_change(cxobj *x, + char *namespace, + char *prefix) +{ + int retval = -1; + cxobj *xp; + char *ns0 = NULL; /* existing namespace */ + char *prefix0 = NULL; /* existing prefix */ + + ns0 = NULL; + if (xml2ns(x, xml_prefix(x), &ns0) < 0) + goto done; + if (strcmp(ns0, namespace) == 0) + goto ok; /* Already has right namespace */ + /* Is namespace already declared? */ + if (xml2prefix(x, namespace, &prefix0) == 1){ + /* Yes it is declared and the prefix is pexists */ + if (xml_prefix_set(x, prefix0) < 0) + goto done; + } + else{ /* Namespace does not exist, add it */ + if ((xp = xml_parent(x)) == NULL){ + clicon_err(OE_XML, ENOENT, "XML node must have parent"); + goto done; + } + if (add_namespace(x, xp, prefix0, namespace) < 0) + goto done; + } + ok: + retval = 0; + done: + return retval; +} + /*! Add default values (if not set) * @param[in] xt XML tree with some node marked * @param[in] arg Ignored diff --git a/lib/src/clixon_xpath.c b/lib/src/clixon_xpath.c index 9f658c2f..385608bf 100644 --- a/lib/src/clixon_xpath.c +++ b/lib/src/clixon_xpath.c @@ -122,11 +122,11 @@ static const map_str2int xpath_tree_map[] = { static const map_str2int axis_type_map[] = { {"NaN", A_NAN}, {"ancestor", A_ANCESTOR}, - {"ancestor-or-selgf", A_ANCESTOR_OR_SELF}, + {"ancestor-or-self", A_ANCESTOR_OR_SELF}, {"attribute", A_ATTRIBUTE}, {"child", A_CHILD}, {"descendant", A_DESCENDANT}, - {"descendeant-or-self", A_DESCENDANT_OR_SELF}, + {"descendant-or-self", A_DESCENDANT_OR_SELF}, {"following", A_FOLLOWING}, {"following-sibling", A_FOLLOWING_SIBLING}, {"namespace", A_NAMESPACE}, diff --git a/lib/src/clixon_xpath_eval.c b/lib/src/clixon_xpath_eval.c index dd3c0965..f712fcad 100644 --- a/lib/src/clixon_xpath_eval.c +++ b/lib/src/clixon_xpath_eval.c @@ -362,8 +362,19 @@ xp_eval_step(xp_ctx *xc0, } ctx_nodeset_replace(xc, vec, veclen); break; - case A_DESCENDANT: case A_DESCENDANT_OR_SELF: + for (i=0; ixc_size; i++){ + xv = xc->xc_nodeset[i]; + if (nodetest_recursive(xv, xs->xs_c0, CX_ELMNT, 0x0, nsc, localonly, &vec, &veclen) < 0) + goto done; + } + for (i=0; ixc_nodeset, &xc->xc_size) < 0) + goto done; + } + break; + case A_DESCENDANT: for (i=0; ixc_size; i++){ xv = xc->xc_nodeset[i]; if (nodetest_recursive(xv, xs->xs_c0, CX_ELMNT, 0x0, nsc, localonly, &vec, &veclen) < 0) diff --git a/lib/src/clixon_yang.c b/lib/src/clixon_yang.c index 09e6a822..9807c62a 100644 --- a/lib/src/clixon_yang.c +++ b/lib/src/clixon_yang.c @@ -87,6 +87,7 @@ #include "clixon_yang.h" #include "clixon_hash.h" #include "clixon_xml.h" +#include "clixon_yang_module.h" #include "clixon_plugin.h" #include "clixon_data.h" #include "clixon_options.h" diff --git a/lib/src/clixon_yang_module.c b/lib/src/clixon_yang_module.c index 4210fa6a..2888fbd3 100644 --- a/lib/src/clixon_yang_module.c +++ b/lib/src/clixon_yang_module.c @@ -74,10 +74,10 @@ #include "clixon_xpath.h" #include "clixon_options.h" #include "clixon_data.h" +#include "clixon_yang_module.h" #include "clixon_plugin.h" #include "clixon_netconf_lib.h" #include "clixon_xml_map.h" -#include "clixon_yang_module.h" #include "clixon_yang_internal.h" /* internal */ modstate_diff_t * @@ -97,6 +97,8 @@ modstate_diff_free(modstate_diff_t *md) { if (md == NULL) return 0; + if (md->md_set_id) + free(md->md_set_id); if (md->md_del) xml_free(md->md_del); if (md->md_mod) diff --git a/lib/src/clixon_yang_type.c b/lib/src/clixon_yang_type.c index 0cd50346..fb59b3e4 100644 --- a/lib/src/clixon_yang_type.c +++ b/lib/src/clixon_yang_type.c @@ -94,6 +94,7 @@ #include "clixon_yang.h" #include "clixon_hash.h" #include "clixon_xml.h" +#include "clixon_yang_module.h" #include "clixon_plugin.h" #include "clixon_options.h" #include "clixon_yang.h" diff --git a/test/test_datastore_repair.sh b/test/test_datastore_repair.sh index 800fb8a7..2a98d1e6 100755 --- a/test/test_datastore_repair.sh +++ b/test/test_datastore_repair.sh @@ -1,7 +1,9 @@ #!/usr/bin/env bash -# Keep a vector of {xpath, namespace} pairs, match xpath in parsed XML and check if symbols belong to namespace -# If not, set that namespace. -# This is a primitive upgrade mechanism if th emore general upgrade mechanism can not be used +# Test of the general-purpose (raw) upgrade mechanism. +# Input is a startup db without mod-state info. +# It has wrong namespace bindings and needs to remove some nodes +# Output is a valid config woith correct namespaces and removed nods +# The code for this is in the main example backend plugin. # Magic line must be first in script (see README.md) s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi @@ -40,6 +42,12 @@ module A{ container y { } } + list remove_me { + key k; + leaf k { + type string; + } + } } EOF @@ -65,7 +73,48 @@ EOF # permission kludges sudo touch $dir/startup_db sudo chmod 666 $dir/startup_db + +# This is how it should look after repair, using prefixes +AFTER=$(cat <foo +EOF +) + +testrun(){ + new "test params: -f $cfg" + # Bring your own backend + if [ $BE -ne 0 ]; then + # kill old backend (if any) + new "kill old backend" + sudo clixon_backend -zf $cfg + if [ $? -ne 0 ]; then + err + fi + new "start backend -s startup -f $cfg" + start_backend -s startup -f $cfg + + new "waiting" + wait_backend + fi + + new "netconf get config" + expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^$AFTER]]>]]>$" + + if [ $BE -ne 0 ]; then + new "Kill backend" + # Check if premature kill + pid=$(pgrep -u root -f clixon_backend) + if [ -z "$pid" ]; then + err "backend already dead" + fi + # kill backend + stop_backend -f $cfg + fi + +} # end testrun + # Create startup db of "old" db with incorrect augment namespace tagging +# without modstate cat < $dir/startup_db @@ -75,44 +124,12 @@ cat < $dir/startup_db + This node is obsolete + this too EOF -# This is how it should look after repair, using prefixes -AFTER=$(cat <foo -EOF -) +new "general-purpose upgrade without modstate" +testrun -# Or using default: -# foo - -new "test params: -f $cfg" -# Bring your own backend -if [ $BE -ne 0 ]; then - # kill old backend (if any) - new "kill old backend" - sudo clixon_backend -zf $cfg - if [ $? -ne 0 ]; then - err - fi - new "start backend -s startup -f $cfg" - start_backend -s startup -f $cfg - - new "waiting" - wait_backend -fi - -new "netconf get config" -expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^$AFTER]]>]]>$" - -new "Kill backend" -# Check if premature kill -pid=$(pgrep -u root -f clixon_backend) -if [ -z "$pid" ]; then - err "backend already dead" -fi -# kill backend -stop_backend -f $cfg - -#rm -rf $dir +rm -rf $dir From cdd22bc33dd4483b8a4647768299fccdc39bbb0f Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Wed, 12 Feb 2020 17:01:25 +0100 Subject: [PATCH 14/19] example typo --- example/main/example_backend.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/example/main/example_backend.c b/example/main/example_backend.c index b2a83b94..27035ce9 100644 --- a/example/main/example_backend.c +++ b/example/main/example_backend.c @@ -419,12 +419,14 @@ example_extension(clicon_handle h, */ static const map_str2str namespace_map[] = { {"/a:x/a:y/a:z/descendant-or-self::node()", "urn:example:b"}, + /* add more paths to be renamed here */ {NULL, NULL} }; /* Remove these paths */ static const char *remove_map[] = { "/a:remove_me", + /* add more paths to be deleted here */ NULL }; @@ -447,7 +449,7 @@ example_upgrade(clicon_handle h, { int retval = -1; cvec *nsc = NULL; /* Canonical namespace */ - yang_stmt *yspec; yang_stmt *yspec; + yang_stmt *yspec; const struct map_str2str *ms; /* map iterator */ cxobj **xvec = NULL; /* vector of result nodes */ size_t xlen; From fa257ebb88cba656b3091f1a6b3cdf6c8cc7ddab Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Wed, 12 Feb 2020 17:37:23 +0100 Subject: [PATCH 15/19] datastore upgrade bugs --- example/main/example_backend.c | 25 +++++++++++++++++-------- lib/src/clixon_datastore_read.c | 1 - lib/src/clixon_xpath_eval.c | 4 ++++ test/test_datastore_repair.sh | 6 +++--- 4 files changed, 24 insertions(+), 12 deletions(-) diff --git a/example/main/example_backend.c b/example/main/example_backend.c index 27035ce9..a12679dc 100644 --- a/example/main/example_backend.c +++ b/example/main/example_backend.c @@ -71,11 +71,15 @@ static int _reset = 0; */ static int _state = 0; -/*! Variable to control upgrade callbacks. +/*! Variable to control module-specific upgrade callbacks. * If set, call test-case for upgrading ietf-interfaces, otherwise call * auto-upgrade */ -static int _upgrade = 0; +static int _module_upgrade = 0; + +/*! Variable to control general-purpose upgrade callbacks. + */ +static int _general_upgrade = 0; /*! Variable to control transaction logging (for debug) * If set, call syslog for every transaction callback @@ -456,9 +460,11 @@ example_upgrade(clicon_handle h, int i; const char **pp; + if (_general_upgrade == 0) + goto ok; if (strcmp(db, "startup") != 0) /* skip other than startup datastore */ goto ok; - if (msd->md_status) /* skip if there is proper module-state in datastore */ + if (msd && msd->md_status) /* skip if there is proper module-state in datastore */ goto ok; yspec = clicon_dbspec_yang(h); /* Get all yangs */ /* Get canonical namespaces for using "normalized" prefixes */ @@ -793,7 +799,7 @@ static clixon_plugin_api api = { .ca_trans_revert=main_revert, /* trans revert */ .ca_trans_end=main_end, /* trans end */ .ca_trans_abort=main_abort, /* trans abort */ - .ca_datastore_upgrade=example_upgrade /* gneral-purpose upgrade. */ + .ca_datastore_upgrade=example_upgrade /* general-purpose upgrade. */ }; /*! Backend plugin initialization @@ -818,7 +824,7 @@ clixon_plugin_init(clicon_handle h) goto done; opterr = 0; optind = 1; - while ((c = getopt(argc, argv, "rsut:")) != -1) + while ((c = getopt(argc, argv, "rsuUt:")) != -1) switch (c) { case 'r': _reset = 1; @@ -826,8 +832,11 @@ clixon_plugin_init(clicon_handle h) case 's': _state = 1; break; - case 'u': - _upgrade = 1; + case 'u': /* module-specific upgrade */ + _module_upgrade = 1; + break; + case 'U': /* general-purpose upgrade */ + _general_upgrade = 1; break; case 't': /* transaction log */ _transaction_log = 1; @@ -885,7 +894,7 @@ clixon_plugin_init(clicon_handle h) /* Upgrade callback: if you start the backend with -- -u you will get the * test interface example. Otherwise the auto-upgrade feature is enabled. */ - if (_upgrade){ + if (_module_upgrade == 1){ if (upgrade_callback_register(h, upgrade_2016, "urn:example:interfaces", 20140508, 20160101, NULL) < 0) goto done; if (upgrade_callback_register(h, upgrade_2018, "urn:example:interfaces", 20160101, 20180220, NULL) < 0) diff --git a/lib/src/clixon_datastore_read.c b/lib/src/clixon_datastore_read.c index ab0d16e1..2bcc10dd 100644 --- a/lib/src/clixon_datastore_read.c +++ b/lib/src/clixon_datastore_read.c @@ -265,7 +265,6 @@ text_read_modstate(clicon_handle h, goto done; } } - continue; /* ignore other tags, such as module-set-id */ if (strcmp(xml_name(xm), "module")) continue; /* ignore other tags, such as module-set-id */ if ((name = xml_find_body(xm, "name")) == NULL) diff --git a/lib/src/clixon_xpath_eval.c b/lib/src/clixon_xpath_eval.c index f712fcad..1c3a7b77 100644 --- a/lib/src/clixon_xpath_eval.c +++ b/lib/src/clixon_xpath_eval.c @@ -373,6 +373,10 @@ xp_eval_step(xp_ctx *xc0, if (cxvec_append(x, &xc->xc_nodeset, &xc->xc_size) < 0) goto done; } + if (vec){ + free(vec); + vec = NULL; + } break; case A_DESCENDANT: for (i=0; ixc_size; i++){ diff --git a/test/test_datastore_repair.sh b/test/test_datastore_repair.sh index 2a98d1e6..640e9b48 100755 --- a/test/test_datastore_repair.sh +++ b/test/test_datastore_repair.sh @@ -81,7 +81,7 @@ EOF ) testrun(){ - new "test params: -f $cfg" + new "test params: -f $cfg -- -U" # Bring your own backend if [ $BE -ne 0 ]; then # kill old backend (if any) @@ -90,8 +90,8 @@ testrun(){ if [ $? -ne 0 ]; then err fi - new "start backend -s startup -f $cfg" - start_backend -s startup -f $cfg + new "start backend -s startup -f $cfg -- -U" + start_backend -s startup -f $cfg -- -U new "waiting" wait_backend From 5dd3243f665a2292df381dc4127be0d2b145ff39 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Fri, 14 Feb 2020 15:16:17 +0100 Subject: [PATCH 16/19] Known Issues: * If you retrieve state _and_ config data using RESTCONF or NETCONF `get`, a severe performance penalty occurs if you have large lists (eg ACLs). Workaround: disable `VALIDATE_STATE_XML` in `include/clixon_custom.h`. --- CHANGELOG.md | 5 ++++- apps/backend/backend_client.c | 10 +++++++++- include/clixon_custom.h | 5 ++++- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 48d42933..585183ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,11 @@ * C-api: Changed `clicon_session_id_get(clicon_handle h, uint32_t *id)` * From a cli perspective this is a revert to 4.1 behaviour, where the cli does not immediately exit on start if the backend is not running, but with the new session-id function +### Known Issues +* If you retrieve state _and_ config data using RESTCONF or NETCONF `get`, a severe performance penalty occurs if you have large lists (eg ACLs). Workaround: disable `VALIDATE_STATE_XML` in `include/clixon_custom.h`. + ### Corrected Bugs -* Fixed: Enabling modstate (CLICON_XMLDB_MODSTATE), changing a revision on a yang, and restarting made the backend daemon exit at start (thanks Matt). +* Fixed: If you enabled modstate (CLICON_XMLDB_MODSTATE), changed a revision in a yang spec, and restarted the backend daemon, it exit at start (thanks Matt). * Also: ensure to load `ietf-yang-library.yang ` if CLICON_XMLDB_MODSTATE is set * Fixed: Pretty-printed XML using prefixes not parsed correctly. * eg ` ` could lead to errors, wheras (` `) works fine. diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index b2ddf5f3..2973ece8 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -979,14 +979,22 @@ from_client_get(clicon_handle h, } /* If not only-state, then read running config * Note xret can be pruned by nacm below and change name and - * metrged with state data, so zero-copy cant be used + * merged with state data, so zero-copy cant be used * Also, must use external namespace context here due to Date: Sat, 15 Feb 2020 14:45:01 +0100 Subject: [PATCH 17/19] 4.3.2 CHANGELOG updated VALIDATE_STATE_XML disabled --- CHANGELOG.md | 10 ++++++++-- include/clixon_custom.h | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 585183ea..d95e61d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,14 +1,20 @@ # Clixon Changelog -## 4.3.2 (Upcoming) +## 4.3.2 (15 February 2020) +### Major New features +* New "general-purpose" datastore upgrade callback added which i called once on startup, intended for low-level general upgrades and as a complement to module-specific upgrade. + * Called on startup after initial XML parsing, but before module-specific upgrades + * Enabled by definign the `.ca_datastore_upgrade` + * [General-purpose upgrade documentation](https://clixon-docs.readthedocs.io/en/latest/backend.html#general-purpose) + ### API changes on existing features (you may need to change your code) * Session-id CLI functionality delayed: "lazy evaluation" * C-api: Changed `clicon_session_id_get(clicon_handle h, uint32_t *id)` * From a cli perspective this is a revert to 4.1 behaviour, where the cli does not immediately exit on start if the backend is not running, but with the new session-id function ### Known Issues -* If you retrieve state _and_ config data using RESTCONF or NETCONF `get`, a severe performance penalty occurs if you have large lists (eg ACLs). Workaround: disable `VALIDATE_STATE_XML` in `include/clixon_custom.h`. +* If you retrieve state _and_ config data using RESTCONF or NETCONF `get`, a performance penalty occurs if you have large lists (eg ACLs). Workaround is: disable `VALIDATE_STATE_XML` in `include/clixon_custom.h` (disabled by default). ### Corrected Bugs * Fixed: If you enabled modstate (CLICON_XMLDB_MODSTATE), changed a revision in a yang spec, and restarted the backend daemon, it exit at start (thanks Matt). diff --git a/include/clixon_custom.h b/include/clixon_custom.h index 02c27d17..0981ec0f 100644 --- a/include/clixon_custom.h +++ b/include/clixon_custom.h @@ -74,4 +74,4 @@ * is recommended to enable it during development and debugging but disable it in production, until * this has been resolved. */ -#define VALIDATE_STATE_XML +#undef VALIDATE_STATE_XML From 4338b681678584eb36b78a08fbc82a4bca6de9b9 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Wed, 19 Feb 2020 16:48:18 +0100 Subject: [PATCH 18/19] Due to increased memory usage in internal XML trees, the [use cbuf for xml value code](https://github.com/clicon/clixon/commit/9575d10887e35079c4a9b227dde6ab0f6f09fa03) is reversed. --- CHANGELOG.md | 5 +++ lib/src/clixon_xml.c | 100 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d95e61d9..bd0f1acf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Clixon Changelog +## 4.3.3 (Expected: February 2020) + +### Minor changes +* Due to increased memory usage in internal XML trees, the [use cbuf for xml value code](https://github.com/clicon/clixon/commit/9575d10887e35079c4a9b227dde6ab0f6f09fa03) is reversed. + ## 4.3.2 (15 February 2020) ### Major New features diff --git a/lib/src/clixon_xml.c b/lib/src/clixon_xml.c index b0982a2e..6d2b6cad 100644 --- a/lib/src/clixon_xml.c +++ b/lib/src/clixon_xml.c @@ -81,6 +81,9 @@ /* Initial length of x_value malloced string */ #define XML_VALUE_MAX_DEFAULT 32 +/* Revert use cbuf for xml value code in patch release */ +#define REVERT_9575d10887e35079c4a9b227dde6ab0f6f09fa03 + /* * Types */ @@ -126,7 +129,13 @@ struct xml{ int x_childvec_len;/* Number of children */ int x_childvec_max;/* Length of allocated vector */ enum cxobj_type x_type; /* type of node: element, attribute, body */ +#ifdef REVERT_9575d10887e35079c4a9b227dde6ab0f6f09fa03 + char *x_value; /* attribute and body nodes have values */ + uint32_t x_value_len; /* Length of x_value string (excluding null) */ + uint32_t x_value_max; /* allocated size for x_value */ +#else cbuf *x_value_cb; /* attribute and body nodes have values */ +#endif int x_flags; /* Flags according to XML_FLAG_* */ yang_stmt *x_spec; /* Pointer to specification, eg yang, by reference, dont free */ @@ -614,6 +623,91 @@ xml_flag_reset(cxobj *xn, return 0; } +#ifdef REVERT_9575d10887e35079c4a9b227dde6ab0f6f09fa03 +/*! Get value of xnode + * @param[in] xn xml node + * @retval value of xml node + */ +char* +xml_value(cxobj *xn) +{ + return xn->x_value; +} + +/*! xml value string reallocator + */ +static int +xml_value_realloc(cxobj *xn, + int len0, + char *val) +{ + int retval = -1; + int len; + int diff; + + if (val==NULL) + goto ok; + len = len0 + strlen(val); + diff = xn->x_value_max - (len + 1); + if (diff <= 0){ + while (diff <= 0){ + if (xn->x_value_max == 0) + xn->x_value_max = XML_VALUE_MAX_DEFAULT; + else + xn->x_value_max *= 2; + diff = xn->x_value_max - (len + 1); + } + if ((xn->x_value = realloc(xn->x_value, xn->x_value_max)) == NULL){ + clicon_err(OE_XML, errno, "realloc"); + goto done; + } + } + strncpy(xn->x_value + len0, val, len-len0+1); + xn->x_value_len = len; + ok: + retval = 0; + done: + return retval; +} + +/*! Set value of xml node, value is copied + * @param[in] xn xml node + * @param[in] val new value, null-terminated string, copied by function + * @retval -1 on error with clicon-err set + * @retval 0 OK + */ +int +xml_value_set(cxobj *xn, + char *val) +{ + int retval = -1; + + if (xn->x_value && strlen(xn->x_value)){ + xn->x_value[0] = '\0'; + xn->x_value_len = 0; + } + if (xml_value_realloc(xn, 0, val) < 0) + goto done; + retval = 0; + done: + return retval; +} + +/*! Append value of xnode, value is copied + * @param[in] xn xml node + * @param[in] val appended value, null-terminated string, copied by function + * @retval NULL on error with clicon-err set, or if value is set to NULL + * @retval new value + */ +int +xml_value_append(cxobj *xn, + char *val) +{ + return xml_value_realloc(xn, xn->x_value_len, val); +} + +#else /* REVERT_9575d10887e35079c4a9b227dde6ab0f6f09fa03 */ + /*! Get value of xnode * @param[in] xn xml node * @retval value of xml node @@ -676,6 +770,7 @@ xml_value_append(cxobj *xn, done: return retval; } +#endif /* REVERT_9575d10887e35079c4a9b227dde6ab0f6f09fa03 */ /*! Get type of xnode * @param[in] xn xml node @@ -1600,8 +1695,13 @@ xml_free(cxobj *x) if (x->x_name) free(x->x_name); +#ifdef REVERT_9575d10887e35079c4a9b227dde6ab0f6f09fa03 + if (x->x_value) + free(x->x_value); +#else /* REVERT_9575d10887e35079c4a9b227dde6ab0f6f09fa03 */ if (x->x_value_cb) cbuf_free(x->x_value_cb); +#endif /* REVERT_9575d10887e35079c4a9b227dde6ab0f6f09fa03 */ if (x->x_prefix) free(x->x_prefix); for (i=0; ix_childvec_len; i++){ From 5918efa7c112430a5872c918d76ffd3ca043dbd8 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Thu, 20 Feb 2020 14:06:10 +0100 Subject: [PATCH 19/19] Clixon 4.3.3 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bd0f1acf..e74ac72b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Clixon Changelog -## 4.3.3 (Expected: February 2020) +## 4.3.3 (20 February 2020) ### Minor changes * Due to increased memory usage in internal XML trees, the [use cbuf for xml value code](https://github.com/clicon/clixon/commit/9575d10887e35079c4a9b227dde6ab0f6f09fa03) is reversed.