[PATCH 0/9] FSADM updates and fixes

[PATCH 0/9] FSADM updates and fixes

[Zdenek Kabelac]

This patchset contains fsadm fixes which might potentially lead
to complete destruction of the filesystem.

The last patch in this set is a new test script so fsadm is regularly
checked whether it still works - probably  xfs & reiserfs tools will
need to be installed on the buildbot machines.

Zdenek Kabelac (9):
  can not -> cannot
  Fix return value from break trap
  Fix dry run exec
  Support for passing Yes flag recursively.
  Fix wrong default value setting for lvm
  Fix problem with fscking of mounted filesystem
  Fix dry run of resize command
  Support for noninteractive shell invocation of fsadm
  Testing script for fsadm and lvresize functionality

 scripts/fsadm.sh |   64 ++++++++++++++++++++---------
 test/t-fsadm.sh  |  119 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 152 insertions(+), 31 deletions(-)

-- 
1.7.3.1

[PATCH 1/9] can not -> cannot

[Zdenek Kabelac]

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 scripts/fsadm.sh |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 64d3184..aa6b2d9 100644
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -228,7 +228,7 @@ yes_no() {
 
 try_umount() {
 	yes_no "Do you want to unmount \"$MOUNTED\"" && dry $UMOUNT "$MOUNTED" && return 0
-	error "Can not proceed with mounted filesystem \"$MOUNTED\""
+	error "Cannot proceed with mounted filesystem \"$MOUNTED\""
 }
 
 validate_parsing() {
@@ -290,7 +290,7 @@ resize_reiser() {
 ########################
 # Resize XFS filesystem
 # - mounted for upsize
-# - can not downsize
+# - cannot downsize
 ########################
 resize_xfs() {
 	detect_mounted
@@ -379,7 +379,7 @@ resize() {
 ###################
 check() {
 	detect_fs "$1"
-	detect_mounted && error "Can not fsck device \"$VOLUME\", filesystem mounted on $MOUNTED"
+	detect_mounted && error "Cannot fsck device \"$VOLUME\", filesystem is mounted on $MOUNTED"
 	case "$FSTYPE" in
 	  "xfs") dry $XFS_CHECK "$VOLUME" ;;
 	  *) dry $FSCK $YES "$VOLUME" ;;
-- 
1.7.3.1

[PATCH 2/9] Fix return value from break trap

[Zdenek Kabelac]

This bug is serious as upon certain conditions it is possible
to break (^C) fsadm before actually resizing filesystem, but the
ongoing lvresize will think resize was succesful and shrinks
partition with unresized filesystem on it.

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 scripts/fsadm.sh |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index aa6b2d9..2c6a21b 100644
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -1,6 +1,6 @@
 #!/bin/bash
 #
-# Copyright (C) 2007-2009 Red Hat, Inc. All rights reserved.
+# Copyright (C) 2007-2010 Red Hat, Inc. All rights reserved.
 #
 # This file is part of LVM2.
 #
@@ -130,7 +130,8 @@ cleanup() {
 	# and allow recursive call of fsadm
 	unset FSADM_RUNNING
 	test "$DO_LVRESIZE" -eq 2 && exec $LVM lvresize $VERB -r -L$(( $NEWSIZE / 1048576 )) $VOLUME
-	exit ${1:-0}
+	# error exit status for break
+	exit ${1:-1}
 }
 
 # convert parameter from Exa/Peta/Tera/Giga/Mega/Kilo/Bytes and blocks
-- 
1.7.3.1

[PATCH 3/9] Fix dry run exec

[Zdenek Kabelac]

Dangerous bug - as user which gives -n/--dry-run flag
could end up with modified lv size but with unresized filesystem.

So make sure recursive exec of lvresize tool has 'dry' prefix.

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 scripts/fsadm.sh |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 2c6a21b..05b6382 100644
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -126,10 +126,13 @@ cleanup() {
 	IFS=$IFS_OLD
 	trap 2
 
-	# start LVRESIZE with the filesystem modification flag
-	# and allow recursive call of fsadm
-	unset FSADM_RUNNING
-	test "$DO_LVRESIZE" -eq 2 && exec $LVM lvresize $VERB -r -L$(( $NEWSIZE / 1048576 )) $VOLUME
+	if [ "$DO_LVRESIZE" -eq 2 ]; then
+		# start LVRESIZE with the filesystem modification flag
+		# and allow recursive call of fsadm
+		unset FSADM_RUNNING
+		dry exec $LVM lvresize $VERB $FORCE -r -L${NEWSIZE}b $VOLUME_ORIG
+	fi
+        
 	# error exit status for break
 	exit ${1:-1}
 }
-- 
1.7.3.1

[PATCH 4/9] Support for passing Yes flag recursively.

[Zdenek Kabelac]

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 scripts/fsadm.sh |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 05b6382..1393bc6 100644
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -52,7 +52,7 @@ XFS_CHECK=xfs_check
 # user may override lvm location by setting LVM_BINARY
 LVM=${LVM_BINARY-lvm}
 
-YES=
+YES=${_FSADM_YES}
 DRY=0
 VERB=
 FORCE=
@@ -129,6 +129,8 @@ cleanup() {
 	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
 		dry exec $LVM lvresize $VERB $FORCE -r -L${NEWSIZE}b $VOLUME_ORIG
 	fi
-- 
1.7.3.1

[PATCH 5/9] Fix wrong default value setting for lvm

[Zdenek Kabelac]

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 scripts/fsadm.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 1393bc6..73b6779 100644
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -50,7 +50,7 @@ FSCK=fsck
 XFS_CHECK=xfs_check
 
 # user may override lvm location by setting LVM_BINARY
-LVM=${LVM_BINARY-lvm}
+LVM=${LVM_BINARY:-lvm}
 
 YES=${_FSADM_YES}
 DRY=0
-- 
1.7.3.1

[PATCH 6/9] Fix problem with fscking of mounted filesystem

[Zdenek Kabelac]

Fix for RH bz #638050.

Update the way how fsadm detects mounted filesystem.
With udev /dev/dm-XXX paths are now returned - but mount or /proc/mounts
prints names in form of /dev/mapper/vg-lv - so the match was not found.

Current solution uses same trick as mount and detects vg-lv name through
/sys where available - this should be reasonable safe.

Instead of calling mount without parameter to get actual mount table,
switch to use /proc/mounts directly. (Not really sure, maybe we want to
use /proc/self/mounts?)

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 scripts/fsadm.sh |   28 +++++++++++++++++++++-------
 1 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 73b6779..2ee24c9 100644
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -66,6 +66,7 @@ BLOCKCOUNT=
 MOUNTPOINT=
 MOUNTED=
 REMOUNT=
+PROCMOUNTS="/proc/mounts"
 
 IFS_OLD=$IFS
 # without bash $'\n'
@@ -165,10 +166,15 @@ decode_size() {
 # detect filesystem on the given device
 # dereference device name if it is symbolic link
 detect_fs() {
-        VOLUME=${1#/dev/}
+	VOLUME_ORIG=$1
+	VOLUME=${1#/dev/}
 	VOLUME=$($READLINK $READLINK_E "/dev/$VOLUME") || error "Cannot get readlink $1"
-	# strip newline from volume name
-	VOLUME=${VOLUME%%$NL}
+	RVOLUME=$VOLUME
+	case "$RVOLUME" in
+	  /dev/dm-[0-9]*)
+		read </sys/block/${RVOLUME#/dev/}/dm/name SYSVOLUME 2>&1 && VOLUME="/dev/mapper/$SYSVOLUME"
+		;;
+	esac
 	# use /dev/null 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 /dev/null -s TYPE "$VOLUME") || error "Cannot get FSTYPE of \"$VOLUME\""
@@ -178,11 +184,19 @@ detect_fs() {
 }
 
 # check if the given device is already mounted and where
+# FIXME: resolve swap usage and device stacking
 detect_mounted()  {
-	$MOUNT >/dev/null || error "Cannot detect mounted device $VOLUME"
-	MOUNTED=$($MOUNT | $GREP "$VOLUME")
-	MOUNTED=${MOUNTED##* on }
-	MOUNTED=${MOUNTED% type *} # allow type in the mount name
+	test -e $PROCMOUNTS || error "Cannot detect mounted device $VOLUME"
+
+	MOUNTED=$($GREP ^"$VOLUME" $PROCMOUNTS)
+
+	# for empty string try again with real volume name
+	test -z "$MOUNTED" && MOUNTED=$($GREP ^"$RVOLUME" $PROCMOUNTS)
+
+	# cut device name prefix and trim everything past mountpoint
+	# echo translates \040 to spaces
+	MOUNTED=${MOUNTED#* }
+	MOUNTED=$(echo -n -e ${MOUNTED%% *})
 	test -n "$MOUNTED"
 }
 
-- 
1.7.3.1

[PATCH 7/9] Fix dry run of resize command

[Zdenek Kabelac]

Resolve from 'dry' position - we do not want to 'dry-run' echo command,
but resize of reiser fs in this case.

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 scripts/fsadm.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 2ee24c9..79bac51 100644
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -301,7 +301,7 @@ resize_reiser() {
 	decode_size $1 $BLOCKSIZE
 	verbose "Resizing \"$VOLUME\" $BLOCKCOUNT -> $NEWBLOCKCOUNT blocks ($NEWSIZE bytes, bs: $NEWBLOCKCOUNT)"
 	if [ -n "$YES" ]; then
-		dry echo y | $RESIZE_REISER -s $NEWSIZE "$VOLUME"
+		echo y | dry $RESIZE_REISER -s $NEWSIZE "$VOLUME"
 	else
 		dry $RESIZE_REISER -s $NEWSIZE "$VOLUME"
 	fi
-- 
1.7.3.1

[PATCH 8/9] Support for noninteractive shell invocation of fsadm

[Zdenek Kabelac]

try to distinguish between the case of  using interactive shell and non
interactive running - different combinations of  '-y' and '-p' option
needs to be used.

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 scripts/fsadm.sh |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 79bac51..945089a 100644
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -402,7 +402,11 @@ check() {
 	detect_mounted && error "Cannot fsck device \"$VOLUME\", filesystem is mounted on $MOUNTED"
 	case "$FSTYPE" in
 	  "xfs") dry $XFS_CHECK "$VOLUME" ;;
-	  *) dry $FSCK $YES "$VOLUME" ;;
+	  *)    # check if executed from interactive shell environment
+		case "$-" in
+		  *i*) dry $FSCK $YES $FORCE "$VOLUME" ;;
+		  *) dry $FSCK $FORCE -p "$VOLUME" ;;
+		esac
 	esac
 }
 
-- 
1.7.3.1

[PATCH 9/9] Testing script for fsadm and lvresize functionality

[Zdenek Kabelac]

Script checks for precence of tools for ext3,xfs,reiserfs and executes
only those tests which are available. In case we do not want to check
some of them - 'skip' needs to be added for given fs type.

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 test/t-fsadm.sh |  119 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 108 insertions(+), 11 deletions(-)

diff --git a/test/t-fsadm.sh b/test/t-fsadm.sh
index 6cff7c5..b89e457 100644
--- a/test/t-fsadm.sh
+++ b/test/t-fsadm.sh
@@ -1,5 +1,5 @@
 #!/bin/bash
-# Copyright (C) 2008 Red Hat, Inc. All rights reserved.
+# Copyright (C) 2008-2010 Red Hat, Inc. All rights reserved.
 #
 # This copyrighted material is made available to anyone wishing to use,
 # modify, copy, or redistribute it subject to the terms and conditions
@@ -9,17 +9,114 @@
 # 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 filesystem resize'
+
 . ./test-utils.sh
 
-exit 200
-# doesn't work right now
-aux prepare_pvs 2
-aux pvcreate --metadatacopies 0 $dev1
-vgcreate -cn $vg $devs
+aux prepare_vg 1 100
+
+# set to "skip" to avoid testing given fs and test warning result
+# i.e. check_reiserfs=skip
+check_ext3=
+check_xfs=
+check_reiserfs=
+
+which mkfs.ext3 || check_ext3=${check_ext3:=mkfs.ext3}
+which fsck.ext3 || check_ext3=${check_ext3:=fsck.ext3}
+which mkfs.xfs || check_xfs=${check_xfs:=mkfs.xfs}
+which xfs_check || check_xfs=${check_xfs:=xfs_check}
+which mkfs.reiserfs || check_reiserfs=${check_reiserfs:=mkfs.reiserfs}
+which reiserfsck || check_reiserfs=${check_reiserfs:=reiserfsck}
+
+vg_lv="$vg/$lv1"
+dev_vg_lv="$DM_DEV_DIR/$vg_lv"
+mount_dir="$TESTDIR/mnt"
+
+test ! -d $mount_dir && mkdir $mount_dir
+
+cleanup_mounted_and_teardown()
+{
+	umount $mount_dir || true
+	teardown
+}
+
+fscheck_ext3()
+{
+	fsck.ext3 -p -F -f $dev_vg_lv
+}
+
+fscheck_xfs()
+{
+	xfs_check $dev_vg_lv
+}
+
+fscheck_reiserfs()
+{
+	reiserfsck --check -p -f $dev_vg_lv </dev/null
+}
+
+check_missing()
+{
+	eval local t=$\check_$1
+	test -z "$t" && return 0
+	test "$t" = skip && return 1
+	# trick for warning test
+	echo "TEST ""WARNING: fsadm skips $1 tests, $t tool is missing"
+	return 1
+}
 
 # Test for block sizes != 1024 (rhbz #480022)
-lvcreate -n$lv1 -L 64M $vg
-mke2fs -b4096 -j $DM_DEV_DIR/$vg/$lv1
-e2fsck -f -y $DM_DEV_DIR/$vg/$lv1
-fsadm --lvresize resize $DM_DEV_DIR/$vg/$lv1 128M
-vgremove -ff $vg
+lvcreate -n $lv1 -L20M $vg
+trap 'aux cleanup_mounted_and_teardown' EXIT
+
+if check_missing ext3; then
+	mkfs.ext3 -b4096 -j $dev_vg_lv
+
+	fsadm --lvresize resize $vg_lv 30M
+	# Fails - not enough space for 4M fs
+	not fsadm --lvresize resize $dev_vg_lv 4M
+	lvresize -L+10M -r $vg_lv
+	lvreduce -L10M -r $vg_lv
+
+	fscheck_ext3
+	mount $dev_vg_lv $mount_dir
+	not fsadm --lvresize resize $vg_lv 9M
+	lvresize -L+20M -r -n $vg_lv
+	umount $mount_dir
+	fscheck_ext3
+
+	lvresize -f -L20M $vg_lv
+fi
+
+if check_missing xfs; then
+	mkfs.xfs -l internal,size=1000b -f $dev_vg_lv
+
+	fsadm --lvresize resize $vg_lv 30M
+	# Fails - not enough space for 4M fs
+	lvresize -L+10M -r $vg_lv
+	not lvreduce -L10M -r $vg_lv
+
+	fscheck_xfs
+	mount $dev_vg_lv $mount_dir
+	lvresize -L+10M -r -n $vg_lv
+	umount $mount_dir
+	fscheck_xfs
+
+	lvresize -f -L20M $vg_lv
+fi
+
+if check_missing reiserfs; then
+	mkfs.reiserfs -s 513 -f $dev_vg_lv
+
+	fsadm --lvresize resize $vg_lv 30M
+	lvresize -L+10M -r $vg_lv
+	fsadm --lvresize -y resize $vg_lv 10M
+
+	fscheck_reiserfs
+	mount $dev_vg_lv $mount_dir
+
+	not fsadm -y --lvresize resize $vg_lv 20M
+	umount $mount_dir
+
+	lvresize -f -L20M $vg_lv
+fi
-- 
1.7.3.1

[PATCH 6/9] Fix problem with fscking of mounted filesystem

[Alasdair G Kergon]

On Thu, Oct 07, 2010 at 04:06:36PM +0200, Zdenek Kabelac wrote:
> Instead of calling mount without parameter to get actual mount table,
> switch to use /proc/mounts directly. (Not really sure, maybe we want to
> use /proc/self/mounts?)
 
/proc/mounts I'd think: If it's mounted anywhere on the system, we need to
be aware, even if it's not visible to the current process.

Alasdair

[PATCH 6/9] Fix problem with fscking of mounted filesystem

[Zdenek Kabelac]

Dne 7.10.2010 19:25, Alasdair G Kergon napsal(a):
> On Thu, Oct 07, 2010 at 04:06:36PM +0200, Zdenek Kabelac wrote:
>> Instead of calling mount without parameter to get actual mount table,
>> switch to use /proc/mounts directly. (Not really sure, maybe we want to
>> use /proc/self/mounts?)
>  
> /proc/mounts I'd think: If it's mounted anywhere on the system, we need to
> be aware, even if it's not visible to the current process.
> 

I've been reading this rhbz:

https://bugzilla.redhat.com/show_bug.cgi?id=491924

Which seems to be suggesting to switch to to use self version.
On the other hand  /proc/mounts is symbolic link to self/mounts anyway.

Zdenek