[Pacemaker] New patch for System Health feature

[Andrew Beekhof]

On Thu, May 14, 2009 at 8:07 PM, Mark Hamzy <hamzy at us.ibm.com> wrote:
> andrew at beekhof.net wrote on 05/13/2009 21:08:36 PM:
>
>> This is missing the modification to char2score that i mentioned (which
>> would also simplify calculate_system_health()).
>> ...
>
>> Oh, and initialize_health_value() should probably just set a something
>> in data_set (which would be passed to char2score) and should obtain a
>> value from data_set->config_hash instead of using an xpath query.
>
> Just to be clear,
>
> hs21c:/usr/src/packages/BUILD # grep -r char2score pacemaker/
> pacemaker/include/crm/common/util.h:extern int char2score(const char
> *score);
> pacemaker/lib/common/utils.c:char2score(const char *score)
> pacemaker/lib/common/xml.c:        int_value = char2score(old_value);
> pacemaker/lib/common/xml.c:        offset = char2score(offset_s);
> pacemaker/lib/pengine/complex.c:           (*rsc)->stickiness =
> char2score(value);
> pacemaker/lib/pengine/complex.c:           (*rsc)->migration_threshold =
> char2score(value);
> pacemaker/lib/pengine/complex.c:           int fail_sticky =
> char2score(value);
> pacemaker/lib/pengine/rules.c:             pair->score = char2score(score);
> pacemaker/lib/pengine/unpack.c:    data_set->default_resource_stickiness =
> char2score(value);
> pacemaker/lib/pengine/utils.c:     fail_count = char2score(value);
> pacemaker/pengine/allocate.c:              *value = char2score (attr_value);
> pacemaker/pengine/constraints.c:   score_i = char2score(score);
> pacemaker/pengine/constraints.c:       int score_i = char2score(score);
> pacemaker/pengine/constraints.c:           score_f = char2score(score);
> pacemaker/pengine/constraints.c:                   score_f =
> char2score(attr_score);
> pacemaker/pengine/constraints.c:           score_f = char2score(score);
> pacemaker/pengine/constraints.c:   local_score = char2score(score_s);
> pacemaker/pengine/constraints.c:   score_i = char2score(score);
> pacemaker/pengine/constraints.c:   local_score = char2score(score_s);
> pacemaker/pengine/constraints.c:   score_i = char2score(score);
> pacemaker/pengine/constraints.c:       score_i = char2score(score);
> pacemaker/pengine/master.c:                score = char2score(attr_value);
>
> char2score appears in many places in the code.  All char2score instances
> should be
> changed to now look like:
>
> char2score(const char *score, pe_working_set_t *data_set)
>
> All of the above instances will just pass in NULL so that the behavior does
> not change.

I was thinking the opposite actually.
But on reflection I think its simpler to create a couple of global
variables to store the values of red/yellow/green (I can do that part)

> If data_set is non-null, then green/yellow/red will convert into numbers
> using three new
> members added to the structure.
>
>> For extra credit, you could also fake the configuration element and
>> pass it to generate_location_rule() instead of using rsc2node_new().
>> This is what I orginally had in mind, but may not be worth the effort.
>
> One of my first attempts had something like:
>
> ...
> +                       sprintf (achXML,
> +                               "<rsc_location id=\"%s_health_location\"
> rsc=\"%s\">\n  <rule id=\"%s_health_rule\" score-attribute=\"#health\">\n
>  <expression attribute=\"#health\" id=\"%s_health_expr\"
> operation=\"defined\"/>\n  </rule>\n</rsc_location>",
> +                               rsc,
> +                               rsc,
> +                               rsc,
> +                               rsc);
> +
> +                       newXML = string2xml(achXML);
> +
> +                       newResources = g_list_append(newResources, newXML);
> ...
> +       g_list_foreach(newResources, add_resource, cib_constraints);

yep, this is roughly what i had in mind (although I was thinking of
creating the xml programatically with create_xml_node() and friends)

> However,
>
> rsc2node_new (id, rsc, health_info->system_health, node, data_set);
>
> seems cleaner.

The former is O(resources) while this is O(resources*nodes).
On balance I'd prefer the first way, but this is also fine for now.