On Tue, Sep 07, 2010 at 03:55:49PM -0400, Michael Smith wrote:
> Allow the LVM RA to handle multiple volume groups.
>
> Correct the documentation to refer to volume groups, not volumes.
>
> Allow "status" operation to skip "vgck" if so configured, in order to
> save time if CLVM is hanging.
>
> Allow an unsuccessful "stop" to be ignored if so configured, as required
> in some CLVM setups.
>
> Remove double-check to simplify "validate-all" routine.
>
> Signed-off-by: Michael Smith <msm...@cbnco.com>
> + if [ -z "$@" ]
That does not work.
"$@" is expanded first, into separate words.
so you get [ -z vg00 vg01 vg02 ],
and that results in
[: too many arguments
rather do [ $# = 0 ]
> + then
> + vgdisplay 2>&1 | grep -q 'Volume group .* not found' && {
> + ocf_log info "No volume groups found"
> + return $OCF_SUCCESS
> + }
But that branch does not look functional to me anyways?
What's that supposed to do?
What if you have a single stale entry in some lvm cache?
I'd rather make "all" as well as "" a configuration error,
and not the default.
Typically, if you use LVM, you'll also have a system VG,
which you won't be able to deactivate.
> + else
> + for i in $@
> + do
> + if vgdisplay $i 2>&1 | grep -q 'Volume group .* not found'
> + then
> + ocf_log info "Volume group $i not found"
> + else
> + vgs_active="$vgs $i"
> + fi
> +
> + if [ -z "$vgs_active" ]
> + then
> + ocf_log info "No volume groups found."
> + return $OCF_SUCCESS
> + fi
> + done
> + fi
how about
vgs_active=$(
# separator / is nice, as it is not allowed in names here.
vgs --separator / -o vg_name,lv_name,lv_attr $* |
grep '.*/.*/....a' | # grep for active lvs
cut -d/ -f1 | # cut the vg_name, and uniq it.
sort -u)
> +
> + ocf_log info "Deactivating logical volumes (groups: ${vgs_active:-all})"
> + ocf_run vgchange --available ln $vgs_active
> + rc=$?
>
> - if
> - LVM_status $1
> + if [ -z "$OCF_RESKEY_checkstop" ] || ocf_is_true "$OCF_RESKEY_checkstop"
And if checkstop is false,
then you are intentionally lying to the cluster manager?
What is the use case for that?
> then
> - ocf_log err "LVM: $1 did not stop correctly"
> - return $OCF_ERR_GENERIC
> + if [ $rc -ne 0 ]
> + then
> + ocf_log err "Volume groups (${vgs_active:-all}) failed to stop"
> + return $OCF_ERR_GENERIC
> + fi
> +
> + if
> + LVM_status $1
uh? why $1, suddenly, no longer $@ ?
If only the first succeeded, you report success anyways!
BTW, why are you using $1 and $@, if VOLUMES is a global variable anyways?
> + then
> + ocf_log err "Volume groups (${vgs_active:-all}) still running after
> stop"
> + return $OCF_ERR_GENERIC
> + fi
> fi
> -
> - # TODO: This MUST run vgexport as well
What about those vgimport/export TODOs?
> -if
> - [ -z "$OCF_RESKEY_volgrpname" ]
> -then
> -# echo "You must identify the volume group name!"
> - ocf_log err "You must identify the volume group name!"
> -# usage
> - exit $OCF_ERR_ARGS
> -fi
I'd leave that in, require that either/or has to be set.
Defaulting to "all" is a bad idea, IMO.
--
: Lars Ellenberg
: LINBIT | Your Way to High Availability
: DRBD/HA support and consulting http://www.linbit.com
DRBD® and LINBIT® are registered trademarks of LINBIT, Austria.
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev@lists.linux-ha.org
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/
