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

[Holger Teutsch]

Hi Dejan,

On Wed, 2011-03-09 at 14:00 +0100, Dejan Muhamedagic wrote:
> Hi Holger,

> > > > 
> > > > 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).
> > > > 
> > > 
> > 
> > I did not pass "set_obj_verify" in semantic check as this variable "only
> > by chance" contains the right values.
> 
> But it's not by chance. As I wrote earlier, it always contains
> the whole CIB. It has to, otherwise crm_verify wouldn't work. It
> should actually be renamed to set_obj_all or similar. Since we
> already have that list created, it's better to reuse it than to
> create another one from scratch. Further, we may need to add more
> semantic checks which would require looking at the whole CIB.
> 

OK, I implemented it this way.

In order to show the intention of the arguments clearer:

Instead of

    def _verify(self, set_obj_semantic, set_obj_all = None):
        if not set_obj_all:
            set_obj_all = set_obj_semantic
        rc1 = set_obj_all.verify()
        if user_prefs.check_frequency != "never":
            rc2 = set_obj_semantic.semantic_check(set_obj_all)
        else:
            rc2 = 0
        return rc1 and rc2 <= 1
    def verify(self,cmd):
        "usage: verify"
        if not cib_factory.is_cib_sane():
            return False
        return self._verify(mkset_obj("xml"))

This way (always passing both args):

    def _verify(self, set_obj_semantic, set_obj_all):
        rc1 = set_obj_all.verify()
        if user_prefs.check_frequency != "never":
            rc2 = set_obj_semantic.semantic_check(set_obj_all)
        else:
            rc2 = 0
        return rc1 and rc2 <= 1
    def verify(self,cmd):
        "usage: verify"
        if not cib_factory.is_cib_sane():
            return False
	set_obj_all = mkset_obj("xml")
        return self._verify(set_obj_all, set_obj_all)


> My only remaining concern is performance. Though the meta-data is
> cached, perhaps it will pay off to save the RAInfo instance with
> the element. But we can worry about that later.
> 

I can work on this as next step.

> Cheers,
> 
> Dejan
> 

- holger

-------------- next part --------------
A non-text attachment was scrubbed...
Name: shell.diff
Type: text/x-patch
Size: 3800 bytes
Desc: not available
URL: <https://lists.clusterlabs.org/pipermail/pacemaker/attachments/20110309/3858a2c8/attachment-0004.bin>