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
# HG changeset patch
# User Holger Teutsch <holger.teut...@web.de>
# Date 1299775617 -3600
# Branch hot
# Node ID 30730ccc0aa09c3a476a18c6d95c680b35999995
# Parent 9fa61ee6e35ef190f4126e163e9bfe6911e35541
Low: Shell: Rename variable set_obj_verify to set_obj_all as it always contains all objects
Simplify usage of this var in [_]verify, pass to CibObjectSet.semantic_check
diff -r 9fa61ee6e35e -r 30730ccc0aa0 shell/modules/cibconfig.py
--- a/shell/modules/cibconfig.py Wed Mar 09 13:41:27 2011 +0100
+++ b/shell/modules/cibconfig.py Thu Mar 10 17:46:57 2011 +0100
@@ -230,7 +230,7 @@
See below for specific implementations.
'''
pass
- def semantic_check(self):
+ def semantic_check(self, set_obj_all):
'''
Test objects for sanity. This is about semantics.
'''
diff -r 9fa61ee6e35e -r 30730ccc0aa0 shell/modules/ui.py.in
--- a/shell/modules/ui.py.in Wed Mar 09 13:41:27 2011 +0100
+++ b/shell/modules/ui.py.in Thu Mar 10 17:46:57 2011 +0100
@@ -1425,12 +1425,10 @@
set_obj = mkset_obj(*args)
err_buf.release() # show them, but get an ack from the user
return set_obj.edit()
- def _verify(self, set_obj_semantic, set_obj_verify = None):
- if not set_obj_verify:
- set_obj_verify = set_obj_semantic
- rc1 = set_obj_verify.verify()
+ 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()
+ rc2 = set_obj_semantic.semantic_check(set_obj_all)
else:
rc2 = 0
return rc1 and rc2 <= 1
@@ -1438,7 +1436,8 @@
"usage: verify"
if not cib_factory.is_cib_sane():
return False
- return self._verify(mkset_obj("xml"))
+ set_obj_all = mkset_obj("xml")
+ return self._verify(set_obj_all, set_obj_all)
def save(self,cmd,*args):
"usage: save [xml] <filename>"
if not cib_factory.is_cib_sane():
# HG changeset patch
# User Holger Teutsch <holger.teut...@web.de>
# Date 1299779740 -3600
# Branch hot
# Node ID 73021c988d92c5dad4c503af9f8826f5d1c34373
# Parent 30730ccc0aa09c3a476a18c6d95c680b35999995
Med: Shell: Check for violations of uniqueness for instance parameters during commit
Implemented in CibObjectSet.semantic_check()
diff -r 30730ccc0aa0 -r 73021c988d92 shell/modules/cibconfig.py
--- a/shell/modules/cibconfig.py Thu Mar 10 17:46:57 2011 +0100
+++ b/shell/modules/cibconfig.py Thu Mar 10 18:55:40 2011 +0100
@@ -230,11 +230,70 @@
See below for specific implementations.
'''
pass
+
+ def __check_unique_clash(self, set_obj_all):
+ 'Check whether resource parameters with attribute "unique" clash'
+
+ def process_primitive(prim, clash_dict):
+ '''
+ Update dict clash_dict with
+ (ra_class, ra_provider, ra_type, name, value) -> [ resourcename ]
+ if parameter "name" should be unique
+ '''
+ 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)
+ if ra == None:
+ return
+ ra_params = ra.params()
+
+ for a in prim.getElementsByTagName("instance_attributes"):
+ # params are in instance_attributes just below the parent
+ # operations may have some as well, e.g. OCF_CHECK_LEVEL
+ if a.parentNode != prim:
+ continue
+
+ for p in a.getElementsByTagName("nvpair"):
+ name = p.getAttribute("name")
+ if ra_params[ name ].get("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
+
+ # we check the whole CIB for clashes as a clash may originate between
+ # an object already committed and a new one
+ clash_dict = {}
+ for obj in set_obj_all.obj_list:
+ node = obj.node
+ if is_primitive(node):
+ process_primitive(node, clash_dict)
+
+ # but we only warn if a 'new' object is involved
+ check_set = set([o.node.getAttribute("id") for o in self.obj_list if is_primitive(o.node)])
+
+ rc = 0
+ for param, resources in clash_dict.items():
+ # at least one new object must be involved
+ if len(resources) > 1 and len(set(resources) & check_set) > 0:
+ rc = 2
+ msg = 'Resources %s violate uniqueness for parameter "%s": "%s"' %\
+ (",".join(sorted(resources)), param[3], param[4])
+ common_warning(msg)
+
+ return rc
+
def semantic_check(self, set_obj_all):
'''
Test objects for sanity. This is about semantics.
'''
- rc = 0
+ rc = self.__check_unique_clash(set_obj_all)
for obj in self.obj_list:
rc |= obj.check_sanity()
return rc
_______________________________________________
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
