diff --git a/CHANGELOG.md b/CHANGELOG.md index 57ab07c6..974201f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,10 @@ ### API changes on existing features (you may need to change your code) +* 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` + * Type `datastore_cache` have values: nocache, cache, or cache-zerocopy + * `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 * These functions are affected: `netconf_trymerge`, `startup_module_state`, `yang_modules_state_get` diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index c5a208ff..9a059027 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -336,7 +336,10 @@ from_client_get_config(clicon_handle h, if ((xfilter = xml_find(xe, "filter")) != NULL) if ((xpath = xml_find_value(xfilter, "select"))==NULL) xpath="/"; - if (xmldb_get(h, db, xpath, &xret, NULL) < 0){ + /* 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 (netconf_operation_failed(cbret, "application", "read registry")< 0) goto done; goto ok; @@ -800,14 +803,16 @@ from_client_get(clicon_handle h, if ((xfilter = xml_find(xe, "filter")) != NULL) if ((xpath = xml_find_value(xfilter, "select"))==NULL) xpath="/"; - /* Get config */ - if (xmldb_get(h, "running", xpath, &xret, NULL) < 0){ + /* Get config + * 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 (netconf_operation_failed(cbret, "application", "read registry")< 0) goto done; goto ok; } /* Get state data from plugins as defined by plugin_statedata(), if any */ - assert(xret); clicon_err_reset(); if ((ret = client_statedata(h, xpath, &xret)) < 0) goto done; diff --git a/apps/backend/backend_commit.c b/apps/backend/backend_commit.c index 6587b581..198fe16d 100644 --- a/apps/backend/backend_commit.c +++ b/apps/backend/backend_commit.c @@ -182,7 +182,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_get1(h, db, "/", &xt, msd) < 0) + if (xmldb_get(h, db, "/", 0, &xt, msd) < 0) goto done; /* Clear flags xpath for get */ xml_apply0(xt, CX_ELMNT, (xml_applyfn_t*)xml_flag_reset, @@ -276,7 +276,7 @@ startup_validate(clicon_handle h, if (ret == 0) goto fail; /* Clear cached trees from default values and marking */ - if (xmldb_get1_clear(h, db) < 0) + if (xmldb_get_clear(h, td->td_target) < 0) goto done; if (xtr){ *xtr = td->td_target; @@ -285,11 +285,7 @@ startup_validate(clicon_handle h, retval = 1; done: if (td){ - if (clicon_option_bool(h, "CLICON_XMLDB_CACHE")){ - /* xmldb_get1 requires free only if not cache */ - td->td_target = NULL; - td->td_src = NULL; - } + xmldb_get_free(h, &td->td_target); transaction_free(td); } return retval; @@ -332,7 +328,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_get1_clear(h, db) < 0) + if (xmldb_get_clear(h, td->td_target) < 0) goto done; /* [Delete and] create running db */ @@ -356,11 +352,7 @@ startup_commit(clicon_handle h, retval = 1; done: if (td){ - if (clicon_option_bool(h, "CLICON_XMLDB_CACHE")){ - /* xmldb_get1 requires free only if not cache */ - td->td_target = NULL; - td->td_src = NULL; - } + xmldb_get_free(h, &td->td_target); transaction_free(td); } return retval; @@ -398,7 +390,7 @@ from_validate_common(clicon_handle h, goto done; } /* This is the state we are going to */ - if (xmldb_get1(h, candidate, "/", &td->td_target, NULL) < 0) + if (xmldb_get(h, candidate, "/", 0, &td->td_target, NULL) < 0) goto done; /* Clear flags xpath for get */ @@ -416,7 +408,7 @@ from_validate_common(clicon_handle h, /* 2. Parse xml trees * This is the state we are going from */ - if (xmldb_get1(h, "running", "/", &td->td_src, NULL) < 0) + if (xmldb_get(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 +513,9 @@ candidate_commit(clicon_handle h, goto done; /* Clear cached trees from default values and marking */ - if (xmldb_get1_clear(h, candidate) < 0) + if (xmldb_get_clear(h, td->td_target) < 0) goto done; - if (xmldb_get1_clear(h, "running") < 0) + if (xmldb_get_clear(h, td->td_src) < 0) goto done; /* Optionally write (potentially modified) tree back to candidate @@ -559,11 +551,8 @@ candidate_commit(clicon_handle h, if (retval < 1 && td) plugin_transaction_abort(h, td); if (td){ - if (clicon_option_bool(h, "CLICON_XMLDB_CACHE")){ - /* xmldb_get1 requires free only if not cache */ - td->td_target = NULL; - td->td_src = NULL; - } + xmldb_get_free(h, &td->td_target); + xmldb_get_free(h, &td->td_src); transaction_free(td); } return retval; @@ -749,9 +738,9 @@ from_client_validate(clicon_handle h, goto ok; } /* Clear cached trees from default values and marking */ - if (xmldb_get1_clear(h, db) < 0) + if (xmldb_get_clear(h, td->td_target) < 0) goto done; - if (xmldb_get1_clear(h, "running") < 0) + if (xmldb_get_clear(h, td->td_src) < 0) goto done; /* Optionally write (potentially modified) tree back to candidate */ @@ -768,12 +757,9 @@ from_client_validate(clicon_handle h, if (retval < 0 && td) plugin_transaction_abort(h, td); if (td){ - if (clicon_option_bool(h, "CLICON_XMLDB_CACHE")){ - /* xmldb_get1 requires free only if not cache */ - td->td_target = NULL; - td->td_src = NULL; - } - transaction_free(td); + xmldb_get_free(h, &td->td_target); + xmldb_get_free(h, &td->td_src); + transaction_free(td); } return retval; } /* from_client_validate */ diff --git a/apps/backend/backend_startup.c b/apps/backend/backend_startup.c index f2270644..8f5d94a8 100644 --- a/apps/backend/backend_startup.c +++ b/apps/backend/backend_startup.c @@ -102,13 +102,12 @@ db_merge(clicon_handle h, cxobj *xt = NULL; /* Get data as xml from db1 */ - if (xmldb_get(h, (char*)db1, NULL, &xt, NULL) < 0) + if (xmldb_get(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: - if (xt) - xml_free(xt); + xmldb_get_free(h, &xt); return retval; } @@ -270,8 +269,7 @@ startup_extraxml(clicon_handle h, ok: retval = 1; done: - if (xt && !clicon_option_bool(h, "CLICON_XMLDB_CACHE")) - xml_free(xt); + xmldb_get_free(h, &xt); if (xmldb_delete(h, db) != 0 && errno != ENOENT) return -1; return retval; diff --git a/lib/clixon/clixon_datastore.h b/lib/clixon/clixon_datastore.h index 61ec86a2..5089d0df 100644 --- a/lib/clixon/clixon_datastore.h +++ b/lib/clixon/clixon_datastore.h @@ -48,9 +48,9 @@ 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, cxobj **xtop, modstate_diff_t *msd); /* in clixon_datastore_read.[ch] */ -int xmldb_get1(clicon_handle h, const char *db, char *xpath, cxobj **xtop, modstate_diff_t *msd); /* in clixon_datastore_read.[ch] */ -int xmldb_get1_clear(clicon_handle h, const char *db); +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); 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/clixon/clixon_options.h b/lib/clixon/clixon_options.h index d0f60bab..5f183d14 100644 --- a/lib/clixon/clixon_options.h +++ b/lib/clixon/clixon_options.h @@ -73,6 +73,15 @@ enum startup_mode_t{ SM_INIT }; +/*! Datastore cache behaviour, see clixon_datastore.[ch] + * See config option type datastore_cache in clixon-config.yang + */ +enum datastore_cache{ + DATASTORE_NOCACHE, + DATASTORE_CACHE, + DATASTORE_CACHE_ZEROCOPY +}; + /* * Prototypes */ @@ -163,6 +172,7 @@ int clicon_sock_family(clicon_handle h); int clicon_sock_port(clicon_handle h); int clicon_autocommit(clicon_handle h); int clicon_startup_mode(clicon_handle h); +enum datastore_cache clicon_datastore_cache(clicon_handle h); /*-- Specific option access functions for non-yang options --*/ int clicon_quiet_mode(clicon_handle h); diff --git a/lib/src/clixon_datastore.c b/lib/src/clixon_datastore.c index d5b918c7..b27ce1b4 100644 --- a/lib/src/clixon_datastore.c +++ b/lib/src/clixon_datastore.c @@ -176,7 +176,6 @@ xmldb_disconnect(clicon_handle h) return retval; } - /*! Copy database from db1 to db2 * @param[in] h Clicon handle * @param[in] from Source database @@ -199,7 +198,7 @@ xmldb_copy(clicon_handle h, cxobj *x2 = NULL; /* to */ /* XXX lock */ - if (clicon_option_bool(h, "CLICON_XMLDB_CACHE")){ + if (clicon_datastore_cache(h) != DATASTORE_NOCACHE){ /* Copy in-memory cache */ /* 1. "to" xml tree in x1 */ if ((de1 = clicon_db_elmnt_get(h, from)) != NULL) @@ -387,7 +386,7 @@ xmldb_delete(clicon_handle h, cxobj *xt = NULL; struct stat sb; - if (clicon_option_bool(h, "CLICON_XMLDB_CACHE")){ + if (clicon_datastore_cache(h) != DATASTORE_NOCACHE){ if ((de = clicon_db_elmnt_get(h, db)) != NULL){ if ((xt = de->de_xml) != NULL){ xml_free(xt); @@ -425,7 +424,7 @@ xmldb_create(clicon_handle h, db_elmnt *de = NULL; cxobj *xt = NULL; - if (clicon_option_bool(h, "CLICON_XMLDB_CACHE")){ /* XXX This should not really happen? */ + if (clicon_datastore_cache(h) != DATASTORE_NOCACHE){ if ((de = clicon_db_elmnt_get(h, db)) != NULL){ if ((xt = de->de_xml) != NULL){ xml_free(xt); diff --git a/lib/src/clixon_datastore_read.c b/lib/src/clixon_datastore_read.c index e92f6fdb..80bf71a6 100644 --- a/lib/src/clixon_datastore_read.c +++ b/lib/src/clixon_datastore_read.c @@ -572,11 +572,11 @@ xmldb_get_cache(clicon_handle h, * @retval -1 Error */ static int -xmldb_get1_cache(clicon_handle h, - const char *db, - char *xpath, - cxobj **xtop, - modstate_diff_t *msd) +xmldb_get_zerocopy(clicon_handle h, + const char *db, + char *xpath, + cxobj **xtop, + modstate_diff_t *msd) { int retval = -1; yang_stmt *yspec; @@ -641,16 +641,29 @@ xmldb_get1_cache(clicon_handle h, * @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"]", &xt, NULL) < 0) + * 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) * err; * xml_free(xt); * @endcode + * * @note if xvec is given, then purge tree, if not return whole tree. * @see xpath_vec */ @@ -658,84 +671,83 @@ int xmldb_get(clicon_handle h, const char *db, char *xpath, + int copy, cxobj **xret, modstate_diff_t *msd) { int retval = -1; - if (clicon_option_bool(h, "CLICON_XMLDB_CACHE")) + switch (clicon_datastore_cache(h)){ + case DATASTORE_NOCACHE: + /* Read from file into created/copy tree, prune non-matching xpath + * Add default values in copy + * Copy deleted by xmldb_free + */ + retval = xmldb_get_nocache(h, db, xpath, xret, msd); + break; + case DATASTORE_CACHE_ZEROCOPY: + /* Get cache (file if empty) mark xpath match in original tree + * add default values in original tree and return that. + * Default values and markings removed in xmldb_clear + */ + if (!copy){ + retval = xmldb_get_zerocopy(h, db, xpath, xret, msd); + break; + } + /* fall through */ + case DATASTORE_CACHE: + /* Get cache (file if empty) mark xpath match and copy marked into copy + * Add default values in copy, return copy + * Copy deleted by xmldb_free + */ retval = xmldb_get_cache(h, db, xpath, xret, msd); - else - retval = xmldb_get_nocache(h, db, xpath, xret, msd); + break; + } 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. - * @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[out] xret Single return XML tree. see note - * @param[out] msd If set, return modules-state differences - * @retval 0 OK - * @retval -1 Error - * @code - * cxobj *xt; - * if (xmldb_get(xh, "running", "/interfaces/interface[name="eth"]", &xt, NULL) < 0) - * err; - * xml_free(xt); - * @endcode - * @note if xvec is given, then purge tree, if not return whole tree. - * @see xmldb_get This version uses direct cache access and needs to be - * cleanued up after use - * @see xmldb_get1_clear Must call after use - * @note If !CLICON_XMLDB_CACHE you need to free xret after use - * @note If CLICON_XMLDB_CACHE mark|change flags set, need to clear after call - */ -int -xmldb_get1(clicon_handle h, - const char *db, - char *xpath, - cxobj **xret, - modstate_diff_t *msd) -{ - int retval = -1; - - if (clicon_option_bool(h, "CLICON_XMLDB_CACHE")) - retval = xmldb_get1_cache(h, db, xpath, xret, msd); - else - retval = xmldb_get_nocache(h, db, xpath, xret, msd); - return retval; -} - -/*! Clear cached tree after accessed by xmldb_get1 +/*! Clear cached xml tree obtained with xmldb_get, if zerocopy * - * @param[in] h Clicon handle - * @param[in] dbname Name of database to search in (filename including dir path - * @see xmldb_get1 + * @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 */ int -xmldb_get1_clear(clicon_handle h, - const char *db) +xmldb_get_clear(clicon_handle h, + cxobj *x) { int retval = -1; - db_elmnt *de = NULL; - if (!clicon_option_bool(h, "CLICON_XMLDB_CACHE")) - goto ok; /* dont bother, tree is a copy */ - de = clicon_db_elmnt_get(h, db); - if (de != NULL && de->de_xml != NULL){ - /* clear XML tree of defaults */ - if (xml_tree_prune_flagged(de->de_xml, XML_FLAG_DEFAULT, 1) < 0) - goto done; - /* clear mark and change */ - xml_apply0(de->de_xml, CX_ELMNT, (xml_applyfn_t*)xml_flag_reset, - (void*)(0xff)); - } + if (clicon_datastore_cache(h) != DATASTORE_CACHE_ZEROCOPY) + goto ok; + if (x == NULL) + goto ok; + /* clear XML tree of defaults */ + if (xml_tree_prune_flagged(x, XML_FLAG_DEFAULT, 1) < 0) + goto done; + /* clear mark and change */ + xml_apply0(x, CX_ELMNT, (xml_applyfn_t*)xml_flag_reset, + (void*)(0xff)); ok: retval = 0; done: return retval; - +} + +/*! Free xml tree obtained with xmldb_get, unless zerocopy + * @param[in] h Clixon handle + * @param[in,out] xp Pointer to XML cache. + * @see xmldb_get + */ +int +xmldb_get_free(clicon_handle h, + cxobj **xp) +{ + if (*xp == NULL || + clicon_datastore_cache(h) == DATASTORE_CACHE_ZEROCOPY) + return 0; + xml_free(*xp); + *xp = NULL; + return 0; } diff --git a/lib/src/clixon_datastore_write.c b/lib/src/clixon_datastore_write.c index cb0407ee..c8a01526 100644 --- a/lib/src/clixon_datastore_write.c +++ b/lib/src/clixon_datastore_write.c @@ -652,8 +652,9 @@ xmldb_put(clicon_handle h, xml_name(x1)); goto done; } - if (clicon_option_bool(h, "CLICON_XMLDB_CACHE")){ - if ((de = clicon_db_elmnt_get(h, db)) != NULL) + + if ((de = clicon_db_elmnt_get(h, db)) != NULL){ + if (clicon_datastore_cache(h) != DATASTORE_NOCACHE) x0 = de->de_xml; } /* If there is no xml x0 tree (in cache), then read it from file */ @@ -713,8 +714,9 @@ xmldb_put(clicon_handle h, if (xml_apply0(x0, -1, xml_sort_verify, NULL) < 0) clicon_log(LOG_NOTICE, "%s: verify failed #3", __FUNCTION__); #endif + /* Write back to datastore cache if first time */ - if (clicon_option_bool(h, "CLICON_XMLDB_CACHE")){ + if (clicon_datastore_cache(h) != DATASTORE_NOCACHE){ db_elmnt de0 = {0,}; if (de != NULL) de0 = *de; @@ -770,7 +772,7 @@ xmldb_put(clicon_handle h, free(dbfile); if (cb) cbuf_free(cb); - if (!clicon_option_bool(h, "CLICON_XMLDB_CACHE") && x0) + if (x0 && clicon_datastore_cache(h) == DATASTORE_NOCACHE) xml_free(x0); return retval; fail: diff --git a/lib/src/clixon_nacm.c b/lib/src/clixon_nacm.c index 2c57da27..02fdf22e 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", &xnacm0, NULL) < 0) + if (xmldb_get(h, "running", "nacm", 1, &xnacm0, NULL) < 0) goto done; } } diff --git a/lib/src/clixon_options.c b/lib/src/clixon_options.c index bf815fd4..b42373b0 100644 --- a/lib/src/clixon_options.c +++ b/lib/src/clixon_options.c @@ -543,30 +543,25 @@ clicon_cli_genmodel_completion(clicon_handle h) return 0; } +static const map_str2int cli_genmodel_map[] = { + {"NONE", GT_NONE}, + {"VARS", GT_VARS}, + {"ALL", GT_ALL}, + {NULL, -1} +}; + /*! How to generate and show CLI syntax: VARS|ALL * @see clixon-config@.yang CLICON_CLI_GENMODEL_TYPE */ enum genmodel_type clicon_cli_genmodel_type(clicon_handle h) { - char *s; - enum genmodel_type gt = GT_ERR; + char *str; - if (!clicon_option_exists(h, "CLICON_CLI_GENMODEL_TYPE")){ - gt = GT_VARS; - goto done; - } - s = clicon_option_str(h, "CLICON_CLI_GENMODEL_TYPE"); - if (strcmp(s, "NONE")==0) - gt = GT_NONE; + if ((str = clicon_option_str(h, "CLICON_CLI_GENMODEL_TYPE")) == NULL) + return GT_VARS; else - if (strcmp(s, "VARS")==0) - gt = GT_VARS; - else - if (strcmp(s, "ALL")==0) - gt = GT_ALL; - done: - return gt; + return clicon_str2int(cli_genmodel_map, str); } /*! Get Dont include keys in cvec in cli vars callbacks @@ -638,6 +633,28 @@ clicon_startup_mode(clicon_handle h) return clicon_str2int(startup_mode_map, mode); } +static const map_str2int datastore_cache_map[] = { + {"nocache", DATASTORE_NOCACHE}, + {"cache", DATASTORE_CACHE}, + {"cache-zerocopy", DATASTORE_CACHE_ZEROCOPY}, + {NULL, -1} +}; + +/*! How to generate and show CLI syntax: VARS|ALL + * @see clixon-config@.yang CLICON_CLI_GENMODEL_TYPE + */ +enum datastore_cache +clicon_datastore_cache(clicon_handle h) +{ + char *str; + + if ((str = clicon_option_str(h, "CLICON_DATASTORE_CACHE")) == NULL) + return DATASTORE_CACHE; + else + return clicon_str2int(datastore_cache_map, str); +} + + /*--------------------------------------------------------------------- * Specific option access functions for non-yang options * Typically dynamic values and more complex datatypes, diff --git a/util/clixon_util_datastore.c b/util/clixon_util_datastore.c index 3b0e5911..0138d6d5 100644 --- a/util/clixon_util_datastore.c +++ b/util/clixon_util_datastore.c @@ -198,11 +198,14 @@ main(int argc, char **argv) xpath = argv[1]; else xpath = "/"; - if (xmldb_get(h, db, xpath, &xt, NULL) < 0) + if (xmldb_get(h, db, xpath, 1, &xt, NULL) < 0) goto done; clicon_xml2file(stdout, xt, 0, 0); - fprintf(stdout, "\n"); + if (xt){ + xml_free(xt); + xt = NULL; + } } else if (strcmp(cmd, "mget")==0){ int nr; @@ -214,15 +217,17 @@ main(int argc, char **argv) else xpath = "/"; for (i=0;i