From 3dba0b537082062c3487d8aec0d54590af4b5014 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Tue, 4 Jun 2019 11:14:03 +0200 Subject: [PATCH] Restore xmldb_get() to original (but removed one parameter Moved all zero-copy xmldb_get functions to xmldb_get0. Clarified CHANGELOG for xmldb_get changes --- CHANGELOG.md | 15 +++++- apps/backend/backend_client.c | 4 +- apps/backend/backend_commit.c | 32 ++++++------- apps/backend/backend_startup.c | 6 +-- example/main/example_backend.c | 2 +- lib/clixon/clixon_datastore.h | 8 ++-- lib/src/clixon_datastore_read.c | 82 +++++++++++++++++++------------- lib/src/clixon_datastore_write.c | 2 - lib/src/clixon_nacm.c | 2 +- lib/src/clixon_xml_map.c | 2 +- util/clixon_util_datastore.c | 4 +- 11 files changed, 93 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4478ca3e..a4ae6f50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,9 +77,18 @@ * Validate-only transactions used to be terminated by `complete` * If a commit user callback fails, a new `revert` callback will be made to plugins that have made a succesful commit. * Clixon used to play the (already made) commit callbacks in reverse order -* Clixon config option `CLICON_XMLDB_CACHE` renamed to `CLICON_DATASTORE_CACHE` and changed type from `boolean` to `datastore_cache` - * Change code from: `clicon_option_bool(h, "CLICON_XMLDB_CACHE")` to `clicon_datastore_cache(h) == DATASTORE_CACHE` +* Datastore cache and xmldb_get() changes: + * You need to remove `msd` (last) parameter of `xmldb_get()`: + * `xmldb_get(h, "running", "/", &xt, NULL)` --> `xmldb_get(h, "running", "/", &xt)` + * New suite of API functions enabling zero-copy: `xmldb_get0`. You can still use `xmldb_get()`. The call sequence of zero-copy is (see reference for more info): +``` + xmldb_get0(xh, "running", "/interfaces/interface[name="eth"]", 0, &xt, NULL); + xmldb_get0_clear(h, xt); # Clear tree from default values and flags + xmldb_get0_free(h, &xt); # Free tree +``` + * Clixon config option `CLICON_XMLDB_CACHE` renamed to `CLICON_DATASTORE_CACHE` and changed type from `boolean` to `datastore_cache` * Type `datastore_cache` have values: nocache, cache, or cache-zerocopy + * Change code from: `clicon_option_bool(h, "CLICON_XMLDB_CACHE")` to `clicon_datastore_cache(h) == DATASTORE_CACHE` * `xmldb_get1` removed (functionality merged with `xmldb_get`) * Non-key list now not accepted in edit-config (before only on validation) * Changed return values in internal functions @@ -157,6 +166,8 @@ ### Minor changes +* JSON parse and print improvements + * Integrated parsing with namespace translation and yang spec lookup * Added CLIgen tab-modes in config option CLICON_CLI_TAB_MODE, which means you can control the behaviour of `` in the CLI. * Yang state get improvements * Integrated state and config into same tree on retrieval, not separate trees diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index e9a827d4..6805dc70 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -342,7 +342,7 @@ from_client_get_config(clicon_handle h, /* Note xret can be pruned by nacm below (and change name), * so zero-copy cant be used */ - if (xmldb_get(h, db, xpath, 1, &xret, NULL) < 0){ + if (xmldb_get(h, db, xpath, &xret) < 0){ if (netconf_operation_failed(cbret, "application", "read registry")< 0) goto done; goto ok; @@ -810,7 +810,7 @@ from_client_get(clicon_handle h, * Note xret can be pruned by nacm below and change name and * metrged with state data, so zero-copy cant be used */ - if (xmldb_get(h, "running", xpath, 1, &xret, NULL) < 0){ + if (xmldb_get(h, "running", xpath, &xret) < 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 b869912d..07de3da7 100644 --- a/apps/backend/backend_commit.c +++ b/apps/backend/backend_commit.c @@ -183,7 +183,7 @@ startup_common(clicon_handle h, if ((msd = modstate_diff_new()) == NULL) goto done; clicon_debug(1, "Reading startup config from %s", db); - if (xmldb_get(h, db, "/", 0, &xt, msd) < 0) + if (xmldb_get0(h, db, "/", 0, &xt, msd) < 0) goto done; /* Clear flags xpath for get */ xml_apply0(xt, CX_ELMNT, (xml_applyfn_t*)xml_flag_reset, @@ -283,7 +283,7 @@ startup_validate(clicon_handle h, } plugin_transaction_end(h, td); /* Clear cached trees from default values and marking */ - if (xmldb_get_clear(h, td->td_target) < 0) + if (xmldb_get0_clear(h, td->td_target) < 0) goto done; if (xtr){ *xtr = td->td_target; @@ -292,7 +292,7 @@ startup_validate(clicon_handle h, retval = 1; done: if (td){ - xmldb_get_free(h, &td->td_target); + xmldb_get0_free(h, &td->td_target); transaction_free(td); } return retval; @@ -335,7 +335,7 @@ startup_commit(clicon_handle h, if (plugin_transaction_commit(h, td) < 0) goto done; /* Clear cached trees from default values and marking */ - if (xmldb_get_clear(h, td->td_target) < 0) + if (xmldb_get0_clear(h, td->td_target) < 0) goto done; /* [Delete and] create running db */ @@ -360,7 +360,7 @@ startup_commit(clicon_handle h, if (td){ if (retval < 1) plugin_transaction_abort(h, td); - xmldb_get_free(h, &td->td_target); + xmldb_get0_free(h, &td->td_target); transaction_free(td); } return retval; @@ -398,7 +398,7 @@ from_validate_common(clicon_handle h, goto done; } /* This is the state we are going to */ - if (xmldb_get(h, candidate, "/", 0, &td->td_target, NULL) < 0) + if (xmldb_get0(h, candidate, "/", 0, &td->td_target, NULL) < 0) goto done; /* Clear flags xpath for get */ @@ -416,7 +416,7 @@ from_validate_common(clicon_handle h, /* 2. Parse xml trees * This is the state we are going from */ - if (xmldb_get(h, "running", "/", 0, &td->td_src, NULL) < 0) + if (xmldb_get0(h, "running", "/", 0, &td->td_src, NULL) < 0) goto done; /* Clear flags xpath for get */ xml_apply0(td->td_src, CX_ELMNT, (xml_applyfn_t*)xml_flag_reset, @@ -521,9 +521,9 @@ candidate_commit(clicon_handle h, goto done; /* Clear cached trees from default values and marking */ - if (xmldb_get_clear(h, td->td_target) < 0) + if (xmldb_get0_clear(h, td->td_target) < 0) goto done; - if (xmldb_get_clear(h, td->td_src) < 0) + if (xmldb_get0_clear(h, td->td_src) < 0) goto done; /* Optionally write (potentially modified) tree back to candidate @@ -559,8 +559,8 @@ candidate_commit(clicon_handle h, if (td){ if (retval < 1) plugin_transaction_abort(h, td); - xmldb_get_free(h, &td->td_target); - xmldb_get_free(h, &td->td_src); + xmldb_get0_free(h, &td->td_target); + xmldb_get0_free(h, &td->td_src); transaction_free(td); } return retval; @@ -744,10 +744,10 @@ from_client_validate(clicon_handle h, goto ok; } /* Clear cached trees from default values and marking */ - if (xmldb_get_clear(h, td->td_target) < 0) + if (xmldb_get0_clear(h, td->td_target) < 0) goto done; - if (xmldb_get_clear(h, td->td_src) < 0 || - xmldb_get_clear(h, td->td_target) < 0){ + if (xmldb_get0_clear(h, td->td_src) < 0 || + xmldb_get0_clear(h, td->td_target) < 0){ plugin_transaction_abort(h, td); goto done; } @@ -769,8 +769,8 @@ from_client_validate(clicon_handle h, if (td){ if (retval < 0) plugin_transaction_abort(h, td); - xmldb_get_free(h, &td->td_target); - xmldb_get_free(h, &td->td_src); + xmldb_get0_free(h, &td->td_target); + xmldb_get0_free(h, &td->td_src); transaction_free(td); } return retval; diff --git a/apps/backend/backend_startup.c b/apps/backend/backend_startup.c index 8f5d94a8..2e5342b3 100644 --- a/apps/backend/backend_startup.c +++ b/apps/backend/backend_startup.c @@ -102,12 +102,12 @@ db_merge(clicon_handle h, cxobj *xt = NULL; /* Get data as xml from db1 */ - if (xmldb_get(h, (char*)db1, NULL, 0, &xt, NULL) < 0) + if (xmldb_get0(h, (char*)db1, NULL, 0, &xt, NULL) < 0) goto done; /* Merge xml into db2. Without commit */ retval = xmldb_put(h, (char*)db2, OP_MERGE, xt, clicon_username_get(h), cbret); done: - xmldb_get_free(h, &xt); + xmldb_get0_free(h, &xt); return retval; } @@ -269,7 +269,7 @@ startup_extraxml(clicon_handle h, ok: retval = 1; done: - xmldb_get_free(h, &xt); + xmldb_get0_free(h, &xt); if (xmldb_delete(h, db) != 0 && errno != ENOENT) return -1; return retval; diff --git a/example/main/example_backend.c b/example/main/example_backend.c index 43f7e936..6823d29f 100644 --- a/example/main/example_backend.c +++ b/example/main/example_backend.c @@ -298,7 +298,7 @@ example_statedata(clicon_handle h, * state information. In this case adding dummy interface operation state * to configured interfaces. * Get config according to xpath */ - if (xmldb_get(h, "running", xpath, 1, &xt, NULL) < 0) + if (xmldb_get(h, "running", xpath, &xt) < 0) goto done; /* From that subset, only get the names */ if (xpath_vec(xt, "/interfaces/interface/name", &xvec, &xlen) < 0) diff --git a/lib/clixon/clixon_datastore.h b/lib/clixon/clixon_datastore.h index 5089d0df..f29903bf 100644 --- a/lib/clixon/clixon_datastore.h +++ b/lib/clixon/clixon_datastore.h @@ -48,9 +48,11 @@ int xmldb_db2file(clicon_handle h, const char *db, char **filename); int xmldb_validate_db(const char *db); int xmldb_connect(clicon_handle h); int xmldb_disconnect(clicon_handle h); -int xmldb_get(clicon_handle h, const char *db, char *xpath, int copy, cxobj **xtop, modstate_diff_t *msd); /* in clixon_datastore_read.[ch] */ -int xmldb_get_clear(clicon_handle h, cxobj *x); -int xmldb_get_free(clicon_handle h, cxobj **xp); + /* in clixon_datastore_read.[ch] */ +int xmldb_get(clicon_handle h, const char *db, char *xpath, cxobj **xtop); +int xmldb_get0(clicon_handle h, const char *db, char *xpath, int copy, cxobj **xtop, modstate_diff_t *msd); +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] */ int xmldb_copy(clicon_handle h, const char *from, const char *to); int xmldb_lock(clicon_handle h, const char *db, int pid); diff --git a/lib/src/clixon_datastore_read.c b/lib/src/clixon_datastore_read.c index 60e762c8..eb099521 100644 --- a/lib/src/clixon_datastore_read.c +++ b/lib/src/clixon_datastore_read.c @@ -636,45 +636,61 @@ xmldb_get_zerocopy(clicon_handle h, return retval; } -/*! Get content of database using xpath. return a set of matching sub-trees - * The function returns a minimal tree that includes all sub-trees that match - * xpath. +/*! Get content of datastore and return a copy of the XML tree * @param[in] h Clicon handle * @param[in] db Name of database to search in (filename including dir path * @param[in] xpath String with XPATH syntax. or NULL for all - * @param[in] copy Force copy. Overrides cache_zerocopy -> cache * @param[out] xret Single return XML tree. Free with xml_free() - * @param[out] msd If set, return modules-state differences - * @retval 0 OK - * @retval -1 Error - * There are two variants of the call: + * @code - * cxobj *xt; - * if (xmldb_get(xh, "running", "/interfaces/interface[name="eth"]", 0, &xt, NULL) < 0) - * err; - * # Code accessing and setting XML flags on xt, sorting, populate w yang spec,... - * xmldb_get_clear(h, xt); # Clear tree from default values and flags - * # (only if cache+zerocopy - * xmldb_get_free(h, &xt); # Free tree (only if not zero-copy) - * @endcode - * The second variant only applies to zerocopy cases where you want to force a copy - * since it may be too difficult to handle marked subtrees. - * @code - * if (xmldb_get(xh, "running", "/interfaces/interface[name="eth"]", 1, &xt, NULL) < 0) + * if (xmldb_get(xh, "running", "/interfaces/interface[name="eth"]", &xt) < 0) * err; * xml_free(xt); * @endcode - * - * @note if xvec is given, then purge tree, if not return whole tree. - * @see xpath_vec + * @see xmldb_get0 Underlying more capable API for enabling zero-copy */ int xmldb_get(clicon_handle h, const char *db, char *xpath, - int copy, - cxobj **xret, - modstate_diff_t *msd) + cxobj **xret) +{ + return xmldb_get0(h, db, xpath, 1, xret, NULL); +} + +/*! Zero-copy variant of get content of database + * + * The function returns a minimal tree that includes all sub-trees that match + * xpath. + * It can be used for zero-copying if CLICON_DATASTORE_CACHE is set + * appropriately. + * The tree returned may be the actual cache, therefore calls for cleaning and + * freeing tree must be made after use. + * @param[in] h Clicon handle + * @param[in] db Name of database to search in (filename including dir path + * @param[in] xpath String with XPATH syntax. or NULL for all + * @param[in] copy Force copy. Overrides cache_zerocopy -> cache + * @param[out] xret Single return XML tree. Free with xml_free() + * @param[out] msd If set, return modules-state differences (upgrade code) + * @retval 0 OK + * @retval -1 Error + * @code + * cxobj *xt; + * if (xmldb_get0(xh, "running", "/interface[name="eth"]", 0, &xt, NULL) < 0) + * err; + * ... + * xmldb_get0_clear(h, xt); # Clear tree from default values and flags + * xmldb_get0_free(h, &xt); # Free tree + * @endcode + * @see xmldb_get for a copy version (old-style) + */ +int +xmldb_get0(clicon_handle h, + const char *db, + char *xpath, + int copy, + cxobj **xret, + modstate_diff_t *msd) { int retval = -1; @@ -707,16 +723,16 @@ xmldb_get(clicon_handle h, return retval; } -/*! Clear cached xml tree obtained with xmldb_get, if zerocopy +/*! Clear cached xml tree obtained with xmldb_get0, if zerocopy * * @param[in] h Clicon handle * @param[in] db Name of datastore * "Clear" an xml tree means removing default values and resetting all flags. - * @see xmldb_get + * @see xmldb_get0 */ int -xmldb_get_clear(clicon_handle h, - cxobj *x) +xmldb_get0_clear(clicon_handle h, + cxobj *x) { int retval = -1; @@ -736,13 +752,13 @@ xmldb_get_clear(clicon_handle h, return retval; } -/*! Free xml tree obtained with xmldb_get, unless zerocopy +/*! Free xml tree obtained with xmldb_get0 * @param[in] h Clixon handle * @param[in,out] xp Pointer to XML cache. - * @see xmldb_get + * @see xmldb_get0 */ int -xmldb_get_free(clicon_handle h, +xmldb_get0_free(clicon_handle h, cxobj **xp) { if (*xp == NULL || diff --git a/lib/src/clixon_datastore_write.c b/lib/src/clixon_datastore_write.c index 35b13bad..3989e816 100644 --- a/lib/src/clixon_datastore_write.c +++ b/lib/src/clixon_datastore_write.c @@ -611,8 +611,6 @@ xml_container_presence(cxobj *x, * if (ret==0) * cbret contains netconf error message * @endcode - * @note that you can add both config data and state data. In comparison, - * xmldb_get has a parameter to get config data only. * @note if xret is non-null, it may contain error message */ int diff --git a/lib/src/clixon_nacm.c b/lib/src/clixon_nacm.c index 02fdf22e..3bb072bd 100644 --- a/lib/src/clixon_nacm.c +++ b/lib/src/clixon_nacm.c @@ -890,7 +890,7 @@ nacm_access_pre(clicon_handle h, goto done; } else if (strcmp(mode, "internal")==0){ - if (xmldb_get(h, "running", "nacm", 1, &xnacm0, NULL) < 0) + if (xmldb_get(h, "running", "nacm", &xnacm0) < 0) goto done; } } diff --git a/lib/src/clixon_xml_map.c b/lib/src/clixon_xml_map.c index 475aa312..2e82e923 100644 --- a/lib/src/clixon_xml_map.c +++ b/lib/src/clixon_xml_map.c @@ -1843,7 +1843,7 @@ api_path_fmt2api_path(char *api_path_fmt, } /*! Transform an xml key format and a vector of values to an XML path - * Used to input xmldb_get() or xmldb_get_vec + * Used to input xmldb_get() * @param[in] api_path_fmt XML key format * @param[in] cvv cligen variable vector, one for every wildchar in api_path_fmt * @param[out] xpath XPATH diff --git a/util/clixon_util_datastore.c b/util/clixon_util_datastore.c index 0138d6d5..678d0fde 100644 --- a/util/clixon_util_datastore.c +++ b/util/clixon_util_datastore.c @@ -198,7 +198,7 @@ main(int argc, char **argv) xpath = argv[1]; else xpath = "/"; - if (xmldb_get(h, db, xpath, 1, &xt, NULL) < 0) + if (xmldb_get(h, db, xpath, &xt) < 0) goto done; clicon_xml2file(stdout, xt, 0, 0); fprintf(stdout, "\n"); @@ -217,7 +217,7 @@ main(int argc, char **argv) else xpath = "/"; for (i=0;i