From 46043b24caf3ccbaeef3fdfaeb7edcc6c99e6a58 Mon Sep 17 00:00:00 2001 From: An-Cheng Huang Date: Mon, 23 Aug 2010 14:48:42 -0700 Subject: move "changed" status handling into library * remove public status modifier as high-level operations no longer need it. * add more information to clarify changed status handling in library. * mark changed status at appropriate places in library. --- src/cli_bin.cpp | 3 - src/common/unionfs.c | 2 +- src/cstore/cstore.cpp | 151 ++++++++++++++++++---------------- src/cstore/cstore.hpp | 7 +- src/cstore/unionfs/cstore-unionfs.cpp | 76 +++++++---------- src/cstore/unionfs/cstore-unionfs.hpp | 4 +- 6 files changed, 118 insertions(+), 125 deletions(-) (limited to 'src') diff --git a/src/cli_bin.cpp b/src/cli_bin.cpp index 25a86ce..5823c02 100644 --- a/src/cli_bin.cpp +++ b/src/cli_bin.cpp @@ -165,8 +165,6 @@ doDiscard(Cstore& cstore, const vector& args) if (!cstore.discardChanges()) { bye("discard failed\n"); } - // special case for discard: don't return (don't want to mark changed) - exit(0); } static void @@ -228,7 +226,6 @@ main(int argc, char **argv) // call the op function OpFunc[op_idx](cstore, path_comps); - cstore.markCfgPathChanged(path_comps); exit(0); } diff --git a/src/common/unionfs.c b/src/common/unionfs.c index 85ea9f1..da4cd7e 100644 --- a/src/common/unionfs.c +++ b/src/common/unionfs.c @@ -730,7 +730,7 @@ common_commit_copy_to_live_config(GNode *node, boolean suppress_piecewise_copy, /* note: return status is not checked and operation continues even if * this fails. this follows the original logic. */ - cstore_unmark_cfg_path_changed(cs, pcomps, ncomps); + cstore_unmark_cfg_path_changed(cs, (const char **) pcomps, ncomps); cstore_free_path_comps(pcomps, ncomps); cstore_free(cs); } diff --git a/src/cstore/cstore.cpp b/src/cstore/cstore.cpp index f146cb0..b7261fe 100644 --- a/src/cstore/cstore.cpp +++ b/src/cstore/cstore.cpp @@ -220,10 +220,10 @@ Cstore::deleteCfgPath(const vector& path_comps, const vtw_def& def) /* assume default value is valid (parser should have validated). * also call unmark_deactivated() in case the node being deleted was * also deactivated. note that unmark_deactivated() succeeds if it's - * not marked deactivated. + * not marked deactivated. also mark "changed". */ bool ret = (write_value(def.def_default) && mark_display_default() - && unmark_deactivated()); + && unmark_deactivated() && mark_changed_with_ancestors()); if (!ret) { output_user("Failed to set default value during delete\n"); } @@ -266,6 +266,10 @@ Cstore::deleteCfgPath(const vector& path_comps, const vtw_def& def) ret = remove_node(); } } + if (ret) { + // mark changed + ret = mark_changed_with_ancestors(); + } RESTORE_PATHS; if (!ret) { output_user("Failed to delete specified config path\n"); @@ -893,7 +897,12 @@ Cstore::renameCfgPath(const vector& args) string otagval = args[1]; string ntagval = args[4]; push_cfg_path(otagnode); - bool ret = rename_child_node(otagval, ntagval); + /* also mark changed. note that it's marking the "tag node" but not the + * "tag values" since one is being "deleted" and the other is being + * "added" anyway. + */ + bool ret = (rename_child_node(otagval, ntagval) + && mark_changed_with_ancestors()); pop_cfg_path(); return ret; } @@ -909,7 +918,11 @@ Cstore::copyCfgPath(const vector& args) string otagval = args[1]; string ntagval = args[4]; push_cfg_path(otagnode); - bool ret = copy_child_node(otagval, ntagval); + /* also mark changed. note that it's marking the "tag node" but not the + * new "tag value" since it is being "added" anyway. + */ + bool ret = (copy_child_node(otagval, ntagval) + && mark_changed_with_ancestors()); pop_cfg_path(); return ret; } @@ -1038,7 +1051,7 @@ Cstore::cfgPathAdded(const vector& path_comps) * (remember this functions is NOT "deactivate-aware") * (1) cfgPathDeleted() * (2) cfgPathAdded() - * (3) marked_changed() + * (3) cfg_node_changed() */ bool Cstore::cfgPathChanged(const vector& path_comps) @@ -1048,7 +1061,7 @@ Cstore::cfgPathChanged(const vector& path_comps) } SAVE_PATHS; append_cfg_path(path_comps); - bool ret = marked_changed(); + bool ret = cfg_node_changed(); RESTORE_PATHS; return ret; } @@ -1208,7 +1221,7 @@ Cstore::cfgPathGetChildNodesStatusDA(const vector& path_comps, } else { SAVE_PATHS; append_cfg_path(ppath); - if (marked_changed()) { + if (cfg_node_changed()) { cmap[work_nodes[i]] = C_NODE_STATUS_CHANGED; } else { cmap[work_nodes[i]] = C_NODE_STATUS_STATIC; @@ -1738,7 +1751,9 @@ Cstore::markCfgPathDeactivated(const vector& path_comps) SAVE_PATHS; append_cfg_path(path_comps); - bool ret = (mark_deactivated() && unmark_deactivated_descendants()); + // note: also mark changed + bool ret = (mark_deactivated() && unmark_deactivated_descendants() + && mark_changed_with_ancestors()); RESTORE_PATHS; return ret; } @@ -1752,17 +1767,16 @@ Cstore::unmarkCfgPathDeactivated(const vector& path_comps) { SAVE_PATHS; append_cfg_path(path_comps); - bool ret = unmark_deactivated(); + // note: also mark changed + bool ret = (unmark_deactivated() && mark_changed_with_ancestors()); RESTORE_PATHS; return ret; } -/* mark specified path as changed in working config. - * the marking is used during commit to check if a node has been changed. - * this should be done after set/delete/activate/deactivate. - * note: if a node is changed, all of its ancestors are also considered - * changed. - * return true if successful. otherwise return false. +/* "changed" status handling. + * the "changed" status is used during commit to check if a node has been + * changed. note that if a node is "changed", all of its ancestors are also + * considered changed (this follows the original logic). * * the original backend implementation only uses the "changed" marker at * "root" to indicate whether the whole config has changed. for the rest @@ -1775,38 +1789,30 @@ Cstore::unmarkCfgPathDeactivated(const vector& path_comps) * in "changes only", so they are _not_ treated as "changed". this creates * problems in various parts of the backend. * - * the new CLI backend/library marks all changed nodes explicitly, and the - * "changed" status depends on such markers. note that the actual marking - * is done by the low-level implementation, so it does not have to be done - * as a "file marker" as long as the low-level implementation can correctly - * answer the "changed" query for a given path. + * the new CLI backend/library "marks" all changed nodes explicitly, and the + * "changed" status depends on such markers. the marking is done using the + * pure virtual mark_changed_with_ancestors() function, which is provided + * by the low-level implementation, so it does not have to be done as a + * "per-node file marker" as long as the low-level implementation can + * correctly answer the "changed" query for a given path. + * + * note that "changed" nodes does not include "added" and "deleted" nodes. + * for the convenience of implementation, the backend must always query + * for "changed" nodes *after* "added" and "deleted" nodes. in other + * words, the backend will only treat a node as "changed" if it is neither + * "added" nor "deleted". currently there are only two places that perform + * changed status query: cfgPathGetChildNodesStatus() and + * cfgPathGetChildNodesStatusDA(). see those two functions for the usage. + * + * what this means is that the backend can choose to either mark or not + * mark "added"/"deleted" nodes as "changed" at its convenience. for + * example, "set" and "delete" always do the marking, but "rename" and + * "copy" do not. + * + * changed status queries are provided by the cfg_node_changed() function, + * and changed markers can be removed by unmarkCfgPathChanged() below (used + * by "commit"). */ -bool -Cstore::markCfgPathChanged(const vector& path_comps) -{ - // first mark the root changed - if (!mark_changed()) { - return false; - } - - // now mark each level as changed - vector ppath; - for (size_t i = 0; i < path_comps.size(); i++) { - ppath.push_back(path_comps[i]); - if (!cfg_path_exists(ppath, false, false)) { - // this level no longer in working. nothing further. - break; - } - SAVE_PATHS; - append_cfg_path(ppath); - bool ret = mark_changed(); - RESTORE_PATHS; - if (!ret) { - return false; - } - } - return true; -} /* unmark "changed" status of specified path in working config. * this is used, e.g., at the end of "commit" to reset a subtree. @@ -2272,6 +2278,10 @@ Cstore::set_cfg_path(const vector& path_comps, bool output) } } } + if (!mark_changed_with_ancestors()) { + ret = false; + break; + } RESTORE_PATHS; } RESTORE_PATHS; // if "break" was hit @@ -2289,29 +2299,32 @@ Cstore::set_cfg_path(const vector& path_comps, bool output) pop_cfg_path(); // only do it if it's previously marked default if (marked_display_default(false)) { - ret = unmark_display_default(); - - /* XXX work around current commit's unionfs implementation problem. - * current commit's unionfs implementation looks at the "changes only" - * directory (i.e., the r/w portion of the union mount), which is wrong. - * - * all config information should be obtained from two directories: - * "active" and "working", e.g., instead of looking at whiteout files - * in "changes only" to find deleted nodes, nodes that are in "active" - * but not in "working" have been deleted. - * - * in this particular case, commit looks at "changes only" to read the - * node.val file. however, since the value didn't change (only the - * "default status" changed), node.val doesn't appear in "changes only". - * here we re-write the file to force it into "changes only" so that - * commit can work correctly. - */ - vector vvec; - read_value_vec(vvec, false); - write_value_vec(vvec); - - // pretend it didn't exist since we changed the status - path_exists = false; + if ((ret = unmark_display_default())) { + /* XXX work around current commit's unionfs implementation problem. + * current commit's unionfs implementation looks at the "changes + * only" directory (i.e., the r/w portion of the union mount), which + * is wrong. + * + * all config information should be obtained from two directories: + * "active" and "working", e.g., instead of looking at whiteout + * files in "changes only" to find deleted nodes, nodes that are in + * "active" but not in "working" have been deleted. + * + * in this particular case, commit looks at "changes only" to read + * the node.val file. however, since the value didn't change (only + * the "default status" changed), node.val doesn't appear in + * "changes only". here we re-write the file to force it into + * "changes only" so that commit can work correctly. + */ + vector vvec; + read_value_vec(vvec, false); + write_value_vec(vvec); + + // pretend it didn't exist since we changed the status + path_exists = false; + // also mark changed + ret = mark_changed_with_ancestors(); + } } RESTORE_PATHS; } diff --git a/src/cstore/cstore.hpp b/src/cstore/cstore.hpp index 4b07fab..0c80ffd 100644 --- a/src/cstore/cstore.hpp +++ b/src/cstore/cstore.hpp @@ -142,8 +142,7 @@ public: virtual bool setupSession() = 0; virtual bool teardownSession() = 0; virtual bool inSession() = 0; - // common - bool markCfgPathChanged(const vector& path_comps); + // commit bool unmarkCfgPathChanged(const vector& path_comps); // XXX load //bool unmarkCfgPathDeactivatedDescendants(const vector& path_comps); @@ -307,14 +306,14 @@ private: virtual bool mark_deactivated() = 0; virtual bool unmark_deactivated() = 0; virtual bool unmark_deactivated_descendants() = 0; + virtual bool mark_changed_with_ancestors() = 0; virtual bool unmark_changed_with_descendants() = 0; - virtual bool mark_changed() = 0; virtual bool remove_comment() = 0; virtual bool set_comment(const string& comment) = 0; virtual bool discard_changes(unsigned long long& num_removed) = 0; // observers for current work path - virtual bool marked_changed() = 0; + virtual bool cfg_node_changed() = 0; // observers for current work path or active path virtual bool read_value_vec(vector& vvec, bool active_cfg) = 0; diff --git a/src/cstore/unionfs/cstore-unionfs.cpp b/src/cstore/unionfs/cstore-unionfs.cpp index 7f14483..5cfd8ee 100644 --- a/src/cstore/unionfs/cstore-unionfs.cpp +++ b/src/cstore/unionfs/cstore-unionfs.cpp @@ -749,6 +749,32 @@ UnionfsCstore::unmark_deactivated_descendants() return ret; } +// mark current work path and all ancestors as "changed" +bool +UnionfsCstore::mark_changed_with_ancestors() +{ + b_fs::path opath = mutable_cfg_path; // use a copy + while (opath.has_parent_path()) { + b_fs::path marker = (work_root / opath); + pop_path(opath); + if (!b_fs_exists(marker) || !b_fs_is_directory(marker)) { + // don't do anything if the node is not there + continue; + } + marker /= C_MARKER_CHANGED; + if (b_fs_exists(marker)) { + // reached a node already marked => done + break; + } + if (!create_file(marker.file_string())) { + output_internal("failed to mark changed [%s]\n", + marker.file_string().c_str()); + return false; + } + } + return true; +} + /* remove all "changed" markers under the current work path. this is used, * e.g., at the end of "commit" to reset a subtree. */ @@ -777,39 +803,6 @@ UnionfsCstore::unmark_changed_with_descendants() return true; } -bool -UnionfsCstore::mark_changed() -{ - if (!mutable_cfg_path.has_parent_path()) { - /* at root, mark changed. root marker is needed by the original - * implementation as an indication of whether the whole config - * has changed. - */ - b_fs::path marker = get_work_path() / C_MARKER_CHANGED; - if (b_fs_exists(marker)) { - // already marked. treat as success. - return true; - } - if (!create_file(marker.file_string())) { - output_internal("failed to mark changed [%s]\n", - get_work_path().file_string().c_str()); - return false; - } - return true; - } - - /* XXX not at root => nop for now. - * we should be marking changed here. however, as commit is still - * using its own unionfs implementation, it will not understand the - * markers and therefore will not perform the necessary cleanup when - * it's done. - * - * for now, don't mark anything besides root. the query function - * will use unionfs-specific implementation (changes-only dir). - */ - return true; -} - // remove the comment at the current work path bool UnionfsCstore::remove_comment() @@ -880,21 +873,12 @@ UnionfsCstore::get_comment(string& comment, bool active_cfg) return read_whole_file(cfile, comment); } +// whether current work path is "changed" bool -UnionfsCstore::marked_changed() +UnionfsCstore::cfg_node_changed() { - /* this function is only called by cfgPathChanged() in base class. - * - * XXX currently just use the changes_only dir for this query. - * see explanation in mark_changed(). - * - * this implementation relies on the fact that cfgPathChanged() - * includes deleted/added nodes (including deactivated/activated - * nodes since it's NOT deactivate-aware). if that is not the case, - * result will be different between deleted nodes (NOT IN - * changes_only) and deactivated nodes (IN changes_only). - */ - return b_fs_exists(get_change_path()); + b_fs::path marker = get_work_path() / C_MARKER_CHANGED; + return b_fs_exists(marker); } /* XXX currently "committed marking" is done inside commit. diff --git a/src/cstore/unionfs/cstore-unionfs.hpp b/src/cstore/unionfs/cstore-unionfs.hpp index bff4844..90f28c5 100644 --- a/src/cstore/unionfs/cstore-unionfs.hpp +++ b/src/cstore/unionfs/cstore-unionfs.hpp @@ -158,14 +158,14 @@ private: bool mark_deactivated(); bool unmark_deactivated(); bool unmark_deactivated_descendants(); + bool mark_changed_with_ancestors(); bool unmark_changed_with_descendants(); - bool mark_changed(); bool remove_comment(); bool set_comment(const string& comment); bool discard_changes(unsigned long long& num_removed); // observers for work path - bool marked_changed(); + bool cfg_node_changed(); // observers for work path or active path bool cfg_node_exists(bool active_cfg); -- cgit v1.2.3 From 3afeb6df69058c605ad251dfc7ae93d4d0ae6f1d Mon Sep 17 00:00:00 2001 From: An-Cheng Huang Date: Tue, 24 Aug 2010 18:54:06 -0700 Subject: add extensible node sorting mechanism * unify node sorting implementation into the backend library. * allow future implementation of per-node, customized sorting policy. --- Makefile.am | 1 + debian/control | 2 +- src/cstore/cstore.cpp | 42 ++++++++++++++++++++++++++++++++++++++++-- src/cstore/cstore.hpp | 41 ++++++++++++++++++++++++++++++++++++++--- 4 files changed, 80 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/Makefile.am b/Makefile.am index 86db597..00a3c80 100644 --- a/Makefile.am +++ b/Makefile.am @@ -23,6 +23,7 @@ src_libvyatta_cfg_la_LIBADD = /usr/lib/libglib-2.0.la src_libvyatta_cfg_la_LIBADD += /usr/lib/libgio-2.0.la src_libvyatta_cfg_la_LIBADD += -lboost_system src_libvyatta_cfg_la_LIBADD += -lboost_filesystem +src_libvyatta_cfg_la_LIBADD += -lapt-pkg src_libvyatta_cfg_la_LDFLAGS = -version-info 1:0:0 src_libvyatta_cfg_la_SOURCES = src/cli_parse.y src/cli_def.l src/cli_val.l src_libvyatta_cfg_la_SOURCES += src/cli_new.c src/cli_path_utils.c diff --git a/debian/control b/debian/control index 2d7e270..6ab3335 100644 --- a/debian/control +++ b/debian/control @@ -3,7 +3,7 @@ Section: contrib/net Priority: extra Maintainer: Vyatta Package Maintainers Build-Depends: debhelper (>= 5), autotools-dev, libglib2.0-dev, - libboost-filesystem1.40-dev + libboost-filesystem1.40-dev, libapt-pkg-dev Standards-Version: 3.7.2 Package: vyatta-cfg diff --git a/src/cstore/cstore.cpp b/src/cstore/cstore.cpp index b7261fe..2c30207 100644 --- a/src/cstore/cstore.cpp +++ b/src/cstore/cstore.cpp @@ -25,6 +25,10 @@ #include #include +// for debian's version comparison algorithm +#define APT_COMPATIBILITY 986 +#include + #include #include #include @@ -58,6 +62,12 @@ const string Cstore::C_ENV_SHAPI_HELP_STRS = "_cli_shell_api_hstrs"; const string Cstore::C_ENUM_SCRIPT_DIR = "/opt/vyatta/share/enumeration"; const string Cstore::C_LOGFILE_STDOUT = "/tmp/cfg-stdout.log"; + +////// static +bool Cstore::_init = false; +map Cstore::_sort_func_map; + + ////// constructors/destructors /* this constructor just returns the generic environment string, * currently the two levels. implementation-specific environment @@ -71,6 +81,8 @@ const string Cstore::C_LOGFILE_STDOUT = "/tmp/cfg-stdout.log"; */ Cstore::Cstore(string& env) { + init(); + string decl = "declare -x "; env = (decl + C_ENV_EDIT_LEVEL + "=/; "); env += (decl + C_ENV_TMPL_LEVEL + "=/;"); @@ -182,6 +194,7 @@ Cstore::tmplGetChildNodes(const vector& path_comps, SAVE_PATHS; append_tmpl_path(path_comps); get_all_tmpl_child_node_names(cnodes); + sort_nodes(cnodes); RESTORE_PATHS; } @@ -1096,6 +1109,7 @@ Cstore::cfgPathGetDeletedChildNodesDA(const vector& path_comps, cnodes.push_back(acnodes[i]); } } + sort_nodes(cnodes); } /* get "deleted" values of specified "multi node" during commit @@ -1145,7 +1159,8 @@ Cstore::cfgPathGetDeletedValuesDA(const vector& path_comps, */ void Cstore::cfgPathGetChildNodesStatus(const vector& path_comps, - map& cmap) + map& cmap, + vector& sorted_keys) { // get a union of active and working map umap; @@ -1166,6 +1181,7 @@ Cstore::cfgPathGetChildNodesStatus(const vector& path_comps, for (; it != umap.end(); ++it) { string c = (*it).first; ppath.push_back(c); + sorted_keys.push_back(c); // note: "changed" includes "deleted" and "added", so check those first. if (cfgPathDeleted(ppath)) { cmap[c] = C_NODE_STATUS_DELETED; @@ -1178,6 +1194,7 @@ Cstore::cfgPathGetChildNodesStatus(const vector& path_comps, } ppath.pop_back(); } + sort_nodes(sorted_keys); } /* DA version of the above function. @@ -1187,12 +1204,14 @@ Cstore::cfgPathGetChildNodesStatus(const vector& path_comps, */ void Cstore::cfgPathGetChildNodesStatusDA(const vector& path_comps, - map& cmap) + map& cmap, + vector& sorted_keys) { // process deleted nodes first vector del_nodes; cfgPathGetDeletedChildNodesDA(path_comps, del_nodes); for (size_t i = 0; i < del_nodes.size(); i++) { + sorted_keys.push_back(del_nodes[i]); cmap[del_nodes[i]] = C_NODE_STATUS_DELETED; } @@ -1202,6 +1221,7 @@ Cstore::cfgPathGetChildNodesStatusDA(const vector& path_comps, vector ppath = path_comps; for (size_t i = 0; i < work_nodes.size(); i++) { ppath.push_back(work_nodes[i]); + sorted_keys.push_back(work_nodes[i]); /* note: in the DA version here, we do NOT check the deactivate state * when considering the state of the child nodes (which include * deactivated ones). the reason is that this DA function is used @@ -1231,6 +1251,7 @@ Cstore::cfgPathGetChildNodesStatusDA(const vector& path_comps, ppath.pop_back(); } + sort_nodes(sorted_keys); } /* check whether specified path is "deactivated" in working config or @@ -1294,6 +1315,7 @@ Cstore::cfgPathGetChildNodesDA(const vector& path_comps, append_cfg_path(path_comps); get_all_child_node_names(cnodes, active_cfg, include_deactivated); RESTORE_PATHS; + sort_nodes(cnodes); } /* get value of specified single-value node. @@ -1575,6 +1597,7 @@ Cstore::cfgPathGetEffectiveChildNodes(const vector& path_comps, } ppath.pop_back(); } + sort_nodes(cnodes); } /* get the "effective" value of specified path during commit operation. @@ -1877,6 +1900,21 @@ Cstore::output_internal(const char *fmt, ...) ////// private functions +bool +Cstore::sort_func_deb_version(string a, string b) +{ + return (pkgVersionCompare(a, b) < 0); +} + +void +Cstore::sort_nodes(vector& nvec, Cstore::SortAlgT sort_alg) +{ + if (_sort_func_map.find(sort_alg) == _sort_func_map.end()) { + return; + } + sort(nvec.begin(), nvec.end(), _sort_func_map[sort_alg]); +} + /* try to append the logical path to template path. * is_tag: (output) whether the last component is a "tag". * return false if logical path is not valid. otherwise return true. diff --git a/src/cstore/cstore.hpp b/src/cstore/cstore.hpp index 0c80ffd..8cb3fd1 100644 --- a/src/cstore/cstore.hpp +++ b/src/cstore/cstore.hpp @@ -38,7 +38,7 @@ using namespace std; class Cstore { public: - Cstore() {}; + Cstore() { init(); }; Cstore(string& env); virtual ~Cstore() {}; @@ -66,6 +66,7 @@ public: static const size_t MAX_CMD_OUTPUT_SIZE = 4096; + ////// the public cstore interface //// functions implemented in this base class // these operate on template path @@ -186,7 +187,13 @@ public: void cfgPathGetDeletedValues(const vector& path_comps, vector& dvals); void cfgPathGetChildNodesStatus(const vector& path_comps, - map& cmap); + map& cmap) { + vector dummy; + cfgPathGetChildNodesStatus(path_comps, cmap, dummy); + }; + void cfgPathGetChildNodesStatus(const vector& path_comps, + map& cmap, + vector& sorted_keys); /* observers for "effective config". can be used both during a config * session and outside a config session. more detailed information @@ -244,7 +251,13 @@ public: vector& dvals, bool include_deactivated = true); void cfgPathGetChildNodesStatusDA(const vector& path_comps, - map& cmap); + map& cmap) { + vector dummy; + cfgPathGetChildNodesStatusDA(path_comps, cmap, dummy); + }; + void cfgPathGetChildNodesStatusDA(const vector& path_comps, + map& cmap, + vector& sorted_keys); /* these are internal API functions and operate on current cfg and @@ -349,6 +362,28 @@ private: virtual string tmpl_path_to_str() = 0; ////// implemented + // for sorting + typedef enum { + SORT_DEFAULT = 0, + SORT_DEB_VERSION = 0, + SORT_NONE + } SortAlgT; + typedef bool (*SortFuncT)(std::string, std::string); + static map _sort_func_map; + + static bool sort_func_deb_version(string a, string b); + void sort_nodes(vector& nvec, SortAlgT sort_alg = SORT_DEFAULT); + + // init + static bool _init; + static void init() { + if (_init) { + return; + } + _init = true; + _sort_func_map[SORT_DEB_VERSION] = &sort_func_deb_version; + } + // begin path modifiers (only these can change path permanently) bool append_tmpl_path(const vector& path_comps, bool& is_tag); bool append_tmpl_path(const vector& path_comps) { -- cgit v1.2.3 From 28b30ff302ed535b1e81902f06d24c9e9fe61c19 Mon Sep 17 00:00:00 2001 From: An-Cheng Huang Date: Wed, 25 Aug 2010 16:41:44 -0700 Subject: switch to unordered_map --- perl_dmod/Cstore/Cstore.xs | 7 +++---- perl_dmod/Cstore/typemap | 2 +- src/cstore/cstore-varref.cpp | 2 +- src/cstore/cstore.cpp | 31 +++++++++++++++++-------------- src/cstore/cstore.hpp | 32 +++++++++++++++++++------------- src/cstore/unionfs/cstore-unionfs.cpp | 19 ++++++++++--------- src/cstore/unionfs/cstore-unionfs.hpp | 4 ++-- 7 files changed, 53 insertions(+), 44 deletions(-) (limited to 'src') diff --git a/perl_dmod/Cstore/Cstore.xs b/perl_dmod/Cstore/Cstore.xs index 4a726e7..ebef08f 100644 --- a/perl_dmod/Cstore/Cstore.xs +++ b/perl_dmod/Cstore/Cstore.xs @@ -25,7 +25,6 @@ #include #include #include -#include /* currently use the UnionfsCstore implementation */ #include @@ -205,7 +204,7 @@ Cstore::cfgPathGetChildNodesStatus(STRVEC *vref) PREINIT: vector arg_strvec; CODE: - map ret_strstrmap; + Cstore::MapT ret_strstrmap; THIS->cfgPathGetChildNodesStatus(arg_strvec, ret_strstrmap); OUTPUT: RETVAL @@ -263,7 +262,7 @@ Cstore::cfgPathGetChildNodesStatusDA(STRVEC *vref) PREINIT: vector arg_strvec; CODE: - map ret_strstrmap; + Cstore::MapT ret_strstrmap; THIS->cfgPathGetChildNodesStatusDA(arg_strvec, ret_strstrmap); OUTPUT: RETVAL @@ -295,7 +294,7 @@ Cstore::getParsedTmpl(STRVEC *vref, bool allow_val) PREINIT: vector arg_strvec; CODE: - map ret_strstrmap; + Cstore::MapT ret_strstrmap; if (!THIS->getParsedTmpl(arg_strvec, ret_strstrmap, allow_val)) { XSRETURN_UNDEF; } diff --git a/perl_dmod/Cstore/typemap b/perl_dmod/Cstore/typemap index f1f6a82..6df545b 100644 --- a/perl_dmod/Cstore/typemap +++ b/perl_dmod/Cstore/typemap @@ -32,7 +32,7 @@ T_STRVEC_REF T_STRSTRMAP_REF HV *href = (HV *) sv_2mortal((SV *) newHV()); - map::iterator it = ret_strstrmap.begin(); + Cstore::MapT::iterator it = ret_strstrmap.begin(); for (; it != ret_strstrmap.end(); ++it) { const char *key = (*it).first.c_str(); const char *val = (*it).second.c_str(); diff --git a/src/cstore/cstore-varref.cpp b/src/cstore/cstore-varref.cpp index f00ed38..02bdb97 100644 --- a/src/cstore/cstore-varref.cpp +++ b/src/cstore/cstore-varref.cpp @@ -225,7 +225,7 @@ bool Cstore::VarRef::getValue(string& value, vtw_type_e& def_type) { vector result; - map added; + Cstore::MapT added; def_type = ERROR_TYPE; for (size_t i = 0; i < _paths.size(); i++) { if (_paths[i].first.size() == 0) { diff --git a/src/cstore/cstore.cpp b/src/cstore/cstore.cpp index 2c30207..25eaae7 100644 --- a/src/cstore/cstore.cpp +++ b/src/cstore/cstore.cpp @@ -21,7 +21,6 @@ #include #include #include -#include #include #include @@ -62,10 +61,14 @@ const string Cstore::C_ENV_SHAPI_HELP_STRS = "_cli_shell_api_hstrs"; const string Cstore::C_ENUM_SCRIPT_DIR = "/opt/vyatta/share/enumeration"; const string Cstore::C_LOGFILE_STDOUT = "/tmp/cfg-stdout.log"; +//// sorting +const unsigned int Cstore::SORT_DEFAULT = 0; +const unsigned int Cstore::SORT_DEB_VERSION = 0; +const unsigned int Cstore::SORT_NONE = 1; ////// static bool Cstore::_init = false; -map Cstore::_sort_func_map; +Cstore::MapT Cstore::_sort_func_map; ////// constructors/destructors @@ -121,7 +124,7 @@ Cstore::validateTmplPath(const vector& path_comps, bool validate_vals, */ bool Cstore::getParsedTmpl(const vector& path_comps, - map& tmap, bool allow_val) + Cstore::MapT& tmap, bool allow_val) { vtw_def def; /* currently this function is used outside actual CLI operations, mainly @@ -1099,7 +1102,7 @@ Cstore::cfgPathGetDeletedChildNodesDA(const vector& path_comps, cfgPathGetChildNodesDA(path_comps, acnodes, true, include_deactivated); vector wcnodes; cfgPathGetChildNodesDA(path_comps, wcnodes, false, include_deactivated); - map cmap; + MapT cmap; for (size_t i = 0; i < wcnodes.size(); i++) { cmap[wcnodes[i]] = true; } @@ -1138,7 +1141,7 @@ Cstore::cfgPathGetDeletedValuesDA(const vector& path_comps, || !cfgPathGetValuesDA(path_comps, nvals, false, include_deactivated)) { return; } - map dmap; + MapT dmap; for (size_t i = 0; i < nvals.size(); i++) { dmap[nvals[i]] = true; } @@ -1159,11 +1162,11 @@ Cstore::cfgPathGetDeletedValuesDA(const vector& path_comps, */ void Cstore::cfgPathGetChildNodesStatus(const vector& path_comps, - map& cmap, + Cstore::MapT& cmap, vector& sorted_keys) { // get a union of active and working - map umap; + MapT umap; vector acnodes; vector wcnodes; cfgPathGetChildNodes(path_comps, acnodes, true); @@ -1177,7 +1180,7 @@ Cstore::cfgPathGetChildNodesStatus(const vector& path_comps, // get the status of each one vector ppath = path_comps; - map::iterator it = umap.begin(); + MapT::iterator it = umap.begin(); for (; it != umap.end(); ++it) { string c = (*it).first; ppath.push_back(c); @@ -1204,7 +1207,7 @@ Cstore::cfgPathGetChildNodesStatus(const vector& path_comps, */ void Cstore::cfgPathGetChildNodesStatusDA(const vector& path_comps, - map& cmap, + Cstore::MapT& cmap, vector& sorted_keys) { // process deleted nodes first @@ -1574,7 +1577,7 @@ Cstore::cfgPathGetEffectiveChildNodes(const vector& path_comps, } // get a union of active and working - map cmap; + MapT cmap; vector acnodes; vector wcnodes; cfgPathGetChildNodes(path_comps, acnodes, true); @@ -1588,7 +1591,7 @@ Cstore::cfgPathGetEffectiveChildNodes(const vector& path_comps, // get only the effective ones from the union vector ppath = path_comps; - map::iterator it = cmap.begin(); + MapT::iterator it = cmap.begin(); for (; it != cmap.end(); ++it) { string c = (*it).first; ppath.push_back(c); @@ -1660,7 +1663,7 @@ Cstore::cfgPathGetEffectiveValues(const vector& path_comps, } // get a union of active and working - map vmap; + MapT vmap; vector ovals; vector nvals; cfgPathGetValues(path_comps, ovals, true); @@ -1674,7 +1677,7 @@ Cstore::cfgPathGetEffectiveValues(const vector& path_comps, // get only the effective ones from the union vector ppath = path_comps; - map::iterator it = vmap.begin(); + MapT::iterator it = vmap.begin(); for (; it != vmap.end(); ++it) { string c = (*it).first; ppath.push_back(c); @@ -1907,7 +1910,7 @@ Cstore::sort_func_deb_version(string a, string b) } void -Cstore::sort_nodes(vector& nvec, Cstore::SortAlgT sort_alg) +Cstore::sort_nodes(vector& nvec, unsigned int sort_alg) { if (_sort_func_map.find(sort_alg) == _sort_func_map.end()) { return; diff --git a/src/cstore/cstore.hpp b/src/cstore/cstore.hpp index 8cb3fd1..5da07fd 100644 --- a/src/cstore/cstore.hpp +++ b/src/cstore/cstore.hpp @@ -18,7 +18,7 @@ #define _CSTORE_H_ #include #include -#include +#include #include @@ -42,6 +42,10 @@ public: Cstore(string& env); virtual ~Cstore() {}; + // types + template + class MapT : public tr1::unordered_map {}; + // constants static const string C_NODE_STATUS_DELETED; static const string C_NODE_STATUS_ADDED; @@ -74,7 +78,7 @@ public: bool validateTmplPath(const vector& path_comps, bool validate_vals, vtw_def& def); bool getParsedTmpl(const vector& path_comps, - map& tmap, bool allow_val = true); + MapT& tmap, bool allow_val = true); void tmplGetChildNodes(const vector& path_comps, vector& cnodes); @@ -187,12 +191,12 @@ public: void cfgPathGetDeletedValues(const vector& path_comps, vector& dvals); void cfgPathGetChildNodesStatus(const vector& path_comps, - map& cmap) { + MapT& cmap) { vector dummy; cfgPathGetChildNodesStatus(path_comps, cmap, dummy); }; void cfgPathGetChildNodesStatus(const vector& path_comps, - map& cmap, + MapT& cmap, vector& sorted_keys); /* observers for "effective config". can be used both during a config @@ -251,12 +255,12 @@ public: vector& dvals, bool include_deactivated = true); void cfgPathGetChildNodesStatusDA(const vector& path_comps, - map& cmap) { + MapT& cmap) { vector dummy; cfgPathGetChildNodesStatusDA(path_comps, cmap, dummy); }; void cfgPathGetChildNodesStatusDA(const vector& path_comps, - map& cmap, + MapT& cmap, vector& sorted_keys); @@ -363,16 +367,18 @@ private: ////// implemented // for sorting - typedef enum { - SORT_DEFAULT = 0, - SORT_DEB_VERSION = 0, - SORT_NONE - } SortAlgT; + /* apparently unordered_map template does not work with "enum" type, so + * change this to simply unsigned ints to allow unifying all map types, + * i.e., "Cstore::MapT". + */ + static const unsigned int SORT_DEFAULT; + static const unsigned int SORT_DEB_VERSION; + static const unsigned int SORT_NONE; typedef bool (*SortFuncT)(std::string, std::string); - static map _sort_func_map; + static MapT _sort_func_map; static bool sort_func_deb_version(string a, string b); - void sort_nodes(vector& nvec, SortAlgT sort_alg = SORT_DEFAULT); + void sort_nodes(vector& nvec, unsigned int sort_alg = SORT_DEFAULT); // init static bool _init; diff --git a/src/cstore/unionfs/cstore-unionfs.cpp b/src/cstore/unionfs/cstore-unionfs.cpp index 5cfd8ee..849d659 100644 --- a/src/cstore/unionfs/cstore-unionfs.cpp +++ b/src/cstore/unionfs/cstore-unionfs.cpp @@ -17,7 +17,6 @@ #include #include #include -#include #include #include @@ -63,8 +62,8 @@ const string UnionfsCstore::C_DEF_NAME = "node.def"; ////// static -static map _fs_escape_chars; -static map _fs_unescape_chars; +static Cstore::MapT _fs_escape_chars; +static Cstore::MapT _fs_unescape_chars; static void _init_fs_escape_chars() { @@ -80,7 +79,7 @@ _init_fs_escape_chars() static string _escape_char(char c) { - map::iterator p = _fs_escape_chars.find(c); + Cstore::MapT::iterator p = _fs_escape_chars.find(c); if (p != _fs_escape_chars.end()) { return _fs_escape_chars[c]; } else { @@ -88,12 +87,13 @@ _escape_char(char c) } } -static map _escape_path_name_cache; +static Cstore::MapT _escape_path_name_cache; static string _escape_path_name(const string& path) { - map::iterator p = _escape_path_name_cache.find(path); + Cstore::MapT::iterator p + = _escape_path_name_cache.find(path); if (p != _escape_path_name_cache.end()) { // found escaped string in cache. just return it. return _escape_path_name_cache[path]; @@ -110,12 +110,13 @@ _escape_path_name(const string& path) return npath; } -static map _unescape_path_name_cache; +static Cstore::MapT _unescape_path_name_cache; static string _unescape_path_name(const string& path) { - map::iterator p = _unescape_path_name_cache.find(path); + Cstore::MapT::iterator p + = _unescape_path_name_cache.find(path); if (p != _unescape_path_name_cache.end()) { // found unescaped string in cache. just return it. return _unescape_path_name_cache[path]; @@ -129,7 +130,7 @@ _unescape_path_name(const string& path) break; } string s = path.substr(i, 3); - map::iterator p = _fs_unescape_chars.find(s); + Cstore::MapT::iterator p = _fs_unescape_chars.find(s); if (p != _fs_unescape_chars.end()) { char c = _fs_unescape_chars[s]; if (path.size() == 3 && c == -1) { diff --git a/src/cstore/unionfs/cstore-unionfs.hpp b/src/cstore/unionfs/cstore-unionfs.hpp index 90f28c5..3942e01 100644 --- a/src/cstore/unionfs/cstore-unionfs.hpp +++ b/src/cstore/unionfs/cstore-unionfs.hpp @@ -83,7 +83,7 @@ private: // path buffers b_fs::path mutable_cfg_path; // mutable part of config path b_fs::path tmpl_path; // whole template path - map > saved_paths; + Cstore::MapT > saved_paths; // saved mutable part of cfg path and whole template path ////// virtual functions defined in base class @@ -122,7 +122,7 @@ private: saved_paths[handle] = p; }; void restore_paths(const void *handle = NULL) { - map >::iterator it + Cstore::MapT >::iterator it = saved_paths.find(handle); if (it == saved_paths.end()) { exit_internal("restore_paths: handle not found\n"); -- cgit v1.2.3 From 797d373bd7403455d249ee9ce9725089a93c8754 Mon Sep 17 00:00:00 2001 From: An-Cheng Huang Date: Wed, 25 Aug 2010 17:20:21 -0700 Subject: remove sorting from unsorted API calls --- src/cstore/cstore.cpp | 220 ++++++++++++++++++++++++++------------------------ src/cstore/cstore.hpp | 20 +++-- 2 files changed, 130 insertions(+), 110 deletions(-) (limited to 'src') diff --git a/src/cstore/cstore.cpp b/src/cstore/cstore.cpp index 25eaae7..30a6d6a 100644 --- a/src/cstore/cstore.cpp +++ b/src/cstore/cstore.cpp @@ -1153,110 +1153,6 @@ Cstore::cfgPathGetDeletedValuesDA(const vector& path_comps, } } -/* this is the equivalent of the listNodeStatus() from the original - * perl API. it provides the "status" ("deleted", "added", "changed", - * or "static") of each child node of specified path. - * cmap: (output) contains the status of child nodes. - * - * note: this function is NOT "deactivate-aware". - */ -void -Cstore::cfgPathGetChildNodesStatus(const vector& path_comps, - Cstore::MapT& cmap, - vector& sorted_keys) -{ - // get a union of active and working - MapT umap; - vector acnodes; - vector wcnodes; - cfgPathGetChildNodes(path_comps, acnodes, true); - cfgPathGetChildNodes(path_comps, wcnodes, false); - for (size_t i = 0; i < acnodes.size(); i++) { - umap[acnodes[i]] = true; - } - for (size_t i = 0; i < wcnodes.size(); i++) { - umap[wcnodes[i]] = true; - } - - // get the status of each one - vector ppath = path_comps; - MapT::iterator it = umap.begin(); - for (; it != umap.end(); ++it) { - string c = (*it).first; - ppath.push_back(c); - sorted_keys.push_back(c); - // note: "changed" includes "deleted" and "added", so check those first. - if (cfgPathDeleted(ppath)) { - cmap[c] = C_NODE_STATUS_DELETED; - } else if (cfgPathAdded(ppath)) { - cmap[c] = C_NODE_STATUS_ADDED; - } else if (cfgPathChanged(ppath)) { - cmap[c] = C_NODE_STATUS_CHANGED; - } else { - cmap[c] = C_NODE_STATUS_STATIC; - } - ppath.pop_back(); - } - sort_nodes(sorted_keys); -} - -/* DA version of the above function. - * cmap: (output) contains the status of child nodes. - * - * note: this follows the original perl API listNodeStatus() implementation. - */ -void -Cstore::cfgPathGetChildNodesStatusDA(const vector& path_comps, - Cstore::MapT& cmap, - vector& sorted_keys) -{ - // process deleted nodes first - vector del_nodes; - cfgPathGetDeletedChildNodesDA(path_comps, del_nodes); - for (size_t i = 0; i < del_nodes.size(); i++) { - sorted_keys.push_back(del_nodes[i]); - cmap[del_nodes[i]] = C_NODE_STATUS_DELETED; - } - - // get all nodes in working config - vector work_nodes; - cfgPathGetChildNodesDA(path_comps, work_nodes, false); - vector ppath = path_comps; - for (size_t i = 0; i < work_nodes.size(); i++) { - ppath.push_back(work_nodes[i]); - sorted_keys.push_back(work_nodes[i]); - /* note: in the DA version here, we do NOT check the deactivate state - * when considering the state of the child nodes (which include - * deactivated ones). the reason is that this DA function is used - * for config output-related operations and should return whether - * each node is actually added/deleted from the config independent - * of its deactivate state. - * - * for "added" state, can't use cfgPathAdded() since it's not DA. - * - * for "changed" state, can't use cfgPathChanged() since it's not DA. - * - * deleted ones already handled above. - */ - if (!cfg_path_exists(ppath, true, true) - && cfg_path_exists(ppath, false, true)) { - cmap[work_nodes[i]] = C_NODE_STATUS_ADDED; - } else { - SAVE_PATHS; - append_cfg_path(ppath); - if (cfg_node_changed()) { - cmap[work_nodes[i]] = C_NODE_STATUS_CHANGED; - } else { - cmap[work_nodes[i]] = C_NODE_STATUS_STATIC; - } - RESTORE_PATHS; - } - - ppath.pop_back(); - } - sort_nodes(sorted_keys); -} - /* check whether specified path is "deactivated" in working config or * active config. * a node is "deactivated" if the node itself or any of its ancestors is @@ -2379,6 +2275,122 @@ Cstore::set_cfg_path(const vector& path_comps, bool output) return ret; } +/* this is the equivalent of the listNodeStatus() from the original + * perl API. it provides the "status" ("deleted", "added", "changed", + * or "static") of each child node of specified path. + * cmap: (output) contains the status of child nodes. + * sorted_keys: (output) contains sorted keys. call with NULL if not needed. + * + * note: this function is NOT "deactivate-aware". + */ +void +Cstore::get_child_nodes_status(const vector& path_comps, + Cstore::MapT& cmap, + vector *sorted_keys) +{ + // get a union of active and working + MapT umap; + vector acnodes; + vector wcnodes; + cfgPathGetChildNodes(path_comps, acnodes, true); + cfgPathGetChildNodes(path_comps, wcnodes, false); + for (size_t i = 0; i < acnodes.size(); i++) { + umap[acnodes[i]] = true; + } + for (size_t i = 0; i < wcnodes.size(); i++) { + umap[wcnodes[i]] = true; + } + + // get the status of each one + vector ppath = path_comps; + MapT::iterator it = umap.begin(); + for (; it != umap.end(); ++it) { + string c = (*it).first; + ppath.push_back(c); + if (sorted_keys) { + sorted_keys->push_back(c); + } + // note: "changed" includes "deleted" and "added", so check those first. + if (cfgPathDeleted(ppath)) { + cmap[c] = C_NODE_STATUS_DELETED; + } else if (cfgPathAdded(ppath)) { + cmap[c] = C_NODE_STATUS_ADDED; + } else if (cfgPathChanged(ppath)) { + cmap[c] = C_NODE_STATUS_CHANGED; + } else { + cmap[c] = C_NODE_STATUS_STATIC; + } + ppath.pop_back(); + } + if (sorted_keys) { + sort_nodes(*sorted_keys); + } +} + +/* DA version of the above function. + * cmap: (output) contains the status of child nodes. + * sorted_keys: (output) contains sorted keys. call with NULL if not needed. + * + * note: this follows the original perl API listNodeStatus() implementation. + */ +void +Cstore::get_child_nodes_status_da(const vector& path_comps, + Cstore::MapT& cmap, + vector *sorted_keys) +{ + // process deleted nodes first + vector del_nodes; + cfgPathGetDeletedChildNodesDA(path_comps, del_nodes); + for (size_t i = 0; i < del_nodes.size(); i++) { + if (sorted_keys) { + sorted_keys->push_back(del_nodes[i]); + } + cmap[del_nodes[i]] = C_NODE_STATUS_DELETED; + } + + // get all nodes in working config + vector work_nodes; + cfgPathGetChildNodesDA(path_comps, work_nodes, false); + vector ppath = path_comps; + for (size_t i = 0; i < work_nodes.size(); i++) { + ppath.push_back(work_nodes[i]); + if (sorted_keys) { + sorted_keys->push_back(work_nodes[i]); + } + /* note: in the DA version here, we do NOT check the deactivate state + * when considering the state of the child nodes (which include + * deactivated ones). the reason is that this DA function is used + * for config output-related operations and should return whether + * each node is actually added/deleted from the config independent + * of its deactivate state. + * + * for "added" state, can't use cfgPathAdded() since it's not DA. + * + * for "changed" state, can't use cfgPathChanged() since it's not DA. + * + * deleted ones already handled above. + */ + if (!cfg_path_exists(ppath, true, true) + && cfg_path_exists(ppath, false, true)) { + cmap[work_nodes[i]] = C_NODE_STATUS_ADDED; + } else { + SAVE_PATHS; + append_cfg_path(ppath); + if (cfg_node_changed()) { + cmap[work_nodes[i]] = C_NODE_STATUS_CHANGED; + } else { + cmap[work_nodes[i]] = C_NODE_STATUS_STATIC; + } + RESTORE_PATHS; + } + + ppath.pop_back(); + } + if (sorted_keys) { + sort_nodes(*sorted_keys); + } +} + /* remove tag at current work path and its subtree. * if specified tag is the last one, also remove the tag node. * return true if successful. otherwise return false. diff --git a/src/cstore/cstore.hpp b/src/cstore/cstore.hpp index 5da07fd..e09c015 100644 --- a/src/cstore/cstore.hpp +++ b/src/cstore/cstore.hpp @@ -192,12 +192,13 @@ public: vector& dvals); void cfgPathGetChildNodesStatus(const vector& path_comps, MapT& cmap) { - vector dummy; - cfgPathGetChildNodesStatus(path_comps, cmap, dummy); + get_child_nodes_status(path_comps, cmap, NULL); }; void cfgPathGetChildNodesStatus(const vector& path_comps, MapT& cmap, - vector& sorted_keys); + vector& sorted_keys) { + get_child_nodes_status(path_comps, cmap, &sorted_keys); + }; /* observers for "effective config". can be used both during a config * session and outside a config session. more detailed information @@ -256,12 +257,13 @@ public: bool include_deactivated = true); void cfgPathGetChildNodesStatusDA(const vector& path_comps, MapT& cmap) { - vector dummy; - cfgPathGetChildNodesStatusDA(path_comps, cmap, dummy); + get_child_nodes_status_da(path_comps, cmap, NULL); }; void cfgPathGetChildNodesStatusDA(const vector& path_comps, MapT& cmap, - vector& sorted_keys); + vector& sorted_keys) { + get_child_nodes_status_da(path_comps, cmap, &sorted_keys); + }; /* these are internal API functions and operate on current cfg and @@ -423,6 +425,12 @@ private: bool cfg_path_exists(const vector& path_comps, bool active_cfg, bool include_deactivated); bool set_cfg_path(const vector& path_comps, bool output); + void get_child_nodes_status(const vector& path_comps, + Cstore::MapT& cmap, + vector *sorted_keys); + void get_child_nodes_status_da(const vector& path_comps, + Cstore::MapT& cmap, + vector *sorted_keys); // these operate on current work path (or active with "active_cfg") bool remove_tag(); -- cgit v1.2.3 From 46ed0425ebde3e026b402026cd633bef7c2b6c5f Mon Sep 17 00:00:00 2001 From: An-Cheng Huang Date: Thu, 26 Aug 2010 13:58:17 -0700 Subject: new implementation for config output * replace the original ConfigOutput perl module. * simplify logic and fix bugs in original code. --- Makefile.am | 1 + etc/bash_completion.d/20vyatta-cfg | 10 +- src/cli_shell_api.cpp | 50 ++++- src/cstore/cstore-node.cpp | 398 +++++++++++++++++++++++++++++++++++++ src/cstore/cstore-node.hpp | 104 ++++++++++ 5 files changed, 547 insertions(+), 16 deletions(-) create mode 100644 src/cstore/cstore-node.cpp create mode 100644 src/cstore/cstore-node.hpp (limited to 'src') diff --git a/Makefile.am b/Makefile.am index 00a3c80..cef450f 100644 --- a/Makefile.am +++ b/Makefile.am @@ -32,6 +32,7 @@ src_libvyatta_cfg_la_SOURCES += src/cli_val_engine.c src/cli_objects.c src_libvyatta_cfg_la_SOURCES += src/cstore/cstore-c.cpp src_libvyatta_cfg_la_SOURCES += src/cstore/cstore.cpp src_libvyatta_cfg_la_SOURCES += src/cstore/cstore-varref.cpp +src_libvyatta_cfg_la_SOURCES += src/cstore/cstore-node.cpp src_libvyatta_cfg_la_SOURCES += src/cstore/unionfs/cstore-unionfs.cpp CLEANFILES = src/cli_parse.c src/cli_parse.h src/cli_def.c src/cli_val.c LDADD = src/libvyatta-cfg.la diff --git a/etc/bash_completion.d/20vyatta-cfg b/etc/bash_completion.d/20vyatta-cfg index 56c1272..f6b02ce 100755 --- a/etc/bash_completion.d/20vyatta-cfg +++ b/etc/bash_completion.d/20vyatta-cfg @@ -84,19 +84,15 @@ done show () { - local show_all='' local -a args=() for arg in "$@"; do if [ "$arg" == "-all" ]; then - show_all='-all' + args+=('--show-show-defaults') else - args[${#args[@]}]="$arg" + args+=("$arg") fi done - local level=$(vyatta_cli_shell_api NOEVAL getEditLevelStr) - ${vyatta_sbindir}/vyatta-output-config.pl ${show_all} \ - $level ${args[@]} \ - | eval "${VYATTA_PAGER:-cat}" + cli-shell-api showCfg "${args[@]}" | eval "${VYATTA_PAGER:-cat}" } commit () diff --git a/src/cli_shell_api.cpp b/src/cli_shell_api.cpp index 1a4f469..0038866 100644 --- a/src/cli_shell_api.cpp +++ b/src/cli_shell_api.cpp @@ -16,14 +16,13 @@ #include #include -#include #include #include -#include -#include +#include #include #include +#include /* This program provides an API for shell scripts (e.g., snippets in * templates, standalone scripts, etc.) to access the CLI cstore library. @@ -48,6 +47,12 @@ print_vec(const vector& vec, const char *sep, const char *quote) } } +//// options +// showCfg options +int op_show_active_only = 0; +int op_show_show_defaults = 0; +int op_show_hide_secrets = 0; + typedef void (*OpFuncT)(const vector& args); typedef struct { @@ -388,6 +393,15 @@ validateTmplValPath(const vector& args) exit(cstore.validateTmplPath(args, true) ? 0 : 1); } +static void +showCfg(const vector& args) +{ + UnionfsCstore cstore(true); + vector nargs(args); + CstoreCfgNode root(cstore, nargs, op_show_active_only); + root.show_as_root(op_show_show_defaults, op_show_hide_secrets); +} + #define OP(name, exact, exact_err, min, min_err) \ { #name, exact, exact_err, min, min_err, &name } @@ -430,6 +444,8 @@ static OpT ops[] = { OP(validateTmplPath, -1, NULL, 1, "Must specify config path"), OP(validateTmplValPath, -1, NULL, 1, "Must specify config path"), + OP(showCfg, -1, NULL, -1, NULL), + {NULL, -1, NULL, -1, NULL, NULL} }; #define OP_exact_args ops[op_idx].op_exact_args @@ -438,16 +454,32 @@ static OpT ops[] = { #define OP_min_error ops[op_idx].op_min_error #define OP_func ops[op_idx].op_func +struct option options[] = { + {"show-active-only", no_argument, &op_show_active_only, 1}, + {"show-show-defaults", no_argument, &op_show_show_defaults, 1}, + {"show-hide-secrets", no_argument, &op_show_hide_secrets, 1}, + {NULL, 0, NULL, 0} +}; + int main(int argc, char **argv) { + // handle options first + int c = 0; + while ((c = getopt_long(argc, argv, "", options, NULL)) != -1) { + // nothing for now + } + int nargs = argc - optind - 1; + char *oname = argv[optind]; + char **nargv = &(argv[optind + 1]); + int i = 0; - if (argc < 2) { + if (nargs < 0) { fprintf(stderr, "Must specify operation\n"); exit(-1); } while (ops[i].op_name) { - if (strcmp(argv[1], ops[i].op_name) == 0) { + if (strcmp(oname, ops[i].op_name) == 0) { op_idx = i; break; } @@ -457,18 +489,18 @@ main(int argc, char **argv) fprintf(stderr, "Invalid operation\n"); exit(-1); } - if (OP_exact_args >= 0 && (argc - 2) != OP_exact_args) { + if (OP_exact_args >= 0 && nargs != OP_exact_args) { fprintf(stderr, "%s\n", OP_exact_error); exit(-1); } - if (OP_min_args >= 0 && (argc - 2) < OP_min_args) { + if (OP_min_args >= 0 && nargs < OP_min_args) { fprintf(stderr, "%s\n", OP_min_error); exit(-1); } vector args; - for (int i = 2; i < argc; i++) { - args.push_back(argv[i]); + for (int i = 0; i < nargs; i++) { + args.push_back(nargv[i]); } // call the op function diff --git a/src/cstore/cstore-node.cpp b/src/cstore/cstore-node.cpp new file mode 100644 index 0000000..85d2fda --- /dev/null +++ b/src/cstore/cstore-node.cpp @@ -0,0 +1,398 @@ +/* + * Copyright (C) 2010 Vyatta, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include +#include +#include +#include +#include + +#include +#include + +using namespace std; + +////// constants +const string CstoreCfgNode::PFX_DIFF_ADD = "+"; // added +const string CstoreCfgNode::PFX_DIFF_DEL = "-"; // deleted +const string CstoreCfgNode::PFX_DIFF_UPD = ">"; // changed +const string CstoreCfgNode::PFX_DIFF_NONE = " "; + +const string CstoreCfgNode::PFX_DEACT_D = "!"; // deactivated +const string CstoreCfgNode::PFX_DEACT_DP = "D"; // deactivate pending +const string CstoreCfgNode::PFX_DEACT_AP = "A"; // activate pending +const string CstoreCfgNode::PFX_DEACT_NONE = " "; + +bool CstoreCfgNode::_init = false; +Cstore::MapT CstoreCfgNode::_st_map; + +////// constructors/destructors +CstoreCfgNode::CstoreCfgNode(Cstore& cstore, vector& path_comps, + bool active_only, const string& name, + NodeStatusT status) + : _cstore(&cstore), _name(name), _status(status), + _pfx_deact(PFX_DEACT_NONE.c_str()), + _is_tag(false), _is_leaf(false), _is_multi(false), _is_value(false), + _is_default(false), _is_invalid(false), _is_empty(false), + _active_only(false), _comment(""), _comment_status(ST_STATIC) +{ + init(); + + if (!_cstore) { + return; + } + + // active_only if specified or not in session + _active_only = (!_cstore->inSession() || active_only); + + /* first get the def (only if path is not empty). if path is empty, treat + * it as an intermediate node. + */ + if (path_comps.size() > 0) { + vtw_def def; + if (_cstore->validateTmplPath(path_comps, false, def)) { + // got the def + if (def.is_value && !def.tag) { + // leaf value. should never happen (recursion should have terminated). + return; + } + _is_tag = (def.tag && !def.is_value); + _is_leaf = (!_is_tag && !def.is_value && def.def_type != ERROR_TYPE); + _is_multi = (_is_leaf && def.multi); + _is_value = def.is_value; + + _is_default = _cstore->cfgPathDefault(path_comps, _active_only); + + bool adeact = _cstore->cfgPathDeactivated(path_comps, true); + /* if active only, pretend deactivate status in working is the same + * as active to get the right output. + */ + bool wdeact = (_active_only + ? adeact + : _cstore->cfgPathDeactivated(path_comps, false)); + if (adeact) { + if (wdeact) { + // deactivated in both active and working => deactivated + _pfx_deact = PFX_DEACT_D.c_str(); + } else { + // deactivated only in active => activate pending + _pfx_deact = PFX_DEACT_AP.c_str(); + } + } else { + if (wdeact) { + // deactivated only in working => deactivate pending + _pfx_deact = PFX_DEACT_DP.c_str(); + } + // 4th case handled by default + } + } else { + // not a valid node. this should only happen at root. + _is_invalid = true; + return; + } + } + + // handle comment + string ac_str; + bool ac = _cstore->cfgPathGetComment(path_comps, ac_str, true); + string wc_str; + bool wc = _cstore->cfgPathGetComment(path_comps, wc_str, false); + if (ac) { + if (wc) { + // has comment in both active and working + _comment = wc_str; + if (ac_str != wc_str) { + _comment_status = ST_CHANGED; + } + } else { + // comment only in active + _comment = ac_str; + _comment_status = ST_DELETED; + } + } else { + if (wc) { + // comment only in working + _comment = wc_str; + _comment_status = ST_ADDED; + } + // 4th case (neither) handled by default + } + + if (_is_leaf) { + /* leaf node. set the name if this is the root (note path_comps must be + * non-empty if this is leaf). + */ + if (_name == "") { + _name = path_comps[path_comps.size() - 1]; + } + if (_is_multi) { + // multi-value node + NodeStatusT st = (_active_only ? ST_STATIC : _status); + if (st == ST_STATIC || st == ST_DELETED || st == ST_ADDED) { + bool get_active = (st != ST_ADDED); + _cstore->cfgPathGetValuesDA(path_comps, _values, get_active, true); + // ignore return value + + // all values have the same status + for (size_t i = 0; i < _values.size(); i++) { + _values_status.push_back(st); + } + } else { + // values changed => need to do a diff between active and working + // this follows the original perl logic + vector ovals; // active values + vector nvals; // working values + _cstore->cfgPathGetValuesDA(path_comps, ovals, true); + _cstore->cfgPathGetValuesDA(path_comps, nvals, false); + Cstore::MapT nmap; + for (size_t i = 0; i < nvals.size(); i++) { + nmap[nvals[i]] = true; + } + Cstore::MapT omap; + for (size_t i = 0; i < ovals.size(); i++) { + omap[ovals[i]] = true; + if (nmap.find(ovals[i]) == nmap.end()) { + _values.push_back(ovals[i]); + _values_status.push_back(ST_DELETED); + } + } + for (size_t i = 0; i < nvals.size(); i++) { + _values.push_back(nvals[i]); + if (omap.find(nvals[i]) == omap.end()) { + // new value not in working + _values_status.push_back(ST_ADDED); + } else if (i < ovals.size() && nvals[i] == ovals[i]) { + // value also in working and in the same position + _values_status.push_back(ST_STATIC); + } else { + // value position has changed + _values_status.push_back(ST_CHANGED); + } + } + } + } else { + // single-value node + // if the node has been deleted, get the value from active too. + bool get_active = (_active_only || _status == ST_DELETED); + _cstore->cfgPathGetValueDA(path_comps, _value, get_active, true); + // ignore return value + } + return; + } + + // intermediate (typeless) or tag node + Cstore::MapT cmap; + vector cnodes; + if (_active_only) { + // only show active config + _cstore->cfgPathGetChildNodesDA(path_comps, cnodes, true, true); + for (size_t i = 0; i < cnodes.size(); i++) { + cmap[cnodes[i]] = Cstore::C_NODE_STATUS_STATIC; + } + } else { + // show config session + _cstore->cfgPathGetChildNodesStatusDA(path_comps, cmap, cnodes); + } + if (cmap.empty()) { + // empty subtree. finished. + _is_empty = true; + return; + } + + for (size_t i = 0; i < cnodes.size(); i++) { + path_comps.push_back(cnodes[i]); + CstoreCfgNode *cn = new CstoreCfgNode(cstore, path_comps, _active_only, + cnodes[i], _st_map[cmap[cnodes[i]]]); + if (_is_tag && !_is_value) { + // tag node + cn->setTagName(_name); + _tag_values.push_back(cn); + } else { + // intermediate node or tag value + _child_nodes.push_back(cn); + } + path_comps.pop_back(); + } +} + + +////// public functions +void +CstoreCfgNode::show_as_root(bool show_default, bool hide_secret) +{ + if (_is_invalid) { + printf("Specified configuration path is not valid\n"); + return; + } + if (_is_empty) { + printf("Configuration under specified path is empty\n"); + return; + } + + show(-1, show_default, hide_secret); +} + + +////// private functions +void +CstoreCfgNode::show(int level, bool show_def, bool hide_secret) +{ + if (_is_leaf) { + // leaf node + if (_is_multi) { + // multi-value node + print_comment(level); + for (size_t i = 0; i < _values.size() + && i < _values_status.size(); i++) { + print_indent(level, _values_status[i]); + printf("%s ", _name.c_str()); + print_value_str(_values[i].c_str(), hide_secret); + printf("\n"); + } + } else { + // handle "default" for single-value node + if (show_def || !_is_default) { + // single-value node + print_comment(level); + print_indent(level); + printf("%s ", _name.c_str()); + print_value_str(_value.c_str(), hide_secret); + printf("\n"); + } + } + return; + } + + if (_is_tag) { + // tag node + for (size_t i = 0; i < _tag_values.size(); i++) { + /* note: if the root is a tag node (level == -1), then need to make + * level 0 when calling tag values' show(). + */ + _tag_values[i]->show((level >= 0 ? level : 0), show_def, hide_secret); + } + } else { + // intermediate node or tag value + bool print_this = (level >= 0 && _name != ""); + if (print_this) { + print_comment(level); + print_indent(level); + if (_is_value && _tag_name != "") { + // at tag value and there is a tag node parent => print node name + printf("%s ", _tag_name.c_str()); + } + printf("%s {\n", _name.c_str()); + } + for (size_t i = 0; i < _child_nodes.size(); i++) { + _child_nodes[i]->show(level + 1, show_def, hide_secret); + } + if (print_this) { + print_indent(level); + printf("}\n"); + } + } +} + +void +CstoreCfgNode::print_indent(int level, NodeStatusT st, bool force_changed) +{ + if (st == ST_CHANGED && !force_changed && !_is_leaf) { + /* normally only output "changed" status for leaf nodes. in special cases + * (currently only for "comment"), "changed" is also needed, so only + * convert to "static" if not forcing "changed". + */ + st = ST_STATIC; + } + print_prefix(st); + for (int i = 0; i < level; i++) { + printf(" "); + }; +} + +void +CstoreCfgNode::print_prefix(NodeStatusT st) +{ + printf("%s ", _pfx_deact); + if (!_active_only) { + /* this follows the original implementation: when outputting the acitve + * configuration only (e.g., in op mode), only generate two columns. + */ + printf("%s", get_prefix_diff(st)); + } +} + +const char * +CstoreCfgNode::get_prefix_diff(NodeStatusT st) +{ + if (st == ST_DELETED) { + return PFX_DIFF_DEL.c_str(); + } else if (st == ST_ADDED) { + return PFX_DIFF_ADD.c_str(); + } else if (st == ST_CHANGED) { + return PFX_DIFF_UPD.c_str(); + } + return PFX_DIFF_NONE.c_str(); +} + +void +CstoreCfgNode::print_comment(int level) +{ + if (_comment == "") { + // no comment + return; + } + // forcing "changed" since it's needed for comment + print_indent(level, _comment_status, true); + printf("/* %s */\n", _comment.c_str()); +} + +/* prints a value string, double-quoting it if necessary. + * this follows the original perl logic, i.e., double quoting is needed if: + * (/^$/ or /[\s\*}{;]/) + * + * also follow the original "secret hiding" logic: + * /^.*(passphrase|password|pre-shared-secret|key)$/ + */ +void +CstoreCfgNode::print_value_str(const char *vstr, bool hide_secret) +{ + // handle secret hiding first + if (hide_secret) { + static const char *sname[] = { "passphrase", "password", + "pre-shared-secret", "key", NULL }; + static size_t slen[] = { 10, 8, 17, 3, 0 }; + size_t nlen = _name.length(); + for (size_t i = 0; sname[i]; i++) { + if (nlen < slen[i]) { + // can't match + continue; + } + if (_name.find(sname[i], nlen - slen[i]) != _name.npos) { + // found secret + printf("****************"); + return; + } + } + } + + const char *quote = ""; + size_t vlen = strlen(vstr); + if (*vstr == 0 || strcspn(vstr, "*}{;\011\012\013\014\015 ") < vlen) { + quote = "\""; + } + printf("%s%s%s", quote, vstr, quote); +} + diff --git a/src/cstore/cstore-node.hpp b/src/cstore/cstore-node.hpp new file mode 100644 index 0000000..a7aa865 --- /dev/null +++ b/src/cstore/cstore-node.hpp @@ -0,0 +1,104 @@ +/* + * Copyright (C) 2010 Vyatta, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef _CSTORE_NODE_H_ +#define _CSTORE_NODE_H_ +#include +#include +#include + +#include + +using namespace std; + +typedef enum { + ST_DELETED, + ST_ADDED, + ST_CHANGED, + ST_STATIC +} NodeStatusT; + +class CstoreCfgNode { +public: + CstoreCfgNode(Cstore& cstore, vector& path_comps, + bool active_only = false, const string& name = "", + NodeStatusT status = ST_STATIC); + ~CstoreCfgNode() {}; + + // diff prefixes + static const string PFX_DIFF_ADD; + static const string PFX_DIFF_DEL; + static const string PFX_DIFF_UPD; + static const string PFX_DIFF_NONE; + + // deactivate prefixes + static const string PFX_DEACT_D; + static const string PFX_DEACT_DP; + static const string PFX_DEACT_AP; + static const string PFX_DEACT_NONE; + + void setTagName(const string& tname) { _tag_name = tname; }; + bool isTag() { return _is_tag; }; + void show_as_root(bool show_default = false, bool hide_secret = false); + +private: + static bool _init; + static Cstore::MapT _st_map; + static void init() { + if (_init) { + return; + } + _st_map[Cstore::C_NODE_STATUS_DELETED] = ST_DELETED; + _st_map[Cstore::C_NODE_STATUS_ADDED] = ST_ADDED; + _st_map[Cstore::C_NODE_STATUS_CHANGED] = ST_CHANGED; + _st_map[Cstore::C_NODE_STATUS_STATIC] = ST_STATIC; + _init = true; + }; + + Cstore *_cstore; + string _name; + NodeStatusT _status; + const char *_pfx_deact; + bool _is_tag; + bool _is_leaf; + bool _is_multi; + bool _is_value; + bool _is_default; + bool _is_invalid; + bool _is_empty; + bool _active_only; + string _tag_name; + string _value; + vector _values; + vector _values_status; + string _comment; + NodeStatusT _comment_status; + vector _tag_values; + vector _child_nodes; + + void show(int level, bool show_def, bool hide_secret); + void print_indent(int level) { + print_indent(level, _status); + }; + void print_indent(int level, NodeStatusT st, bool force_changed = false); + void print_prefix(NodeStatusT st); + const char *get_prefix_diff(NodeStatusT st); + void print_comment(int level); + void print_value_str(const char *vstr, bool hide_secret); +}; + +#endif /* _CSTORE_NODE_H_ */ + -- cgit v1.2.3 From 80459cf8380301800e4d024a5eb2bc4a63cdd919 Mon Sep 17 00:00:00 2001 From: An-Cheng Huang Date: Thu, 26 Aug 2010 14:49:04 -0700 Subject: mark changed ancestors up to the root --- src/cstore/unionfs/cstore-unionfs.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/cstore/unionfs/cstore-unionfs.cpp b/src/cstore/unionfs/cstore-unionfs.cpp index 849d659..eebabfe 100644 --- a/src/cstore/unionfs/cstore-unionfs.cpp +++ b/src/cstore/unionfs/cstore-unionfs.cpp @@ -755,9 +755,15 @@ bool UnionfsCstore::mark_changed_with_ancestors() { b_fs::path opath = mutable_cfg_path; // use a copy - while (opath.has_parent_path()) { - b_fs::path marker = (work_root / opath); - pop_path(opath); + bool done = false; + while (!done) { + b_fs::path marker = work_root; + if (opath.has_parent_path()) { + marker /= opath; + pop_path(opath); + } else { + done = true; + } if (!b_fs_exists(marker) || !b_fs_is_directory(marker)) { // don't do anything if the node is not there continue; -- cgit v1.2.3 From 17f19681974fb4399d36ac43006efac9828dc5b9 Mon Sep 17 00:00:00 2001 From: An-Cheng Huang Date: Thu, 26 Aug 2010 16:46:07 -0700 Subject: don't show extra empty level for typeless leaf nodes. --- src/cstore/cstore-node.cpp | 23 ++++++++++++++++------- src/cstore/cstore-node.hpp | 1 + 2 files changed, 17 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/cstore/cstore-node.cpp b/src/cstore/cstore-node.cpp index 85d2fda..a67f291 100644 --- a/src/cstore/cstore-node.cpp +++ b/src/cstore/cstore-node.cpp @@ -47,6 +47,7 @@ CstoreCfgNode::CstoreCfgNode(Cstore& cstore, vector& path_comps, _pfx_deact(PFX_DEACT_NONE.c_str()), _is_tag(false), _is_leaf(false), _is_multi(false), _is_value(false), _is_default(false), _is_invalid(false), _is_empty(false), + _is_leaf_typeless(false), _active_only(false), _comment(""), _comment_status(ST_STATIC) { init(); @@ -208,6 +209,12 @@ CstoreCfgNode::CstoreCfgNode(Cstore& cstore, vector& path_comps, } if (cmap.empty()) { // empty subtree. finished. + vector tcnodes; + _cstore->tmplGetChildNodes(path_comps, tcnodes); + if (tcnodes.size() == 0) { + // typeless leaf node + _is_leaf_typeless = true; + } _is_empty = true; return; } @@ -294,14 +301,16 @@ CstoreCfgNode::show(int level, bool show_def, bool hide_secret) // at tag value and there is a tag node parent => print node name printf("%s ", _tag_name.c_str()); } - printf("%s {\n", _name.c_str()); - } - for (size_t i = 0; i < _child_nodes.size(); i++) { - _child_nodes[i]->show(level + 1, show_def, hide_secret); + printf("%s%s\n", _name.c_str(), (_is_leaf_typeless ? "" : " {")); } - if (print_this) { - print_indent(level); - printf("}\n"); + if (!_is_leaf_typeless) { + for (size_t i = 0; i < _child_nodes.size(); i++) { + _child_nodes[i]->show(level + 1, show_def, hide_secret); + } + if (print_this) { + print_indent(level); + printf("}\n"); + } } } } diff --git a/src/cstore/cstore-node.hpp b/src/cstore/cstore-node.hpp index a7aa865..f58da4e 100644 --- a/src/cstore/cstore-node.hpp +++ b/src/cstore/cstore-node.hpp @@ -79,6 +79,7 @@ private: bool _is_default; bool _is_invalid; bool _is_empty; + bool _is_leaf_typeless; bool _active_only; string _tag_name; string _value; -- cgit v1.2.3 From aaee996343034a3ab286ec1c2807e5548f2039f4 Mon Sep 17 00:00:00 2001 From: An-Cheng Huang Date: Thu, 26 Aug 2010 17:07:28 -0700 Subject: mark the root as "changed" after "comment" operation. --- src/cstore/cstore.cpp | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src') diff --git a/src/cstore/cstore.cpp b/src/cstore/cstore.cpp index 30a6d6a..8afa3ee 100644 --- a/src/cstore/cstore.cpp +++ b/src/cstore/cstore.cpp @@ -970,6 +970,10 @@ Cstore::commentCfgPath(const vector& args, const vtw_def& def) } } RESTORE_PATHS; + if (ret) { + // mark the root as changed for "comment" + ret = mark_changed_with_ancestors(); + } return ret; } -- cgit v1.2.3 From efe1f2b5aea0065f94394b916f1fa8b447229d5a Mon Sep 17 00:00:00 2001 From: An-Cheng Huang Date: Thu, 26 Aug 2010 19:04:24 -0700 Subject: fix for bug 5960 * change "commit" to use syscalls (instead of system("...") calls) for mount/umount. * this is a temporary fix, and such low-level details need to be moved into the backend library. --- debian/vyatta-cfg.postinst.in | 2 +- src/common/unionfs.c | 65 +++++++++++++++++++------------------------ 2 files changed, 30 insertions(+), 37 deletions(-) (limited to 'src') diff --git a/debian/vyatta-cfg.postinst.in b/debian/vyatta-cfg.postinst.in index c007538..bb747d4 100644 --- a/debian/vyatta-cfg.postinst.in +++ b/debian/vyatta-cfg.postinst.in @@ -28,7 +28,7 @@ if [ "$sysconfdir" != "/etc" ]; then fi # capability stuff -for bin in my_cli_bin my_cli_shell_api; do +for bin in my_cli_bin my_cli_shell_api my_commit; do touch -ac $sbindir/$bin setcap cap_sys_admin=pe $sbindir/$bin done diff --git a/src/common/unionfs.c b/src/common/unionfs.c index da4cd7e..9fe2497 100644 --- a/src/common/unionfs.c +++ b/src/common/unionfs.c @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -23,6 +24,10 @@ extern vtw_path t_path; * behavior so no status is returned (since system() return code was not * checked). this should probably be changed so each invocation is checked * for error. + * + * XXX these (and many other things here) will need to be moved into the + * library to consolidate the usage of all low-level implementation + * details. */ static inline void sys_mkdir_p(const char *path) @@ -54,6 +59,25 @@ sys_cp(const char *src_file, const char *dst_file) } } +static inline void +sys_umount_session() +{ + if (umount(get_mdirp()) != 0) { + perror("umount"); + } +} + +static inline void +sys_mount_session() +{ + char mopts[MAX_LENGTH_DIR_PATH * 2]; + snprintf(mopts, MAX_LENGTH_DIR_PATH * 2, "dirs=%s=rw:%s=ro", + get_cdirp(), get_adirp()); + if (mount("unionfs", get_mdirp(), "unionfs", 0, mopts) != 0) { + perror("mount"); + } +} + void set_path(char *path, boolean config); @@ -743,9 +767,6 @@ common_commit_copy_to_live_config(GNode *node, boolean suppress_piecewise_copy, static const char format1point1[]="mv -f %s/* -t %s"; /*mdirp, tmpp*/ static const char format1point5[]="rm -fr '%s'/*"; /*tmpp*/ - static const char format2[]="sudo umount %s"; //mdirp - static const char format8[]="sudo mount -t unionfs -o dirs=%s=rw:%s=ro unionfs %s"; //cdirp, adirp, mdirp - set_echo(TRUE); char mbuf[MAX_LENGTH_DIR_PATH]; @@ -802,15 +823,9 @@ common_commit_copy_to_live_config(GNode *node, boolean suppress_piecewise_copy, system(command); } - //unmount temp (i.e. rm merge) - sprintf(command, format2, mbuf_root); - if (g_debug) { - printf("%s\n",command); - syslog(LOG_DEBUG,"%s\n",command); - fflush(NULL); - } + // unmount the session dir if (test_mode == FALSE) { - system(command); + sys_umount_session(); } if (suppress_piecewise_copy) { @@ -837,14 +852,8 @@ common_commit_copy_to_live_config(GNode *node, boolean suppress_piecewise_copy, piecewise_copy(node, test_mode); } - sprintf(command, format8, cbuf_root,abuf_root,mbuf_root); - if (g_debug) { - printf("%s\n",command); - syslog(LOG_DEBUG,"%s\n",command); - fflush(NULL); - } if (test_mode == FALSE) { - system(command); + sys_mount_session(); } fflush(NULL); @@ -876,10 +885,7 @@ common_commit_clean_temp_config(GNode *root_node, boolean test_mode) char *command; command = malloc(MAX_LENGTH_DIR_PATH); /* XXX must ... remove ... this ... */ - static const char format2[]="sudo umount %s"; //mdirp static const char format5[]="rm -fr '%s'/{.*,*} >&/dev/null ; /bin/true"; /*cdirp*/ - static const char format7[]="sudo mount -t unionfs -o dirs=%s=rw:%s=ro unionfs %s"; //cdirp, adirp, mdirp - char tbuf[MAX_LENGTH_DIR_PATH]; sprintf(tbuf,"%s",get_tmpp()); @@ -892,15 +898,8 @@ common_commit_clean_temp_config(GNode *root_node, boolean test_mode) set_echo(TRUE); - sprintf(command, format2, mbuf); - if (g_debug) { - printf("%s\n",command); - syslog(LOG_DEBUG,"%s\n",command); - fflush(NULL); - } - if (test_mode == FALSE) { - system(command); + sys_umount_session(); } /* @@ -939,14 +938,8 @@ common_commit_clean_temp_config(GNode *root_node, boolean test_mode) system(command); } - sprintf(command, format7, cbuf,abuf,mbuf); - if (g_debug) { - printf("%s\n",command); - syslog(LOG_DEBUG,"%s\n",command); - fflush(NULL); - } if (test_mode == FALSE) { - system(command); + sys_mount_session(); } /* notify other users in config mode */ -- cgit v1.2.3 From fd6bde80731e56d6bd35586ddaaf06536bd9df8f Mon Sep 17 00:00:00 2001 From: Michael Larson Date: Fri, 23 Jul 2010 13:17:24 -0700 Subject: allow commit hook commands to write to stdout. (cherry picked from commit a213dd1c48581294fc967e7198d78ca270c6ae46) --- src/commit2.c | 1 + 1 file changed, 1 insertion(+) (limited to 'src') diff --git a/src/commit2.c b/src/commit2.c index d45b7c0..884d2fe 100644 --- a/src/commit2.c +++ b/src/commit2.c @@ -369,6 +369,7 @@ main(int argc, char** argv) setenv(ENV_COMMIT_STATUS,"FAILURE",1); } struct dirent *dirp = NULL; + restore_output(); while ((dirp = readdir(dp)) != NULL) { if (strcmp(dirp->d_name, ".") != 0 && strcmp(dirp->d_name, "..") != 0) { -- cgit v1.2.3 From cfa746fb0d41a70843a851561136dc003745f14f Mon Sep 17 00:00:00 2001 From: Michael Larson Date: Tue, 3 Aug 2010 13:48:11 -0700 Subject: fix for bug 5982. upped max_length_buffer length from 1024 to 4096. Should leave open and reassign to mendocino so that a proper string length check can be added to the commit. (cherry picked from commit df3c80575ea539e560afc3357eb322e3e9df64bf) --- src/common/unionfs.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/common/unionfs.h b/src/common/unionfs.h index 7881594..362972e 100644 --- a/src/common/unionfs.h +++ b/src/common/unionfs.h @@ -30,8 +30,8 @@ #define WHITEOUT_DISABLE_FILE ".wh..disable" #define DELETED_NODE ".wh." -#define MAX_LENGTH_DIR_PATH 1024 -#define MAX_LENGTH_HELP_STR 1024 +#define MAX_LENGTH_DIR_PATH 4096 +#define MAX_LENGTH_HELP_STR 4096 boolean value_exists(char *path); -- cgit v1.2.3