commit 4442cca0155fc1b84f676d3c32d3fdf5d6fa289a
Author: Gao,Yan <ygao@suse.com>
Date:   Sat May 16 14:29:07 2015 +0200

    Refactor: cib: Calculate and add digest for a patchset after accepting changes for the target xml
    
    This would be very important in case some day we want to switch back to
    libxml2 xmlNodeDump() function to dump xml string

diff --git a/include/crm/common/xml.h b/include/crm/common/xml.h
index 968c922..e128275 100644
--- a/include/crm/common/xml.h
+++ b/include/crm/common/xml.h
@@ -267,9 +267,11 @@ void xml_log_patchset(uint8_t level, const char *function, xmlNode *xml);
 bool xml_patch_versions(xmlNode *patchset, int add[3], int del[3]);
 
 xmlNode *xml_create_patchset(
-    int format, xmlNode *source, xmlNode *target, bool *config, bool manage_version, bool with_digest);
+    int format, xmlNode *source, xmlNode *target, bool *config, bool manage_version);
 int xml_apply_patchset(xmlNode *xml, xmlNode *patchset, bool check_version);
 
+void patchset_process_digest(xmlNode *patch, xmlNode *source, xmlNode *target, bool with_digest);
+
 void save_xml_to_file(xmlNode * xml, const char *desc, const char *filename);
 char *xml_get_path(xmlNode *xml);
 
diff --git a/lib/cib/cib_utils.c b/lib/cib/cib_utils.c
index ea37c4b..28b8e81 100644
--- a/lib/cib/cib_utils.c
+++ b/lib/cib/cib_utils.c
@@ -302,6 +302,7 @@ cib_perform_op(const char *op, int call_options, cib_op_t * fn, gboolean is_quer
     const char *new_version = NULL;
     static struct qb_log_callsite *diff_cs = NULL;
     const char *user = crm_element_value(req, F_CIB_USER);
+    bool with_digest = FALSE;
 
     crm_trace("Begin %s%s op", is_query ? "read-only " : "", op);
 
@@ -449,26 +450,30 @@ cib_perform_op(const char *op, int call_options, cib_op_t * fn, gboolean is_quer
          * The v1 format would barf on this, but we know the v2 patch
          * format only needs it for the top-level version fields
          */
-        local_diff = xml_create_patchset(2, current_cib, scratch, (bool*)config_changed, manage_counters, FALSE);
+        local_diff = xml_create_patchset(2, current_cib, scratch, (bool*)config_changed, manage_counters);
 
     } else {
         static time_t expires = 0;
         time_t tm_now = time(NULL);
-        bool with_digest = FALSE;
 
         if (expires < tm_now) {
             expires = tm_now + 60;  /* Validate clients are correctly applying v2-style diffs at most once a minute */
             with_digest = TRUE;
         }
 
-        local_diff = xml_create_patchset(0, current_cib, scratch, (bool*)config_changed, manage_counters, with_digest);
+        local_diff = xml_create_patchset(0, current_cib, scratch, (bool*)config_changed, manage_counters);
     }
 
+    xml_log_changes(LOG_TRACE, __FUNCTION__, scratch);
+    xml_accept_changes(scratch);
+
     if (diff_cs == NULL) {
         diff_cs = qb_log_callsite_get(__PRETTY_FUNCTION__, __FILE__, "diff-validation", LOG_DEBUG, __LINE__, crm_trace_nonlog);
     }
 
     if(local_diff) {
+        patchset_process_digest(local_diff, current_cib, scratch, with_digest);
+
         xml_log_patchset(LOG_INFO, __FUNCTION__, local_diff);
         crm_log_xml_trace(local_diff, "raw patch");
     }
@@ -494,9 +499,6 @@ cib_perform_op(const char *op, int call_options, cib_op_t * fn, gboolean is_quer
         free_xml(c);
     }
 
-    xml_log_changes(LOG_TRACE, __FUNCTION__, scratch);
-    xml_accept_changes(scratch);
-
     if (safe_str_eq(section, XML_CIB_TAG_STATUS)) {
         /* Throttle the amount of costly validation we perform due to status updates
          * a) we don't really care whats in the status section
diff --git a/lib/common/xml.c b/lib/common/xml.c
index 716f053..1a4d3f8 100644
--- a/lib/common/xml.c
+++ b/lib/common/xml.c
@@ -1439,9 +1439,9 @@ xml_repair_v1_diff(xmlNode * last, xmlNode * next, xmlNode * local_diff, gboolea
 }
 
 static xmlNode *
-xml_create_patchset_v1(xmlNode *source, xmlNode *target, bool config, bool with_digest)
+xml_create_patchset_v1(xmlNode *source, xmlNode *target, bool config, bool suppress)
 {
-    xmlNode *patchset = diff_xml_object(source, target, !with_digest);
+    xmlNode *patchset = diff_xml_object(source, target, suppress);
 
     if(patchset) {
         CRM_LOG_ASSERT(xml_document_dirty(target));
@@ -1527,7 +1527,7 @@ static gboolean patch_legacy_mode(void)
 }
 
 xmlNode *
-xml_create_patchset(int format, xmlNode *source, xmlNode *target, bool *config_changed, bool manage_version, bool with_digest)
+xml_create_patchset(int format, xmlNode *source, xmlNode *target, bool *config_changed, bool manage_version)
 {
     int counter = 0;
     bool config = FALSE;
@@ -1573,8 +1573,7 @@ xml_create_patchset(int format, xmlNode *source, xmlNode *target, bool *config_c
 
     switch(format) {
         case 1:
-            with_digest = TRUE;
-            patch = xml_create_patchset_v1(source, target, config, with_digest);
+            patch = xml_create_patchset_v1(source, target, config, FALSE);
             break;
         case 2:
             patch = xml_create_patchset_v2(source, target);
@@ -1584,13 +1583,36 @@ xml_create_patchset(int format, xmlNode *source, xmlNode *target, bool *config_c
             return NULL;
     }
 
-    if(patch && with_digest) {
-        char *digest = calculate_xml_versioned_digest(target, FALSE, TRUE, version);
+    return patch;
+}
+
+void
+patchset_process_digest(xmlNode *patch, xmlNode *source, xmlNode *target, bool with_digest)
+{
+    int format = 1;
+    const char *version = NULL;
+    char *digest = NULL;
 
-        crm_xml_add(patch, XML_ATTR_DIGEST, digest);
-        free(digest);
+    if (patch == NULL || source == NULL || target == NULL) {
+        return;
     }
-    return patch;
+
+    /* NOTE: We should always call xml_accept_changes() before calculating digest. */
+    /* Otherwise, with an on-tracking dirty target, we could get a wrong digest. */
+    CRM_LOG_ASSERT(xml_document_dirty(target) == FALSE);
+
+    crm_element_value_int(patch, "format", &format);
+    if (format > 1 && with_digest == FALSE) {
+        return;
+    }
+
+    version = crm_element_value(source, XML_ATTR_CRM_VERSION);
+    digest = calculate_xml_versioned_digest(target, FALSE, TRUE, version);
+
+    crm_xml_add(patch, XML_ATTR_DIGEST, digest);
+    free(digest);
+
+    return;
 }
 
 static void
diff --git a/tools/cib_shadow.c b/tools/cib_shadow.c
index 56912c3..d68c1be 100644
--- a/tools/cib_shadow.c
+++ b/tools/cib_shadow.c
@@ -433,7 +433,7 @@ main(int argc, char **argv)
             goto done;
         }
 
