From 64ad04d23107e97832ec81c0cc32a95214196d6d Mon Sep 17 00:00:00 2001
From: An-Cheng Huang <ancheng@vyatta.com>
Date: Fri, 20 Aug 2010 13:42:12 -0700
Subject: remove workaround for "changed" status handling
---
src/cstore/cstore.cpp | 17 +++++++++++++
src/cstore/unionfs/cstore-unionfs.cpp | 47 ++++++++---------------------------
2 files changed, 27 insertions(+), 37 deletions(-)
diff --git a/src/cstore/cstore.cpp b/src/cstore/cstore.cpp
index 0634988..f146cb0 100644
--- a/src/cstore/cstore.cpp
+++ b/src/cstore/cstore.cpp
@@ -1763,6 +1763,23 @@ Cstore::unmarkCfgPathDeactivated(const vector<string>& path_comps)
* note: if a node is changed, all of its ancestors are also considered
* changed.
* return true if successful. otherwise return false.
+ *
+ * the original backend implementation only uses the "changed" marker at
+ * "root" to indicate whether the whole config has changed. for the rest
+ * of the config hierarchy, the original implementation treated all nodes
+ * that are present in the unionfs "changes only" directory as "changed".
+ *
+ * this worked until the introduction of "deactivate". since deactivated
+ * nodes are also present in the "changes only" directory, the backend
+ * treat them as "changed". on the other hand, deleted nodes don't appear
+ * 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.
*/
bool
Cstore::markCfgPathChanged(const vector<string>& path_comps)
diff --git a/src/cstore/unionfs/cstore-unionfs.cpp b/src/cstore/unionfs/cstore-unionfs.cpp
index 7f14483..46611ed 100644
--- a/src/cstore/unionfs/cstore-unionfs.cpp
+++ b/src/cstore/unionfs/cstore-unionfs.cpp
@@ -780,33 +780,16 @@ UnionfsCstore::unmark_changed_with_descendants()
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;
- }
+ b_fs::path marker = get_work_path() / C_MARKER_CHANGED;
+ if (b_fs_exists(marker)) {
+ // already marked. treat as success.
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).
- */
+ 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;
}
@@ -883,18 +866,8 @@ UnionfsCstore::get_comment(string& comment, bool active_cfg)
bool
UnionfsCstore::marked_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.
--
cgit v1.2.3