Skip to contentSkip to conversation

fsadm update

RFC PATCH Superseded Lukas Czerner 4 participants 46 messages Sep 27, 2011 – Oct 5, 2011

lore
Share & tools
lore
#0 lore raw

v3 cover fsadm update

Hello again,

this is third update of the fsadm.

Changes:

version 2:
The main difference is that all the big features are collapsed into one patch for better review (since some of the features depends on each other and has been developed in that way). Also one new test is added to exercise "remove" command.

version 3:
In version 2 some patches was unintentionally squashed together, which is fixed in this version.

Here is a link to the first version of the patch set:
http://www.redhat.com/archives/linux-lvm/2011-September/msg00046.html

Comments are welcomed.

Thanks!
-Lukas

Signature · 18 lines
[PATCH v3 01/18] fsadm: Add new commands create,list,add and remove
[PATCH v3 02/18] fsadm: Make all internal math in kilobytes
[PATCH v3 03/18] fsadm: Add simple configuration file
[PATCH v3 04/18] fsadm: Use DEFAULT_DEVICE_POOL when creating volume
[PATCH v3 05/18] fsadm: Add LVOL_PREFIX configuration option
[PATCH v3 06/18] fsadm: Fsck extN before resize only if it is not
[PATCH v3 07/18] fsadm: Only use readlink if link is provided
[PATCH v3 08/18] fsadm: Remove unnecessary modification of PATH
[PATCH v3 09/18] fsadm: Allow to specify lv in vg/lv format
[PATCH v3 10/18] fsadm: Umount ext2 file system prior resize
[PATCH v3 11/18] fsadm: Add help for new commands and update man
[PATCH v3 12/18] lvresize: Specify --resize-fs-only when going to
[PATCH v3 13/18] fsadm: remove -y (YES) option
[PATCH v3 14/18] test: add helper to compute aligned lv size
[PATCH v3 15/18] test: Add test for fsadm add command
[PATCH v3 16/18] test: Add test for fsadm create command
[PATCH v3 17/18] test: Add test for fsadm resize command
[PATCH v3 18/18] test: Add test for fsadm remove command
#1 lore raw

PATCH v3 1/18 fsadm: Add new commands create, list, add and remove

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.

---
 scripts/fsadm.sh |  978 ++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 908 insertions(+), 70 deletions(-)
scripts/fsadm.sh +908 -70
index c8cc5e0..f0da586 100755
@@ -14,10 +14,16 @@
 #
 # Author: Zdenek Kabelac <[email protected]>
 #
-# Script for resizing devices (usable for LVM resize)
+# Author: Lukas Czerner <[email protected]>
+#    - Significantly extended fsadm functionality
+#    - Implemented new commands "add", "create", "list", "remove"
+#    - Rework of "resize" command
+#
+# Utility to manage logical volumes
 #
 # Needed utilities:
-#   mount, umount, grep, readlink, blockdev, blkid, fsck, xfs_check
+#   mount, umount, grep, readlink, blockdev, blkid, fsck, xfs_check, bc, df
+#   xfs_db, mktemp
 #
 # ext2/ext3/ext4: resize2fs, tune2fs
 # reiserfs: resize_reiserfs, reiserfstune
@@ -29,7 +35,7 @@
 #   2 break detected
 #   3 unsupported online filesystem check for given mounted fs
 
-TOOL=fsadm
+TOOL=$(basename "$0")
 
 _SAVEPATH=$PATH
 PATH=/sbin:/usr/sbin:/bin:/usr/sbin:$PATH
@@ -54,6 +60,8 @@ READLINK=readlink
 READLINK_E="-e"
 FSCK=fsck
 XFS_CHECK=xfs_check
+BC=bc
+DF=df
 
 # user may override lvm location by setting LVM_BINARY
 LVM=${LVM_BINARY:-lvm}
@@ -63,7 +71,6 @@ DRY=0
 VERB=
 FORCE=
 EXTOFF=0
-DO_LVRESIZE=0
 FSTYPE=unknown
 VOLUME=unknown
 TEMPDIR="${TMPDIR:-/tmp}/${TOOL}_${RANDOM}$$/m"
@@ -73,10 +80,16 @@ BLOCKCOUNT=
 MOUNTPOINT=
 MOUNTED=
 REMOUNT=
+IN_GROUP=
+NOT_IN_GROUP=
+GROUP=
 PROCMOUNTS="/proc/mounts"
+PROCDEVICES="/proc/devices"
+PROCPARTITIONS="/proc/partitions"
 NULL="$DM_DEV_DIR/null"
 
-IFS_OLD=$IFS
+MAX_VGS=999
+
 # without bash $'\n'
 NL='
 '
@@ -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,"
@@ -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 "$*"
+}
+
+dry() {
+	dry_nofail "$@" || error "FAILED. Exitting!"
+}
+
+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
 }
 
 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
-
 	# 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"
+}
+
+#############################
+# 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
+			case "$i" in
+				"blocksize ="*) bsize=${i##* } ;;
+				"fdblocks ="*) fbcount=${i##* } ;;
+				"dblocks ="*) bcount=${i##* } ;;
+				"logblocks ="*) lbcount=${i##* } ;;
+				"agcount ="*) agcount=${i##* } ;;
+			esac
+		done
+		bcount=$(($bcount-$lbcount))
+		fbcount=$(($fbcount-(4+(4*$agcount))))
+
+		total=$(($bcount*$bsize))
+		TOTAL=$(humanize_size $total)
+		used=$((($bcount-$fbcount)*$bsize))
+		USED=$(humanize_size $used)
+		free=$(($fbcount*$bsize))
+		FREE=$(humanize_size $free)
+		return
+	fi
+	line=$("$DF" -k "$VOLUME" | "$GREP" -e "$MOUNTED$" | sed -e 's/  */ /g')
+	line=${line#* }
+	total=$(echo $line | cut -d' ' -f1)
+	TOTAL=$(humanize_size $total)
+	free=$(echo $line | cut -d' ' -f3)
+	FREE=$(humanize_size $free)
+	used=$(echo $line | cut -d' ' -f2)
+	USED=$(humanize_size $used)
+}
+
+detect_fs_size() {
+	if [ -z "$FSTYPE" ]; then
+		return
+	fi
+	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 )) ;;
 	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
+	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\""
 	RVOLUME=$VOLUME
 	case "$RVOLUME" in
           # hardcoded /dev  since udev does not create these entries elsewhere
