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
_______________________________________________
Pacemaker mailing list: Pacemaker@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
