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
diff -r a35d8d6d0ab1 shell/modules/cibconfig.py
--- a/shell/modules/cibconfig.py Wed Mar 09 11:21:03 2011 +0100
+++ b/shell/modules/cibconfig.py Wed Mar 09 19:53:50 2011 +0100
@@ -230,11 +230,68 @@
See below for specific implementations.
'''
pass
- def semantic_check(self):
+
+ 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()
+
+ 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
+
+ # 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
diff -r a35d8d6d0ab1 shell/modules/ui.py.in
--- a/shell/modules/ui.py.in Wed Mar 09 11:21:03 2011 +0100
+++ b/shell/modules/ui.py.in Wed Mar 09 19:53:50 2011 +0100
@@ -1425,12 +1425,12 @@
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 = 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()
+ rc2 = set_obj_semantic.semantic_check(set_obj_all)
else:
rc2 = 0
return rc1 and rc2 <= 1
_______________________________________________
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
