[lvm-devel] [PATCH v3 01/18] fsadm: Add new commands create, list, add and remove
Zdenek Kabelac
zkabelac at redhat.com
Tue Oct 4 09:41:35 UTC 2011
Dne 27.9.2011 15:42, Lukas Czerner napsal(a):
> This commit adds new functionality in the form of new commands. Namely
> it is create, list, add and remove. This commit also changes the way how
> are commands recognised an executed. The new approach is more suitable
> for bigger number of commands.
>
> Resize command is also significantly reworked. Unlike in the old
> approach fsadm will always attempt to resize the logical volume as well,
> since this is what it is expected. Leave the --lvresize option for
> backwards compatibility, but remove it from the help. If no file system
> resides on the logical volume, only the volume will be resized.
>
> * Create command provides the functionality of creating a new logical
> volumes including defined file system.
> * List command provides the functionality of listing useful information
> about the logical volumes, file systems and devices.
> * Add command allows to add devices into volume groups (pool).
> * Remove command allows to remove the volumes or volume groups from the
> system.
>
> Signed-off-by: Lukas Czerner<lczerner at redhat.com>
> @@ -96,7 +109,6 @@ tool_usage() {
> echo " -e | --ext-offline unmount filesystem before ext2/ext3/ext4 resize"
> echo " -f | --force Bypass sanity checks"
> echo " -n | --dry-run Print commands without running them"
> - echo " -l | --lvresize Resize given device (if it is LVM device)"
> echo " -y | --yes Answer \"yes\" at any prompts"
> echo
> echo " new_size - Absolute number of filesystem blocks to be in the filesystem,"
Again - no way to remove already supported option.
> @@ -115,13 +127,37 @@ error() {
> cleanup 1
> }
>
> -dry() {
> +warn() {
> + echo "$TOOL: $@">&2
> +}
> +
> +dry_nofail() {
> if [ "$DRY" -ne 0 ]; then
> verbose "Dry execution $@"
> return 0
> fi
> verbose "Executing $@"
> - $@
> + local IFS=" "
> + eval "$*"
> +}
That looks weird ?
Dry execution has it's limits - if something fails - it cannot be be
simulated. Going with _nofail seems to be dangerous.
So for now skip it.
> +float_math() {
> + if [ $# -le 0 ]; then
> + return
> + fi
> + result=$(LANG=C echo "scale=2; $@" | "$BC" -q 2> /dev/null)
> + echo "$result"
> +}
> +
> +is_natural() {
> + case "$1" in
> + "" | *[!0-9]*) return 1;;
> + *) return 0;;
> + esac
> }
Please - split whole float_math in separate patch - I think this one could be
easily upstreamed. Though it's adding new deps - however 'bc' is very common
command available probably everywhere - so it should not present a problem.
(Though needs new packaging deps to be properly handled - wondering how we
could make this easier for package maintainers)
>
> cleanup() {
> @@ -132,54 +168,140 @@ cleanup() {
> verbose "Remounting unmounted filesystem back"
> dry "$MOUNT" "$VOLUME" "$MOUNTED"
> fi
> - IFS=$IFS_OLD
> trap 2
>
> test "$1" -eq 2&& verbose "Break detected"
>
> - if [ "$DO_LVRESIZE" -eq 2 ]; then
> - # start LVRESIZE with the filesystem modification flag
> - # and allow recursive call of fsadm
> - _FSADM_YES=$YES
> - export _FSADM_YES
> - unset FSADM_RUNNING
> - test -n "$LVM_BINARY"&& PATH=$_SAVEPATH
> - dry exec "$LVM" lvresize $VERB $FORCE -r -L${NEWSIZE}b "$VOLUME_ORIG"
> - fi
> -
Again - please do not remove things you do not understand.
And also thing like this needs to be in a separate patch - not bundled in
hundreds patch lines.
> # error exit status for break
> exit ${1:-1}
> }
>
> +#####################################
> +# Convet the size into human readable
> +# form. size in Bytes expected
> +#####################################
> +humanize_size() {
> + size="$1"
> + count=0
> + while [ ${size%%.*} -ge 1024 ]&& [ $count -lt 7 ]; do
> + size=$(float_math $size/1024)
> + count=$(($count+1))
> + done
> + case $count in
> + 0) unit="B" ;;
> + 1) unit="KiB" ;;
> + 2) unit="MiB" ;;
> + 3) unit="GiB" ;;
> + 4) unit="TiB" ;;
> + 5) unit="PiB" ;;
> + 6) unit="EiB" ;;
> + 7) unit="ZiB" ;;
> + 8) unit="YiB" ;;
> + *) unit="???" ;;
> + esac
> + echo "$size $unit"
> +}
> +
We need to make sure - using are handled in the very same way by both commands
(lvm & fsadm - might probably require to parse lvm.conf for
'si_unit_consistency' option - so the user will not be confused.
(As fsadm is part of lvm2 package and this nice extensions might bring some
unwanted confusion to users with differently configure lvm2).
> +#############################
> +# Get size of entN filesystem
> +# by reading tune2fs output
> +#############################
> +get_ext_size() {
> + local IFS=$NL
> + for i in $(LANG=C $TUNE_EXT -l "$VOLUME"); do
> + case "$i" in
> + "Block size"*) bsize=${i##* } ;;
> + "Block count"*) bcount=${i##* } ;;
> + "Reserved block count"*) rbcount=${i##* } ;;
> + "Free blocks"*) fbcount=${i##* } ;;
> + esac
> + done
> +
> + total=$(($bcount*$bsize))
> + TOTAL=$(humanize_size $total)
> + used=$((($bcount-$fbcount)*$bsize))
> + USED=$(humanize_size $used)
> + free=$((($fbcount-$rbcount)*$bsize))
> + FREE=$(humanize_size $free)
> +}
> +
> +############################
> +# Get size of xfs file system
> +# by reading the df output or
> +# examine file system with
> +# xfs_db tool
> +#############################
> +get_xfs_size() {
> + local IFS=$NL
> + if [ -z "$MOUNTED" ]; then
> +
> + for i in $(LANG=C xfs_db -c 'sb' -c 'print blocksize fdblocks dblocks logblocks agcount' $VOLUME); do
"$XFS_DB"
"$VOLUME"
> + line=$("$DF" -k "$VOLUME" | "$GREP" -e "$MOUNTED$" | sed -e 's/ */ /g')
"$SED"
> + line=${line#* }
> + total=$(echo $line | cut -d' ' -f1)
"$CUT"
Though maybe there is proably native shell way to extract things like this
without extra execution command.
> +detect_fs_size() {
> + if [ -z "$FSTYPE" ]; then
> + return
> + fi
Why ?
(Imho it's handled by case below via return 1
> + case "$FSTYPE" in
> + ext[234]) get_ext_size ;;
> + xfs) get_xfs_size ;;
> + *) return 1 ;;
> + esac
> + return 0
> +}
> +
> # convert parameter from Exa/Peta/Tera/Giga/Mega/Kilo/Bytes and blocks
> # (2^(60/50/40/30/20/10/0))
> decode_size() {
> case "$1" in
> - *[eE]) NEWSIZE=$(( ${1%[eE]} * 1152921504606846976 )) ;;
> - *[pP]) NEWSIZE=$(( ${1%[pP]} * 1125899906842624 )) ;;
> - *[tT]) NEWSIZE=$(( ${1%[tT]} * 1099511627776 )) ;;
> - *[gG]) NEWSIZE=$(( ${1%[gG]} * 1073741824 )) ;;
> - *[mM]) NEWSIZE=$(( ${1%[mM]} * 1048576 )) ;;
> - *[kK]) NEWSIZE=$(( ${1%[kK]} * 1024 )) ;;
> + *[eE]) NEWSIZE=$(float_math "${1%[eE]} * 1152921504606846976") ;;
> + *[pP]) NEWSIZE=$(float_math "${1%[pP]} * 1125899906842624") ;;
> + *[tT]) NEWSIZE=$(float_math "${1%[tT]} * 1099511627776") ;;
> + *[gG]) NEWSIZE=$(float_math "${1%[gG]} * 1073741824") ;;
> + *[mM]) NEWSIZE=$(float_math "${1%[mM]} * 1048576") ;;
> + *[kK]) NEWSIZE=$(float_math "${1%[kK]} * 1024") ;;
> *[bB]) NEWSIZE=${1%[bB]} ;;
> *) NEWSIZE=$(( $1 * $2 )) ;;
Yep - separate patch for math operation.
> esac
> - #NEWBLOCKCOUNT=$(round_block_size $NEWSIZE $2)
> - NEWBLOCKCOUNT=$(( $NEWSIZE / $2 ))
>
> - if [ $DO_LVRESIZE -eq 1 ]; then
> - # start lvresize, but first cleanup mounted dirs
> - DO_LVRESIZE=2
> - cleanup 0
> - fi
Recursion stays for now.
> + NEWSIZE=${NEWSIZE%%.*}
> + NEWBLOCKCOUNT=$(float_math "$NEWSIZE / $2")
> + NEWBLOCKCOUNT=${NEWBLOCKCOUNT%%.*}
> }
>
> # detect filesystem on the given device
> # dereference device name if it is symbolic link
> detect_fs() {
> - VOLUME_ORIG=$1
> + VOLUME_ORIG="$1"
> VOLUME=${1/#"${DM_DEV_DIR}/"/}
> - VOLUME=$("$READLINK" $READLINK_E "$DM_DEV_DIR/$VOLUME") || error "Cannot get readlink \"$1\""
> + VOLUME=$("$READLINK" "$READLINK_E" "$DM_DEV_DIR/$VOLUME") || error "Cannot get readlink \"$1\""
Do not modify same thing in multiple patches
(The advantage of shuffling these things in front of the whole set.)
> - MOUNTED=$("$GREP" ^"$VOLUME" "$PROCMOUNTS")
> + MOUNTED=$("$GREP" -e "^$VOLUME " "$PROCMOUNTS")
Candidate for separate patch in front.
Assuming we currently do not handle well VGs starting with letter '-'.
So it's hidden bugfix (grep -e)
> @@ -295,17 +417,19 @@ resize_ext() {
> if [ "$NEWBLOCKCOUNT" -lt "$BLOCKCOUNT" -o "$EXTOFF" -eq 1 ]; then
> detect_mounted&& verbose "$RESIZE_EXT needs unmounted filesystem"&& try_umount
> REMOUNT=$MOUNTED
> - if test -n "$MOUNTED" ; then
> - # Forced fsck -f for umounted extX filesystem.
> - case "$-" in
> - *i*) dry "$FSCK" $YES -f "$VOLUME" ;;
> - *) dry "$FSCK" -f -p "$VOLUME" ;;
> - esac
> - fi
> + fi
> +
> + # We should really do the fsck before every resize.
Well user is always the master - if he is brave to go without it, let him do
what he wants (i.e. destroy his fs).
> - dry "$RESIZE_REISER" -s $NEWSIZE "$VOLUME"
> + dry "$RESIZE_REISER" -s "$NEWSIZE" "$VOLUME"
If a variable is under our full control, and we know the content could never
have a space inside - we do not need to put all of them to "".
Though I do not really mind about this one.
> +make_ext() {
> + device="$1"
> + fstyp="$2"
> + bsize=4
> +
> + if [ "$YES" ]; then
> + force="-F"
> + fi
> +
> + dry "mkfs.$fstyp" "$force" -b "$(($bsize*1024))" $device
I'd prefer to stay with upper case shell variable names.
> ####################
> # Resize filesystem
> ####################
> -resize() {
> +resize_fs() {
> NEWSIZE=$2
> - detect_fs "$1"
> + detect_fs "$1" || error "Cannot get FSTYPE of \"$VOLUME\""
> + verbose "\"$FSTYPE\" filesystem found on \"$VOLUME\""
> + if [ "$NEWSIZE" ]; then
> + is_natural $NEWSIZE || error "$NEWSIZE is not valid number for file system size"
> + fi
> detect_device_size
> verbose "Device \"$VOLUME\" size is $DEVSIZE bytes"
> # if the size parameter is missing use device size
> #if [ -n "$NEWSIZE" -a $NEWSIZE<
> test -z "$NEWSIZE"&& NEWSIZE=${DEVSIZE}b
> - IFS=$NL
> + local IFS=$NL
> case "$FSTYPE" in
> "ext3"|"ext2"|"ext4") resize_ext $NEWSIZE ;;
> "reiserfs") resize_reiser $NEWSIZE ;;
> "xfs") resize_xfs $NEWSIZE ;;
> *) error "Filesystem \"$FSTYPE\" on device \"$VOLUME\" is not supported by this tool" ;;
> esac || error "Resize $FSTYPE failed"
> - cleanup 0
Your recursion removal patch must be separate patch proposal.
> + case $i in
> + "size="*) [ -z "$size" ]&& size=${i##*=};;
> + [[:digit:]]*) [ -z "$size" ]&& size=${i##*=};;
> + +[[:digit:]]*) [ -z "$size" ]&& size=${i##*=};;
> + -[[:digit:]]*) [ -z "$size" ]&& size=${i##*=};;
> + *) error "Wrong option $i. (see: $TOOL --help)";;
> + esac
Another separate patch - to add support for +-
> +
> + [ "$GROUP" ]&& [ "$GROUP" != "$vgname" ]&& error "Some devices are in different"\
> + "group than the logical volume"\
> + "($lvname). Please provide unused"\
test -n -a != &&
> + # Avoid callinf lvresize in recurse
> + if [ "$FSADM_RESIZE_RECURSE" ]; then
> + error "Recursive lvresize call detected! Exiting."
> + fi
Recursion - separate patch...
> +#################################
> +# Check the device list to detect
> +# if there is not multiple groups
are
> +#################################
> +detect_device_group() {
> + ret=0
> + prev_vgname=""
> + vgs=""
> + tmp=$(mktemp)
> +
> + LANG=C "$LVM" vgs -o vg_name,pv_name --separator ' ' --noheadings --nosuffix 2> /dev/null | sed -e 's/^ *//' | sort | uniq> $tmp
> + NOT_IN_GROUP=
> + IN_GROUP=
> + for i in "$@"; do
> + cur_vgname=$("$GREP" -e "$i$" "$tmp" | cut -d' ' -f1)
> +
> + if [ -z "$cur_vgname" ]; then
> + NOT_IN_GROUP+="$i "
> + continue
> + else
> + IN_GROUP+="$i "
> + fi
> +
> + if [ -z "$prev_vgname" ]; then
> + prev_vgname=$cur_vgname
> + vgs=$cur_vgname
> + continue;
> + fi
> +
> + if [ "$cur_vgname" != "$prev_vgname" ]; then
> + ret=1
> + vgs+=" $cur_vgname"
Avoid using bashisms as much as you could - I think we could make the shell
script usable with traditional posix shell.
(look for checkbashism perl script)
i.e. vgs="$vgs $cur_name" - like you do elsewhere.
> + if [ $lines -eq 1 ]; then
> + vgname=$(echo $vgroups | sed -e 's/^ *//' | cut -d' ' -f1)
Assuming shell has enough power to do this on its own.
> + dmnumber=$(cat "$PROCDEVICES" | "$GREP" device-mapper | sed -e 's/^ *//')
"$CAT"
More information about the lvm-devel
mailing list