HomeClusterLabs Projects

Fix: libcib: Don't incorrectly expand "++" and "+=" in XML attr values

Description

Fix: libcib: Don't incorrectly expand "++" and "+=" in XML attr values

Drop fix_plus_plus_recursive() call in cib_perform_op. Suppose we have
an XML attribute "score" with initial value "9".

\# cibadmin --query --scope constraints | grep loc_cons

<rsc_location id="loc_cons" rsc="dummy" node="laptop" score="9"/>

Now suppose a CIB replace operation sets the value of "score" to
"score++". fix_plus_plus_recursive() calls expand_plus_plus(), but it
doesn't have access to the old value. So it treats the old value as 0
and expands "score++" to "1" no matter what.

\# cibadmin --replace --scope constraints --xml-text \
'<constraints><rsc_location id="loc_cons" rsc="dummy" node="laptop" score="score++"/></constraints>'
\# cibadmin --query --scope constraints | grep loc_cons

<rsc_location id="loc_cons" rsc="dummy" node="laptop" score="1"/>

Likewise, "score+=5" gets expanded to 5.

\# cibadmin --replace --scope constraints --xml-text \
'<constraints><rsc_location id="loc_cons" rsc="dummy" node="laptop" score="score+=5"/></constraints>'
\# cibadmin --query --scope constraints | grep loc_cons

<rsc_location id="loc_cons" rsc="dummy" node="laptop" score="5"/>

(CIB modify operations expand "++" and "+=" correctly via
pcmk__xml_update().)

Node attributes are nvpairs, so this can't expand node attribute values.
If we update the value of a node attribute test_attr to "test_attr++",
we're not setting an XML attribute "test_attr" to "test_attr++". Rather,
we're updating an <nvpair> to have "name='test_attr'" and
"value='test_attr++'". This is a mismatch as far as
fix_plus_plus_recursive() and expand_plus_plus() are concerned.

There is only one internal function (update_failcount()) that sets
"<attr>++" for any XML attribute value, and there are no internal
functions that set "<attr>+=<int>". update_failcount() is setting a node
attribute, so as explained above, fix_plus_plus_recursive() has no
effect. (attrd_expand_value() expands the value in this update.)

So the fix_plus_plus_recursive() behavior is a bug when processing CIB
operations. It's better to set the new value unexpanded than to pretend
the old value was 0. It's not worth the effort of implementing correct
value expansion when processing a CIB operation unless there's demand
for it.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>

Details

Provenance
nrwahl2Authored on Apr 3 2024, 1:09 PM
Parents
rPcf7d0e6c9773: Test: cts-cli: Update for pcmk__inject_failcount() setting integer value
Branches
Unknown
Tags
Unknown