[Pacemaker] [PATCH] This is an alternate fix for Bug #2528 based on a patch to the

[Andrew Beekhof]

This part:

+    } else if (hash_entry->timer_id != 0) {
+        crm_debug_2("Update already scheduled");
+        return;

is definitely wrong.  Subsequent changes to an attribute value are
intended to reset the timer.

On Thu, Sep 15, 2011 at 7:50 AM, Rainer Weikusat
<rweikusat at mobileactivedefense.com> wrote:
> # HG changeset patch
> # User Rainer Weikusat <rweikusat at mobileactivedefense.com>
> # Date 1316036167 -3600
> # Branch stable-1.0
> # Node ID ea611ef8c1e6a9d294d9d0dff6db2f317232292b
> # Parent  a15ead49e20f047e129882619ed075a65c1ebdfe
> This is an alternate fix for Bug #2528 based on a patch to the
> Debian Squeeze pacemaker package used to provide 'high availability'
> for the product I'm presently paid to work on. As opposed to the
> change documented at
>
> http://hg.clusterlabs.org/pacemaker/stable-1.0/rev/76bd1e3370b8
>
> it doesn't test equality of value and hash_entry->value twice in
> order to determine if an identical updated was already scheduled
> and it avoids 'recalculating' hash_entry->value if its value is
> still current.
>
> diff -r a15ead49e20f -r ea611ef8c1e6 tools/attrd.c
> --- a/tools/attrd.c     Thu Aug 25 16:49:59 2011 +1000
> +++ b/tools/attrd.c     Wed Sep 14 22:36:07 2011 +0100
> @@ -764,49 +764,47 @@
>
>        crm_debug("Supplied: %s, Current: %s, Stored: %s",
>                  value, hash_entry->value, hash_entry->stored_value);
> -
> -       if(safe_str_eq(value, hash_entry->value)
> -          && safe_str_eq(value, hash_entry->stored_value)) {
> -           crm_debug_2("Ignoring non-change");
> -           return;
>
> -       } else if(value) {
> -           int offset = 1;
> -           int int_value = 0;
> -           int value_len = strlen(value);
> -           if(value_len < (plus_plus_len + 2)
> -              || value[plus_plus_len] != '+'
> -              || (value[plus_plus_len+1] != '+' && value[plus_plus_len+1] != '=')) {
> -               goto set_unexpanded;
> -           }
> +       if (!safe_str_eq(value, hash_entry->value)) {
> +               if (value) {
> +                       int offset = 1;
> +                       int int_value = 0;
> +                       int value_len = strlen(value);
> +                       if(value_len < (plus_plus_len + 2)
> +                          || value[plus_plus_len] != '+'
> +                          || (value[plus_plus_len+1] != '+' && value[plus_plus_len+1] != '=')) {
> +                               goto set_unexpanded;
> +                       }
>
> -           int_value = char2score(hash_entry->value);
> -           if(value[plus_plus_len+1] != '+') {
> -               const char *offset_s = value+(plus_plus_len+2);
> -               offset = char2score(offset_s);
> -           }
> -           int_value += offset;
> +                       int_value = char2score(hash_entry->value);
> +                       if(value[plus_plus_len+1] != '+') {
> +                               const char *offset_s = value+(plus_plus_len+2);
> +                               offset = char2score(offset_s);
> +                       }
> +                       int_value += offset;
> +
> +                       if(int_value > INFINITY) {
> +                               int_value = INFINITY;
> +                       }
>
> -           if(int_value > INFINITY) {
> -               int_value = INFINITY;
> -           }
> -
> -           crm_info("Expanded %s=%s to %d", attr, value, int_value);
> -           crm_xml_add_int(msg, F_ATTRD_VALUE, int_value);
> -           value = crm_element_value(msg, F_ATTRD_VALUE);
> -       }
> -
> -  set_unexpanded:
> -       if(safe_str_eq(value, hash_entry->value) && hash_entry->timer_id) {
> -           /* We're already waiting to set this value */
> -           return;
> -       }
> -
> -       crm_free(hash_entry->value);
> -       hash_entry->value = NULL;
> -       if(value != NULL) {
> -               hash_entry->value = crm_strdup(value);
> -               crm_debug("New value of %s is %s", attr, value);
> +                       crm_info("Expanded %s=%s to %d", attr, value, int_value);
> +                       crm_xml_add_int(msg, F_ATTRD_VALUE, int_value);
> +                       value = crm_element_value(msg, F_ATTRD_VALUE);
> +               }
> +
> +       set_unexpanded:
> +               crm_free(hash_entry->value);
> +               hash_entry->value = NULL;
> +               if(value != NULL) {
> +                       hash_entry->value = crm_strdup(value);
> +                       crm_debug("New value of %s is %s", attr, value);
> +               }
> +       } else if (safe_str_eq(value, hash_entry->stored_value)) {
> +               crm_debug_2("Ignoring non-change");
> +               return;
> +       } else if (hash_entry->timer_id != 0) {
> +               crm_debug_2("Update already scheduled");
> +               return;
>        }
>
>        stop_attrd_timer(hash_entry);
>
> _______________________________________________
> Pacemaker mailing list: Pacemaker at oss.clusterlabs.org
> http://oss.clusterlabs.org/mailman/listinfo/pacemaker
>
> Project Home: http://www.clusterlabs.org
> Getting started: http://www.clusterlabs.org/doc/Cluster_from_Scratch.pdf
> Bugs: http://developerbugs.linux-foundation.org/enter_bug.cgi?product=Pacemaker
>