-        diff = xml_create_patchset(0, old_config, new_config, NULL, FALSE, FALSE);
+        diff = xml_create_patchset(0, old_config, new_config, NULL, FALSE);
         if (diff != NULL) {
             xml_log_patchset(0, "  ", diff);
             rc = 1;
diff --git a/tools/xml_diff.c b/tools/xml_diff.c
index 4226c8c..cd3cb29 100644
--- a/tools/xml_diff.c
+++ b/tools/xml_diff.c
@@ -203,7 +203,14 @@ main(int argc, char **argv)
         xml_calculate_changes(object_1, object_2);
         crm_log_xml_debug(object_2, xml_file_2?xml_file_2:"target");
 
-        output = xml_create_patchset(0, object_1, object_2, NULL, FALSE, as_cib);
+        output = xml_create_patchset(0, object_1, object_2, NULL, FALSE);
+
+        xml_log_changes(LOG_INFO, __FUNCTION__, object_2);
+        xml_accept_changes(object_2);
+
+        if (output) {
+            patchset_process_digest(output, object_1, object_2, as_cib);
+        }
 
         if(as_cib && output) {
             int add[] = { 0, 0, 0 };
@@ -267,7 +274,6 @@ main(int argc, char **argv)
                 }
             }
         }
-        xml_log_changes(LOG_INFO, __FUNCTION__, object_2);
         xml_log_patchset(LOG_NOTICE, __FUNCTION__, output);
     }
 
