handle "changed" status properly

* original backend implementation uses unionfs-specific "changes only" dir to determine "changed" status. this breaks when it involves deactivated nodes.
* new library design uses explicit per-node "changed" marker. however, since previously "commit" only handles a root "changed" marker, the new library could not implement this scheme and used a workaround instead.
* now add API functions for "commit" to properly clean up "changed" markers.
* modify "commit" to use these API functions and remove the workaround from the new library.

7 files changed, 89 insertions, 20 deletions

diff --git a/src/common/unionfs.c b/src/common/unionfs.c
index e7157c0..85ea9f1 100644
--- a/src/common/unionfs.c
+++ b/src/common/unionfs.c
@@ -10,6 +10,8 @@
 #include "common/defs.h"
 #include "common/unionfs.h"
 
+#include <cstore/cstore-c.h>
+
 boolean g_debug;
 
 extern vtw_path m_path;
@@ -715,7 +717,26 @@ common_commit_copy_to_live_config(GNode *node, boolean suppress_piecewise_copy,
     printf("common_commit_copy_to_live_config(): %s\n",path);
     syslog(LOG_DEBUG,"common_commit_copy_to_live_config(): %s\n",path);
   }
+
+  /* this function is called for each "subtree" that has been successfully
+   * committed. before doing anything else, remove the "changed" status
+   * from any changed nodes in this subtree first (since this subtree is
+   * going into the active config).
+   */
+  {
+    void *cs = cstore_init();
+    int ncomps;
+    char **pcomps = cstore_path_string_to_path_comps(path, &ncomps);
+    /* 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_free_path_comps(pcomps, ncomps);
+    cstore_free(cs);
+  }
+
   char *command = malloc(MAX_LENGTH_DIR_PATH);
+  /* XXX must ... remove ... this ... */
   static const char format0[]="mkdir -p %s ; /bin/true";
   static const char formatpoint5[]="rm -fr '%s'"; /*tmpp*/
   static const char format1[]="cp -r -f %s/* %s"; /*mdirp, tmpp*/
@@ -854,8 +875,8 @@ 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 format3[]="rm -f %s/" MOD_NAME " >&/dev/null ; /bin/true"; /*tmpp*/
   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
 
@@ -902,26 +923,11 @@ common_commit_clean_temp_config(GNode *root_node, boolean test_mode)
 		    (GNodeTraverseFunc)delete_wh_func,
 		    (gpointer)&sd);
   }
-  
-  sprintf(command, format3, tbuf);
-  if (g_debug) {
-    printf("%s\n",command);
-    syslog(LOG_DEBUG,"%s\n",command);
-    fflush(NULL);
-  }
-  if (test_mode == FALSE) {
-    system(command);
-  }
 
