xml: allow full xpath when applying applying patch sets

[lge]

Pacemaker-1.1.12, more specifically d170314 xml: Use an optimized xpath search function for applying patch sets introduced __xml_find_path(), which expects the path in elements of cib-diffs to be of a sequence of strictly "/xml-tag-name" plus optional "[@id='xml-id']" (which apparently all internally generated cib-diffs are).

Now, if I wanted to implement a "compare-exchange" type of cib modification, "change node attribute FOO of node #uname=ava to value 2, but only if it is still the expected value 1" I could handcraft a cib diff like so:

   <diff format="2">
     <change operation="modify" path="/cib/configuration/nodes/node[@uname=&apos;ava&apos;]/instance_attributes/nvpair[@name=&apos;FOO&apos; and @value=&apos;1&apos;]">
       <change-result>
         <nvpair id="nodes-ava-FOO" name="FOO" value="2"/>
       </change-result>
     </change>
   </diff>

If xml_apply_patchset_v2() would use the "real" get_xpath_object(), it would even work. Use case would be e.g. a "fencing-via-cib" shoot-out of two scripts triggered on two nodes at the same time.

I understand that using the optimized __xml_find_path() variant improves performance quite a bit, so what I do is to check the xpath argument, and only use the "full" get_xpath_object() if it contains anything more fancy than the above mentioned sequence of "/xml-tag-name[@id='xml-id']".

If the double sscanf(), first during the new check help function, then during __xml_find_path(), seems like a performance degradation, it can be integrated into __xml_find_path(), and do a fallback to get_xpath_object() as soon as the xpath does not fully match the expected input form.

[kgaillot]

If the double sscanf(), first during the new check help function, then during __xml_find_path(), seems like a performance degradation, it can be integrated into __xml_find_path(), and do a fallback to get_xpath_object() as soon as the xpath does not fully match the expected input form.

That does sound cleaner as well. The caller doesn't have to care about which implementation gets used and can't make a mistake about it.

As for the example use case, I'm not sure how a CIB shoot-out could work in a split-brain.

[kgaillot]

Was it an accident to remove the position argument?

[lge]

absolutely, patch created against older version of pacemaker, then "pull --rebase"d, and not re-compiled-tested. sorry for that.

This was really meant as a RFC at first stage anyways. I'll updated the patch.

Re: how shoot-out works during cluster-partition: It still works, if you require quorum before you update, or wait for the "other nodes" to be marked as clean down (successfully fenced) before you do the update, or both. Because you will either survive, and be able to shoot, or you won't, and not be.

But the idea is that some resource has problems, but the cluster communication is still intact, so the resource can arbitrate using the cib.

What I did until now is create a location constraint with a pre-determined xml-id. Only one will succeed, the other will fail, because "already exists".

Now, with more than two DRBD nodes (yes, obviously DRBD is my example), I can have a sequence of resource internal degradation that would cause the constraint to already exist, but still need to be updated to reflect the new situation.

I'm sure other use cases for a "compare exchange" will be found soon. Using a "static" constraint on some node attribute, and "compare-exchange" the node attributes instead, seemed like a good idea. Not new, btw, I think I discussed similar ideas with beekhof about three years ago.

Re: the caller should not have to care which implementation gets used ... there is only one caller of __xml_find_path() right now. But yes, the check should probably go into __xml_find_path() anyways.

Maybe revert the logic, and "always" use get_xpath_object(), but internally to it, check if it can get away with __xml_find_xpath()?

[kgaillot]

Maybe revert the logic, and "always" use get_xpath_object(), but internally to it, check if it can get away with __xml_find_xpath()?

Hmm, interesting ... but I think get_xpath_object() is too widely used. I'd rather not add more overhead to it to benefit a narrow use case that is well isolated code-wise.

[beekhof]

Just make sure you run it through valgrind. There is some fun that happens between getXpathResult() and freeXpathObject() that was needed to make get_xpath_object() work. You'd better be sure it still hold for this usage :)