@@ -189,10 +311,10 @@ detect_fs() {
 	esac
 	# use null device as cache file to be sure about the result
 	# not using option '-o value' to be compatible with older version of blkid
-	FSTYPE=$("$BLKID" -c "$NULL" -s TYPE "$VOLUME") || error "Cannot get FSTYPE of \"$VOLUME\""
+	FSTYPE=$("$BLKID" -c "$NULL" -s TYPE "$VOLUME") || return 1
 	FSTYPE=${FSTYPE##*TYPE=\"} # cut quotation marks
 	FSTYPE=${FSTYPE%%\"*}
-	verbose "\"$FSTYPE\" filesystem found on \"$VOLUME\""
+	return 0
 }
 
 # check if the given device is already mounted and where
@@ -200,10 +322,10 @@ detect_fs() {
 detect_mounted()  {
 	test -e "$PROCMOUNTS" || error "Cannot detect mounted device \"$VOLUME\""
 
-	MOUNTED=$("$GREP" ^"$VOLUME" "$PROCMOUNTS")
+	MOUNTED=$("$GREP" -e "^$VOLUME " "$PROCMOUNTS")
 
 	# for empty string try again with real volume name
-	test -z "$MOUNTED" && MOUNTED=$("$GREP" ^"$RVOLUME" "$PROCMOUNTS")
+	test -z "$MOUNTED" && MOUNTED=$("$GREP" -e "^$RVOLUME " "$PROCMOUNTS")
 
 	# cut device name prefix and trim everything past mountpoint
 	# echo translates \040 to spaces
@@ -212,8 +334,8 @@ detect_mounted()  {
 
 	# for systems with different device names - check also mount output
 	if test -z "$MOUNTED" ; then
-		MOUNTED=$(LANG=C "$MOUNT" | "$GREP" ^"$VOLUME")
-		test -z "$MOUNTED" && MOUNTED=$(LANG=C "$MOUNT" | "$GREP" ^"$RVOLUME")
+		MOUNTED=$(LANG=C "$MOUNT" | "$GREP" -e "^$VOLUME ")
+		test -z "$MOUNTED" && MOUNTED=$(LANG=C "$MOUNT" | "$GREP" -e "^$RVOLUME ")
 		MOUNTED=${MOUNTED##* on }
 		MOUNTED=${MOUNTED% type *} # allow type in the mount name
 	fi
@@ -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.
+	if [ -z "$MOUNTED" ] || [ "$REMOUNT" ]; then
+		# Forced fsck -f for umounted extX filesystem.
+		case "$-" in
+		  *i*) dry "$FSCK" "$YES" -f "$VOLUME" ;;
+		  *) dry "$FSCK" -f -p "$VOLUME" ;;
+		esac
 	fi
 
 	verbose "Resizing filesystem on device \"$VOLUME\" to $NEWSIZE bytes ($BLOCKCOUNT -> $NEWBLOCKCOUNT blocks of $BLOCKSIZE bytes)"
-	dry "$RESIZE_EXT" $FSFORCE "$VOLUME" $NEWBLOCKCOUNT
+	dry "$RESIZE_EXT" "$FSFORCE" "$VOLUME" "$NEWBLOCKCOUNT"
 }
 
 #############################
@@ -327,9 +451,9 @@ resize_reiser() {
 	decode_size $1 $BLOCKSIZE
 	verbose "Resizing \"$VOLUME\" $BLOCKCOUNT -> $NEWBLOCKCOUNT blocks ($NEWSIZE bytes, bs: $NEWBLOCKCOUNT)"
 	if [ -n "$YES" ]; then
-		echo y | dry "$RESIZE_REISER" -s $NEWSIZE "$VOLUME"
+		echo y | dry "$RESIZE_REISER" -s "$NEWSIZE" "$VOLUME"
 	else
-		dry "$RESIZE_REISER" -s $NEWSIZE "$VOLUME"
+		dry "$RESIZE_REISER" -s "$NEWSIZE" "$VOLUME"
 	fi
 }
 
@@ -357,7 +481,7 @@ resize_xfs() {
 	decode_size $1 $BLOCKSIZE
 	if [ $NEWBLOCKCOUNT -gt $BLOCKCOUNT ]; then
 		verbose "Resizing Xfs mounted on \"$MOUNTPOINT\" to fill device \"$VOLUME\""
-		dry "$RESIZE_XFS" $MOUNTPOINT
+		dry "$RESIZE_XFS" "$MOUNTPOINT"
 	elif [ $NEWBLOCKCOUNT -eq $BLOCKCOUNT ]; then
 		verbose "Xfs filesystem already has the right size"
 	else
@@ -365,25 +489,736 @@ resize_xfs() {
 	fi
 }
 
+#####################################
+# Create extN filesystem with respect
+# to the striped volume
+#####################################
+make_ext() {
+	device="$1"
+	fstyp="$2"
+	bsize=4
+
+	if [ "$YES" ]; then
+		force="-F"
+	fi
+
+	dry "mkfs.$fstyp" "$force" -b "$(($bsize*1024))" $device
+}
+
+############################################
+# Create a file system just using mkfs.fstyp
+############################################
+generic_make_fs() {
+	device=$1
+	fstyp=$2
+	bsize=4096
+
+	if [ "$YES" ]; then
+		force="-f"
+	fi
+
+	dry "mkfs.$fstyp" $force "$device"
+}
+
 ####################
 # 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
+}
+
+resize_lvolume() {
+	lvname=$1
+	newsize=$2
+	lvsize=$(LANG=C "$LVM" lvs -o lv_size --separator ' ' --noheadings --nosuffix --units b $lvname 2> /dev/null | sed -e 's/^ *//')
+	if [ $lvsize != $newsize ]; then
+		dry "$LVM" lvresize "$VERB" "$FORCE" -L"${newsize}"b "$lvname"
+	else
+		verbose "Logical volume already is of the size you're trying to resize it to"
+	fi
+}
+
+resize() {
+	local size=
+
+	# Special case for the situation we have been called from within the lvresize code.
+	if [ "$RESIZE_FS_ONLY" ]; then
+		resize_fs "$@"
+		return
+	fi
+
+	for i in "$@"; do
+		if [ -b "$i" ]; then
+			if [ -z "$devcount" ]; then
+				"$LVM" lvs "$i" &> /dev/null || error "$i is not valid logical volume"
+				lvname=$i
+				devcount=0
+				continue
+			fi
+			devices="$devices $i"
+			devcount=$(($devcount+1))
+			continue
+		fi
+		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
+	done
+
+	if [ -z "$size" ]; then
+		error "Please provide the size argument. (see: $TOOL --help )"
+	fi
+
+	[ -z $lvname ] && error "Logical volume to resize was not specified"
+	[ $devcount -gt 0 ] && [ "$shrink" ] && warn "While shrinking the file system we "\
+						     "do not need additional devices. "\
+						     "Those provided will be ignored"
+
+	# We need to add provided devices into particular group
+	if [ $devcount -gt 0 ] && [ -z $shrink ]; then
+		# Get the volume group of the logical volume
+		vgname=$(detect_volume_group $lvname)
+		[ -z $vgname ] && error "Wrong argument $lvname. (see: $TOOL --help)"
+
+		# Determine whether the devices are in the group
+		detect_device_group $devices ||
+			error "Devices are in different groups. Please run "\
+				"\"$TOOL list\" to get more information on layout."
+
+		[ "$GROUP" ] && [ "$GROUP" != "$vgname" ] && error "Some devices are in different"\
+								   "group than the logical volume"\
+								   "($lvname). Please provide unused"\
+								   "devices"
+		# Add missing devices into the group
+		if [ "$NOT_IN_GROUP" ]; then
+			dry "$LVM" vgextend "$VERB" $vgname $NOT_IN_GROUP
+		fi
+	fi
+
+	resizefs=
+	detect_fs $lvname
+	[ "$FSTYPE" ] && resizefs="-r"
+
+	# Avoid callinf lvresize in recurse
+	if [ "$FSADM_RESIZE_RECURSE" ]; then
+		error "Recursive lvresize call detected! Exiting."
+	fi
+
+	# We are going to call lvresize with -r parameter which will not only
+	# resize the logical volume, but also the file system (if present)
+	# by calling fsadm recursively with --resize-fs-only argument.
+	# To be able to do this we should set some appropriate variables.
+	export _FSADM_YES=$YES
+	export FSADM_RESIZE_RECURSE=1
+	unset FSADM_RUNNING
+
+	dry exec "$LVM" lvresize $VERB $FORCE $resizefs -L${size} "$lvname"
+
+}
+
+#################################
+# Check the device list to detect
+# if there is not multiple groups
+#################################
+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"
+		fi
+	done
+	rm -f $tmp
+	GROUP=$vgs
+	return $ret
+}
+
+#############################
+# Create a new logical volume
+#############################
+create() {
+	devcount=0
+	vg_create=0
+
+	for i in "$@"; do
+		if [ -b $i ]; then
+			devices="$devices $i"
+			devcount=$(($devcount+1))
+			continue
+		fi
+		case $i in
+			"stripesize"*) stripesize=${i##*=} ;;
+			"name"*) name=${i##*=} ;;
+			"fstyp"* | "fs"*) fstyp=${i##*=} ;;
+			"size"*) size=${i##*=} ;;
+			"stripes"*) stripes=${i##*=} ;;
+			*) if [ -z $vgname ]; then vgname=$i; else
+				error "Wrong option $i. (see: $TOOL --help)"
+			   fi ;;
+		esac
+	done
+
+	detect_device_group $devices ||
+		error "Devices are in different groups. Please run "\
+			"\"$TOOL list\" to get more information on layout."
+
+	if [ "$GROUP" ] && [ "$vgname" ];then
+		if [ "$vgname" != "$GROUP" ]; then
+			error "You specified group ($vgname) and devices which does "\
+				"belong to a different group ($GROUP). Either remove "\
+				"devices first, or specify devices which are free."
+		fi
+	elif [ "$GROUP" ]; then
+		vgname=$GROUP
+	fi
+
+	# Devices are not in any group so we should find some.
+	# Either create a new one, or use the existing one, if
+	# there is only one vgroup, or use the one with enough
+	# free space in it
+	if [ -z "$vgname" ]; then
+		vgroups=$("$LVM" vgs -o vg_name,vg_free --separator ' ' --noheadings --units b 2> /dev/null)
+		lines=$("$LVM" vgs -o vg_name,vg_free --separator ' ' --noheadings --units b 2> /dev/null | wc -l)
+
+		if [ $lines -eq 1 ]; then
+			vgname=$(echo $vgroups | sed -e 's/^ *//' | cut -d' ' -f1)
+		elif [ $lines -gt 1 ]; then
+			if [ -z $size ]; then
+				error "Not enough information to create" \
+					"logical volume"
+			fi
+			decode_size $size 1
+			new_size=$NEWBLOCKCOUNT
+			IFS=$NL
+			for i in $vgroups; do
+				vgsize=$(echo $i | sed -e 's/^ *//' | cut -d' ' -f2)
+				vgsize=${vgsize%%B}
+				if [ $vgsize -ge $new_size ]; then
+					vgname=$(echo $i | sed -e 's/^ *//' | cut -d' ' -f1)
+					break;
+				fi
+			done
+		fi
+	fi
+	IFS=$IFS_OLD
+
+	if [ -z "$vgname" ]; then
+		[ $devcount -eq 0 ] && error "No suitable device specified."
+		for i in $(seq -w $MAX_VGS); do
+			"$LVM" vgs vg${i} &> /dev/null
+			if [ $? -ne 0 ]; then
+				vgname="vg${i}"
+				break;
+			fi
+		done
+		vg_create=1
+	fi
+
+	[ -z "$vgname" ] && error "No suitable name for volume group found."
+
+	if [ "$stripesize" ] || [ "$stripes" ]; then
+		if [ -z "$stripes" ] && [ $devcount -eq 0 ]; then
+			error "Stripe size specified ($stripesize), but " \
+			      "neither devices, nor number of stripes " \
+			      "has been provided"
+		fi
+		[ $devcount -gt 0 ] && [ -z "$stripes" ] && stripes=$devcount
+		if [ "$stripesize" ]; then
+			striped="-i $stripes -I $stripesize"
+		else
+			striped="-i $stripes"
+		fi
+	fi
+
+	if [ "$name" ]; then
+		lvname="--name $name"
+	else
+		tmp=$(mktemp)
+		"$LVM" lvs "$vgname" --separator ' ' --noheadings > $tmp
+		for i in $(seq -w $MAX_VGS); do
+			"$GREP" -e " lvol${i} " $tmp &> /dev/null
+			if [ $? -ne 0 ]; then
+				name="lvol${i}"
+				lvname="--name $name"
+				break;
+			fi
+		done
+		rm -f $tmp
+	fi
+
+	[ -z "$lvname" ] && error "No suitable name for the logical volume."
+
+	if [ $vg_create -eq 1 ]; then
+		dry "$LVM" vgcreate "$VERB" $vgname $devices
+	elif [ ! -z "$NOT_IN_GROUP" ]; then
+		dry "$LVM" vgextend "$VERB" $vgname $NOT_IN_GROUP
+	fi
+
+
+	if [ -z $size ] && [ -n "$devices" ]; then
+		size="-l 100%PVS"
+	elif [ -z $size ]; then
+		size="-l 100%FREE"
+	else
+		size="-L $size"
+	fi
+	dry "$LVM" lvcreate "$VERB" $lvname $size $striped "$vgname" $devices
+	device="/dev/${vgname}/${name}"
+
+	guess=0
+	if [ -z $fstyp ]; then
+		fstyp=$(echo $TOOL | sed 's/^.*\.//g')
+		guess=1
+	fi
+
+	case $fstyp in
+		ext[234]) make_ext $device $fstyp;;
+		xfs|reiserfs) generic_make_fs $device $fstyp;;
+		*)	if [ $guess -eq 1 ]; then
+				return 0
+			else
+				error "Filesystem $fstyp is not supported"
+			fi
+			;;
+	esac
+}
+
+###############################
+# Remove device form the group
+###############################
+remove_device() {
+	vgname=$("$LVM" pvs "$1" -o vg_name --separator ' ' --noheadings) 2> /dev/null || return 1
+	vgname=$(echo $vgname | sed 's/[ \t]*//')
+	if [ "$vgname" ]; then
+		dry_nofail "$LVM" vgreduce "$vgname" "$device"
+	else
+		warn "Device $1 is not in any pool."
+	fi
+	return 0
+}
+
+#############################
+# Remove the logical volume
+# volume group, or the device
+# from the group
+#############################
+do_remove() {
+	item="${1%% }"
+	device=
+	MOUNTED=
+
+	# Block device has been given
+	if [ -b "$item" ]; then
+		device=$item
+	fi
+
+	# Mount point has been given
+	if [ -z "$device" ] && [ -d "$item" ]; then
+		mp=$(echo "$item" | sed -e 's/\/$//')
+
+		count=$("$GREP" " $mp " "$PROCMOUNTS" | wc -l)
+
+		if [ $count -eq 1 ]; then
+			device=$("$GREP" " $mp " "$PROCMOUNTS" | cut -d' ' -f1)
+		else
+			count=$(LANG=C "$MOUNT" | "$GREP" " $mp " | wc -l)
+			[ $count -ne 1 ] && warn "Could not find device mounted at $mp" && return
+			device=$(LANG=C "$MOUNT" | "$GREP" " $mp " | cut -d' ' -f1)
+		fi
+		VOLUME=$device
+	fi
+
+	if [ -z "$device" ]; then
+		"$LVM" vgs "$item" &> /dev/null
+		# Volume group has been given
+		if [ $? -eq 0 ]; then
+			dry "$LVM" vgremove "$FORCE" "$item"
+			return
+		fi
+	fi
+
+	[ -z "$device" ] && warn "$item is not a valid mount point, dm device nor volume group name."&& return
+
+	# Try to remove device from the group
+	remove_device "$device" && return 0
+
+	detect_mounted && try_umount
+	dry "$LVM" lvremove "$FORCE" "$device"
+	return
+}
+
+###############################
+# Iterate through the arguments
+# and do_remove on them
+###############################
+remove() {
+	if [ $# -eq 0 ]; then
+		error "Please specify what do you want to remove"\
+		      "(see $TOOL remove --help)."
+	fi
+	# help
+	if [ "$1" == "help" ]; then
+		echo "Usage: $TOOL remove [mount point | dm device | voulume group | device | --all]"
+		exit 0
+	elif [ "$1" == "--all" ]; then
+		list="$(LANG=C $LVM vgs -o vg_name --separator ' ' --noheadings --nosuffix --units b 2> /dev/null)"
+	else
+		list="$@"
+	fi
+
+	for item in $list; do
+		do_remove "$item"
+	done
+}
+
+#############################
+# List all file systems built
+# on top of DM device
+#############################
+list_filesystems() {
+	local IFS=$NL
+	local c=0
+	for line in $(LANG=C "$LVM" lvs -o lv_path,lv_size,segtype --noheadings --separator ' ' --nosuffix --units b 2> /dev/null); do
+		line=$(echo $line | sed -e 's/^ *\//\//')
+		volume=$(echo $line | cut -d' ' -f1)
+		[ "$volume" == "$last_volume" ] && continue
+		c=$((c+1))
+		local volumes[$c]=$volume
+		local segtype[$c]=$(echo $line | cut -d' ' -f3)
+		local lvsize[$c]=$(humanize_size $(echo $line | cut -d' ' -f2))
+		detect_fs "$volume"
+		detect_mounted
+		detect_fs_size
+		local total[$c]=$TOTAL
+		local fstype[$c]=$FSTYPE
+		local free[$c]=$FREE
+		local used[$c]=$USED
+		local mounted[$c]=$MOUNTED
+		FSTYPE=
+		FREE=
+		USED=
+		TOTAL=
+		MOUNTED=
+		last_volume=$volume
+	done
+
+	if [ $c -eq 0 ]; then
+		warn " No file systems suitable for managing by $TOOL found."
+		return
+	fi
+
+	len_volume=6
+	len_fstype=2
+	len_free=4
+	len_used=4
+	len_total=5
+	len_mounted=11
+	len_segtype=4
+	len_lvsize=4
+	for i in $(seq $c); do
+		local _volume=${volumes[$i]}
+		local _fstype=${fstype[$i]}
+		local _total=${total[$i]}
+		local _free=${free[$i]}
+		local _used=${used[$i]}
+		local _segtype=${segtype[$i]}
+		local _lvsize=${lvsize[$i]}
+		local _mounted=${mounted[$i]}
+		[ ${#_volume} -gt "0$len_volume" ] && len_volume=${#_volume}
+		[ ${#_fstype} -gt "0$len_fstype" ] && len_fstype=${#_fstype}
+		[ ${#_total} -gt "0$len_total" ] && len_total=${#_total}
+		[ ${#_free} -gt "0$len_free" ] && len_free=${#_free}
+		[ ${#_used} -gt "0$len_used" ] && len_used=${#_used}
+		[ ${#_segtype} -gt "0$len_segtype" ] && len_segtype=${#_segtype}
+		[ ${#_lvsize} -gt "0$len_lvsize" ] && len_lvsize=${#_lvsize}
+		[ ${#_mounted} -gt "0$len_mounted" ] && len_mounted=${#_mounted}
+	done
+
+	format="%-$(($len_volume+2))s %$(($len_lvsize))s  %-$(($len_fstype+2))s%$(($len_free))s  %$(($len_used))s  %$(($len_total))s  %-$(($len_segtype+2))s%-$(($len_mounted))s\n"
+	header=$(printf "$format" "Volume" "LV size" "FS" "Free" "Used" "Total" "Type" "Mount point")
+	separator=""
+	for i in $(seq ${#header}); do
+		separator+="-"
+	done
+	echo $separator
+	printf "$format" "Volume" "LV size" "FS" "Free" "Used" "Total" "Type" "Mount point"
+	echo $separator
+
+	for i in $(seq $c); do
+		printf "$format" "${volumes[$i]}" "${lvsize[$i]}" "${fstype[$i]}" "${free[$i]}" "${used[$i]}" "${total[$i]}" "${segtype[$i]}" "${mounted[$i]}"
+	done
+
+	echo $separator
+}
+
+###########################
+# List all non DM devices
+###########################
+list_devices() {
+	local IFS=$NL
+	tmp=$(mktemp)
+
+	c=0
+	dmnumber=$(cat "$PROCDEVICES" | "$GREP" device-mapper | sed -e 's/^  *//')
+	dmnumber=${dmnumber%% *}
+	LANG=C "$LVM" pvs -o pv_name,vg_name,pv_size,pv_free,pv_used --separator ' ' --noheadings --nosuffix --units b > $tmp
+	for line in $(cat "$PROCPARTITIONS" | tail -n +3 | sed -e 's/^  *//' | "$GREP" -v -e "^$dmnumber"); do
+		c=$((c+1))
+		line=$(echo $line | sed -e 's/  */ /g')
+		_total=$(($(echo $line | cut -d' ' -f3)*1024))
+		local total[$c]=$(humanize_size ${_total%%.*})
+		VOLUME=$(echo $line | cut -d' ' -f4)
+		RVOLUME="/dev/$(echo $line | cut -d' ' -f4)"
+		line=$("$GREP" -e " $RVOLUME " $tmp)
+		detect_mounted
+		if [ -z $MOUNTED ]; then
+			count=$("$GREP" "$VOLUME" "$PROCPARTITIONS" | wc -l)
+			[ $count -gt 1 ] && MOUNTED="PARTITIONED"
+		fi
+
+		if [ ! -z $line ]; then
+			line=$(echo $line | sed -e 's/^ *\//\//')
+			local group[$c]=$(echo $line | cut -d' ' -f2)
+			_total=$(echo $line | cut -d' ' -f3)
+			local total[$c]=$(humanize_size ${_total%%.*})
+			_free=$(echo $line | cut -d' ' -f4)
+			local free[$c]=$(humanize_size ${_free%%.*})
+			_used=$(echo $line | cut -d' ' -f5)
+			local used[$c]=$(humanize_size ${_used%%.*})
+		fi
+		local volumes[$c]=$RVOLUME
+		local mounted[$c]=$MOUNTED
+		free=
+		used=
+		total=
+		MOUNTED=
+	done
+	rm -f $tmp
+
+	if [ $c -eq 0 ]; then
+		warn " No devices found."
+		return
+	fi
+
+	len_volume=6
+	len_free=4
+	len_used=4
+	len_total=5
+	len_group=5
+	len_mounted=11
+	for i in $(seq $c); do
+		local _volume=${volumes[$i]}
+		local _free=${free[$i]}
+		local _used=${used[$i]}
+		local _total=${total[$i]}
+		local _group=${group[$i]}
+		local _mounted=${mounted[$i]}
+		[ ${#_volume} -gt "0$len_volume" ] && len_volume=${#_volume}
+		[ ${#_free} -gt "0$len_free" ] && len_free=${#_free}
+		[ ${#_used} -gt "0$len_used" ] && len_used=${#_used}
+		[ ${#_total} -gt "0$len_total" ] && len_total=${#_total}
+		[ ${#_group} -gt "0$len_group" ] && len_group=${#_group}
+		[ ${#_mounted} -gt "0$len_mounted" ] && len_mounted=${#_mounted}
+	done
+
+	format="%-$(($len_volume+2))s%$(($len_free))s  %$(($len_used))s  %$(($len_total))s  %-$(($len_group+2))s%-$(($len_mounted))s\n"
+	header=$(printf "$format" "Device" "Free" "Used" "Total" "Group" "Mount point")
+	separator=""
+	for i in $(seq ${#header}); do
+		separator+="-"
+	done
+	echo $separator
+	printf "$format" "Device" "Free" "Used" "Total" "Group" "Mount point"
+	echo $separator
+
+	for i in $(seq $c); do
+		printf "$format" "${volumes[$i]}" "${free[$i]}" "${used[$i]}" "${total[$i]}" "${group[$i]}" "${mounted[$i]}"
+	done
+
+	echo $separator
+}
+
+################################
+# List all pools (volume groups)
+################################
+list_pool() {
+	local IFS=$NL
+	c=0
+	for line in $(LANG=C "$LVM" vgs -o vg_name,pv_count,vg_size,vg_free --separator ' ' --noheadings --nosuffix --units b 2> /dev/null); do
+		c=$((c+1))
+		line=$(echo $line | sed -e 's/^ *//')
+		local group[$c]=$(echo $line | cut -d' ' -f1)
+		local devices[$c]=$(echo $line | cut -d' ' -f2)
+		_total=$(echo $line | cut -d' ' -f3)
+		_free=$(echo $line | cut -d' ' -f4)
+		_used=$((${_total%%.*}-${_free%%.*}))
+		local used[$c]=$(humanize_size ${_used%%.*})
+		local total[$c]=$(humanize_size ${_total%%.*})
+		local free[$c]=$(humanize_size ${_free%%.*})
+	done
+
+	if [ $c -eq 0 ]; then
+		warn " No pools found on the system."
+		return
+	fi
+
+	len_group=5
+	len_devices=6
+	len_free=4
+	len_used=4
+	len_total=5
+	for i in $(seq $c); do
+		local _group=${group[$i]}
+		local _devices=${devices[$i]}
+		local _free=${free[$i]}
+		local _used=${used[$i]}
+		local _total=${total[$i]}
+		[ ${#_group} -gt "0$len_group" ] && len_group=${#_group}
+		[ ${#_devices} -gt "0$len_devices" ] && len_devices=${#_devices}
+		[ ${#_free} -gt "0$len_free" ] && len_free=${#_free}
+		[ ${#_used} -gt "0$len_used" ] && len_used=${#_used}
+		[ ${#_total} -gt "0$len_total" ] && len_total=${#_total}
+	done
+
+	format="%-$(($len_group+2))s%-$(($len_devices+2))s%$(($len_free))s  %$(($len_used))s  %$(($len_total))s\n"
+	header=$(printf "$format" "Group" "Devices" "Free" "Used" "Total")
+	separator=""
+	for i in $(seq ${#header}); do
+		separator+="-"
+	done
+	echo $separator
+	printf "$format" "Group" "Devices" "Free" "Used" "Total"
+	echo $separator
+
+	for i in $(seq $c); do
+		printf "$format" "${group[$i]}" "${devices[$i]}" "${free[$i]}" "${used[$i]}" "${total[$i]}"
+	done
+	echo $separator
+}
+
+#############################
+# List the available storage
+# and file systems
+#############################
+list() {
+	for i in "$@"; do
+		case "$i" in
+			"filesystems" | "fs") list_filesystems ;;
+			"devices" | "dev") list_devices ;;
+			"pool") list_pool ;;
+			*) error "Wrong option $i. (see: $TOOL --help)"
+		esac
+	done
+
+	if [ $# -eq 0 ]; then
+		list_devices
+		echo ""
+		list_pool
+		echo ""
+		list_filesystems
+	fi
+}
+
+detect_volume_group() {
+	_vg=$("$LVM" lvs -o vg_name --separator ' ' --noheadings $1 2> /dev/null)
+	ret=$?
+	echo "$_vg" | sed -e 's/^ *//'
+	return $ret
+}
+
+############################
+# Add devices into the group
+############################
+add() {
+	vg_create=0
+	devcount=0
+	tmp=$(mktemp)
+
+	for i in "$@"; do
+		if [ -b "$i" ]; then
+			devices="$devices $i"
+			devcount=$(($devcount+1))
+			continue
+		elif [ -z "$vgname" ]; then vgname=$i; else
+			error "Wrong argument $i. (see: $TOOL --help)"
+		fi
+	done
+
+	[ $devcount -eq 0 ] && error "No suitable device specified."
+	"$LVM" vgs -o vg_name --separator ' ' --noheadings | sed -e 's/^ *//' > $tmp
+
+	if [ -z "$vgname" ]; then
+		lines=$(wc -l $tmp)
+		lines=${lines%% *}
+
+		if [ $lines -eq 1 ]; then
+			vgname=$(cut -d' ' -f1 $tmp)
+		elif [ $lines -gt 1 ]; then
+			list_pool
+			rm -f $tmp
+			error "Please, specify group you want to add the device into"
+		elif [ $lines -eq 0 ]; then
+			vgname="vg001"
+			vg_create=1
+		fi
+	else
+		"$GREP" -e "$vgname" $tmp &> /dev/null
+		vg_create=$?
+	fi
+	rm -f $tmp
+
+	[ -z "$vgname" ] && error "No suitable name for volume group found."
+	detect_device_group $devices
+
+	if [ $vg_create -eq 1 ]; then
+		dry "$LVM" vgcreate "$VERB" "$vgname" $devices
+	elif [ ! -z "$NOT_IN_GROUP" ]; then
+		dry "$LVM" vgextend "$VERB" "$vgname" $NOT_IN_GROUP
+	else
+		warn "Nothing to do"
+	fi
 }
 
 ####################################
@@ -399,7 +1234,8 @@ diff_dates() {
 # Check filesystem
 ###################
 check() {
-	detect_fs "$1"
+	detect_fs "$1" || error "Cannot get FSTYPE of \"$VOLUME\""
+	verbose "\"$FSTYPE\" filesystem found on \"$VOLUME\""
 	if detect_mounted ; then
 		verbose "Skipping filesystem check for device \"$VOLUME\" as the filesystem is mounted on $MOUNTED";
 		cleanup 3
@@ -407,8 +1243,7 @@ check() {
 
 	case "$FSTYPE" in
 	  "ext2"|"ext3"|"ext4")
-		IFS_CHECK=$IFS
-		IFS=$NL
+		local IFS=$NL
 		for i in $(LANG=C "$TUNE_EXT" -l "$VOLUME"); do
 			case "$i" in
 			  "Last mount"*) LASTMOUNT=${i##*: } ;;
@@ -425,15 +1260,14 @@ check() {
 			fi
 			;;
 		esac
-		IFS=$IFS_CHECK
 	esac
 
 	case "$FSTYPE" in
 	  "xfs") dry "$XFS_CHECK" "$VOLUME" ;;
 	  *)    # check if executed from interactive shell environment
 		case "$-" in
-		  *i*) dry "$FSCK" $YES $FORCE "$VOLUME" ;;
-		  *) dry "$FSCK" $FORCE -p "$VOLUME" ;;
+		  *i*) dry "$FSCK" "$YES" "$FORCE" "$VOLUME" ;;
+		  *) dry "$FSCK" "$FORCE" -p "$VOLUME" ;;
 		esac
 	esac
 }
@@ -475,20 +1309,24 @@ do
 	  "-n"|"--dry-run") DRY=1 ;;
 	  "-f"|"--force") FORCE="-f" ;;
 	  "-e"|"--ext-offline") EXTOFF=1 ;;
+	  "-o"|"--resize-fs-only") RESIZE_FS_ONLY=1 ;;
 	  "-y"|"--yes") YES="-y" ;;
-	  "-l"|"--lvresize") DO_LVRESIZE=1 ;;
-	  "check") CHECK="$2" ; shift ;;
-	  "resize") RESIZE="$2"; NEWSIZE="$3" ; shift 2 ;;
+	  "-l"|"--lvresize") ;;
+	  "check") COMMAND=$1; shift; ARGS="$@"; break ;;
+	  "resize") COMMAND=$1; shift; ARGS="$@"; break ;;
+	  "create") COMMAND=$1; shift; ARGS="$@"; break ;;
+	  "list") COMMAND=$1; shift; ARGS="$@"; break ;;
+	  "add") COMMAND=$1; shift; ARGS="$@"; break ;;
+	  "remove") COMMAND=$1; shift; ARGS="$@"; break ;;
 	  *) error "Wrong argument \"$1\". (see: $TOOL --help)"
 	esac
 	shift
 done
 
-if [ -n "$CHECK" ]; then
-	check "$CHECK"
-elif [ -n "$RESIZE" ]; then
-	export FSADM_RUNNING="fsadm"
-	resize "$RESIZE" "$NEWSIZE"
-else
+if [ -z "$COMMAND" ]; then
 	error "Missing command. (see: $TOOL --help)"
 fi
+
+export FSADM_RUNNING="fsadm"
+$COMMAND $ARGS
+cleanup 0
Signature · 1 line
1.7.4.4
#2 lore raw

PATCH v3 1/18 fsadm: Add new commands create, list, add and remove

Dne 27.9.2011 15:42, Lukas Czerner napsal(a):

Quoted 22 lines

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.

scripts/fsadm.sh tool_usage() { · line 114

Quoted 7 lines
scripts/fsadm.sh +0 -1
@@ -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.

Quoted 9 lines
scripts/fsadm.sh +4 -1
@@ -115,13 +127,37 @@ error() {
  	cleanup 1
  }
 
-dry() {
+warn() {
+	echo "$TOOL: $@">&2
+}
+

scripts/fsadm.sh error() { · line 142

Quoted 10 lines
scripts/fsadm.sh +4 -1
+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.

Quoted 4 lines
scripts/fsadm.sh +4 -0
+float_math() {
+	if [ $# -le 0 ]; then
+		return
+	fi

scripts/fsadm.sh error() { · line 161

Quoted 10 lines
scripts/fsadm.sh +9 -0
+	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)

Quoted 11 lines
scripts/fsadm.sh +0 -1
  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"
 

scripts/fsadm.sh cleanup() { · line 149

Quoted 10 lines
scripts/fsadm.sh +0 -10
-	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.

Quoted 20 lines
scripts/fsadm.sh +16 -0
  	# 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" ;;

scripts/fsadm.sh cleanup() { · line 204

Quoted 10 lines
scripts/fsadm.sh +10 -0
+		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).

Quoted 24 lines
scripts/fsadm.sh +24 -0
+#############################
+# 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)
+}
+
+############################

scripts/fsadm.sh cleanup() { · line 238

Quoted 10 lines
scripts/fsadm.sh +10 -0
+# 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"

scripts/fsadm.sh cleanup() { · line 258

Quoted 1 line
scripts/fsadm.sh +1 -0
+	line=$("$DF" -k "$VOLUME" | "$GREP" -e "$MOUNTED$" | sed -e 's/  */ /g')

"$SED"

scripts/fsadm.sh cleanup() { · line 260

Quoted 2 lines
scripts/fsadm.sh +2 -0
+	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.

scripts/fsadm.sh cleanup() { · line 271

Quoted 4 lines
scripts/fsadm.sh +4 -0
+detect_fs_size() {
+	if [ -z "$FSTYPE" ]; then
+		return
+	fi

Why ?
(Imho it's handled by case below via return 1

Quoted 16 lines
scripts/fsadm.sh +8 -4
+	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 )) ;;

scripts/fsadm.sh cleanup() { · line 291

Quoted 10 lines
scripts/fsadm.sh +6 -2
-	 *[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.

scripts/fsadm.sh cleanup() { · line 174

Quoted 9 lines
scripts/fsadm.sh +0 -7
  	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.

scripts/fsadm.sh cleanup() { · line 304

Quoted 13 lines
scripts/fsadm.sh +5 -2
+	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.)

scripts/fsadm.sh detect_fs() { · line 325

Quoted 2 lines
scripts/fsadm.sh +1 -1
-	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)

Quoted 4 lines
scripts/fsadm.sh +0 -0
@@ -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

scripts/fsadm.sh resize_ext() { · line 422

Quoted 10 lines
scripts/fsadm.sh +3 -7
-		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).

scripts/fsadm.sh resize_reiser() { · line 456

Quoted 2 lines
scripts/fsadm.sh +1 -1
-		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.

scripts/fsadm.sh resize_xfs() { · line 505

Quoted 10 lines
scripts/fsadm.sh +10 -0
+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.

Quoted 16 lines
scripts/fsadm.sh +6 -2
  ####################
  # 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<

scripts/fsadm.sh resize_xfs() { · line 386

Quoted 10 lines
scripts/fsadm.sh +1 -2
  	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.

Review

Quoted 7 lines
(quoted diff) +7 -0
+		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 +-

scripts/fsadm.sh resize_xfs() { · line 610

Quoted 4 lines
scripts/fsadm.sh +4 -0
+
+		[ "$GROUP" ]&&  [ "$GROUP" != "$vgname" ]&&  error "Some devices are in different"\
+								   "group than the logical volume"\
+								   "($lvname). Please provide unused"\

test -n -a != &&

scripts/fsadm.sh resize_xfs() { · line 625

Quoted 4 lines
scripts/fsadm.sh +4 -0
+	# Avoid callinf lvresize in recurse
+	if [ "$FSADM_RESIZE_RECURSE" ]; then
+		error "Recursive lvresize call detected! Exiting."
+	fi

Recursion - separate patch...

scripts/fsadm.sh resize_xfs() { · line 641

Quoted 3 lines
scripts/fsadm.sh +3 -0
+#################################
+# Check the device list to detect
+# if there is not multiple groups

are

Quoted 19 lines
scripts/fsadm.sh +19 -0
+#################################
+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

scripts/fsadm.sh resize_xfs() { · line 670

Quoted 10 lines
scripts/fsadm.sh +10 -0
+
+		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.

scripts/fsadm.sh resize_xfs() { · line 726

Quoted 2 lines
scripts/fsadm.sh +2 -0
+		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.

scripts/fsadm.sh resize_xfs() { · line 1004

Quoted 1 line
scripts/fsadm.sh +1 -0
+	dmnumber=$(cat "$PROCDEVICES" | "$GREP" device-mapper | sed -e 's/^  *//')

"$CAT"

#3 lore raw

PATCH v3 1/18 fsadm: Add new commands create, list, add and remove

Quoted 22 lines · Zdenek Kabelac

Dne 27.9.2011 15:42, Lukas Czerner napsal(a):

Quoted 20 lines

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.

Quoted 14 lines
Quoted 11 lines
(quoted diff) +0 -0
@@ -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"
(quoted diff) +0 -1
-	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.

This is only removed from the tool_usage(), as you can see. I am not removing it from the code, but only from the documentation since it is not needed in the new code.

Quoted 19 lines
Quoted 19 lines
scripts/fsadm.sh +5 -2
@@ -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 $@"
-	$@
Quoted 8 lines

scripts/fsadm.sh error() { · line 142

Quoted 3 lines
scripts/fsadm.sh +3 -0
+	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.

Actually what looks weird that we call function we are executing commands with "dry()" because it really is dry only if --dry-run has been specified. I only coped with that, but I would be really in favour of changing it to run() or similar.

dry_nofail() is different from dry() only in the way it handles failures of invoked command. In dry() we are calling error() in that case, but dry_nofail() is not calling error, but rather report the return value, so its user can handle the failure by himself. Please see where it is used.

Quoted 16 lines

So for now skip it.

Quoted 11 lines
scripts/fsadm.sh +11 -0
+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;;
Quoted 8 lines

scripts/fsadm.sh error() { · line 161

Quoted 3 lines
scripts/fsadm.sh +2 -0
+		*) 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.

I can do that, however it is just one step in allowing float math in fsadm. I have done that so we have easier work in the future and because it is useful, but I have no intention of making fsadm fully float number aware, at least for now.

Quoted 19 lines

(Though needs new packaging deps to be properly handled - wondering how we could make this easier for package maintainers)

Quoted 11 lines
scripts/fsadm.sh +0 -1
  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"
 
Quoted 13 lines

scripts/fsadm.sh cleanup() { · line 149

Quoted 11 lines
scripts/fsadm.sh +0 -8
-	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"

scripts/fsadm.sh +0 -2
-	fi
-

Again - please do not remove things you do not understand.

Oh, but I do understand. Please read the new code, before trying to insult me. But maybe it would be better to separate resize command changes into separate patch.

Quoted 35 lines

And also thing like this needs to be in a separate patch - not bundled in hundreds patch lines.

Quoted 28 lines
  	# error exit status for break
  	exit ${1:-1}
  }
scripts/fsadm.sh +24 -0
+#####################################
+# 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"
Quoted 8 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+}
+

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).

Regardless on what I think of this "option" I'll try to look at this.

Quoted 151 lines
Quoted 37 lines
scripts/fsadm.sh +34 -0
+#############################
+# 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
Quoted 1 line

dblocks logblocks agcount' $VOLUME); do

"$XFS_DB"

"$VOLUME"

Quoted 2 lines
(quoted diff) +1 -0
+	line=$("$DF" -k "$VOLUME" | "$GREP" -e "$MOUNTED$" | sed -e 's/  */

/g')

"$SED"

scripts/fsadm.sh cleanup() { · line 260

Quoted 2 lines
scripts/fsadm.sh +2 -0
+	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.

scripts/fsadm.sh cleanup() { · line 271

Quoted 4 lines
scripts/fsadm.sh +4 -0
+detect_fs_size() {
+	if [ -z "$FSTYPE" ]; then
+		return
+	fi

Why ?
(Imho it's handled by case below via return 1

Quoted 16 lines
scripts/fsadm.sh +8 -4
+	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 )) ;;

scripts/fsadm.sh cleanup() { · line 291

Quoted 10 lines
scripts/fsadm.sh +6 -2
-	 *[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.

scripts/fsadm.sh cleanup() { · line 174

Quoted 9 lines

esac

scripts/fsadm.sh +0 -7
-	#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.

Quoted 14 lines
scripts/fsadm.sh +4 -2
+	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\""

(quoted diff) +1 -0
+	VOLUME=$("$READLINK" "$READLINK_E" "$DM_DEV_DIR/$VOLUME") || error
Quoted 1 line

"Cannot get readlink \"$1\""

Do not modify same thing in multiple patches
(The advantage of shuffling these things in front of the whole set.)

scripts/fsadm.sh detect_fs() { · line 325

Quoted 2 lines
scripts/fsadm.sh +1 -1
-	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)

Quoted 10 lines
(quoted diff) +0 -0
@@ -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

scripts/fsadm.sh +0 -5
-		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" ;;
Quoted 8 lines

scripts/fsadm.sh resize_ext() { · line 422

Quoted 5 lines
scripts/fsadm.sh +3 -2
-			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).

You're missing the point, resize2fs requires that the file system is checked with e2fsck -f prior the resize, if on of the conditions happen

1. file system was not cleanly umounted
2. file system contain errors, or is invalid
3. it was not checked since the last time it was mounted

It is just to make sure that the file system is consistent and we will not make everything worse by messing with metadata further.

Thanks!
-Lukas

#4 lore raw

PATCH v3 1/18 fsadm: Add new commands create, list, add and remove

Dne 4.10.2011 14:13, Lukas Czerner napsal(a):

Quoted 39 lines · Zdenek Kabelac
Quoted 23 lines

Dne 27.9.2011 15:42, Lukas Czerner napsal(a):

Quoted 20 lines

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.

Quoted 14 lines
Quoted 11 lines
(quoted diff) +0 -0
@@ -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"
(quoted diff) +0 -1
-	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.

Quoted 4 lines

This is only removed from the tool_usage(), as you can see. I am not removing it from the code, but only from the documentation since it is not needed in the new code.

Ok this made it clear that such functional change than needs a separate patch, which will demonstrate how it's going to be used and how it is matching current behavior.

Quoted 14 lines
Quoted 5 lines
Quoted 5 lines
scripts/fsadm.sh +0 -5
-	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
Quoted 8 lines

scripts/fsadm.sh cleanup() { · line 149

Quoted 6 lines
(quoted diff) +0 -3
-		unset FSADM_RUNNING
-		test -n "$LVM_BINARY"&&   PATH=$_SAVEPATH
-		dry exec "$LVM" lvresize $VERB $FORCE -r -L${NEWSIZE}b

"$VOLUME_ORIG"

scripts/fsadm.sh +0 -2
-	fi
-

Again - please do not remove things you do not understand.

Quoted 7 lines

Oh, but I do understand. Please read the new code, before trying to insult me. But maybe it would be better to separate resize command changes into separate patch.

Quoted 3 lines

And also thing like this needs to be in a separate patch - not bundled in hundreds patch lines.

It's nothing about insulting - this is very tricky change - so unless this is extracted to separate patch to be properly reviewed for all combination and whether all cases keep all VG locks properly.
(Thus patch needs to explain how it will resize filesystem and LV when user calls fsadm resize and this operation could not be broken with another parallel lvm operation in both directions - upsize/downsize while current syntax is still usable)

Quoted 6 lines
Quoted 2 lines

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).

You're missing the point, resize2fs requires that the file system is checked with e2fsck -f prior the resize, if on of the conditions happen

Quoted 7 lines

1. file system was not cleanly umounted
2. file system contain errors, or is invalid
3. it was not checked since the last time it was mounted

It is just to make sure that the file system is consistent and we will not make everything worse by messing with metadata further.

If the user wants to risk resize2fs without fsck - it's his decision (and current -f with fsadm allows it) - and since we are not like gnome3 - we allow users to kill their systems in number of ways already, so no reason to eliminate this one.

i.e. mkfs, mount, umount, resize LV, resize2fs -f works without fsck and
makes errorless result - so we support.

Zdenek

#5 lore raw

PATCH v3 1/18 fsadm: Add new commands create, list, add and remove

Quoted 43 lines · Zdenek Kabelac

Dne 4.10.2011 14:13, Lukas Czerner napsal(a):

Quoted 42 lines · Zdenek Kabelac
Quoted 40 lines

Dne 27.9.2011 15:42, Lukas Czerner napsal(a):

Quoted 29 lines

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.

(quoted diff) +0 -0
@@ -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"

(quoted diff) +0 -1
-	echo "    -l | --lvresize     Resize given device (if it is LVM
Quoted 6 lines

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.

Quoted 9 lines
Quoted 4 lines

This is only removed from the tool_usage(), as you can see. I am not removing it from the code, but only from the documentation since it is not needed in the new code.

Ok this made it clear that such functional change than needs a separate patch, which will demonstrate how it's going to be used and how it is matching current behavior.

Agreed, as I said I'll separate resize update into separate patch.

Quoted 45 lines
Quoted 16 lines
Quoted 15 lines

scripts/fsadm.sh cleanup() { · line 149

Quoted 13 lines
scripts/fsadm.sh +0 -8
-	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"

scripts/fsadm.sh +0 -2
-	fi
-

Again - please do not remove things you do not understand.

Quoted 7 lines

Oh, but I do understand. Please read the new code, before trying to insult me. But maybe it would be better to separate resize command changes into separate patch.

Quoted 3 lines

And also thing like this needs to be in a separate patch - not bundled in hundreds patch lines.

It's nothing about insulting - this is very tricky change - so unless this is extracted to separate patch to be properly reviewed for all combination and whether all cases keep all VG locks properly.
(Thus patch needs to explain how it will resize filesystem and LV when user calls fsadm resize and this operation could not be broken with another parallel lvm operation in both directions - upsize/downsize while current syntax is still usable)

Quoted 11 lines
Quoted 3 lines

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).

You're missing the point, resize2fs requires that the file system is checked with e2fsck -f prior the resize, if on of the conditions happen

1. file system was not cleanly umounted
2. file system contain errors, or is invalid
3. it was not checked since the last time it was mounted

Quoted 8 lines
Quoted 3 lines

It is just to make sure that the file system is consistent and we will not make everything worse by messing with metadata further.

If the user wants to risk resize2fs without fsck - it's his decision (and current -f with fsadm allows it) - and since we are not like gnome3 - we allow users to kill their systems in number of ways already, so no reason to eliminate this one.

Well, this is just wrong in so many ways. Running fsck.ext4 -f to force the full check is not the same as skipping the fsck before resize in the tool which is suppose to do-it-all. We have to pick the safe way here.

Quoted 3 lines

i.e. mkfs, mount, umount, resize LV, resize2fs -f works without fsck and
makes errorless result - so we support.

Also it makes me think that you do not really see what is in the code. There is nothing about about 'force', fsck is done anyway (in the old code - your code) in the case that

[ "$NEWBLOCKCOUNT" -lt "$BLOCKCOUNT" -o "$EXTOFF" -eq 1 ]

and

if test -n "$MOUNTED"

so please, read before you write.

Also your arguments are out of the line. There are different levels of granularity of force, and if user force things he does not expect things will be done in wrong way. That said, when I use force I do not want to be asked about stupid questions, but I certaily do not want fsck before resize2fs to be skipped.

You just have to understand that we are using multiple tools in fsadm and just passing force to all of them would be incredibly stupid, even if you properly document it.

User is just not able to guess what different sort of tools we are using in the path he is about to invoke, and guess where we are going to use his 'force' and what consequences will that have. We just HAVE to pick the most logical and most safe way of doing things. But your way of doing thing is just to get as many option as you can aggregate and throw it to the user while screaming RTFM, that is easy, but it is also wrong.

-Lukas

Quoted 5 lines

Zdenek

#6 lore raw

PATCH v3 1/18 fsadm: Add new commands create, list, add and remove

Dne 5.10.2011 10:02, Lukas Czerner napsal(a):

Quoted 22 lines · Zdenek Kabelac
Quoted 21 lines

Dne 4.10.2011 14:13, Lukas Czerner napsal(a):

Quoted 18 lines · Zdenek Kabelac
Quoted 6 lines

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).

You're missing the point, resize2fs requires that the file system is checked with e2fsck -f prior the resize, if on of the conditions happen

1. file system was not cleanly umounted
2. file system contain errors, or is invalid
3. it was not checked since the last time it was mounted

It is just to make sure that the file system is consistent and we will not make everything worse by messing with metadata further.

Quoted 9 lines
Quoted 4 lines

If the user wants to risk resize2fs without fsck - it's his decision (and current -f with fsadm allows it) - and since we are not like gnome3 - we allow users to kill their systems in number of ways already, so no reason to eliminate this one.

Well, this is just wrong in so many ways. Running fsck.ext4 -f to force the full check is not the same as skipping the fsck before resize in the tool which is suppose to do-it-all. We have to pick the safe way here.

Tool is not meant to be used only in a 'safe' way - tool is supposed to fully usable even various force scenarios - i.e. when fsck support options '-y' we should be able to use - even if some extX developers think it's very dangerous option and should never ever be used - it's there and it has its use. (I've used it already many times, and nothing unexpectedly bad happened).

What we could do here better is to draw some state diagram and eventually add few more options - but we will not strip down the usefulness of fsadm only for clean filesystem and leave the user to run fsck -y on its - as we can do that for him.

Quoted 8 lines
Quoted 3 lines

i.e. mkfs, mount, umount, resize LV, resize2fs -f works without fsck and
makes errorless result - so we support.

Also it makes me think that you do not really see what is in the code. There is nothing about about 'force', fsck is done anyway (in the old code - your code) in the case that

Quoted 7 lines

[ "$NEWBLOCKCOUNT" -lt "$BLOCKCOUNT" -o "$EXTOFF" -eq 1 ]

and

if test -n "$MOUNTED"

so please, read before you write.

man lvresize --nofsck

Quoted 5 lines

Also your arguments are out of the line. There are different levels of granularity of force, and if user force things he does not expect things will be done in wrong way. That said, when I use force I do not want to be asked about stupid questions, but I certaily do not want fsck before resize2fs to be skipped.

We already allow that - since there are situation you may need to use it.

Quoted 3 lines

You just have to understand that we are using multiple tools in fsadm and just passing force to all of them would be incredibly stupid, even if you properly document it.

Certainly personal opinion.

As already said - provide state diagram and suggest more options (in lvm we use -ff for some situations - so maybe more levels of force)

But we need to support what we already can do with fsadm tool.

If the user puts --force anywhere on the command line - he surely needs to expect, that some things may cause damage.

Quoted 2 lines

his 'force' and what consequences will that have. We just HAVE to pick the most logical and most safe way of doing things. But your way of

There is no 'safe' way with --force - that's the purpose of force. However there could be multiple force levels.

Zdenek

#7 lore raw

PATCH v3 1/18 fsadm: Add new commands create, list, add and remove

Quoted 35 lines · Zdenek Kabelac

Dne 5.10.2011 10:02, Lukas Czerner napsal(a):

Quoted 29 lines · Zdenek Kabelac
Quoted 22 lines

Dne 4.10.2011 14:13, Lukas Czerner napsal(a):

Quoted 12 lines · Zdenek Kabelac
Quoted 7 lines

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).

You're missing the point, resize2fs requires that the file system is checked with e2fsck -f prior the resize, if on of the conditions happen

Quoted 7 lines

1. file system was not cleanly umounted
2. file system contain errors, or is invalid
3. it was not checked since the last time it was mounted

It is just to make sure that the file system is consistent and we will not make everything worse by messing with metadata further.

Quoted 5 lines

If the user wants to risk resize2fs without fsck - it's his decision (and current -f with fsadm allows it) - and since we are not like gnome3 - we allow
users to kill their systems in number of ways already, so no reason to eliminate this one.

Quoted 4 lines

Well, this is just wrong in so many ways. Running fsck.ext4 -f to force the full check is not the same as skipping the fsck before resize in the tool which is suppose to do-it-all. We have to pick the safe way here.

Quoted 5 lines

Tool is not meant to be used only in a 'safe' way - tool is supposed to fully usable even various force scenarios - i.e. when fsck support options '-y' we should be able to use - even if some extX developers think it's very dangerous option and should never ever be used - it's there and it has its use. (I've used it already many times, and nothing unexpectedly bad happened).

No you're wrong, we are aggregating option from multiple tools, and using them only in the way it is useful for that tool. We certainly do not want to map every possible options, and this is even more obvious if this option is not safe, or needed for that matter.

I have never said that -y should never be used, I just said that user needs to know what is he doing, which is not the case when you're using -y to allow automatic umount and doing fsck -y .

Quoted 5 lines

What we could do here better is to draw some state diagram and eventually add few more options - but we will not strip down the usefulness of fsadm only for clean filesystem and leave the user to run fsck -y on its - as we can do that for him.

Yeah, doing fsck -y on your laptop, where you probably does not care about your data wary much is fine if you're ok with it. But if you're in enterprise environment, it definitely is not a smart thing to do, because you do not want to lose your data and ideally you want to find the root cause of the problem to avoid the corruption on future, hence fsck -y is just dumbest thing to do. Not talking about the fact that '-y' does not mean "Please fix my file system', but rather 'Answer yes to everything', which does not make sense if you do not know what happened. On the other hand '-p' means 'Please, fix my file system in the bast way you can'.

Quoted 12 lines
Quoted 12 lines
Quoted 5 lines

i.e. mkfs, mount, umount, resize LV, resize2fs -f works without fsck and
makes errorless result - so we support.

Also it makes me think that you do not really see what is in the code. There is nothing about about 'force', fsck is done anyway (in the old code - your code) in the case that

[ "$NEWBLOCKCOUNT" -lt "$BLOCKCOUNT" -o "$EXTOFF" -eq 1 ]

Quoted 7 lines
Quoted 5 lines

and

if test -n "$MOUNTED"

so please, read before you write.

man lvresize --nofsck

That is so funny, it just proves how little you know about your own code, this is the second time I am saying you that it just does not work that way, and fsck will be run if the above conditions are met, regardless on whether you do run lvresize --nofsck of fsadm -f.

Quoted 9 lines
Quoted 7 lines

Also your arguments are out of the line. There are different levels of granularity of force, and if user force things he does not expect things will be done in wrong way. That said, when I use force I do not want to be asked about stupid questions, but I certaily do not want fsck before resize2fs to be skipped.

We already allow that - since there are situation you may need to use it.

No you do not. You allow umount without interaction, but that will imply insane fsck -y, hence using that in script is really bad thing.

Quoted 7 lines
Quoted 4 lines

You just have to understand that we are using multiple tools in fsadm and just passing force to all of them would be incredibly stupid, even if you properly document it.

Certainly personal opinion.

Quoted 7 lines

As already said - provide state diagram and suggest more options (in lvm we use -ff for some situations - so maybe more levels of force)

But we need to support what we already can do with fsadm tool.

If the user puts --force anywhere on the command line - he surely needs to expect, that some things may cause damage.

nope, that is not true for all the tools generally. fsck.extN -f fill not do anything bad.

Quoted 6 lines
Quoted 3 lines

his 'force' and what consequences will that have. We just HAVE to pick the most logical and most safe way of doing things. But your way of

There is no 'safe' way with --force - that's the purpose of force. However there could be multiple force levels.

Please do not spread your absolutely undocumented '-f' and '-ff' lvm options which no user knows what is actually does, any further.

And I do not have to remind you that force does not usually imply "skip fsck".

Quoted 3 lines

Zdenek

-Lukas

#8 lore raw

PATCH v3 1/18 fsadm: Add new commands create, list, add and remove

Quoted 5 lines · Lukas Czerner
Quoted 4 lines · Zdenek Kabelac

Dne 5.10.2011 10:02, Lukas Czerner napsal(a):

Quoted 3 lines · Zdenek Kabelac
Quoted 2 lines

Dne 4.10.2011 14:13, Lukas Czerner napsal(a):

Quoted 1 line · Zdenek Kabelac

You both lost me several messages ago!

For *changes* to functionality, what I'm looking for is:

Description of how it works today in *all* relevant code paths/option
combinations/invocation methods;

Description/explanation of how it will work *after* the change has been made
in all the same cases as now plus any new ones

Explanation of/justification for the difference.

Currently in lvm:

--yes is meant to mean 'answer yes to all questions'
These are generally informative questions telling the user what the tool will do and seeking confirmation. On its own, they should not be dangerous or risk corruption. Example: removing an LV that is active but not currently being used. The idea of such questions is to point out to the user the actual consequences of the command to help to make sure they understand what will happen.

--force is meant to mean 'permit operations that might not always be wise but
sometimes do need to be done - assume the user knows what they are doing'. Normally a warning of what will happen is issued and confirmation is required, but --yes can be used to avoid that confirmation.

For dangerous things that wouldn't normally make sense but occasionally can be justified, like destroying PVs while they are in (inactive) VGs, --force can be required to be repeated twice.

Now, let's try to understand:
How these options are currently used in fsadm-related tools; What's good/bad here and perhaps ought to change.

Alasdair

#9 lore raw

PATCH v3 1/18 fsadm: Add new commands create, list, add and remove

Quoted 7 lines · Alasdair G Kergon
Quoted 5 lines · Lukas Czerner
Quoted 4 lines · Zdenek Kabelac

Dne 5.10.2011 10:02, Lukas Czerner napsal(a):

Quoted 3 lines · Zdenek Kabelac
Quoted 2 lines

Dne 4.10.2011 14:13, Lukas Czerner napsal(a):

Quoted 1 line · Zdenek Kabelac

Hi Alasdair,

Quoted 21 lines

You both lost me several messages ago!

For *changes* to functionality, what I'm looking for is:

Description of how it works today in *all* relevant code paths/option
combinations/invocation methods;

Description/explanation of how it will work *after* the change has been made
in all the same cases as now plus any new ones

Explanation of/justification for the difference.

Currently in lvm:

--yes is meant to mean 'answer yes to all questions'
These are generally informative questions telling the user what the tool will do and seeking confirmation. On its own, they should not be dangerous or risk corruption. Example: removing an LV that is active but not currently being used. The idea of such questions is to point out to the user the actual consequences of the command to help to make sure they understand what will happen.

Quoted 4 lines

--force is meant to mean 'permit operations that might not always be wise but
sometimes do need to be done - assume the user knows what they are doing'. Normally a warning of what will happen is issued and confirmation is required, but --yes can be used to avoid that confirmation.

Do you mean --force in the last sentence ?

Quoted 6 lines

For dangerous things that wouldn't normally make sense but occasionally can be justified, like destroying PVs while they are in (inactive) VGs, --force can be required to be repeated twice.

Quoted 6 lines

Now, let's try to understand:
How these options are currently used in fsadm-related tools; What's good/bad here and perhaps ought to change.

Alasdair

The first misunderstanding was the use of -f in lvm toos. Because I can not see -y|--yes in the documentation I simply used force for not asking questions about active volumes to be removed. This can be fixed, if '-y' really works.

The other is, that using -y option to answer yes to umount mounted file system to be checked is ok, but then running fsck with that '-y' option does not make sense and it is also dangerous. But Zdenek is convinced that it has to stay there (I say it is a bug).

And lastly, the problem with using --force to skip the fsck before attempt to resize the file system. Using force in my opinion does not imply skipping fsck, and I believe that is how it is implemented in lvresize as well, where you have to use "--nofsck" (which is broken btw).

I believe that fscking file system before the resize is the right thing to do and I do not see any reason to allow it to be overridden in fsadm, although we probably should check for 'last mount' and 'last check' fs attributes to see if it was checked after the last mount and fsck it otherwise.

Generally the problem is that we could not agree on almost anything and due to 'it stays as it is, deal with it' altitude it seems to be that even the discussion is not allowed.

Honestly, when I started to poke the fsadm I quickly realized that if I am going to improve the functionality in the bigger scale, it will be more and more painful to do this in Bash. So maybe it will be easier for both sides to just forget about fsadm and make a separate project for example in Python. Because after all I would like this tool to be able to manage btrfs as well someday and since that does not have anything to do with lvm, I fear that it will be a major problem to get this into upstream fsadm. So what people think ?

Thanks!
-Lukas

#10 lore raw

PATCH v3 1/18 fsadm: Add new commands create, list, add and remove

Quoted 32 lines · Lukas Czerner
Quoted 7 lines · Alasdair G Kergon
Quoted 5 lines · Lukas Czerner
Quoted 4 lines · Zdenek Kabelac

Dne 5.10.2011 10:02, Lukas Czerner napsal(a):

Quoted 3 lines · Zdenek Kabelac
Quoted 2 lines

Dne 4.10.2011 14:13, Lukas Czerner napsal(a):

Quoted 1 line · Zdenek Kabelac

Hi Alasdair,

Quoted 20 lines

You both lost me several messages ago!

For *changes* to functionality, what I'm looking for is:

Description of how it works today in *all* relevant code paths/option
combinations/invocation methods;

Description/explanation of how it will work *after* the change has been made
in all the same cases as now plus any new ones

Explanation of/justification for the difference.

Currently in lvm:

--yes is meant to mean 'answer yes to all questions'
These are generally informative questions telling the user what the tool will do and seeking confirmation. On its own, they should not be dangerous or risk corruption. Example: removing an LV that is active but not currently being used. The idea of such questions is to point out to the user the actual consequences of the command to help to make sure they understand what will happen.

Btw, there is no such option in lvm. It is not documented and it does not work, at least for vgremove and lvremove. However it does work in pvremove, also it is documented there.

Quoted 58 lines
Quoted 5 lines

--force is meant to mean 'permit operations that might not always be wise but
sometimes do need to be done - assume the user knows what they are doing'. Normally a warning of what will happen is issued and confirmation is required, but --yes can be used to avoid that confirmation.

Do you mean --force in the last sentence ?

Quoted 6 lines

For dangerous things that wouldn't normally make sense but occasionally can be justified, like destroying PVs while they are in (inactive) VGs, --force can be required to be repeated twice.

Quoted 6 lines

Now, let's try to understand:
How these options are currently used in fsadm-related tools; What's good/bad here and perhaps ought to change.

Alasdair

The first misunderstanding was the use of -f in lvm toos. Because I can not see -y|--yes in the documentation I simply used force for not asking questions about active volumes to be removed. This can be fixed, if '-y' really works.

The other is, that using -y option to answer yes to umount mounted file system to be checked is ok, but then running fsck with that '-y' option does not make sense and it is also dangerous. But Zdenek is convinced that it has to stay there (I say it is a bug).

And lastly, the problem with using --force to skip the fsck before attempt to resize the file system. Using force in my opinion does not imply skipping fsck, and I believe that is how it is implemented in lvresize as well, where you have to use "--nofsck" (which is broken btw).

I believe that fscking file system before the resize is the right thing to do and I do not see any reason to allow it to be overridden in fsadm, although we probably should check for 'last mount' and 'last check' fs attributes to see if it was checked after the last mount and fsck it otherwise.

Generally the problem is that we could not agree on almost anything and due to 'it stays as it is, deal with it' altitude it seems to be that even the discussion is not allowed.

Honestly, when I started to poke the fsadm I quickly realized that if I am going to improve the functionality in the bigger scale, it will be more and more painful to do this in Bash. So maybe it will be easier for both sides to just forget about fsadm and make a separate project for example in Python. Because after all I would like this tool to be able to manage btrfs as well someday and since that does not have anything to do with lvm, I fear that it will be a major problem to get this into upstream fsadm. So what people think ?

Thanks!
-Lukas

#11 lore raw

PATCH v3 1/18 fsadm: Add new commands create, list, add and remove

In reply to Lukas Czerner
Quoted 54 lines · Lukas Czerner
Quoted 7 lines · Alasdair G Kergon
Quoted 5 lines · Lukas Czerner
Quoted 4 lines · Zdenek Kabelac

Dne 5.10.2011 10:02, Lukas Czerner napsal(a):

Quoted 3 lines · Zdenek Kabelac
Quoted 2 lines

Dne 4.10.2011 14:13, Lukas Czerner napsal(a):

Quoted 1 line · Zdenek Kabelac

Hi Alasdair,

Quoted 21 lines

You both lost me several messages ago!

For *changes* to functionality, what I'm looking for is:

Description of how it works today in *all* relevant code paths/option
combinations/invocation methods;

Description/explanation of how it will work *after* the change has been made
in all the same cases as now plus any new ones

Explanation of/justification for the difference.

Currently in lvm:

--yes is meant to mean 'answer yes to all questions'
These are generally informative questions telling the user what the tool will do and seeking confirmation. On its own, they should not be dangerous or risk corruption. Example: removing an LV that is active but not currently being used. The idea of such questions is to point out to the user the actual consequences of the command to help to make sure they understand what will happen.

Quoted 4 lines

--force is meant to mean 'permit operations that might not always be wise but
sometimes do need to be done - assume the user knows what they are doing'. Normally a warning of what will happen is issued and confirmation is required, but --yes can be used to avoid that confirmation.

Do you mean --force in the last sentence ?

Quoted 11 lines

For dangerous things that wouldn't normally make sense but occasionally can be justified, like destroying PVs while they are in (inactive) VGs, --force can be required to be repeated twice.

Now, let's try to understand:
How these options are currently used in fsadm-related tools; What's good/bad here and perhaps ought to change.

Alasdair

The first misunderstanding was the use of -f in lvm toos. Because I can not see -y|--yes in the documentation I simply used force for not asking questions about active volumes to be removed. This can be fixed, if '-y' really works.

Quoted 4 lines

The other is, that using -y option to answer yes to umount mounted file system to be checked is ok, but then running fsck with that '-y' option does not make sense and it is also dangerous. But Zdenek is convinced that it has to stay there (I say it is a bug).

fsck should *never* be run on a mounted file system. If that is what you are saying happens, that is 100% a bug and will cause data loss.

Quoted 11 lines

And lastly, the problem with using --force to skip the fsck before attempt to resize the file system. Using force in my opinion does not imply skipping fsck, and I believe that is how it is implemented in lvresize as well, where you have to use "--nofsck" (which is broken btw).

I believe that fscking file system before the resize is the right thing to do and I do not see any reason to allow it to be overridden in fsadm, although we probably should check for 'last mount' and 'last check' fs attributes to see if it was checked after the last mount and fsck it otherwise.

We should definitely follow the rules of the file system for when it is safe to resize. Having a clean file system verified by fsck or the checks you mention would both be fine, over riding those checks is not.

I think in general our tools should "do the right thing" and fall back to having a human do manual fsck checks and so on. Running fsck in automatic "yes" mode in a script seems quite iffy to me.

Quoted 14 lines

Generally the problem is that we could not agree on almost anything and due to 'it stays as it is, deal with it' altitude it seems to be that even the discussion is not allowed.

Honestly, when I started to poke the fsadm I quickly realized that if I am going to improve the functionality in the bigger scale, it will be more and more painful to do this in Bash. So maybe it will be easier for both sides to just forget about fsadm and make a separate project for example in Python. Because after all I would like this tool to be able to manage btrfs as well someday and since that does not have anything to do with lvm, I fear that it will be a major problem to get this into upstream fsadm. So what people think ?

Quoted 2 lines

Thanks!
-Lukas

If we do need to expand beyond the boundaries of fsadm, that is fine with me. I think that the goal should be to present a pluggable & flexible user command that will work for all local file systems including btrfs so we should not require lvm to be there.

thanks!

ric

#12 lore raw

PATCH v3 1/18 fsadm: Add new commands create, list, add and remove

In reply to Lukas Czerner
Quoted 7 lines · Lukas Czerner
Quoted 4 lines · Alasdair G Kergon

--force is meant to mean 'permit operations that might not always be wise but
sometimes do need to be done - assume the user knows what they are doing'. Normally a warning of what will happen is issued and confirmation is required, but --yes can be used to avoid that confirmation.

Do you mean --force in the last sentence ?

No, I meant --yes.

It works like this:

If you use --yes, you will not be asked any questions.
Where the code would otherwise have issued a question, it proceeds as if you
answered 'yes, do it'.

If you use --force, it can open up *extra* things the code allows to happen. Without it, the code would just give an error 'I'm not going to let you do that'.

So, the number of --force options supplied controls *what* the code lets you do.

--yes just controls whether or not you get interactive prompts advising you what will happen and giving you chances to bail out.

Quoted 4 lines

The first misunderstanding was the use of -f in lvm toos. Because I can not see -y|--yes in the documentation I simply used force for not asking questions about active volumes to be removed. This can be fixed, if '-y' really works.

I can see things missing from the man pages...

Quoted 4 lines

The other is, that using -y option to answer yes to umount mounted file system to be checked is ok, but then running fsck with that '-y' option does not make sense and it is also dangerous. But Zdenek is convinced that it has to stay there (I say it is a bug).

Well what we could do there is:
--yes only: we do NOT supply -y to fsck. (Ideally we'd like to choose
non-interactive options though i.e. if it hits something that would normally require an interactive response, then instead of prompting, fail the operation. Perhaps -y should map to 'fsck -n' OR 'fsck -p'? It should be non-interactive AND safe.)

--force --yes: This could perhaps map to -y, if there's a case
for that. Or perhaps '--force --force --yes'

Quoted 1 line

lvresize as well, where you have to use "--nofsck" (which is broken btw).

The reason for --nofsck is if you *know* the filesystem is ok because you already ran a check (for example). But if the fs is indeed ok, won't a basic fsck be fast anyway, (just checking fs state in superblock rather than performing a full check) such that it doesn't matter if it gets run again?

Alasdair

#13 lore raw

PATCH v3 1/18 fsadm: Add new commands create, list, add and remove

Quoted 17 lines · Alasdair G Kergon
Quoted 7 lines · Lukas Czerner
Quoted 4 lines · Alasdair G Kergon

--force is meant to mean 'permit operations that might not always be wise but
sometimes do need to be done - assume the user knows what they are doing'. Normally a warning of what will happen is issued and confirmation is required, but --yes can be used to avoid that confirmation.

Do you mean --force in the last sentence ?

No, I meant --yes.

It works like this:

If you use --yes, you will not be asked any questions.
Where the code would otherwise have issued a question, it proceeds as if you
answered 'yes, do it'.

Quoted 8 lines

If you use --force, it can open up *extra* things the code allows to happen. Without it, the code would just give an error 'I'm not going to let you do that'.

So, the number of --force options supplied controls *what* the code lets you do.

--yes just controls whether or not you get interactive prompts advising you what will happen and giving you chances to bail out.

Ok, now it is clear to me. However --yes option does not exist in some lvm tolls (e.g. lvremove, vgremove)

Quoted 13 lines
Quoted 5 lines

The first misunderstanding was the use of -f in lvm toos. Because I can not see -y|--yes in the documentation I simply used force for not asking questions about active volumes to be removed. This can be fixed, if '-y' really works.

I can see things missing from the man pages...

Quoted 4 lines

The other is, that using -y option to answer yes to umount mounted file system to be checked is ok, but then running fsck with that '-y' option does not make sense and it is also dangerous. But Zdenek is convinced that it has to stay there (I say it is a bug).

Quoted 6 lines

Well what we could do there is:
--yes only: we do NOT supply -y to fsck. (Ideally we'd like to choose
non-interactive options though i.e. if it hits something that would normally require an interactive response, then instead of prompting, fail the operation. Perhaps -y should map to 'fsck -n' OR 'fsck -p'? It should be non-interactive AND safe.)

Non interactive would be fsck -p which actually will try to fix the file system, not answer yes to all questions, however I am not sure if we want it to map it to --yes option.

Quoted 3 lines

--force --yes: This could perhaps map to -y, if there's a case
for that. Or perhaps '--force --force --yes'

The problem here is, that --yes is also used to umount the file system prior to the fsck. I would be in favor of just not supporting fsck -y in fsadm, but having fsck -p for non interactive use.

Quoted 7 lines
Quoted 2 lines

lvresize as well, where you have to use "--nofsck" (which is broken btw).

The reason for --nofsck is if you *know* the filesystem is ok because you already ran a check (for example). But if the fs is indeed ok, won't a basic fsck be fast anyway, (just checking fs state in superblock rather than performing a full check) such that it doesn't matter if it gets run again?

We can easily get the information whether the file system has been checked since the last umount and skip the check automatically, since resize2fs will not require it. However basic check is not very useful, since as you said it'll only check the state of the super block, which is not enough and resize2fs will refuse to run in the case that the file system has not received full check from the last mount.

So as you can see, --nofsck is actually not needed for the case you've described and I would prefer not allowing user to override that behaviour and always do fsck prior to the resize2fs, but I can not insist on it - it just can not be done with --force, but rather with --nofsck.

Quoted 2 lines

Alasdair

-Lukas

#14 lore raw

PATCH v3 2/18 fsadm: Make all internal math in kilobytes

In reply to Lukas Czerner

We should make all math in fsadm to count with kilobytes (if possible) because it will bypass the problem of having too big numbers. Also the smaller granularity might have troubles with 512B alignment.

---
 scripts/fsadm.sh |   79 ++++++++++++++++++++++++++---------------------------
 1 files changed, 39 insertions(+), 40 deletions(-)
scripts/fsadm.sh +39 -40
index f0da586..58ff0ab 100755
@@ -188,15 +188,14 @@ humanize_size() {
 		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" ;;
+		0) unit="KiB" ;;
+		1) unit="MiB" ;;
+		2) unit="GiB" ;;
+		3) unit="TiB" ;;
+		4) unit="PiB" ;;
+		5) unit="EiB" ;;
+		6) unit="ZiB" ;;
+		7) unit="YiB" ;;
 		*) unit="???" ;;
 	esac
 	echo "$size $unit"
@@ -217,11 +216,11 @@ get_ext_size() {
 		esac
 	done
 
-	total=$(($bcount*$bsize))
+	total=$(($bcount*$bsize/1024))
 	TOTAL=$(humanize_size $total)
-	used=$((($bcount-$fbcount)*$bsize))
+	used=$((($bcount-$fbcount)*$bsize/1024))
 	USED=$(humanize_size $used)
-	free=$((($fbcount-$rbcount)*$bsize))
+	free=$((($fbcount-$rbcount)*$bsize/1024))
 	FREE=$(humanize_size $free)
 }
 
@@ -247,11 +246,11 @@ get_xfs_size() {
 		bcount=$(($bcount-$lbcount))
 		fbcount=$(($fbcount-(4+(4*$agcount))))
 
-		total=$(($bcount*$bsize))
+		total=$(($bcount*$bsize/1024))
 		TOTAL=$(humanize_size $total)
-		used=$((($bcount-$fbcount)*$bsize))
+		used=$((($bcount-$fbcount)*$bsize/1024))
 		USED=$(humanize_size $used)
-		free=$(($fbcount*$bsize))
+		free=$(($fbcount*$bsize/1024))
 		FREE=$(humanize_size $free)
 		return
 	fi
@@ -278,21 +277,20 @@ detect_fs_size() {
 }
 
 # convert parameter from Exa/Peta/Tera/Giga/Mega/Kilo/Bytes and blocks
-# (2^(60/50/40/30/20/10/0))
+# (2^(60/50/40/30/20/10/0)) into Kilobytes (KiB)
 decode_size() {
 	case "$1" in
-	 *[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 )) ;;
+	 *[eE]) NEWSIZE=$(float_math "${1%[eE]} * 1125899906842624") ;;
+	 *[pP]) NEWSIZE=$(float_math "${1%[pP]} * 1099511627776") ;;
+	 *[tT]) NEWSIZE=$(float_math "${1%[tT]} * 1073741824") ;;
+	 *[gG]) NEWSIZE=$(float_math "${1%[gG]} * 1048576") ;;
+	 *[mM]) NEWSIZE=$(float_math "${1%[mM]} * 1024") ;;
+	 *[kK]) NEWSIZE=${1%[kK]} ;;
+	 *[bB]) NEWSIZE=$(float_math "${1%[bB]} / 1024") ;;
 	esac
 
 	NEWSIZE=${NEWSIZE%%.*}
-	NEWBLOCKCOUNT=$(float_math "$NEWSIZE / $2")
+	NEWBLOCKCOUNT=$(float_math "($NEWSIZE / $2) * 1024")
 	NEWBLOCKCOUNT=${NEWBLOCKCOUNT%%.*}
 }
 
@@ -428,8 +426,8 @@ resize_ext() {
 		esac
 	fi
 
-	verbose "Resizing filesystem on device \"$VOLUME\" to $NEWSIZE bytes ($BLOCKCOUNT -> $NEWBLOCKCOUNT blocks of $BLOCKSIZE bytes)"
-	dry "$RESIZE_EXT" "$FSFORCE" "$VOLUME" "$NEWBLOCKCOUNT"
+	verbose "Resizing filesystem on device \"$VOLUME\" to $NEWSIZE KiB ($BLOCKCOUNT -> $NEWBLOCKCOUNT blocks of $BLOCKSIZE bytes)"
+	dry "$RESIZE_EXT" $FSFORCE "$VOLUME" "$NEWBLOCKCOUNT"
 }
 
 #############################
@@ -449,11 +447,11 @@ resize_reiser() {
 	done
 	validate_parsing "$TUNE_REISER"
 	decode_size $1 $BLOCKSIZE
-	verbose "Resizing \"$VOLUME\" $BLOCKCOUNT -> $NEWBLOCKCOUNT blocks ($NEWSIZE bytes, bs: $NEWBLOCKCOUNT)"
+	verbose "Resizing \"$VOLUME\" $BLOCKCOUNT -> $NEWBLOCKCOUNT blocks ($NEWSIZE KiB, bs: $NEWBLOCKCOUNT)"
 	if [ -n "$YES" ]; then
-		echo y | dry "$RESIZE_REISER" -s "$NEWSIZE" "$VOLUME"
+		echo y | dry "$RESIZE_REISER" -s "${NEWSIZE}K" "$VOLUME"
 	else
-		dry "$RESIZE_REISER" -s "$NEWSIZE" "$VOLUME"
+		dry "$RESIZE_REISER" -s "${NEWSIZE}K" "$VOLUME"
 	fi
 }
 
@@ -547,9 +545,10 @@ resize_fs() {
 resize_lvolume() {
 	lvname=$1
 	newsize=$2
-	lvsize=$(LANG=C "$LVM" lvs -o lv_size --separator ' ' --noheadings --nosuffix --units b $lvname 2> /dev/null | sed -e 's/^ *//')
+
+	lvsize=$(LANG=C "$LVM" lvs -o lv_size --separator ' ' --noheadings --nosuffix --units k $lvname 2> /dev/null | sed -e 's/^ *//')
 	if [ $lvsize != $newsize ]; then
-		dry "$LVM" lvresize "$VERB" "$FORCE" -L"${newsize}"b "$lvname"
+		dry "$LVM" lvresize "$VERB" "$FORCE" -L "${newsize}k" "$lvname"
 	else
 		verbose "Logical volume already is of the size you're trying to resize it to"
 	fi
@@ -719,8 +718,8 @@ create() {
 	# there is only one vgroup, or use the one with enough
 	# free space in it
 	if [ -z "$vgname" ]; then
-		vgroups=$("$LVM" vgs -o vg_name,vg_free --separator ' ' --noheadings --units b 2> /dev/null)
-		lines=$("$LVM" vgs -o vg_name,vg_free --separator ' ' --noheadings --units b 2> /dev/null | wc -l)
+		vgroups=$("$LVM" vgs -o vg_name,vg_free --separator ' ' --noheadings --units k 2> /dev/null)
+		lines=$("$LVM" vgs -o vg_name,vg_free --separator ' ' --noheadings --units k 2> /dev/null | wc -l)
 
 		if [ $lines -eq 1 ]; then
 			vgname=$(echo $vgroups | sed -e 's/^ *//' | cut -d' ' -f1)
@@ -903,7 +902,7 @@ remove() {
 		echo "Usage: $TOOL remove [mount point | dm device | voulume group | device | --all]"
 		exit 0
 	elif [ "$1" == "--all" ]; then
-		list="$(LANG=C $LVM vgs -o vg_name --separator ' ' --noheadings --nosuffix --units b 2> /dev/null)"
+		list="$(LANG=C $LVM vgs -o vg_name --separator ' ' --noheadings --nosuffix --units k 2> /dev/null)"
 	else
 		list="$@"
 	fi
@@ -920,7 +919,7 @@ remove() {
 list_filesystems() {
 	local IFS=$NL
 	local c=0
-	for line in $(LANG=C "$LVM" lvs -o lv_path,lv_size,segtype --noheadings --separator ' ' --nosuffix --units b 2> /dev/null); do
+	for line in $(LANG=C "$LVM" lvs -o lv_path,lv_size,segtype --noheadings --separator ' ' --nosuffix --units k 2> /dev/null); do
 		line=$(echo $line | sed -e 's/^ *\//\//')
 		volume=$(echo $line | cut -d' ' -f1)
 		[ "$volume" == "$last_volume" ] && continue
@@ -1003,11 +1002,11 @@ list_devices() {
 	c=0
 	dmnumber=$(cat "$PROCDEVICES" | "$GREP" device-mapper | sed -e 's/^  *//')
 	dmnumber=${dmnumber%% *}
-	LANG=C "$LVM" pvs -o pv_name,vg_name,pv_size,pv_free,pv_used --separator ' ' --noheadings --nosuffix --units b > $tmp
-	for line in $(cat "$PROCPARTITIONS" | tail -n +3 | sed -e 's/^  *//' | "$GREP" -v -e "^$dmnumber"); do
+	LANG=C "$LVM" pvs -o pv_name,vg_name,pv_size,pv_free,pv_used --separator ' ' --noheadings --nosuffix --units k > $tmp
+	for line in $(cat "$PROCPARTITIONS" | tail -n +3 | sed -e 's/^  *//' | grep -v -e "^$dmnumber"); do
 		c=$((c+1))
 		line=$(echo $line | sed -e 's/  */ /g')
-		_total=$(($(echo $line | cut -d' ' -f3)*1024))
+		_total=$(echo $line | cut -d' ' -f3)
 		local total[$c]=$(humanize_size ${_total%%.*})
 		VOLUME=$(echo $line | cut -d' ' -f4)
 		RVOLUME="/dev/$(echo $line | cut -d' ' -f4)"
@@ -1086,7 +1085,7 @@ list_devices() {
 list_pool() {
 	local IFS=$NL
 	c=0
-	for line in $(LANG=C "$LVM" vgs -o vg_name,pv_count,vg_size,vg_free --separator ' ' --noheadings --nosuffix --units b 2> /dev/null); do
+	for line in $(LANG=C "$LVM" vgs -o vg_name,pv_count,vg_size,vg_free --separator ' ' --noheadings --nosuffix --units k 2> /dev/null); do
 		c=$((c+1))
 		line=$(echo $line | sed -e 's/^ *//')
 		local group[$c]=$(echo $line | cut -d' ' -f1)
Signature · 1 line
1.7.4.4
#15 lore raw

PATCH v3 2/18 fsadm: Make all internal math in kilobytes

Dne 27.9.2011 15:42, Lukas Czerner napsal(a):

Quoted 3 lines

We should make all math in fsadm to count with kilobytes (if possible) because it will bypass the problem of having too big numbers. Also the smaller granularity might have troubles with 512B alignment.

I don't like this one - if you have problems with big numbers - bugreport bash. You are probably covering up different bug here.

Zdenek

#16 lore raw

PATCH v3 2/18 fsadm: Make all internal math in kilobytes

Quoted 8 lines · Zdenek Kabelac

Dne 27.9.2011 15:42, Lukas Czerner napsal(a):

Quoted 3 lines

We should make all math in fsadm to count with kilobytes (if possible) because it will bypass the problem of having too big numbers. Also the smaller granularity might have troubles with 512B alignment.

I don't like this one - if you have problems with big numbers - bugreport bash. You are probably covering up different bug here.

I am not sure you do not "like" this one, it does not seem like a real reason, could you be more specific ? This patch just forces internal math to be done in kilobytes, since there is no real reason to do it in bytes.

It has no implications for the user interface.

Also, please when you reply to my patches, could you cc me as well ? This is not the only list I am sending patches to so I can not just check all lists for replies.

Thanks!
-Lukas

Quoted 9 lines

Zdenek

--
lvm-devel mailing list
lvm-devel at redhat.com
https://www.redhat.com/mailman/listinfo/lvm-devel

#17 lore raw

PATCH v3 3/18 fsadm: Add simple configuration file

In reply to Lukas Czerner
---
 scripts/fsadm.sh |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)
scripts/fsadm.sh +30 -0
index 58ff0ab..98921d3 100755
@@ -88,6 +88,9 @@ PROCDEVICES="/proc/devices"
 PROCPARTITIONS="/proc/partitions"
 NULL="$DM_DEV_DIR/null"
 
+CONFIG_PATHS="/etc/fsadm.conf ~/fsadm.conf"
+CONFIG=
+
 MAX_VGS=999
 
 # without bash $'\n'
@@ -1271,6 +1274,22 @@ check() {
 	esac
 }
 
+set_default_config() {
+	[ -z "$DEFAULT_DEVICE_POOL" ] && DEFAULT_DEVICE_POOL="device_pool"
+	[ -z "$LVOL_PREFIX" ] && LVOL_PREFIX="lvol"
+}
+
+parse_config() {
+	for line in $(cat "$1"); do
+		case "$line" in
+		  "DEFAULT_DEVICE_POOL"*) DEFAULT_DEVICE_POOL=${line##*=} ;;
+		  "LVOL_PREFIX"*) LVOL_PREFIX=${line##*=} ;;
+		  "#"*) continue ;;
+		  *) error "Unknown value \"$line\" in configuration file"
+		esac
+	done
+}
+
 #############################
 # start point of this script
 # - parsing parameters
@@ -1311,6 +1330,7 @@ do
 	  "-o"|"--resize-fs-only") RESIZE_FS_ONLY=1 ;;
 	  "-y"|"--yes") YES="-y" ;;
 	  "-l"|"--lvresize") ;;
+	  "-c"|"--config") CONFIG=$2; shift 1 ;;
 	  "check") COMMAND=$1; shift; ARGS="$@"; break ;;
 	  "resize") COMMAND=$1; shift; ARGS="$@"; break ;;
 	  "create") COMMAND=$1; shift; ARGS="$@"; break ;;
@@ -1326,6 +1346,16 @@ if [ -z "$COMMAND" ]; then
 	error "Missing command. (see: $TOOL --help)"
 fi
 
+set_default_config
+if [ "$CONFIG" ]; then
+	[ ! -f "$CONFIG" ] && error "\"$CONFIG\" is not proper configuration file"
+	CONFIG_PATHS+=" $CONFIG"
+fi
+for i in "$CONFIG_PATHS"; do
+	[ ! -f "$i" ] && continue
+	parse_config "$i"
+done
+
 export FSADM_RUNNING="fsadm"
 $COMMAND $ARGS
 cleanup 0
Signature · 1 line
1.7.4.4
#18 lore raw

PATCH v3 3/18 fsadm: Add simple configuration file

Dne 27.9.2011 15:42, Lukas Czerner napsal(a):

Quoted 10 lines
---
 scripts/fsadm.sh |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)
scripts/fsadm.sh +0 -0
index 58ff0ab..98921d3 100755

scripts/fsadm.sh PROCDEVICES="/proc/devices" · line 93

Quoted 6 lines
scripts/fsadm.sh +3 -0
@@ -88,6 +88,9 @@ PROCDEVICES="/proc/devices"
 PROCPARTITIONS="/proc/partitions"
 NULL="$DM_DEV_DIR/null"
 
+CONFIG_PATHS="/etc/fsadm.conf ~/fsadm.conf"
+CONFIG=
+

Looks too complicated - sourcing config file with some defines looks like better approach here if you really think it's useful.

Zdenek

#19 lore raw

PATCH v3 3/18 fsadm: Add simple configuration file

Quoted 23 lines · Zdenek Kabelac

Dne 27.9.2011 15:42, Lukas Czerner napsal(a):

Quoted 10 lines
---
 scripts/fsadm.sh |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)
scripts/fsadm.sh +0 -0
index 58ff0ab..98921d3 100755

scripts/fsadm.sh PROCDEVICES="/proc/devices" · line 93

Quoted 6 lines
scripts/fsadm.sh +3 -0
@@ -88,6 +88,9 @@ PROCDEVICES="/proc/devices"
 PROCPARTITIONS="/proc/partitions"
 NULL="$DM_DEV_DIR/null"
 
+CONFIG_PATHS="/etc/fsadm.conf ~/fsadm.conf"
+CONFIG=
+

Looks too complicated - sourcing config file with some defines looks like better approach here if you really think it's useful.

Quoted 1 line

Zdenek

I do not think we should or can support setting all the different variables we are using in fsadm via configuration file. That is why I choose to only have strictly defined options in configuration file. This way we have absolute control over what is configured via the configuration file, which is the right approach.

Thanks!
-Lukas

Quoted 6 lines

--
lvm-devel mailing list
lvm-devel at redhat.com
https://www.redhat.com/mailman/listinfo/lvm-devel

#20 lore raw

PATCH v3 4/18 fsadm: Use DEFAULT_DEVICE_POOL when creating volume group

In reply to Lukas Czerner

I user did not specified group, use DEFAULT_DEVICE_POOL from configuration file.

---
 scripts/fsadm.sh |   58 +++--------------------------------------------------
 1 files changed, 4 insertions(+), 54 deletions(-)
scripts/fsadm.sh +4 -54
index 98921d3..d1b17e5 100755
@@ -716,49 +716,10 @@ create() {
 		vgname=$GROUP
 	fi
 
-	# Devices are not in any group so we should find some.
-	# Either create a new one, or use the existing one, if
-	# there is only one vgroup, or use the one with enough
-	# free space in it
 	if [ -z "$vgname" ]; then
-		vgroups=$("$LVM" vgs -o vg_name,vg_free --separator ' ' --noheadings --units k 2> /dev/null)
-		lines=$("$LVM" vgs -o vg_name,vg_free --separator ' ' --noheadings --units k 2> /dev/null | wc -l)
-
-		if [ $lines -eq 1 ]; then
-			vgname=$(echo $vgroups | sed -e 's/^ *//' | cut -d' ' -f1)
-		elif [ $lines -gt 1 ]; then
-			if [ -z $size ]; then
-				error "Not enough information to create" \
-					"logical volume"
-			fi
-			decode_size $size 1
-			new_size=$NEWBLOCKCOUNT
-			IFS=$NL
-			for i in $vgroups; do
-				vgsize=$(echo $i | sed -e 's/^ *//' | cut -d' ' -f2)
-				vgsize=${vgsize%%B}
-				if [ $vgsize -ge $new_size ]; then
-					vgname=$(echo $i | sed -e 's/^ *//' | cut -d' ' -f1)
-					break;
-				fi
-			done
-		fi
+		vgname=$DEFAULT_DEVICE_POOL
+		"$LVM" vgs "$vgname" &> /dev/null || vg_create=1
 	fi
-	IFS=$IFS_OLD
-
-	if [ -z "$vgname" ]; then
-		[ $devcount -eq 0 ] && error "No suitable device specified."
-		for i in $(seq -w $MAX_VGS); do
-			"$LVM" vgs vg${i} &> /dev/null
-			if [ $? -ne 0 ]; then
-				vgname="vg${i}"
-				break;
-			fi
-		done
-		vg_create=1
-	fi
-
-	[ -z "$vgname" ] && error "No suitable name for volume group found."
 
 	if [ "$stripesize" ] || [ "$stripes" ]; then
 		if [ -z "$stripes" ] && [ $devcount -eq 0 ]; then
@@ -1192,19 +1153,8 @@ add() {
 	"$LVM" vgs -o vg_name --separator ' ' --noheadings | sed -e 's/^ *//' > $tmp
 
 	if [ -z "$vgname" ]; then
-		lines=$(wc -l $tmp)
-		lines=${lines%% *}
-
-		if [ $lines -eq 1 ]; then
-			vgname=$(cut -d' ' -f1 $tmp)
-		elif [ $lines -gt 1 ]; then
-			list_pool
-			rm -f $tmp
-			error "Please, specify group you want to add the device into"
-		elif [ $lines -eq 0 ]; then
-			vgname="vg001"
-			vg_create=1
-		fi
+		vgname=$DEFAULT_DEVICE_POOL
+		"$GREP" "$vgname" $tmp &> /dev/null || vg_create=1
 	else
 		"$GREP" -e "$vgname" $tmp &> /dev/null
 		vg_create=$?
Signature · 1 line
1.7.4.4
#21 lore raw

PATCH v3 5/18 fsadm: Add LVOL_PREFIX configuration option

In reply to Lukas Czerner

This commit adds new configuration option LVOL_PREFIX which defines a prefix for new logical volumes. The prefix will be concatenated with a sequential number of the logical volume.

---
 scripts/fsadm.sh |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)
scripts/fsadm.sh +4 -6
index d1b17e5..d4901d1 100755
@@ -741,12 +741,10 @@ create() {
 		tmp=$(mktemp)
 		"$LVM" lvs "$vgname" --separator ' ' --noheadings > $tmp
 		for i in $(seq -w $MAX_VGS); do
-			"$GREP" -e " lvol${i} " $tmp &> /dev/null
-			if [ $? -ne 0 ]; then
-				name="lvol${i}"
-				lvname="--name $name"
-				break;
-			fi
+			"$GREP" -e " ${LVOL_PREFIX}${i} " $tmp &> /dev/null && continue
+			name="${LVOL_PREFIX}${i}"
+			lvname="--name $name"
+			break;
 		done
 		rm -f $tmp
 	fi
Signature · 1 line
1.7.4.4
#22 lore raw

PATCH v3 6/18 fsadm: Fsck extN before resize only if it is not mounted

In reply to Lukas Czerner
---
 scripts/fsadm.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
scripts/fsadm.sh +2 -1
index d4901d1..f43d72e 100755
@@ -415,8 +415,9 @@ resize_ext() {
 	decode_size $1 $BLOCKSIZE
 	FSFORCE=$FORCE
 
+	detect_mounted
 	if [ "$NEWBLOCKCOUNT" -lt "$BLOCKCOUNT" -o "$EXTOFF" -eq 1 ]; then
-		detect_mounted && verbose "$RESIZE_EXT needs unmounted filesystem" && try_umount
+		[ "$MOUNTED" ] && verbose "$RESIZE_EXT needs unmounted filesystem" && try_umount
 		REMOUNT=$MOUNTED
 	fi
 
Signature · 1 line
1.7.4.4
#23 lore raw

PATCH v3 6/18 fsadm: Fsck extN before resize only if it is not mounted

Dne 27.9.2011 15:42, Lukas Czerner napsal(a):

Quoted 10 lines
---
  scripts/fsadm.sh |    3 ++-
  1 files changed, 2 insertions(+), 1 deletions(-)
scripts/fsadm.sh +0 -0
index d4901d1..f43d72e 100755

scripts/fsadm.sh resize_ext() { · line 423

Quoted 10 lines
scripts/fsadm.sh +2 -1
@@ -415,8 +415,9 @@ resize_ext() {
  	decode_size $1 $BLOCKSIZE
  	FSFORCE=$FORCE
 
+	detect_mounted
  	if [ "$NEWBLOCKCOUNT" -lt "$BLOCKCOUNT" -o "$EXTOFF" -eq 1 ]; then
-		detect_mounted&&  verbose "$RESIZE_EXT needs unmounted filesystem"&&  try_umount
+		[ "$MOUNTED" ]&&  verbose "$RESIZE_EXT needs unmounted filesystem"&&  try_umount
  		REMOUNT=$MOUNTED
  	fi
 

Patches like these need to go in the front of the patch set so they could be eaily applied upstream.

Zdenek

#24 lore raw

PATCH v3 7/18 fsadm: Only use readlink if link is provided

In reply to Lukas Czerner
---
 scripts/fsadm.sh |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
scripts/fsadm.sh +4 -2
index f43d72e..2d5c6f9 100755
@@ -301,8 +301,10 @@ decode_size() {
 # dereference device name if it is symbolic link
 detect_fs() {
 	VOLUME_ORIG="$1"
-	VOLUME=${1/#"${DM_DEV_DIR}/"/}
-	VOLUME=$("$READLINK" "$READLINK_E" "$DM_DEV_DIR/$VOLUME") || error "Cannot get readlink \"$1\""
+	VOLUME=${DM_DEV_DIR}/${1/#"${DM_DEV_DIR}/"/}
+	if [ -h "$VOLUME" ]; then
+		VOLUME=$("$READLINK" "$READLINK_E" "$VOLUME") || error "Cannot get readlink \"$VOLUME\""
+	fi
 	RVOLUME=$VOLUME
 	case "$RVOLUME" in
           # hardcoded /dev  since udev does not create these entries elsewhere
Signature · 1 line
1.7.4.4
#25 lore raw

PATCH v3 8/18 fsadm: Remove unnecessary modification of PATH variable

In reply to Lukas Czerner

User (or the system) should be set properly so there is no need to modify PATH variable. It also prevent us from setting appropriate PATH before calling the fsadm.

---
 scripts/fsadm.sh |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)
scripts/fsadm.sh +0 -3
index 2d5c6f9..a1044f7 100755
@@ -37,9 +37,6 @@
 
 TOOL=$(basename "$0")
 
-_SAVEPATH=$PATH
-PATH=/sbin:/usr/sbin:/bin:/usr/sbin:$PATH
-
 # utilities
 TUNE_EXT=tune2fs
 RESIZE_EXT=resize2fs
Signature · 1 line
1.7.4.4
#26 lore raw

PATCH v3 8/18 fsadm: Remove unnecessary modification of PATH variable

Dne 27.9.2011 15:42, Lukas Czerner napsal(a):

Quoted 14 lines

User (or the system) should be set properly so there is no need to modify PATH variable. It also prevent us from setting appropriate PATH before calling the fsadm.

---
  scripts/fsadm.sh |    3 ---
  1 files changed, 0 insertions(+), 3 deletions(-)
scripts/fsadm.sh +0 -0
index 2d5c6f9..a1044f7 100755

scripts/fsadm.sh line 42

Quoted 9 lines
scripts/fsadm.sh +0 -3
@@ -37,9 +37,6 @@
 
  TOOL=$(basename "$0")
 
-_SAVEPATH=$PATH
-PATH=/sbin:/usr/sbin:/bin:/usr/sbin:$PATH
-
  # utilities
  TUNE_EXT=tune2fs
  RESIZE_EXT=resize2fs

Move this patch out of the future set - it's for separate discussion.

Zdenek

#27 lore raw

PATCH v3 8/18 fsadm: Remove unnecessary modification of PATH variable

Quoted 28 lines · Zdenek Kabelac

Dne 27.9.2011 15:42, Lukas Czerner napsal(a):

Quoted 14 lines

User (or the system) should be set properly so there is no need to modify PATH variable. It also prevent us from setting appropriate PATH before calling the fsadm.

---
  scripts/fsadm.sh |    3 ---
  1 files changed, 0 insertions(+), 3 deletions(-)
scripts/fsadm.sh +0 -0
index 2d5c6f9..a1044f7 100755

scripts/fsadm.sh line 42

Quoted 9 lines
scripts/fsadm.sh +0 -3
@@ -37,9 +37,6 @@
 
  TOOL=$(basename "$0")
 
-_SAVEPATH=$PATH
-PATH=/sbin:/usr/sbin:/bin:/usr/sbin:$PATH
-
  # utilities
  TUNE_EXT=tune2fs
  RESIZE_EXT=resize2fs

Move this patch out of the future set - it's for separate discussion.

Quoted 1 line

Zdenek

So let the separate discussion begin!

-Lukas

#28 lore raw

PATCH v3 9/18 fsadm: Allow to specify lv in vg/lv format

In reply to Lukas Czerner

Various lvm tools does allow to specify logical volume in vg/lv format, which meand no real path is provided.

---
 scripts/fsadm.sh |   21 ++++++++++++++-------
 1 files changed, 14 insertions(+), 7 deletions(-)
scripts/fsadm.sh +14 -7
index a1044f7..0a2c8cc 100755
@@ -294,10 +294,7 @@ decode_size() {
 	NEWBLOCKCOUNT=${NEWBLOCKCOUNT%%.*}
 }
 
-# detect filesystem on the given device
-# dereference device name if it is symbolic link
-detect_fs() {
-	VOLUME_ORIG="$1"
+get_volume() {
 	VOLUME=${DM_DEV_DIR}/${1/#"${DM_DEV_DIR}/"/}
 	if [ -h "$VOLUME" ]; then
 		VOLUME=$("$READLINK" "$READLINK_E" "$VOLUME") || error "Cannot get readlink \"$VOLUME\""
@@ -309,6 +306,12 @@ detect_fs() {
 		read </sys/block/${RVOLUME#/dev/}/dm/name SYSVOLUME 2>&1 && VOLUME="$DM_DEV_DIR/mapper/$SYSVOLUME"
 		;;
 	esac
+}
+
+# detect filesystem on the given device
+# dereference device name if it is symbolic link
+detect_fs() {
+	get_volume "$1"
 	# use null device as cache file to be sure about the result
 	# not using option '-o value' to be compatible with older version of blkid
 	FSTYPE=$("$BLKID" -c "$NULL" -s TYPE "$VOLUME") || return 1
@@ -529,6 +532,7 @@ resize_fs() {
 	detect_fs "$1" ||  error "Cannot get FSTYPE of \"$VOLUME\""
 	verbose "\"$FSTYPE\" filesystem found on \"$VOLUME\""
 	if [ "$NEWSIZE" ]; then
+		decode_size $NEWSIZE
 		is_natural $NEWSIZE ||  error "$NEWSIZE is not valid number for file system size"
 	fi
 	detect_device_size
@@ -559,6 +563,7 @@ resize_lvolume() {
 
 resize() {
 	local size=
+	devcount=
 
 	# Special case for the situation we have been called from within the lvresize code.
 	if [ "$RESIZE_FS_ONLY" ]; then
@@ -567,7 +572,8 @@ resize() {
 	fi
 
 	for i in "$@"; do
-		if [ -b "$i" ]; then
+		get_volume "$i"
+		if [ -b "$VOLUME" ]; then
 			if [ -z "$devcount" ]; then
 				"$LVM" lvs "$i" &> /dev/null || error "$i is not valid logical volume"
 				lvname=$i
@@ -810,9 +816,10 @@ do_remove() {
 	device=
 	MOUNTED=
 
+	get_volume "$item"
 	# Block device has been given
-	if [ -b "$item" ]; then
-		device=$item
+	if [ -b "$VOLUME" ]; then
+		device=$VOLUME
 	fi
 
 	# Mount point has been given
Signature · 1 line
1.7.4.4
#29 lore raw

PATCH v3 10/18 fsadm: Umount ext2 file system prior resize

In reply to Lukas Czerner

ext2 does NOT support online resize, so we should attempt to umount the file system first.

---
 scripts/fsadm.sh |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
scripts/fsadm.sh +5 -0
index 0a2c8cc..7220f98 100755
@@ -417,6 +417,11 @@ resize_ext() {
 	decode_size $1 $BLOCKSIZE
 	FSFORCE=$FORCE
 
+	# ext2 does NOT support online resize
+	if [ "$FSTYPE" == "ext2" ]; then
+		EXTOFF=1
+	fi
+
 	detect_mounted
 	if [ "$NEWBLOCKCOUNT" -lt "$BLOCKCOUNT" -o "$EXTOFF" -eq 1 ]; then
 		[ "$MOUNTED" ] && verbose "$RESIZE_EXT needs unmounted filesystem" && try_umount
Signature · 1 line
1.7.4.4
#30 lore raw

PATCH v3 11/18 fsadm: Add help for new commands and update man page

In reply to Lukas Czerner

This commit updates fsadm manual pages with new fsadm commands and functionality it provides. It also adds --help for the new commands directly into fsadm, so we can specify --help for each command specifically.

---
 man/fsadm.8.in   |  323 ++++++++++++++++++++++++++++++++++++++++++++++++++----
 scripts/fsadm.sh |  134 +++++++++++++++++++---
 2 files changed, 417 insertions(+), 40 deletions(-)
man/fsadm.8.in +301 -22
index b7bd0a0..0de2c9e 100644
@@ -1,24 +1,119 @@
 .TH "FSADM" "8" "LVM TOOLS #VERSION#" "Red Hat, Inc" "\""
 .SH "NAME"
-fsadm \- utility to resize or check filesystem on a device
+fsadm \- utility to manage logical volumes
 .SH SYNOPSIS
 .B fsadm
-.RI [ options ]
+.RI "[ " "OPTIONS" " ] " "COMMAND" " [ " "COMMAND_OPTIONS" " ] "
+.R [ ... ]
+.PP
+.B fsadm
+.RI "[ " "OPTIONS" " ]"
 .B check
+[
+.B --help
+]
 .I device
-.sp
+.PP
 .B fsadm
-.RI [ options ]
+.RI "[ " "OPTIONS" " ]"
 .B resize
+[
+.B --help
+]
+[
+.B size=\fISize\fP
+|
+.B size=+\fISize\fP
+|
+.B size=-\fISize\fP
+]
+[
+.I device...
+]
+.I volume
+.PP
+.B fsadm
+.RI "[ " "OPTIONS" " ]
+.B create
+[
+.B --help
+]
+[
+.B stripesize=\fIStripeSize\fP
+]
+[
+.B name=\fIName\fP
+]
+[
+.B fs=\fIType\fP
+|
+.B fstyp=\fIType\fP
+]
+[
+.B size=\fISize\fP
+]
+[
+.B stripes\fIStripes\fP
+]
+[
+.I device...
+]
+[
+.I pool
+]
+.PP
+.B fsadm
+.RI "[ " "OPTIONS" " ]
+.B list
+[
+.B --help
+]
+[
+.B filesystems
+|
+.B fs
+]
+[
+.B device
+|
+.B dev
+]
+[
+.B pool
+]
+.PP
+.B fsadm
+.RI "[ " "OPTIONS" " ]
+.B add
+[
+.B --help
+]
 .I device
-.RI [ new_size [ BKMGTEP ]]
-.sp
+[
+.I device...
+]
+[
+.I pool
+]
+.PP
+.B fsadm
+.RI "[ " "OPTIONS" " ]
+.B remove
+[
+.B --help
+]
+[
+.B --all
+]
+.I item
+[
+.I item...
+]
+.PP
 .SH DESCRIPTION
 .B fsadm
-utility checks or resizes the filesystem on a device.
-It tries to use the same API for 
-.IR ext2 , ext3 , ext4 , ReiserFS
-and \fIXFS\fP filesystem.
+utility to manage logical volumes and pools (volume groups).
+It tries to use the same API for ext3, ext4 and xfs file system.
 .SH OPTIONS
 .TP
 .BR \-h ", " \-\-help
@@ -39,11 +134,117 @@ Print commands without running them.
 .BR \-y ", " \-\-yes
 Answer "yes" at any prompts.
 .TP
-.I new_size
-Absolute number of filesystem blocks to be in the filesystem,
-or an absolute size using a suffix (in powers of 1024).
-If new_size is not supplied, the whole device is used.
-
+.B \-c ", " \-\-config\fP  \fIpath\fP
+Specify the path to the configuration file. If not provided, default
+location will be used. The default locations are
+.B /etc/fsadm.conf
+and
+.B ~/fsadm.conf
+, but options in the second one will override options in the first one.
+.SH COMMANDS
+.TP
+.B check
+Check the file system on
+.I device
+using fsck.
+.TP
+.B resize
+Change the size of the logical
+.I volume
+and file system on it. You can specify
+.B size=\fP\fISize
+to resize to given
+.IR Size ,
+or
+.B size=+\fP\fISize
+to extend the volume by the given
+.IR Size ,
+or
+.B size=-\fP\fISize
+to shrink the volume by the given
+.IR Size .
+If no
+.I Size
+is provided the volume will be resized to its maximum size.
+You can also specify one or more
+.I devices
+to use for extending the
+.IR volume .
+If the device is not in any pool, it will be added the volume's
+pool prior the resize. Note that some file system (namely
+.BR xfs )
+does not support shrinking. Also some file system does not support online
+resize, it means that it will be unmounted before the resize and then mounted
+back afterwards. It will also happen when you specify
+.BR -e ", " --ext-offline
+for extN file systems, or if you're trying to shrink the file system since none
+of supported file system supports online shrinking. And finally, some file
+systems (namely
+.BR xfs )
+does not support offline resize, it means, that the file system will be mounted
+prior to resize and then unmounted afterwards.
+.TP
+.B create
+Create a new logical volume from the
+.IR pool ,
+optionally with the defined file system. You can specify
+.I Type
+of the file system which will be automatically created on the new logical
+volume. Currently only
+.BR ext3 ", " ext4 " and " xfs
+file systems are supported. You can create striped volume by defining the
+.IR StripeSize .
+In that case, if
+.IR Stripes
+is not defined, then number of provided
+.I devices
+will be used. Either
+.I devices
+or
+.I Stripes
+has to be defined if
+.I StripeSize
+is provided, otherwise
+.B fsadm
+does not have enough information to proceed in creating new logical volume.
+Note that if no pool is specified the default pool will be used.
+.TP
+.B list
+List devices, file systems and pools in your system. You can select to list
+all logical volumes by specifying
+.BR filesystems ,
+or
+.BR fs.
+Or you can select to list all pools in the system by specifying
+.BR pool.
+Or you can also select to list all devices in the system by specifying
+.BR devices ,
+or
+.BR dev ,
+however note that this will not list any DM device.
+Optionally you can specify any combination of the above options to list whatever
+you desire, or you can simply omit the option. In that case it will list
+everything as if all options has been specified.
+.TP
+.B add
+Add one, or more
+.I devices
+into the
+.IR pool .
+If no
+.I pool
+is specified, provided
+.I devices
+will be added into the default pool. Note that, if any device is already part
+of the same, or different pool, it will be skipped.
+.TP
+.B remove
+Remove one, or more logical volumes, devices, or pools defined
+by
+.IR items .
+You can also specify
+.B --all
+to remove all pools corresponding volumes from your system.
 .SH DIAGNOSTICS
 On successful completion, the status code is 0.
 A status code of 2 indicates the operation was interrupted by the user.
@@ -51,14 +252,76 @@ A status code of 3 indicates the requested check operation could not be performe
 because the filesystem is mounted and does not support an online 
 .BR fsck (8).
 A status code of 1 is used for other failures.
-
 .SH EXAMPLES
-Resize the filesystem on logical volume /dev/vg/test to 1000 megabytes.
-If /dev/vg/test contains ext2/ext3/ext4
-filesystem it will be unmounted prior the resize.
-All [y|n] questions will be answered 'y'.
-.sp
-.B fsadm \-e \-y resize /dev/vg/test 1000M
+To
+.B add
+device
+.I /dev/sdb
+into the default pool run this command:
+.PP
+    \fB fsadm add /dev/sdb
+.PP
+You can also add mode devices into another pool called
+.I mypool
+.PP
+    \fB fsadm add /dev/sdc /dev/sdd /dev/sde mypool
+.PP
+To create a 300GB linear logical volume with
+.B ext4
+file system using devices
+.B /dev/sda
+and
+.B /dev/sdb
+you can use the following command:
+.PP
+    \fB fsadm create fs=ext4 size=300G /dev/sda /dev/sdb
+.PP
+Of course, we are assuming that /dev/sda and /dev/sdb does have at least 300GB
+of space when combined, otherwise the volume creation would fail.
+.PP
+Now let's create 500GB striped volume with stripe size of 16KB and
+.B xfs
+file system using four devices. It means that
+.I Stripes
+will be equal to 4, however note that we do not need to define
+.I Stripes
+manually if we are listing devices as follows:
+.PP
+    \fB fsadm create fs=xfs size=500G stripesize=16 /dev/sda /dev/sdb /dev/sdc /dev/sdd
+.PP
+Now, if we assume that we already have at least four devices in the default
+pool, we can achieve the same result by calling:
+.PP
+    \fB fsadm create fs=xfs size=500G stripesize=16 stripes=4
+.PP
+To
+.B shrink
+the
+.B device_pool/lvol001
+logical volume by 10G we can simply call
+.PP
+    \fB fsadm resize size=-10G device_pool/lvol001
+.PP
+Or we can
+.B extend
+it by 1T using more devices which are not in the pool just yet:
+.PP
+    \fB fsadm resize size=+1T device_pool/lvol001 /dev/sde /dev/sdf
+.PP
+To
+.B remove
+the above logical volume we can use the following command:
+.PP
+    \fB fsadm remove device_pool/lvol001
+.PP
+or we can simply remove the whole pool, wiping all logical volumes in it:
+.PP
+    \fB fsadm remove device_pool
+.PP
+Alternatively we can remove all pools in the system by calling:
+.PP
+    \fB fsadm remove --all
+.PP
 .SH ENVIRONMENT VARIABLES
 .TP
 .B TMPDIR
@@ -67,6 +330,22 @@ The temporary directory name for mount points. Defaults to "/tmp".
 .B DM_DEV_DIR
 The device directory name.
 Defaults to "/dev" and must be an absolute path.
+.TP
+.B DEFAULT_DEVICE_POOL
+Default pool that will be used in
+.B fsadm
+command if not pool is specified. It defaults to "device_pool" and can
+also be set in configuration file (see
+.B \-c ", " \-\-config\fP  \fIpath\fP
+).
+.TP
+.B LVOL_PREFIX
+Defines the name of the newly created volume if no
+.B name=
+is provided. Defaults to "lvol" and will be concatenated with the number
+of the volume. It can also be set in configuration file (see
+.B \-c ", " \-\-config\fP  \fIpath\fP
+).
 
 .SH SEE ALSO
 .BR lvm (8),
scripts/fsadm.sh +116 -18

182 lines not on this page · raw message

Signature · 1 line
1.7.4.4
#31 lore raw

PATCH v3 12/18 lvresize: Specify --resize-fs-only when going to use fsadm resize

In reply to Lukas Czerner

When lvresize is calling fsadm to resize the file system, originally it was the default action. However now fsadm support much more features and it is doing the lvresize by itself. Luckily there is still a way to tell fsadm to resize only the file system ane leave the rest up to the lvresize - provide "--resize-fs-only" argument.

This commit adds "--resize-fs-only" argument when we are going to use fsadm to resize the fs from within the lvresize.

---
 tools/lvresize.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)
tools/lvresize.c +10 -2
index 96e623e..44f934e 100644
@@ -128,7 +128,7 @@ static int _request_confirmation(struct cmd_context *cmd,
 
 enum fsadm_cmd_e { FSADM_CMD_CHECK, FSADM_CMD_RESIZE };
 #define FSADM_CMD "fsadm"
-#define FSADM_CMD_MAX_ARGS 6
+#define FSADM_CMD_MAX_ARGS 7
 #define FSADM_CHECK_FAILS_FOR_MOUNTED 3 /* shell exist status code */
 
 /*
@@ -157,7 +157,15 @@ static int _fsadm_cmd(struct cmd_context *cmd,
 	if (arg_count(cmd, force_ARG))
 		argv[i++] = "--force";
 
-	argv[i++] = (fcmd == FSADM_CMD_RESIZE) ? "resize" : "check";
+	switch (fcmd) {
+		case FSADM_CMD_RESIZE:
+			argv[i++] = "--resize-fs-only";
+			argv[i++] = "resize";
+			break;
+		case FSADM_CMD_CHECK:
+			argv[i++] = "check";
+			break;
+	}
 
 	if (dm_snprintf(lv_path, PATH_MAX, "%s%s/%s", cmd->dev_dir, lp->vg_name,
 			lp->lv_name) < 0) {
Signature · 1 line
1.7.4.4
#32 lore raw

PATCH v3 12/18 lvresize: Specify --resize-fs-only when going to use fsadm resize

Dne 27.9.2011 15:42, Lukas Czerner napsal(a):

Quoted 8 lines

When lvresize is calling fsadm to resize the file system, originally it was the default action. However now fsadm support much more features and it is doing the lvresize by itself. Luckily there is still a way to tell fsadm to resize only the file system ane leave the rest up to the lvresize - provide "--resize-fs-only" argument.

This commit adds "--resize-fs-only" argument when we are going to use fsadm to resize the fs from within the lvresize.

There is a big reason to call lvresize for this operation - it's the cluster atomicity, so the resize operation (while using lvm tools) provide consistent results - this breaks it - so NACK for now.

Zdenek

#33 lore raw

PATCH v3 12/18 lvresize: Specify --resize-fs-only when going to use fsadm resize

Quoted 11 lines · Zdenek Kabelac

Dne 27.9.2011 15:42, Lukas Czerner napsal(a):

Quoted 8 lines

When lvresize is calling fsadm to resize the file system, originally it was the default action. However now fsadm support much more features and it is doing the lvresize by itself. Luckily there is still a way to tell fsadm to resize only the file system ane leave the rest up to the lvresize - provide "--resize-fs-only" argument.

This commit adds "--resize-fs-only" argument when we are going to use fsadm to resize the fs from within the lvresize.

Quoted 5 lines

There is a big reason to call lvresize for this operation - it's the cluster atomicity, so the resize operation (while using lvm tools) provide consistent results - this breaks it - so NACK for now.

Zdenek

Sure, the patch actually does not change the atomicity, however the description might be a bit badly worded, I'll change that.

Thanks!
-Lukas

Quoted 6 lines

--
lvm-devel mailing list
lvm-devel at redhat.com
https://www.redhat.com/mailman/listinfo/lvm-devel

#34 lore raw

PATCH v3 13/18 fsadm: remove -y (YES) option

In reply to Lukas Czerner

There is some confusion in using -y (YES) and -f (FORCE) options in fsadm. In some cases we are asked for yes/no question which can be override by -f option, but not by -y option. Usually most of the questions tools ask for are yes/no and it can be overridden by forcing it with -f (e.g. fsck.(extN|xfs), lvm and others...) so it make sense to get rid of -y option and use only -f instead.

Also I do not think it is wise to use -y option in fsck.extN since people using fsadm would probably not know how it works, so we should NOT provide them with that option, but rather let them use "real" fsck instead (and let them read man page if needed). Also running fsck with -y when you have corrupted file system is probably not a good idea from multiple reasons. This is also fixed by this commit.

This commit removes '-y' option and use '-f' instead. With exception of fsck.

---
 man/fsadm.8.in   |    5 +----
 scripts/fsadm.sh |   23 +++++++++++------------
 2 files changed, 12 insertions(+), 16 deletions(-)
man/fsadm.8.in +1 -4
index 0de2c9e..4f187ae 100644
@@ -126,14 +126,11 @@ Be more verbose.
 Unmount ext2/ext3/ext4 filesystem before doing resize.
 .TP
 .BR \-f ", " \-\-force
-Bypass some sanity checks.
+Force all commands without asking questions.
 .TP
 .BR \-n ", " \-\-dry\-run
 Print commands without running them.
 .TP
-.BR \-y ", " \-\-yes
-Answer "yes" at any prompts.
-.TP
 .B \-c ", " \-\-config\fP  \fIpath\fP
 Specify the path to the configuration file. If not provided, default
 location will be used. The default locations are
scripts/fsadm.sh +11 -12
index eb2be44..c8f6999 100755
@@ -63,7 +63,7 @@ DF=df
 # user may override lvm location by setting LVM_BINARY
 LVM=${LVM_BINARY:-lvm}
 
-YES=${_FSADM_YES}
+FORCE=${_FSADM_FORCE}
 DRY=0
 VERB=
 FORCE=
@@ -190,9 +190,8 @@ tool_usage() {
 	echo "    -h | --help           Show this help message"
 	echo "    -v | --verbose        Be verbose"
 	echo "    -e | --ext-offline    unmount filesystem before ext2/ext3/ext4 resize"
-	echo "    -f | --force          Bypass sanity checks"
+	echo "    -f | --force          Force all commands without asking questions"
 	echo "    -n | --dry-run        Print commands without running them"
-	echo "    -y | --yes            Answer \"yes\" at any prompts"
 	echo "    -c | --config path    Specify the path to the configuration file"
 	echo
 	exit
@@ -460,8 +459,9 @@ temp_umount() {
 yes_no() {
 	echo -n "$@? [Y|n] "
 
-	if [ -n "$YES" ]; then
-		echo y ; return 0
+	if [ -n "$FORCE" ]; then
+		echo y
+		return 0
 	fi
 
 	while read -r -s -n 1 ANS ; do
@@ -512,7 +512,7 @@ resize_ext() {
 	if [ -z "$MOUNTED" ] || [ "$REMOUNT" ]; then
 		# Forced fsck -f for umounted extX filesystem.
 		case "$-" in
-		  *i*) dry "$FSCK" "$YES" -f "$VOLUME" ;;
+		  *i*) dry "$FSCK" -f "$VOLUME" ;;
 		  *) dry "$FSCK" -f -p "$VOLUME" ;;
 		esac
 	fi
@@ -539,7 +539,7 @@ resize_reiser() {
 	validate_parsing "$TUNE_REISER"
 	decode_size $1 $BLOCKSIZE
 	verbose "Resizing \"$VOLUME\" $BLOCKCOUNT -> $NEWBLOCKCOUNT blocks ($NEWSIZE KiB, bs: $NEWBLOCKCOUNT)"
-	if [ -n "$YES" ]; then
+	if [ -n "$FORCE" ]; then
 		echo y | dry "$RESIZE_REISER" -s "${NEWSIZE}K" "$VOLUME"
 	else
 		dry "$RESIZE_REISER" -s "${NEWSIZE}K" "$VOLUME"
@@ -587,7 +587,7 @@ make_ext() {
 	fstyp="$2"
 	bsize=4
 
-	if [ "$YES" ]; then
+	if [ "$FORCE" ]; then
 		force="-F"
 	fi
 
@@ -602,7 +602,7 @@ generic_make_fs() {
 	fstyp=$2
 	bsize=4096
 
-	if [ "$YES" ]; then
+	if [ "$FORCE" ]; then
 		force="-f"
 	fi
 
@@ -725,7 +725,7 @@ resize() {
 	# resize the logical volume, but also the file system (if present)
 	# by calling fsadm recursively with --resize-fs-only argument.
 	# To be able to do this we should set some appropriate variables.
-	export _FSADM_YES=$YES
+	export _FSADM_FORCE=$FORCE
 	export FSADM_RESIZE_RECURSE=1
 	unset FSADM_RUNNING
 
@@ -1326,7 +1326,7 @@ check() {
 	  "xfs") dry "$XFS_CHECK" "$VOLUME" ;;
 	  *)    # check if executed from interactive shell environment
 		case "$-" in
-		  *i*) dry "$FSCK" "$YES" "$FORCE" "$VOLUME" ;;
+		  *i*) dry "$FSCK" "$FORCE" "$VOLUME" ;;
 		  *) dry "$FSCK" "$FORCE" -p "$VOLUME" ;;
 		esac
 	esac
@@ -1386,7 +1386,6 @@ do
 	  "-f"|"--force") FORCE="-f" ;;
 	  "-e"|"--ext-offline") EXTOFF=1 ;;
 	  "-o"|"--resize-fs-only") RESIZE_FS_ONLY=1 ;;
-	  "-y"|"--yes") YES="-y" ;;
 	  "-l"|"--lvresize") ;;
 	  "-c"|"--config") CONFIG=$2; shift 1 ;;
 	  "check") COMMAND=$1; shift; ARGS="$@"; break ;;
Signature · 1 line
1.7.4.4
#35 lore raw

PATCH v3 13/18 fsadm: remove -y (YES) option

Dne 27.9.2011 15:42, Lukas Czerner napsal(a):

Quoted 14 lines

There is some confusion in using -y (YES) and -f (FORCE) options in fsadm. In some cases we are asked for yes/no question which can be override by -f option, but not by -y option. Usually most of the questions tools ask for are yes/no and it can be overridden by forcing it with -f (e.g. fsck.(extN|xfs), lvm and others...) so it make sense to get rid of -y option and use only -f instead.

Also I do not think it is wise to use -y option in fsck.extN since people using fsadm would probably not know how it works, so we should NOT provide them with that option, but rather let them use "real" fsck instead (and let them read man page if needed). Also running fsck with -y when you have corrupted file system is probably not a good idea from multiple reasons. This is also fixed by this commit.

Quoted 3 lines

This commit removes '-y' option and use '-f' instead. With exception of fsck.

NACK

-f and -y are different.

While fsck will proceed with -f on mounted file system (leading to certain damage) -y option will stop here and just answer -y to question about umount.

And also you would change command line syntax and change the behavior of already written scripts.

Zdenek

#36 lore raw

PATCH v3 13/18 fsadm: remove -y (YES) option

Quoted 21 lines · Zdenek Kabelac

Dne 27.9.2011 15:42, Lukas Czerner napsal(a):

Quoted 14 lines

There is some confusion in using -y (YES) and -f (FORCE) options in fsadm. In some cases we are asked for yes/no question which can be override by -f option, but not by -y option. Usually most of the questions tools ask for are yes/no and it can be overridden by forcing it with -f (e.g. fsck.(extN|xfs), lvm and others...) so it make sense to get rid of -y option and use only -f instead.

Also I do not think it is wise to use -y option in fsck.extN since people using fsadm would probably not know how it works, so we should NOT provide them with that option, but rather let them use "real" fsck instead (and let them read man page if needed). Also running fsck with -y when you have corrupted file system is probably not a good idea from multiple reasons. This is also fixed by this commit.

Quoted 3 lines

This commit removes '-y' option and use '-f' instead. With exception of fsck.

NACK

Quoted 4 lines

-f and -y are different.

While fsck will proceed with -f on mounted file system (leading to certain damage) -y option will stop here and just answer -y to question about umount.

I am not saying that -f should be used instead of -y.

That is why having both options for the fsadm does not make sense, because it is not just fsck which fsadm is using internally. Force means force, we are trying to simplify things here not map every argument of every tool into fsadm.

When user force things, he should know what is he doing, since he really is using "force" :). -f option is quite generic and most of the tools does have it, however -y options is specific, that is why fsadm does not have -n option, or even -p option.

Generally I am in favour of changing check command to only check for file system consistency and if problems are found report to the user that he should run proper fsck by itself, since it is a bit delicate situation and the user should really know what is he doing if he does not want to lose data.

Finally I think that having both '-f' and '-y' option which are really inconsistent among the tools does not make sense.

Thanks!
-Lukas

Quoted 11 lines

And also you would change command line syntax and change the behavior of already written scripts.

Zdenek

--
lvm-devel mailing list
lvm-devel at redhat.com
https://www.redhat.com/mailman/listinfo/lvm-devel

#37 lore raw

PATCH v3 13/18 fsadm: remove -y (YES) option

Dne 3.10.2011 18:39, Lukas Czerner napsal(a):

Quoted 40 lines · Zdenek Kabelac
Quoted 22 lines

Dne 27.9.2011 15:42, Lukas Czerner napsal(a):

Quoted 14 lines

There is some confusion in using -y (YES) and -f (FORCE) options in fsadm. In some cases we are asked for yes/no question which can be override by -f option, but not by -y option. Usually most of the questions tools ask for are yes/no and it can be overridden by forcing it with -f (e.g. fsck.(extN|xfs), lvm and others...) so it make sense to get rid of -y option and use only -f instead.

Also I do not think it is wise to use -y option in fsck.extN since people using fsadm would probably not know how it works, so we should NOT provide them with that option, but rather let them use "real" fsck instead (and let them read man page if needed). Also running fsck with -y when you have corrupted file system is probably not a good idea from multiple reasons. This is also fixed by this commit.

Quoted 3 lines

This commit removes '-y' option and use '-f' instead. With exception of fsck.

NACK

Quoted 4 lines

-f and -y are different.

While fsck will proceed with -f on mounted file system (leading to certain damage) -y option will stop here and just answer -y to question about umount.

I am not saying that -f should be used instead of -y.

That is why having both options for the fsadm does not make sense, because it is not just fsck which fsadm is using internally. Force means force, we are trying to simplify things here not map every argument of every tool into fsadm.

When user force things, he should know what is he doing, since he really is using "force" :). -f option is quite generic and most of the tools does have it, however -y options is specific, that is why fsadm does not have -n option, or even -p option.

Quoted 8 lines

Generally I am in favour of changing check command to only check for file system consistency and if problems are found report to the user that he should run proper fsck by itself, since it is a bit delicate situation and the user should really know what is he doing if he does not want to lose data.

Finally I think that having both '-f' and '-y' option which are really inconsistent among the tools does not make sense.

Well think more about the difference between -y and -f.

If something does not make sense to you, the first solution is to remove it, the second (harder) is to try to think about it deeply and try to understand for multiple reasons of this NACK - and from your other thread replies, it also applies to your other 'does not make sense to me' comments...

Zdenek

#38 lore raw

PATCH v3 13/18 fsadm: remove -y (YES) option

Quoted 53 lines · Zdenek Kabelac

Dne 3.10.2011 18:39, Lukas Czerner napsal(a):

Quoted 43 lines · Zdenek Kabelac
Quoted 23 lines

Dne 27.9.2011 15:42, Lukas Czerner napsal(a):

Quoted 15 lines

There is some confusion in using -y (YES) and -f (FORCE) options in fsadm. In some cases we are asked for yes/no question which can be override by -f option, but not by -y option. Usually most of the questions
tools ask for are yes/no and it can be overridden by forcing it with -f (e.g. fsck.(extN|xfs), lvm and others...) so it make sense to get rid of -y option and use only -f instead.

Also I do not think it is wise to use -y option in fsck.extN since people using fsadm would probably not know how it works, so we should NOT provide them with that option, but rather let them use "real" fsck instead (and let them read man page if needed). Also running fsck with -y when you have corrupted file system is probably not a good idea from multiple reasons. This is also fixed by this commit.

Quoted 3 lines

This commit removes '-y' option and use '-f' instead. With exception of fsck.

NACK

Quoted 6 lines

-f and -y are different.

While fsck will proceed with -f on mounted file system (leading to certain
damage) -y option will stop here and just answer -y to question about umount.

I am not saying that -f should be used instead of -y.

That is why having both options for the fsadm does not make sense, because it is not just fsck which fsadm is using internally. Force means force, we are trying to simplify things here not map every argument of every tool into fsadm.

When user force things, he should know what is he doing, since he really is using "force" :). -f option is quite generic and most of the tools does have it, however -y options is specific, that is why fsadm does not have -n option, or even -p option.

Quoted 8 lines

Generally I am in favour of changing check command to only check for file system consistency and if problems are found report to the user that he should run proper fsck by itself, since it is a bit delicate situation and the user should really know what is he doing if he does not want to lose data.

Finally I think that having both '-f' and '-y' option which are really inconsistent among the tools does not make sense.

Quoted 9 lines

Well think more about the difference between -y and -f.

If something does not make sense to you, the first solution is to remove it, the second (harder) is to try to think about it deeply and try to understand for multiple reasons of this NACK - and from your other thread replies, it also applies to your other 'does not make sense to me' comments...

Zdenek

I think I gave you multiple reasons why it does not make sense, if you do not want to listen, I do not really care :) but it does to change the fact that it is not consistent and it is confusing. No talking about the fact that doing fsck with -y might be really bad idea if you do not know what you're doing, or if you at least do not have metadata backup.

-Lukas

#39 lore raw

PATCH v3 13/18 fsadm: remove -y (YES) option

Dne 4.10.2011 08:29, Lukas Czerner napsal(a):

Quoted 62 lines · Zdenek Kabelac
Quoted 61 lines

Dne 3.10.2011 18:39, Lukas Czerner napsal(a):

Quoted 43 lines · Zdenek Kabelac
Quoted 23 lines

Dne 27.9.2011 15:42, Lukas Czerner napsal(a):

Quoted 15 lines

There is some confusion in using -y (YES) and -f (FORCE) options in fsadm. In some cases we are asked for yes/no question which can be override by -f option, but not by -y option. Usually most of the questions
tools ask for are yes/no and it can be overridden by forcing it with -f (e.g. fsck.(extN|xfs), lvm and others...) so it make sense to get rid of -y option and use only -f instead.

Also I do not think it is wise to use -y option in fsck.extN since people using fsadm would probably not know how it works, so we should NOT provide them with that option, but rather let them use "real" fsck instead (and let them read man page if needed). Also running fsck with -y when you have corrupted file system is probably not a good idea from multiple reasons. This is also fixed by this commit.

Quoted 3 lines

This commit removes '-y' option and use '-f' instead. With exception of fsck.

NACK

Quoted 6 lines

-f and -y are different.

While fsck will proceed with -f on mounted file system (leading to certain
damage) -y option will stop here and just answer -y to question about umount.

I am not saying that -f should be used instead of -y.

That is why having both options for the fsadm does not make sense, because it is not just fsck which fsadm is using internally. Force means force, we are trying to simplify things here not map every argument of every tool into fsadm.

When user force things, he should know what is he doing, since he really is using "force" :). -f option is quite generic and most of the tools does have it, however -y options is specific, that is why fsadm does not have -n option, or even -p option.

Quoted 8 lines

Generally I am in favour of changing check command to only check for file system consistency and if problems are found report to the user that he should run proper fsck by itself, since it is a bit delicate situation and the user should really know what is he doing if he does not want to lose data.

Finally I think that having both '-f' and '-y' option which are really inconsistent among the tools does not make sense.

Well think more about the difference between -y and -f.

If something does not make sense to you, the first solution is to remove it, the second (harder) is to try to think about it deeply and try to understand for multiple reasons of this NACK - and from your other thread replies, it also applies to your other 'does not make sense to me' comments...

Quoted 9 lines
Quoted 2 lines

Zdenek

I think I gave you multiple reasons why it does not make sense, if you do not want to listen, I do not really care :) but it does to change the fact that it is not consistent and it is confusing. No talking about the fact that doing fsck with -y might be really bad idea if you do not know what you're doing, or if you at least do not have metadata backup.

Well simply take it as a fact -y and -f will stay.

And please try to read what has been written (or even said) about the reasons these options have to stay supported - like all other options which are already part of fsadm.

Zdenek

#40 lore raw

PATCH v3 14/18 test: add helper to compute aligned lv size

In reply to Lukas Czerner

For test purposes add a helper function to compute lv size as it would be done by lvm tools.

---
 test/lib/utils.sh |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)
test/lib/utils.sh +23 -0
index 4c3cd93..c555e45 100644
@@ -177,6 +177,29 @@ kernel_at_least() {
     return 1
 }
 
+align_size_up() {
+    size=$1
+    [ -z $2 ] && stripes=0
+    [ -z $3 ] && extent=4
+
+    [ -z $size ] || [ -z $stripes ] || [ -z $extent ] && exit 1
+
+    tmp=$((size%extent))
+    if [ $tmp -ne 0 ]; then
+        size=$(($size+($extent-$tmp)))
+    fi
+    if [ $stripes -eq 0 ]; then
+        echo "$size"
+        return 0
+    fi
+    extents=$(($size/$extent))
+    tmp=$(($extents%$stripes))
+    if [ $tmp -ne 0 ]; then
+        extents=$(($extents-$tmp+$stripes))
+    fi
+    echo "$(($extents*$extent))"
+}
+
 . lib/paths || { echo >&2 you must run make first; exit 1; }
 
 PATH=$abs_top_builddir/test/lib:$PATH
Signature · 1 line
1.7.4.4
#41 lore raw

PATCH v3 15/18 test: Add test for fsadm add command

In reply to Lukas Czerner

The tests exercises the fsadm "add" functionality, by trying various combinations of arguments and checking the desired results.

---
 test/t-fsadm-add.sh |   53 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 53 insertions(+), 0 deletions(-)
 create mode 100644 test/t-fsadm-add.sh
test/t-fsadm-add.sh new file +53 -0
new file mode 100644
index 0000000..4c4eb2e
@@ -0,0 +1,53 @@
+#!/bin/bash
+# Copyright 2011 (C) Red Hat, Inc., Lukas Czerner <[email protected]>
+#
+# This copyrighted material is made available to anyone wishing to use,
+# modify, copy, or redistribute it subject to the terms and conditions
+# of the GNU General Public License v.2.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software Foundation,
+# Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+test_description='Exercise fsadm add'
+
+. lib/test
+
+DEV_COUNT=10
+aux prepare_devs $DEV_COUNT 10
+TEST_DEVS=$(cat DEVICES)
+export DEFAULT_DEVICE_POOL=$vg1
+
+pool1=$vg2
+pool2=$vg3
+
+# Create default pool with all devices at once
+fsadm add $TEST_DEVS
+check vg_field $DEFAULT_DEVICE_POOL pv_count $DEV_COUNT
+fsadm -f remove $DEFAULT_DEVICE_POOL
+
+# Create default pool by adding devices one per a call
+for i in $TEST_DEVS; do
+        fsadm add $i
+done
+check vg_field $DEFAULT_DEVICE_POOL pv_count $DEV_COUNT
+fsadm -f remove $DEFAULT_DEVICE_POOL
+
+# Create different groups from different devices
+fsadm add $dev1 $dev2 $dev3 $pool1
+fsadm add $dev4 $dev5 $dev6
+fsadm add $dev7 $dev8 $dev9 $pool2
+fsadm add $dev10 $pool2
+not fsadm add $dev10 $pool2 $pool1
+check vg_field $DEFAULT_DEVICE_POOL pv_count 3
+check vg_field $pool1 pv_count 3
+check vg_field $pool2 pv_count 4
+fsadm -f remove --all
+
+fsadm add --help
+
+# Some cases which should fail
+not fsadm _garbage_
+not fsadm add
+not fsadm add _somepool
+not fsadm add $dev1 $dev2 $dev3 $pool1 _otherpool
Signature · 1 line
1.7.4.4
#42 lore raw

PATCH v3 16/18 test: Add test for fsadm create command

In reply to Lukas Czerner

The tests exercises the fsadm "create" functionality, by trying various combinations of arguments and checking the desired results. It also tries to create volumes with various supported file systems.

---
 test/t-fsadm-create.sh |  128 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 128 insertions(+), 0 deletions(-)
 create mode 100644 test/t-fsadm-create.sh
test/t-fsadm-create.sh new file +128 -0
new file mode 100644
index 0000000..4b735f8
@@ -0,0 +1,128 @@
+#!/bin/bash
+# Copyright 2011 (C) Red Hat, Inc., Lukas Czerner <[email protected]>
+#
+# This copyrighted material is made available to anyone wishing to use,
+# modify, copy, or redistribute it subject to the terms and conditions
+# of the GNU General Public License v.2.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software Foundation,
+# Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+test_description='Exercise fsadm create'
+
+. lib/test
+
+DEV_COUNT=10
+DEV_SIZE=10
+TEST_MAX_SIZE=$(($DEV_COUNT*$DEV_SIZE))
+aux prepare_devs $DEV_COUNT $DEV_SIZE
+TEST_DEVS=$(cat DEVICES)
+export DEFAULT_DEVICE_POOL=$vg1
+export LVOL_PREFIX="lvol"
+lvol1=${LVOL_PREFIX}001
+lvol2=${LVOL_PREFIX}002
+lvol3=${LVOL_PREFIX}003
+
+pool1=$vg2
+pool2=$vg3
+
+TEST_FS=
+which mkfs.ext2 && TEST_FS+="ext2 "
+which mkfs.ext3 && TEST_FS+="ext3 "
+which mkfs.ext4 && TEST_FS+="ext4 "
+which mkfs.xfs  && TEST_FS+="xfs"
+
+_do_fsck() {
+	case $1 in
+		ext[234]) call="fsck.$1 -F -n" ;;
+		xfs) call="xfs_check" ;;
+		*) return 1 ;;
+	esac
+	$call $DM_DEV_DIR/$2
+}
+
+# Create volume with all devices at once
+fsadm create $TEST_DEVS
+not fsadm create $TEST_DEVS
+not fsadm create $TEST_DEVS $pool1
+not fsadm create size=$DEV_SIZE $TEST_DEVS
+check lv_field $DEFAULT_DEVICE_POOL/$lvol1 pv_count $DEV_COUNT
+fsadm -f remove $DEFAULT_DEVICE_POOL
+
+# Create the group first and then create volume using the whole group
+fsadm add $TEST_DEVS
+fsadm create
+check lv_field $DEFAULT_DEVICE_POOL/$lvol1 pv_count $DEV_COUNT
+fsadm -f remove $DEFAULT_DEVICE_POOL
+
+# Create a logical volume of fixed size
+size=$(($DEV_SIZE*6))
+fsadm create size=${size}M $TEST_DEVS
+not fsadm create size=$TEST_MAX_SIZE
+size=$(align_size_up $size)
+check lv_field $DEFAULT_DEVICE_POOL/$lvol1 lv_size ${size}.00m
+fsadm -f remove $DEFAULT_DEVICE_POOL
+
+# Create a striped logical volume
+fsadm create stripesize=32 $TEST_DEVS
+not fsadm create stripesize=32 size=$TEST_MAX_SIZE
+check lv_field $DEFAULT_DEVICE_POOL/$lvol1 stripesize 32.00k
+fsadm  -f remove $DEFAULT_DEVICE_POOL
+
+# Create several volumes with different parameters
+fsadm  add $TEST_DEVS
+fsadm create stripesize=8 stripes=$(($DEV_COUNT/2)) size=$(($DEV_SIZE*2))M
+fsadm create stripes=$(($DEV_COUNT)) size=$(($DEV_SIZE))M
+not fsadm create stripesize=32 size=$(($DEV_SIZE*2))M
+fsadm list
+fsadm create
+check lv_field $DEFAULT_DEVICE_POOL/$lvol1 stripesize 8.00k
+check lv_field $DEFAULT_DEVICE_POOL/$lvol1 stripes $(($DEV_COUNT/2))
+check lv_field $DEFAULT_DEVICE_POOL/$lvol2 stripes $DEV_COUNT
+check lv_field $DEFAULT_DEVICE_POOL/$lvol3 segtype linear
+fsadm  -f remove $DEFAULT_DEVICE_POOL
+
+# Create several volumes with different parameters from different groups
+fsadm add $dev1 $dev2 $dev3 $pool1
+fsadm create $dev1 $dev2
+fsadm add $dev4 $dev5 $dev6 $pool2
+fsadm create stripesize=32 stripes=3 size=$(($DEV_SIZE*2))M $pool2
+fsadm create $dev7 $dev8 $dev9 stripesize=8
+not fsadm create size=$DEV_SIZE $pool1
+check lv_field $DEFAULT_DEVICE_POOL/$lvol1 stripesize 8.00k
+check lv_field $pool1/$lvol1 seg_count 2
+check lv_field $pool2/$lvol1 stripesize 32.00k
+check lv_field $pool2/$lvol1 stripes 3
+fsadm  -f remove --all
+
+# Create logical volumes with file system
+for fs in $TEST_FS; do
+	fsadm create fs=$fs size=$(($DEV_SIZE*6))M $TEST_DEVS
+	check lv_field $DEFAULT_DEVICE_POOL/$lvol1 pv_count $DEV_COUNT
+	fsadm -f check ${DEFAULT_DEVICE_POOL}/$lvol1
+	_do_fsck $fs ${DEFAULT_DEVICE_POOL}/$lvol1
+	fsadm  -f remove $DEFAULT_DEVICE_POOL
+
+	fsadm create fs=$fs stripesize=32 size=$(($DEV_SIZE*6))M $TEST_DEVS
+	check lv_field $DEFAULT_DEVICE_POOL/$lvol1 pv_count $DEV_COUNT
+	check lv_field $DEFAULT_DEVICE_POOL/$lvol1 stripesize 32.00k
+	_do_fsck $fs ${DEFAULT_DEVICE_POOL}/$lvol1
+	fsadm  -f remove $DEFAULT_DEVICE_POOL
+
+	fsadm add $TEST_DEVS
+	fsadm create fs=$fs stripesize=8 stripes=$((DEV_COUNT/5)) size=$(($DEV_SIZE*2))M
+	check lv_field $DEFAULT_DEVICE_POOL/$lvol1 stripes $(($DEV_COUNT/5))
+	check lv_field $DEFAULT_DEVICE_POOL/$lvol1 stripesize 8.00k
+	_do_fsck $fs ${DEFAULT_DEVICE_POOL}/$lvol1
+	fsadm  -f remove $DEFAULT_DEVICE_POOL
+done
+
+fsadm create --help
+
+# Some cases which should fail
+not fsadm create
+fsadm add $TEST_DEVS
+not fsadm create $pool1
+not fsadm create stripesize=16 stripes=3 $dev1 $dev2
+fsadm  -f remove --all
Signature · 1 line
1.7.4.4
#43 lore raw

PATCH v3 17/18 test: Add test for fsadm resize command

In reply to Lukas Czerner

The tests exercises the fsadm "resize" functionality, by trying various combinations of arguments and checking the desired results. It also tries to create and resize volumes with various supported file systems, checking the resulting size.

---
 test/t-fsadm-resize.sh |  125 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 125 insertions(+), 0 deletions(-)
 create mode 100644 test/t-fsadm-resize.sh
test/t-fsadm-resize.sh new file +125 -0
new file mode 100644
index 0000000..130bd54
@@ -0,0 +1,125 @@
+#!/bin/bash
+# Copyright 2011 (C) Red Hat, Inc., Lukas Czerner <[email protected]>
+#
+# This copyrighted material is made available to anyone wishing to use,
+# modify, copy, or redistribute it subject to the terms and conditions
+# of the GNU General Public License v.2.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software Foundation,
+# Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+test_description='Exercise fsadm create'
+
+. lib/test
+
+DEV_COUNT=10
+DEV_SIZE=10
+TEST_MAX_SIZE=$(($DEV_COUNT*$DEV_SIZE))
+aux prepare_devs $DEV_COUNT $DEV_SIZE
+TEST_DEVS=$(cat DEVICES)
+export DEFAULT_DEVICE_POOL=$vg1
+export LVOL_PREFIX="lvol"
+lvol1=${LVOL_PREFIX}001
+lvol2=${LVOL_PREFIX}002
+lvol3=${LVOL_PREFIX}003
+
+pool1=$vg2
+pool2=$vg3
+DEFAULT_VOLUME=${DEFAULT_DEVICE_POOL}/$lvol1
+
+TEST_FS=
+which mkfs.ext2 && TEST_FS+="ext2 "
+which mkfs.ext3 && TEST_FS+="ext3 "
+which mkfs.ext4 && TEST_FS+="ext4 "
+which mkfs.xfs  && TEST_FS+="xfs"
+
+TEST_MNT=$TESTDIR/mnt
+
+_test_resize()
+{
+	size=$((TEST_MAX_SIZE/2))
+	fsadm -f resize size=${size}M ${DM_DEV_DIR}/$DEFAULT_VOLUME
+	size=$(align_size_up $size)
+	check lv_field $DEFAULT_DEVICE_POOL/$lvol1 lv_size ${size}.00m
+
+	# xfs does not support shrinking (xfs only grows big!! :))
+	if [ "$fs" != "xfs" ]; then
+		fsadm -f -v resize size=-$(($TEST_MAX_SIZE/4))M $DEFAULT_VOLUME
+		size=$(align_size_up $(($size-($TEST_MAX_SIZE/4))))
+		check lv_field $DEFAULT_DEVICE_POOL/$lvol1 lv_size ${size}.00m
+	fi
+	fsadm -f -v resize size=+$(($TEST_MAX_SIZE/5))M $DEFAULT_VOLUME
+	size=$(align_size_up $(($size+($TEST_MAX_SIZE/5))))
+	check lv_field $DEFAULT_DEVICE_POOL/$lvol1 lv_size ${size}.00m
+}
+
+fsadm add $TEST_DEVS
+size=$((TEST_MAX_SIZE/3))
+fsadm create size=${size}M $TEST_DEVS
+size=$(align_size_up $size)
+check lv_field $DEFAULT_DEVICE_POOL/$lvol1 lv_size ${size}.00m
+
+fsadm -f resize size=+$(($TEST_MAX_SIZE/3))M ${DM_DEV_DIR}/$DEFAULT_VOLUME
+size=$(align_size_up $(($size+($TEST_MAX_SIZE/3))))
+check lv_field $DEFAULT_DEVICE_POOL/$lvol1 lv_size ${size}.00m
+
+fsadm -f resize size=-$(($TEST_MAX_SIZE/2))M $DEFAULT_VOLUME
+size=$(align_size_up $(($size-($TEST_MAX_SIZE/2))))
+check lv_field $DEFAULT_DEVICE_POOL/$lvol1 lv_size ${size}.00m
+
+fsadm -f resize size=$(($TEST_MAX_SIZE/2))M $DEFAULT_VOLUME
+size=$(align_size_up $(($TEST_MAX_SIZE/2)))
+check lv_field $DEFAULT_DEVICE_POOL/$lvol1 lv_size ${size}.00m
+fsadm -f remove $DEFAULT_DEVICE_POOL
+
+fsadm create $dev1
+check lv_field $DEFAULT_DEVICE_POOL/$lvol1 pv_count 1
+devs=${TEST_DEVS##*$dev1 }
+fsadm resize size=+$((TEST_MAX_SIZE/2))M $DEFAULT_VOLUME $devs
+check lv_field $DEFAULT_DEVICE_POOL/$lvol1 pv_count $DEV_COUNT
+fsadm -f remove $DEFAULT_DEVICE_POOL
+
+fsadm create $dev1
+check lv_field $DEFAULT_DEVICE_POOL/$lvol1 pv_count 1
+devs=${TEST_DEVS##*$dev1 }
+fsadm resize size=$((TEST_MAX_SIZE/2))M ${DM_DEV_DIR}/$DEFAULT_VOLUME $devs
+check lv_field $DEFAULT_DEVICE_POOL/$lvol1 pv_count $DEV_COUNT
+fsadm -f remove $DEFAULT_DEVICE_POOL
+
+[ ! -d $TEST_MNT ] && mkdir $TEST_MNT &> /dev/null
+for fs in $TEST_FS; do
+	# umounted test
+	fsadm add $TEST_DEVS
+	size=$((TEST_MAX_SIZE/4))
+	fsadm create fs=$fs size=${size}M $TEST_DEVS
+
+	_test_resize
+	fsadm -f check $DEFAULT_VOLUME
+	fsadm -f remove $DEFAULT_DEVICE_POOL
+
+	# mounted test
+	fsadm add $TEST_DEVS
+	size=$((TEST_MAX_SIZE/4))
+	fsadm create fs=$fs size=${size}M $TEST_DEVS
+
+	mount ${DM_DEV_DIR}/$DEFAULT_VOLUME $TEST_MNT
+
+	_test_resize
+
+	umount $TEST_MNT
+	fsadm -f check $DEFAULT_VOLUME
+	fsadm -f remove $DEFAULT_DEVICE_POOL
+done
+fsadm  -f remove --all
+
+fsadm resize --help
+
+# Some cases which should fail
+not fsadm resize
+not fsadm resize _garbage_
+not fsadm resize $dev1
+fsadm create $TEST_DEVS
+not fsadm resize $DEFAULT_VOLUME
+not fsadm -f resize size=+10G $DEFAULT_VOLUME
+not fsadm -f resize size=-10G $DEFAULT_VOLUME
Signature · 1 line
1.7.4.4
#44 lore raw

PATCH v3 18/18 test: Add test for fsadm remove command

In reply to Lukas Czerner

The tests exercises the fsadm "remove" functionality, by trying various combinations of arguments and checking the desired results.

---
 test/t-fsadm-remove.sh |  123 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 123 insertions(+), 0 deletions(-)
 create mode 100644 test/t-fsadm-remove.sh
test/t-fsadm-remove.sh new file +123 -0
new file mode 100644
index 0000000..c7f71fb
@@ -0,0 +1,123 @@
+#!/bin/bash
+# Copyright 2011 (C) Red Hat, Inc., Lukas Czerner <[email protected]>
+#
+# This copyrighted material is made available to anyone wishing to use,
+# modify, copy, or redistribute it subject to the terms and conditions
+# of the GNU General Public License v.2.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software Foundation,
+# Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+test_description='Exercise fsadm remove'
+
+. lib/test
+
+DEV_COUNT=10
+DEV_SIZE=10
+TEST_MAX_SIZE=$(($DEV_COUNT*$DEV_SIZE))
+aux prepare_devs $DEV_COUNT $DEV_SIZE
+TEST_DEVS=$(cat DEVICES)
+export DEFAULT_DEVICE_POOL=$vg1
+export LVOL_PREFIX="lvol"
+lvol1=${LVOL_PREFIX}001
+lvol2=${LVOL_PREFIX}002
+lvol3=${LVOL_PREFIX}003
+
+pool1=$vg2
+pool2=$vg3
+DEFAULT_VOLUME=${DEFAULT_DEVICE_POOL}/$lvol1
+
+_FS=
+which mkfs.ext2 && _FS="ext2"
+which mkfs.ext3 && _FS="ext3"
+which mkfs.ext4 && _FS="ext4"
+which mkfs.xfs  && _FS="xfs"
+
+
+TEST_MNT=$TESTDIR/mnt
+
+# Remove logical volume
+fsadm create $TEST_DEVS
+check lv_field $DEFAULT_VOLUME lv_name $lvol1
+fsadm -f remove $DEFAULT_VOLUME
+not check lv_field $DEFAULT_VOLUME lv_name $lvol1
+
+# Remove volume group
+fsadm create $TEST_DEVS
+check vg_field $DEFAULT_DEVICE_POOL vg_name $DEFAULT_DEVICE_POOL
+fsadm -f remove $DEFAULT_DEVICE_POOL
+not check vg_field $DEFAULT_DEVICE_POOL vg_name $DEFAULT_DEVICE_POOL
+
+if [ "$_FS" ]; then
+	[ ! -d $TEST_MNT ] && mkdir $TEST_MNT &> /dev/null
+	# Remove mounted logical volume by mount point
+	fsadm create fs=$_FS $TEST_DEVS
+	mount ${DM_DEV_DIR}/$DEFAULT_VOLUME $TEST_MNT
+	check lv_field $DEFAULT_VOLUME lv_name $lvol1
+	fsadm -f remove $TEST_MNT
+	not check lv_field $DEFAULT_VOLUME lv_name $lvol1
+	fsadm -f remove $DEFAULT_DEVICE_POOL
+
+	# Remove mounted logical volume by lv
+	fsadm create fs=$_FS $TEST_DEVS
+	mount ${DM_DEV_DIR}/$DEFAULT_VOLUME $TEST_MNT
+	check lv_field $DEFAULT_VOLUME lv_name $lvol1
+	fsadm -f remove $DEFAULT_VOLUME
+	not check lv_field $DEFAULT_VOLUME lv_name $lvol1
+	fsadm -f remove $DEFAULT_DEVICE_POOL
+fi
+
+# Remove unused devices from the pool
+fsadm create $dev1 $dev2 $dev3
+fsadm add $TEST_DEVS
+check vg_field $DEFAULT_DEVICE_POOL pv_count $DEV_COUNT
+fsadm -f remove $TEST_DEVS
+check vg_field $DEFAULT_DEVICE_POOL pv_count 3
+fsadm -f remove --all
+
+# Remove multiple things
+fsadm add $dev1 $dev2 $pool1
+fsadm add $dev3 $dev4 $pool2
+fsadm create $pool2
+fsadm create $dev5 $dev6
+fsadm create $dev7 $dev8
+fsadm add $dev9
+check vg_field $pool1 pv_count 2
+check vg_field $pool2 pv_count 2
+check vg_field $pool2 lv_count 1
+check vg_field $DEFAULT_DEVICE_POOL pv_count 5
+check vg_field $DEFAULT_DEVICE_POOL lv_count 2
+check vg_field $pool1 vg_name $pool1
+check lv_field ${pool2}/$lvol1 lv_name $lvol1
+fsadm -f remove $pool1 ${pool2}/$lvol1 $DEFAULT_VOLUME $dev9
+not check vg_field $pool1 vg_name $pool1
+not check lv_field ${pool2}/$lvol1 lv_name $lvol1
+not check lv_field $DEFAULT_VOLUME lv_name $lvol1
+check vg_field $DEFAULT_DEVICE_POOL pv_count 4
+fsadm -f remove --all
+
+# Remove all
+fsadm add $dev1 $dev2 $pool1
+fsadm add $dev3 $dev4 $pool2
+fsadm create $pool2
+fsadm create $dev5 $dev6
+fsadm create $dev7 $dev8
+fsadm add $dev9
+check vg_field $pool1 pv_count 2
+check vg_field $pool2 pv_count 2
+check vg_field $pool2 lv_count 1
+check vg_field $DEFAULT_DEVICE_POOL pv_count 5
+check vg_field $DEFAULT_DEVICE_POOL lv_count 2
+check vg_field $pool1 vg_name $pool1
+check vg_field $pool2 vg_name $pool2
+check vg_field $DEFAULT_DEVICE_POOL vg_name $DEFAULT_DEVICE_POOL
+fsadm -f remove --all
+not check vg_field $pool1 vg_name $pool1
+not check vg_field $pool2 vg_name $pool2
+not check vg_field $DEFAULT_DEVICE_POOL vg_name $DEFAULT_DEVICE_POOL
+
+fsadm remove --help
+
+# Some cases which should fail
+not fsadm remove
Signature · 1 line
1.7.4.4
#45 lore raw

v3 cover fsadm update

In reply to Lukas Czerner
Quoted 12 lines · Lukas Czerner

Hello again,

this is third update of the fsadm.

Changes:

version 2:
The main difference is that all the big features are collapsed into one patch for better review (since some of the features depends on each other and has been developed in that way). Also one new test is added to exercise "remove" command.

Quoted 8 lines

version 3:
In version 2 some patches was unintentionally squashed together, which is fixed in this version.

Here is a link to the first version of the patch set:
http://www.redhat.com/archives/linux-lvm/2011-September/msg00046.html

Comments are welcomed.

Optionally you can get the whole patched fsadm script from:

http://people.redhat.com/lczerner/fsadm/fsadm.sh

-Lukas

Quoted 5 lines

Thanks!
-Lukas