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

[Holger Teutsch]

Hi Dejan,
On Thu, 2011-03-10 at 10:14 +0100, Dejan Muhamedagic wrote:
> Hi Holger,
> 
> On Wed, Mar 09, 2011 at 07:58:02PM +0100, Holger Teutsch wrote:
> > Hi Dejan,
> > 
> > On Wed, 2011-03-09 at 14:00 +0100, Dejan Muhamedagic wrote:
> > > Hi Holger,
> > 

> > 
> > 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)

See patch set_obj_all.diff

> 
> > > 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.
> 
> I'll do some testing on really big configurations and try to
> gauge the impact.

OK

> 
> The patch makes some regression tests blow:
> 
> +  File "/usr/lib64/python2.6/site-packages/crm/ui.py", line 1441, in verify
> +    return self._verify(mkset_obj("xml"))
> +  File "/usr/lib64/python2.6/site-packages/crm/ui.py", line 1433, in _verify
> +    rc2 = set_obj_semantic.semantic_check(set_obj_all)
> +  File "/usr/lib64/python2.6/site-packages/crm/cibconfig.py", line 294, in semantic_check
> +    rc = self.__check_unique_clash(set_obj_all)
> +  File "/usr/lib64/python2.6/site-packages/crm/cibconfig.py", line 274, in __check_unique_clash
> +    process_primitive(node, clash_dict)
> +  File "/usr/lib64/python2.6/site-packages/crm/cibconfig.py", line 259, in process_primitive
> +    if ra_params[ name ]['unique'] == '1':
> +KeyError: 'OCF_CHECK_LEVEL'
> 
> Can't recall why OCF_CHECK_LEVEL appears here. There must be some
> good explanation :)

The good explanation is: Not only params are in <instance_atributes ...>
but OCF_CHECK_LEVEL as well within <operations ...>

The latest version no longer blows the test -> semantic_check.diff

Regards
Holger
-------------- next part --------------
A non-text attachment was scrubbed...
Name: set_obj_all.diff
Type: text/x-patch
Size: 2047 bytes
Desc: not available
URL: <https://lists.clusterlabs.org/pipermail/pacemaker/attachments/20110310/efb7efd5/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: semantic_check.diff
Type: text/x-patch
Size: 3391 bytes
Desc: not available
URL: <https://lists.clusterlabs.org/pipermail/pacemaker/attachments/20110310/efb7efd5/attachment-0007.bin>