[Pacemaker] Patch for bugzilla 2541: Shell should warn if parameter uniqueness is violated

[Holger Teutsch]

Hi Dejan,

On Tue, 2011-03-08 at 12:07 +0100, Dejan Muhamedagic wrote:
> Hi Holger,
> 
> On Tue, Mar 08, 2011 at 09:15:01AM +0100, Holger Teutsch wrote:
> > On Fri, 2011-03-04 at 13:06 +0100, Holger Teutsch wrote:
> > > On Thu, 2011-03-03 at 10:55 +0100, Florian Haas wrote:
> > > > On 2011-03-03 10:43, Holger Teutsch wrote:
> > > > > Hi,
> > > > > I submit a patch for
> > > > > "bugzilla 2541: Shell should warn if parameter uniqueness is violated"
> > > > > for discussion.
> > > > 
...
> It looks good, just a few notes. The check function should move
> to the CibObjectSetRaw class and be invoked from

Will move it there.

> semantic_check(). There's
> 
>         rc1 = set_obj_verify.verify()
>         if user_prefs.check_frequency != "never":
> 			rc2 = set_obj_semantic.semantic_check()
> 
> The last should be changed to:
> 
> 			rc2 = set_obj_semantic.semantic_check(set_obj_verify)
> 
> set_obj_verify always contains all CIB elements (well, that means
> that its name should probably be changed too :). Now, the code
> should check _only_ new and changed primitives which are
> contained in set_obj_semantic. That's because we don't want to
> repeatedly print warnings for all objects on commit, but only for
> those which were added/changed in the meantime. On the other
> hand, verify is an explicit check and in that case the whole CIB
> is always verified.
> 
> > 
> > +            ra_class = prim.getAttribute("class")
> > +            ra_provider = prim.getAttribute("provider")
> > +            ra_type = prim.getAttribute("type")
> > +            ra_id = prim.getAttribute("id")
> > +
> > +            ra = RAInfo(ra_class, ra_type, ra_provider)
> 
> There's a convenience function get_ra(node) for this.
> 

I did not use this as I need all ra_XXX value anyhow later in the code
for building k.

> > +            if ra == None:
> > +                return
> > +            ra_params = ra.params()
> > +
> > +            attributes = prim.getElementsByTagName("instance_attributes")
> > +            if len(attributes) == 0:
> > +                return
> > +
> > +            for p in attributes[0].getElementsByTagName("nvpair"):
> > +                name = p.getAttribute("name")
> > +                if ra_params[ name ]['unique'] == '1':
> > +                    value = p.getAttribute("value")
> > +                    k = (ra_class, ra_provider, ra_type, name, value)
> > +                    try:
> > +                        clash_dict[k].append(ra_id)
> > +                    except:
> > +                        clash_dict[k] = [ra_id]
> > +            return
> > +
> > +        clash_dict = {}
> > +        for p in cib_factory.mkobj_list("xml","type:primitive"):
> 
> This would become:
> 
>            for p in all_obj_list: # passed from _verify()
> 			   if is_primitive(p.node):
> 
> > +            process_primitive(p.node, clash_dict)
> 
> Or perhaps to loop through self.obj_list and build clash_dict
> against all elements? Otherwise, you'll need to skip elements
> which don't pass the check but are not new/changed (in
> self.obj_list).
> 

The typical occurrences of clashes will originate from "old" objects and
"new/changed" objects.

I think I have to loop over all objects to build clash dict  and
then ... 

> > +
> > +        no_clash = 1
> > +        for param, resources in clash_dict.items():
> > +            if len(resources) > 1:

... only emit a warning if the intersection of a "clash set" with
"changed objects" is not empty. 

> > +                no_clash = 0
> > +                msg = 'Resources %s violate uniqueness for parameter "%s": "%s"' %\
> > +                        (",".join(sorted(resources)), param[3], param[4])
> > +                common_warning(msg)
> > +
> > +        return no_clash
> > +

I will submit an updated version later this week.

-holger