Lars Ellenberg
2013-05-14 12:54:41 UTC
On Tue, May 14, 2013 at 01:22:08PM +0200, Lars Ellenberg wrote:
[ on linux-ha,
a lot of stuff about why I think we don't even want that feature,
or at least not how it is implemented ]
This post is related, but about the implementation bugs in the current
implementation as I spotted them. So this time to linux-ha-dev.
If you want me to trimm the Cc, just say so ;-)
So assuming we even want this feature, here are some bugs I found:
------
in lvm_vg.sh,
vg_stop_tagged()
probably only strip, if you are the owner,
NOT if you "could" claim it.
vg_tag_owner
-if [ $? -ne 0 ] ; then
+if [ $? = 1 ] ; then
strip_tags
------
I see quite a number of
lvs --options | while read line; do
some_variable=whatever
possibly return $SOMETHING
done
refer to $some_variable
e.g in vg_start_exclusive().
THIS DOES NOT WORK.
the whole while loop, because it reads from a pipe,
executes in a subshell.
variable assignments do not propagate to the main script,
return values do not return from the current function,
but only from the subshell, changing the exit code of that pipeline only.
Note that I think most (all?) of these slipped in
with the attempt to
Low: LVM: Restore LVM agent portability
so I guess the original is probably ok, doing
result=($(subcommand));
while [ ! -z ${result[iterator]} ] ...
See the example further down for how to do that
same thing in portable shell.
----
# BROKEN
t() {
var=initial-value
echo bla | while read line; do
var=other-value
return 42
done
echo "still here"
}
t
echo "return code: $?"
echo "var=$var"
----
output:
----
still here
return code: 0
var=initial-value
----
D'oh. All that fine error handling for nothing :-(
Assuming they are otherwise correct,
just convert all those into
for some_var in $(lvs --options); do
done
That should do what I think you mean.
----
# OK
t() {
var=initial-value
for line in $(echo bla); do
var=other-value
return 42
done
echo "still here?"
}
t
echo "return code: $?"
echo "var=$var"
----
output:
----
return code: 42
var=other-value
----
For the two (or more) field case,
where the original used bash array variables,
you can use the positional argument array.
Do something like this:
t()
{
local result name attr
# doing it this way, first declare local,
# then do the "backtick assign"
# preserves the exit code
result=$(lvs --noheadings -o name,attr $whatever)
# so you can now check the exit status, if you want to.
# then assign to positional argument "array"
set -- $result
# if $# % 2 != 0 then panic ;-)
while [ $# -ge 2 ]; do
name=$1 attr=$2
shift 2
do-what-you-have-to
case $attr in
[rR]???????p)
it-is-a-partial ;;
[mM]???????p)
a-partial-we-may-need-to-repair ;;
*a?)
: it-is-active-great ;;
*)
not-activated-now-what ;;
esac
done
}
shell can split lines to words and do pattern matching just fine...
no need to call out to echo | awk | grep, unless word splitting and
patterns are not sufficient.
Or, of course, if we decide we so deperatly want this feature,
maybe just ignore the /bin/bash != /bin/sh is a regression argument,
use the hopefully "proven" bash code from the rgmanager branch,
drop that compat commit,
and ignore people whining about bash being bloated.
This is Linux specific code, and I very much doubt there is a linux
shipping LVM + pacemaker but no bash.
---------
I have to stop this now.
But if I come back later,
I'll followup with whatever else I find.
Cheers,
Lars
[ on linux-ha,
a lot of stuff about why I think we don't even want that feature,
or at least not how it is implemented ]
This post is related, but about the implementation bugs in the current
implementation as I spotted them. So this time to linux-ha-dev.
If you want me to trimm the Cc, just say so ;-)
So assuming we even want this feature, here are some bugs I found:
------
in lvm_vg.sh,
vg_stop_tagged()
probably only strip, if you are the owner,
NOT if you "could" claim it.
vg_tag_owner
-if [ $? -ne 0 ] ; then
+if [ $? = 1 ] ; then
strip_tags
------
I see quite a number of
lvs --options | while read line; do
some_variable=whatever
possibly return $SOMETHING
done
refer to $some_variable
e.g in vg_start_exclusive().
THIS DOES NOT WORK.
the whole while loop, because it reads from a pipe,
executes in a subshell.
variable assignments do not propagate to the main script,
return values do not return from the current function,
but only from the subshell, changing the exit code of that pipeline only.
Note that I think most (all?) of these slipped in
with the attempt to
Low: LVM: Restore LVM agent portability
so I guess the original is probably ok, doing
result=($(subcommand));
while [ ! -z ${result[iterator]} ] ...
See the example further down for how to do that
same thing in portable shell.
----
# BROKEN
t() {
var=initial-value
echo bla | while read line; do
var=other-value
return 42
done
echo "still here"
}
t
echo "return code: $?"
echo "var=$var"
----
output:
----
still here
return code: 0
var=initial-value
----
D'oh. All that fine error handling for nothing :-(
Assuming they are otherwise correct,
just convert all those into
for some_var in $(lvs --options); do
done
That should do what I think you mean.
----
# OK
t() {
var=initial-value
for line in $(echo bla); do
var=other-value
return 42
done
echo "still here?"
}
t
echo "return code: $?"
echo "var=$var"
----
output:
----
return code: 42
var=other-value
----
For the two (or more) field case,
where the original used bash array variables,
you can use the positional argument array.
Do something like this:
t()
{
local result name attr
# doing it this way, first declare local,
# then do the "backtick assign"
# preserves the exit code
result=$(lvs --noheadings -o name,attr $whatever)
# so you can now check the exit status, if you want to.
# then assign to positional argument "array"
set -- $result
# if $# % 2 != 0 then panic ;-)
while [ $# -ge 2 ]; do
name=$1 attr=$2
shift 2
do-what-you-have-to
case $attr in
[rR]???????p)
it-is-a-partial ;;
[mM]???????p)
a-partial-we-may-need-to-repair ;;
*a?)
: it-is-active-great ;;
*)
not-activated-now-what ;;
esac
done
}
shell can split lines to words and do pattern matching just fine...
no need to call out to echo | awk | grep, unless word splitting and
patterns are not sufficient.
Or, of course, if we decide we so deperatly want this feature,
maybe just ignore the /bin/bash != /bin/sh is a regression argument,
use the hopefully "proven" bash code from the rgmanager branch,
drop that compat commit,
and ignore people whining about bash being bloated.
This is Linux specific code, and I very much doubt there is a linux
shipping LVM + pacemaker but no bash.
---------
I have to stop this now.
But if I come back later,
I'll followup with whatever else I find.
Cheers,
Lars