-  sprintf(command, format3, cbuf);
-  if (g_debug) {
-    printf("%s\n",command);
-    syslog(LOG_DEBUG,"%s\n",command);
-    fflush(NULL);
-  }
-  if (test_mode == FALSE) {
-    system(command);
-  }
+  /* originally the root "changed" marker was being removed here. this is now
+   * handled in common_commit_copy_to_live_config() since we need to do it
+   * subtree-by-subtree (and also remove the markers from any descendants).
+   */
 
   sprintf(command, format5, cbuf);
   if (g_debug) {
diff --git a/src/cstore/cstore-c.cpp b/src/cstore/cstore-c.cpp
index fd818a5..c835efe 100644
--- a/src/cstore/cstore-c.cpp
+++ b/src/cstore/cstore-c.cpp
@@ -143,6 +143,19 @@ cstore_cfg_path_get_effective_value(void *handle, const char *path_comps[],
   return NULL;
 }
 
+int
+cstore_unmark_cfg_path_changed(void *handle, const char *path_comps[],
+                               int num_comps)
+{
+  if (handle) {
+    vector<string> vs;
+    _get_str_vec(vs, path_comps, num_comps);
+    Cstore *cs = (Cstore *) handle;
+    return (cs->unmarkCfgPathChanged(vs) ? 1 : 0);
+  }
+  return 0;
+}
+
 char **
 cstore_path_string_to_path_comps(const char *path_str, int *num_comps)
 {
diff --git a/src/cstore/cstore-c.h b/src/cstore/cstore-c.h
index a2ad844..4f33672 100644
--- a/src/cstore/cstore-c.h
+++ b/src/cstore/cstore-c.h
@@ -38,6 +38,9 @@ char *cstore_cfg_path_get_effective_value(void *handle,
                                           const char *path_comps[],
                                           int num_comps);
 
+int cstore_unmark_cfg_path_changed(void *handle, const char *path_comps[],
+                                   int num_comps);
+
 /* the following are internal APIs for the library. they can only be used
  * during cstore operations since they operate on "current" paths constructed
  * by the operations.
diff --git a/src/cstore/cstore.cpp b/src/cstore/cstore.cpp
index ebf9585..0634988 100644
--- a/src/cstore/cstore.cpp
+++ b/src/cstore/cstore.cpp
@@ -1791,6 +1791,22 @@ Cstore::markCfgPathChanged(const vector<string>& path_comps)
   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.
+ * note: unmarking a node means all of its descendants are also unmarked,
+ *       i.e., they become "unchanged".
+ * return true if successful. otherwise return false.
+ */
+bool
+Cstore::unmarkCfgPathChanged(const vector<string>& path_comps)
+{
+  SAVE_PATHS;
+  append_cfg_path(path_comps);
+  bool ret = unmark_changed_with_descendants();
+  RESTORE_PATHS;
+  return ret;
+}
+
 
 ////// protected functions
 void
diff --git a/src/cstore/cstore.hpp b/src/cstore/cstore.hpp
index 37a3e06..4b07fab 100644
--- a/src/cstore/cstore.hpp
+++ b/src/cstore/cstore.hpp
@@ -144,6 +144,7 @@ public:
   virtual bool inSession() = 0;
   // common
   bool markCfgPathChanged(const vector<string>& path_comps);
+  bool unmarkCfgPathChanged(const vector<string>& path_comps);
   // XXX load
   //bool unmarkCfgPathDeactivatedDescendants(const vector<string>& path_comps);
 
@@ -306,6 +307,7 @@ private:
   virtual bool mark_deactivated() = 0;
   virtual bool unmark_deactivated() = 0;
   virtual bool unmark_deactivated_descendants() = 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;
diff --git a/src/cstore/unionfs/cstore-unionfs.cpp b/src/cstore/unionfs/cstore-unionfs.cpp
index 860d553..7f14483 100644
--- a/src/cstore/unionfs/cstore-unionfs.cpp
+++ b/src/cstore/unionfs/cstore-unionfs.cpp
@@ -749,6 +749,34 @@ UnionfsCstore::unmark_deactivated_descendants()
   return ret;
 }
 
+/* remove all "changed" markers under the current work path. this is used,
+ * e.g., at the end of "commit" to reset a subtree.
+ */
+bool
+UnionfsCstore::unmark_changed_with_descendants()
+{
+  try {
+    vector<b_fs::path> markers;
+    b_fs::recursive_directory_iterator di(get_work_path());
+    for (; di != b_fs::recursive_directory_iterator(); ++di) {
+      if (!b_fs_is_regular(di->path())
+          || di->path().filename() != C_MARKER_CHANGED) {
+        // not marker
+        continue;
+      }
+      markers.push_back(di->path());
+    }
+    for (size_t i = 0; i < markers.size(); i++) {
+      b_fs::remove(markers[i]);
+    }
+  } catch (...) {
+    output_internal("failed to unmark changed with descendants [%s]\n",
+                    get_work_path().file_string().c_str());
+    return false;
+  }
+  return true;
+}
+
 bool
 UnionfsCstore::mark_changed()
 {
diff --git a/src/cstore/unionfs/cstore-unionfs.hpp b/src/cstore/unionfs/cstore-unionfs.hpp
index 9e49064..bff4844 100644
--- a/src/cstore/unionfs/cstore-unionfs.hpp
+++ b/src/cstore/unionfs/cstore-unionfs.hpp
@@ -158,6 +158,7 @@ private:
   bool mark_deactivated();
   bool unmark_deactivated();
   bool unmark_deactivated_descendants();
+  bool unmark_changed_with_descendants();
   bool mark_changed();
   bool remove_comment();
   bool set_comment(const string& comment);