All of lore.kernel.org
 help / color / mirror / Atom feed
* [linux-lvm] [RFC][PATCH 00/35] fsadm update
@ 2011-09-21 16:45 Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 01/35] fsadm: Add "create" command Lukas Czerner
                   ` (34 more replies)
  0 siblings, 35 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: dchinner, rwheeler, linux-lvm

Hello everyone,

as I mentioned before I started a project to extend existing fsadm
script to provide user friendly tool to build the storage all the way up
the stack from dm, md to file systems. Basically the tool with the ease
of use of btrfs-progs. This is my first attempt to merge my changes of
fsadm into upstream.

I am not going to (once again) describe all the changes I have made. The
best way to see the new fsadm feature is to look at updated manual
page (patch 31), fo your convenience I am going to put some parts into the
mail as well.

Not only I have made quite a lot of changes, but I have also added tests to
validate the new features so we will be able to catch regressions. One thing
might be a bit confusing though. It is the fact that there are so many
patches with some functionality scattered between more of them. The reason
is, that while I was adding more features the fsadm infrastructure needed
to change as well. It was not possible for me to simply add one functionality,
polish it and the go to the other. However I have tried to clean the commits
as much as I cold. If you have any troubles with that, please let me know!

Note that fsadm update is not quite complete yet. It is just the first
attempt to push the changes I have upstream to be able to continue to make
more changes without the painful rebase at the end and also to collect some
very much needed feedback. Note that features like support for mirrors,
snapshots and raids will follow.

So here is description of some features ripped of the manual page.

Comments are welcomed.

Thanks!

USAGE:

fsadm [ OPTIONS ] COMMAND [ COMMAND_OPTIONS ] [ ... ]

fsadm [ OPTIONS ] check [ --help ] device

fsadm  [  OPTIONS  ]  resize [ --help ] [ size=Size | size=+Size | size=-Size ] [ device...  ]
volume

fsadm [ OPTIONS ] create [ --help ] [ stripesize=StripeSize  ]  [  name=Name  ]  [  fs=Type  |
fstyp=Type ] [ size=Size ] [ stripesStripes ] [ device...  ] [ pool ]

fsadm [ OPTIONS ] list [ --help ] [ filesystems | fs ] [ device | dev ] [ pool ]

fsadm [ OPTIONS ] add [ --help ] device [ device...  ] [ pool ]

fsadm [ OPTIONS ] remove [ --help ] [ --all ] item [ item...  ]

COMMANDS:

*check*
Check the file system on device using fsck.

*resize*
Change the size of the logical volume and file system on it. You can specify  size=Size
to  resize  to  given  Size,  or  size=+Size to extend the volume by the given Size, or
size=-Size to shrink the volume by the given Size.  If no Size is provided  the  volume
will  be  resized to its maximum size.  You can also specify one or more devices to use
for extending the volume.  If the device is not in any pool, it will be added the  vol‐
ume's  pool  prior the resize. Note that some file system (namely 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 -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 xfs) does not support offline resize, it means, that
the file system will be mounted prior to resize and then unmounted afterwards.

*create*
Create a new logical volume from the pool, optionally with the defined file system. You
can specify Type of the file system which will be automatically created on the new log‐
ical volume. Currently only ext3, ext4 and xfs file systems are supported. You can cre‐
ate striped volume by defining the  StripeSize.   In  that  case,  if  Stripes  is  not
defined, then number of provided devices will be used. Either devices or Stripes has to
be defined if StripeSize is provided, otherwise 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.

*list*
List devices, file systems and pools in your system. You can select to list all logical
volumes  by  specifying filesystems, or fs.  Or you can select to list all pools in the
system by specifying pool.  Or you can also select to list all devices in the system by
specifying  devices,  or  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.

*add*
Add one, or more devices into the pool.  If no pool is specified, provided 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.

*remove*
Remove one, or more logical volumes, devices, or pools defined by items.  You can  also
specify --all to remove all pools corresponding volumes from your system.

EXAMPLES:

To add device /dev/sdb into the default pool run this command:

    fsadm add /dev/sdb

You can also add mode devices into another pool called mypool

    fsadm add /dev/sdc /dev/sdd /dev/sde mypool

To create a 300GB linear logical volume with ext4  file  system  using  devices  /dev/sda  and
/dev/sdb you can use the following command:

    fsadm create fs=ext4 size=300G /dev/sda /dev/sdb

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.

Now let's create 500GB striped volume with stripe size of 16KB and xfs file system using  four
devices.  It means that Stripes will be equal to 4, however note that we do not need to define
Stripes manually if we are listing devices as follows:

    fsadm create fs=xfs size=500G stripesize=16 /dev/sda /dev/sdb /dev/sdc /dev/sdd

Now, if we assume that we already have at least four devices  in  the  default  pool,  we  can
achieve the same result by calling:

    fsadm create fs=xfs size=500G stripesize=16 stripes=4

To shrink the default_pool/lvol001 logical volume by 10G we can simply call

    fsadm resize size=-10G default_pool/lvol001

Or we can extend it by 1T using more devices which are not in the pool just yet:

    fsadm resize size=+1T default_pool/lvol001 /dev/sde /dev/sdf

To remove the above logical volume we can use the following command:

    fsadm remove default_pool/lvol001

or we can simply remove the whole pool, wiping all logical volumes in it:

    fsadm remove default_pool

Alternatively we can remove all pools in the system by calling:

    fsadm remove --all


PATCHES:

[PATCH 01/35] fsadm: Add "create" command
[PATCH 02/35] fsadm: Add "destroy" command
[PATCH 03/35] fsadm: Add "list" command
[PATCH 04/35] fsadm: Make "create" command more vg aware
[PATCH 05/35] fsadm: Teach "destroy" command to take more arguments
[PATCH 06/35] fsadm: Simple cleanup and comment update
[PATCH 07/35] fsadm: Create "add" command
[PATCH 08/35] fsadm: Update "list" command for better alignment
[PATCH 09/35] fsadm: Specify number of stripes when no device is
[PATCH 10/35] fsadm: Print type of the volume in filesystem listing
[PATCH 11/35] fsadm: Add "remove" command
[PATCH 12/35] fsadm: Try to avoid calling LVM in the loops
[PATCH 13/35] fsadm: Merge "destroy" and "remove" into one command
[PATCH 14/35] fsadm: Allow to remove all volume groups
[PATCH 15/35] fsadm: Make all internal math in kilobytes
[PATCH 16/35] fsadm: Use warn for warnings in list command
[PATCH 17/35] fsadm: Handle resize if there is no file system
[PATCH 18/35] fsadm: Fsck extN before resize only if it is not
[PATCH 19/35] fsadm: Align numbers to the decimal point
[PATCH 20/35] fsadm: Add simple configuration file
[PATCH 21/35] fsadm: Use DEFAULT_POOL when creating volume group
[PATCH 22/35] fsadm: Add LVOL_PREFIX configuration option
[PATCH 23/35] fsadm: Only use readlink if link is provided
[PATCH 24/35] fsadm: Remove unnecessary modification of PATH
[PATCH 25/35] fsadm: Allow to specify size without "size=" prefix in
[PATCH 26/35] fsadm: Allow to specify lv in vg/lv format
[PATCH 27/35] fsadm: error out when no size is provided in resize
[PATCH 28/35] fsadm: Umount ext2 file system prior resize
[PATCH 29/35] lvresize: Specify --resize-fs-only when going to use
[PATCH 30/35] test: add helper to compute aligned lv size
[PATCH 31/35] fsadm: Add help for new commands and update man page
[PATCH 32/35] fsadm: Update authorship of the fsadm
[PATCH 33/35] test: Add test for fsadm add command
[PATCH 34/35] test: Add test for fsadm create command
[PATCH 35/35] test: Add test for fsadm resize command

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 01/35] fsadm: Add "create" command
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 19:00   ` Stephane Chazelas
  2011-09-21 16:45 ` [linux-lvm] [PATCH 02/35] fsadm: Add "destroy" command Lukas Czerner
                   ` (33 subsequent siblings)
  34 siblings, 1 reply; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

Create command provides the functionality of creating a new logical
volumes including defined file system.

This commit also changes the way how are commands recognised an
executed. The new approach is more suitable for bigger number of
commands.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |  140 ++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 131 insertions(+), 9 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index c8cc5e0..42c7da4 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -29,7 +29,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
@@ -76,6 +76,8 @@ REMOUNT=
 PROCMOUNTS="/proc/mounts"
 NULL="$DM_DEV_DIR/null"
 
+MAX_VGS=999
+
 IFS_OLD=$IFS
 # without bash $'\n'
 NL='
@@ -122,6 +124,14 @@ dry() {
 	fi
 	verbose "Executing $@"
 	$@
+	if [ $? -ne 0 ]; then
+		error "FAILED. Exitting!"
+	fi
+}
+
+is_natural() {
+	test "$1" -ge 0 &> /dev/null && return 1
+	return 0
 }
 
 cleanup() {
@@ -365,12 +375,42 @@ resize_xfs() {
 	fi
 }
 
+make_ext() {
+	device=$1
+	fstyp=$2
+	stripe=$3
+	stripesize=$4
+	bsize=4
+
+	if [ "$YES" ]; then
+		force="-F"
+	fi
+	stride=$(($stripesize/$bsize))
+	stripewidth=$(($stride*$stripe))
+
+	dry mkfs.$fstyp $force -b$(($bsize*1024)) -E stride=${stride},stripe-width=${stripewidth} $device
+}
+
+generic_make_fs() {
+	device=$1
+	fstyp=$2
+	bsize=4096
+
+	if [ "$YES" ]; then
+		force="-f"
+	fi
+
+	dry mkfs.$fstyp $force $device
+}
+
 ####################
 # Resize filesystem
 ####################
 resize() {
 	NEWSIZE=$2
 	detect_fs "$1"
+	is_natural $NEWSIZE
+	[ $? -ne 1 ] && error "$NEWSIZE is not valid number for file system size"
 	detect_device_size
 	verbose "Device \"$VOLUME\" size is $DEVSIZE bytes"
 	# if the size parameter is missing use device size
@@ -386,6 +426,89 @@ resize() {
 	cleanup 0
 }
 
+name_exists() {
+	cmd=$1
+	search=$2
+	$LVM $cmd --separator ' ' 2>&1 | tail -n-1 | cut -d' ' -f3 | grep $search 2>&1> /dev/null
+	if [ $? -eq 0 ]; then
+		return 1
+	fi
+	return 0
+}
+
+#############################
+# Create a new logical volume
+#############################
+create() {
+	size="-l 100%FREE"
+	devcount=0
+
+	for i in $@; do
+		if [ -b $i ]; then
+			devices="$devices $i"
+			devcount=$(($devcount+1))
+			continue
+		fi
+		case $i in
+			"stripesize"* | "chunksize"*) stripesize=${i##*=} ;;
+			"name"*) name="--name ${i##*=}" ;;
+			"fstyp"*) fstyp=${i##*=} ;;
+			"size"*) size="-L ${i##*=}" ;;
+			*) error "Wrong option $i. (see: $TOOL --help)"
+		esac
+	done
+
+	for i in $(seq -w $MAX_VGS); do
+		name_exists vgs vg${i}
+		if [ $? -eq 0 ]; then
+			vgname="vg${i}"
+			break;
+		fi
+	done
+
+	[ -z "$vgname" ] && error "No suitable name for volume group found."
+	[ $devcount -eq 0 ] && error "No suitable device specified."
+
+	if [ "$stripesize" ]; then
+		striped="-i $devcount -I $stripesize"
+	fi
+
+	if [ "$name" ]; then
+		lvname="--name $name"
+	else
+		for i in $(seq -w $MAX_VGS); do
+			name_exists "vgs $vgname" lvol${i}
+			if [ $? -eq 0 ]; then
+				name="lvol${i}"
+				lvname="--name $name"
+				break;
+			fi
+		done
+	fi
+
+	dry $LVM vgcreate $VERB $vgname $devices
+	dry $LVM lvcreate $VERB $lvname $size $striped $vgname
+	device="/dev/${vgname}/${name}"
+
+	guess=0
+	echo "$fstyp"
+	if [ -z $fstyp ]; then
+		fstyp=$(echo $TOOL | sed 's/^.*\.//g')
+		guess=1
+	fi
+
+	case $fstyp in
+		ext[234]) make_ext $device $fstyp $devcount $stripesize ;;
+		xfs|reiserfs) generic_make_fs $device $fstyp;;
+		*)	if [ $guess -eq 1 ]; then
+				return 0
+			else
+				error "Filesystem $fstyp is not supported"
+			fi
+			;;
+	esac
+}
+
 ####################################
 # Calclulate diff between two dates
 #  LANG=C input is expected the
@@ -477,18 +600,17 @@ do
 	  "-e"|"--ext-offline") EXTOFF=1 ;;
 	  "-y"|"--yes") YES="-y" ;;
 	  "-l"|"--lvresize") DO_LVRESIZE=1 ;;
-	  "check") CHECK="$2" ; shift ;;
-	  "resize") RESIZE="$2"; NEWSIZE="$3" ; shift 2 ;;
+	  "check") COMMAND=$1; shift; ARGS=$@; break ;;
+	  "resize") COMMAND=$1; shift; ARGS=$@; break ;;
+	  "create") 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
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 02/35] fsadm: Add "destroy" command
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 01/35] fsadm: Add "create" command Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 03/35] fsadm: Add "list" command Lukas Czerner
                   ` (32 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

This commit adds new command "destroy" which provides the functionality
of removing logical volumes or volume group.

Several fixes for filesystem creation are also involved including fix
for creating extn file system on striped logical volume.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |   67 +++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 62 insertions(+), 5 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 42c7da4..2895371 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -385,10 +385,14 @@ make_ext() {
 	if [ "$YES" ]; then
 		force="-F"
 	fi
-	stride=$(($stripesize/$bsize))
-	stripewidth=$(($stride*$stripe))
 
-	dry mkfs.$fstyp $force -b$(($bsize*1024)) -E stride=${stride},stripe-width=${stripewidth} $device
+	if [ ! -z $stripesize ]; then
+		stride=$(($stripesize/$bsize))
+		stripewidth=$(($stride*$stripe))
+		extended="-E stride=${stride},stripe-width=${stripewidth}"
+	fi
+
+	dry mkfs.$fstyp $force -b$(($bsize*1024)) $extended $device
 }
 
 generic_make_fs() {
@@ -452,7 +456,7 @@ create() {
 		case $i in
 			"stripesize"* | "chunksize"*) stripesize=${i##*=} ;;
 			"name"*) name="--name ${i##*=}" ;;
-			"fstyp"*) fstyp=${i##*=} ;;
+			"fstyp"* | "fs"*) fstyp=${i##*=} ;;
 			"size"*) size="-L ${i##*=}" ;;
 			*) error "Wrong option $i. (see: $TOOL --help)"
 		esac
@@ -491,7 +495,6 @@ create() {
 	device="/dev/${vgname}/${name}"
 
 	guess=0
-	echo "$fstyp"
 	if [ -z $fstyp ]; then
 		fstyp=$(echo $TOOL | sed 's/^.*\.//g')
 		guess=1
@@ -509,6 +512,59 @@ create() {
 	esac
 }
 
+#############################
+# Remove the logical volume
+# of the whole volume group
+#############################
+destroy() {
+	if [ $# -gt 2 ]; then
+		error "Wrong option for destroy. (see: $TOOL --help)"
+	fi
+
+	# help
+	if [ "$1" == "help" ]; then
+		echo "Usage: $TOOL destroy [mount point | dm device | voulume group]"
+		exit 0
+	fi
+
+	# Block device has been given
+	if [ -b $1 ]; then
+		device=$1
+	fi
+
+	# Mount point has been given
+	if [ -z $device ] && [ -d $1 ]; then
+		mp=$(echo $1 | 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 ] && error "Could not find device mounted at $mp"
+			device=$(LANG=C $MOUNT | $GREP " $mp " | cut -d' ' -f1)
+		fi
+		MOUNTED=$device
+	fi
+
+	if [ -z $device ]; then
+		$LVM vgs $1
+		# Volume group has been given
+		if [ $? -eq 0 ]; then
+			dry $LVM vgremove $FORCE $1
+			return 0
+		fi
+	fi
+
+	[ -z $device ] && error "$1 is not a valid mount point, dm device nor volume group name."
+
+	if [ $MOUNTED ]; then
+		try_umount
+	fi
+	dry $LVM lvremove $FORCE $device
+}
+
 ####################################
 # Calclulate diff between two dates
 #  LANG=C input is expected the
@@ -603,6 +659,7 @@ do
 	  "check") COMMAND=$1; shift; ARGS=$@; break ;;
 	  "resize") COMMAND=$1; shift; ARGS=$@; break ;;
 	  "create") COMMAND=$1; shift; ARGS=$@; break ;;
+	  "destroy") COMMAND=$1; shift; ARGS=$@; break ;;
 	  *) error "Wrong argument \"$1\". (see: $TOOL --help)"
 	esac
 	shift
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 03/35] fsadm: Add "list" command
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 01/35] fsadm: Add "create" command Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 02/35] fsadm: Add "destroy" command Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 04/35] fsadm: Make "create" command more vg aware Lukas Czerner
                   ` (31 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

List command provides the functionality of listing useful information
about the logical volumes, file systems and devices.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |  231 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 229 insertions(+), 2 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 2895371..803c6a5 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -17,7 +17,8 @@
 # Script for resizing devices (usable for LVM resize)
 #
 # Needed utilities:
-#   mount, umount, grep, readlink, blockdev, blkid, fsck, xfs_check
+#   mount, umount, grep, readlink, blockdev, blkid, fsck, xfs_check, bc, df
+#   xfs_db
 #
 # ext2/ext3/ext4: resize2fs, tune2fs
 # reiserfs: resize_reiserfs, reiserfstune
@@ -54,6 +55,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}
@@ -74,6 +77,8 @@ MOUNTPOINT=
 MOUNTED=
 REMOUNT=
 PROCMOUNTS="/proc/mounts"
+PROCDEVICES="/proc/devices"
+PROCPARTITIONS="/proc/partitions"
 NULL="$DM_DEV_DIR/null"
 
 MAX_VGS=999
@@ -202,7 +207,6 @@ detect_fs() {
 	FSTYPE=$("$BLKID" -c "$NULL" -s TYPE "$VOLUME") || error "Cannot get FSTYPE of \"$VOLUME\""
 	FSTYPE=${FSTYPE##*TYPE=\"} # cut quotation marks
 	FSTYPE=${FSTYPE%%\"*}
-	verbose "\"$FSTYPE\" filesystem found on \"$VOLUME\""
 }
 
 # check if the given device is already mounted and where
@@ -413,6 +417,7 @@ generic_make_fs() {
 resize() {
 	NEWSIZE=$2
 	detect_fs "$1"
+	verbose "\"$FSTYPE\" filesystem found on \"$VOLUME\""
 	is_natural $NEWSIZE
 	[ $? -ne 1 ] && error "$NEWSIZE is not valid number for file system size"
 	detect_device_size
@@ -565,6 +570,226 @@ destroy() {
 	dry $LVM lvremove $FORCE $device
 }
 
+float_math() {
+	if [ $# -le 0 ]; then
+		return
+	fi
+	result=$(LANG=C echo "scale=2; $@" | $BC -q 2> /dev/null)
+	echo $result
+}
+
+#####################################
+# Convet the size into human readable
+# form. size in KiB 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="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"
+}
+
+get_ext_size() {
+	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
+
+	bsize=$(($bsize/1024))
+	total=$(($bcount*$bsize))
+	TOTAL=$(humanize_size $total)
+	used=$((($bcount-$fbcount)*$bsize))
+	USED=$(humanize_size $used)
+	free=$((($fbcount-$rbcount)*$bsize))
+	FREE=$(humanize_size $free)
+	IFS=$OLD_IFS
+}
+
+get_xfs_size() {
+	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
+		bsize=$(($bsize/1024))
+		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)
+	IFS=$OLD_IFS
+}
+
+detect_fs_size() {
+	case $FSTYPE in
+		ext[234]) get_ext_size ;;
+		xfs) get_xfs_size ;;
+		*) error "Filesystem $FSTYM is not supported by $TOOL"
+	esac
+}
+
+list_filesystems() {
+	IFS=$NL
+	format="%-20s%-8s%-13s%-13s%-13s%s\n"
+	header=$(printf $format "Volume" "Type" "Free" "Used" "Total" "Mount point")
+	separator=""
+	for i in $(seq ${#header}); do
+		separator+="-"
+	done
+	echo $separator
+	echo $header
+	echo $separator
+	IFS=$NL
+	c=0
+	for line in $($LVM lvs -o lv_path,lv_size --noheadings --separator ' ' --nosuffix --units k 2> /dev/null); do
+		line=$(echo $line | sed -e 's/^ *\//\//')
+		volume=$(echo $line | cut -d' ' -f1)
+		detect_fs $volume
+		detect_mounted
+		detect_fs_size
+		printf $format $volume $FSTYPE $FREE $USED $TOTAL $MOUNTED
+		c=$((c+1))
+	done
+	if [ $c -eq 0 ]; then
+		echo " No file systems suitable for managing by $TOOL found."
+	fi
+	echo $separator
+	IFS=$OLD_IFS
+}
+
+list_devices() {
+	IFS=$NL
+
+	format="%-13s%-13s%-13s%-13s%s\n"
+	header=$(printf $format "Device" "Free" "Used" "Total" "Group")
+	separator=""
+	for i in $(seq ${#header}); do
+		separator+="-"
+	done
+	echo $separator
+	echo $header
+	echo $separator
+	c=0
+	for line in $(LANG=C $LVM pvs -o pv_name,vg_name,pv_size,pv_free,pv_used --separator ' ' --noheadings --nosuffix --units k); do
+		line=$(echo $line | sed -e 's/^ *\//\//')
+		device=$(echo $line | cut -d' ' -f1)
+		group=$(echo $line | cut -d' ' -f2)
+		total=$(echo $line | cut -d' ' -f3)
+		total=$(humanize_size ${total%%.*})
+		free=$(echo $line | cut -d' ' -f4)
+		free=$(humanize_size ${free%%.*})
+		used=$(echo $line | cut -d' ' -f5)
+		used=$(humanize_size ${used%%.*})
+
+		printf $format $device $free $used $total $group
+		c=$((c+1))
+	done
+	if [ $c -eq 0 ]; then
+		echo " No devices found in the pool."
+	fi
+	echo $separator
+	IFS=$OLD_IFS
+}
+
+list_pool() {
+	IFS=$NL
+
+	format="%-10s%-9s%-13s%-13s%s\n"
+	header=$(printf $format "Group" "Devices" "Free" "Used" "Total")
+	separator=""
+	for i in $(seq ${#header}); do
+		separator+="-"
+	done
+	echo $separator
+	echo $header
+	echo $separator
+	c=0
+	for line in $(LANG=C $LVM vgs -o vg_name,pv_count,vg_size,vg_free --separator ' ' --noheadings --nosuffix --units k); do
+		line=$(echo $line | sed -e 's/^ *//')
+		group=$(echo $line | cut -d' ' -f1)
+		devices=$(echo $line | cut -d' ' -f2)
+		total=$(echo $line | cut -d' ' -f3)
+		free=$(echo $line | cut -d' ' -f4)
+
+		used=$((${total%%.*}-${free%%.*}))
+		used=$(humanize_size ${used%%.*})
+		total=$(humanize_size ${total%%.*})
+		free=$(humanize_size ${free%%.*})
+		printf $format $group $devices $total $free $used
+		c=$((c+1))
+	done
+	if [ $c -eq 0 ]; then
+		echo " Pool is empty."
+	fi
+	echo $separator
+	IFS=$OLD_IFS
+}
+
+#############################
+# 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
+}
+
+
 ####################################
 # Calclulate diff between two dates
 #  LANG=C input is expected the
@@ -579,6 +804,7 @@ diff_dates() {
 ###################
 check() {
 	detect_fs "$1"
+	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
@@ -660,6 +886,7 @@ do
 	  "resize") COMMAND=$1; shift; ARGS=$@; break ;;
 	  "create") COMMAND=$1; shift; ARGS=$@; break ;;
 	  "destroy") COMMAND=$1; shift; ARGS=$@; break ;;
+	  "list") COMMAND=$1; shift; ARGS=$@; break ;;
 	  *) error "Wrong argument \"$1\". (see: $TOOL --help)"
 	esac
 	shift
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 04/35] fsadm: Make "create" command more vg aware
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (2 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 03/35] fsadm: Add "list" command Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 05/35] fsadm: Teach "destroy" command to take more arguments Lukas Czerner
                   ` (30 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

Update create command so we can also specify volume group (pool) from
which we want to create logical volume, or, if neither device with
sufficient space, nor volume group is specified, it decides
automatically ho the logical volume should be created.

Also fix the list command so it will not print wrong data in case that
none was loaded.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |  207 +++++++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 167 insertions(+), 40 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 803c6a5..00b5019 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -76,6 +76,9 @@ BLOCKCOUNT=
 MOUNTPOINT=
 MOUNTED=
 REMOUNT=
+IN_GROUP=
+NOT_IN_GROUP=
+GROUP=
 PROCMOUNTS="/proc/mounts"
 PROCDEVICES="/proc/devices"
 PROCPARTITIONS="/proc/partitions"
@@ -134,6 +137,14 @@ dry() {
 	fi
 }
 
+float_math() {
+	if [ $# -le 0 ]; then
+		return
+	fi
+	result=$(LANG=C echo "scale=2; $@" | $BC -q 2> /dev/null)
+	echo $result
+}
+
 is_natural() {
 	test "$1" -ge 0 &> /dev/null && return 1
 	return 0
@@ -204,9 +215,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%%\"*}
+	return 0
 }
 
 # check if the given device is already mounted and where
@@ -417,6 +429,7 @@ generic_make_fs() {
 resize() {
 	NEWSIZE=$2
 	detect_fs "$1"
+	[ $? -eq 1 ] && error "Cannot get FSTYPE of \"$VOLUME\""
 	verbose "\"$FSTYPE\" filesystem found on \"$VOLUME\""
 	is_natural $NEWSIZE
 	[ $? -ne 1 ] && error "$NEWSIZE is not valid number for file system size"
@@ -438,19 +451,52 @@ resize() {
 name_exists() {
 	cmd=$1
 	search=$2
-	$LVM $cmd --separator ' ' 2>&1 | tail -n-1 | cut -d' ' -f3 | grep $search 2>&1> /dev/null
+	$LVM $cmd --separator ' ' --noheadings --nosuffix 2>&1 | cut -d' ' -f3 | grep $search 2>&1> /dev/null
 	if [ $? -eq 0 ]; then
 		return 1
 	fi
 	return 0
 }
 
+detect_device_group() {
+	devices=$@
+	ret=0
+	prev_vgname=""
+	vgs=""
+
+	NOT_IN_GROUP=
+	IN_GROUP=
+	for i in $devices; do
+		cur_vgname=$(LANG=C $LVM pvs -o vg_name --separator ' ' --noheadings --nosuffix --units k $i 2> /dev/null | sed -e 's/^ *//')
+
+		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
+	GROUP=$vgs
+	return $ret
+}
+
 #############################
 # Create a new logical volume
 #############################
 create() {
-	size="-l 100%FREE"
 	devcount=0
+	vg_create=0
 
 	for i in $@; do
 		if [ -b $i ]; then
@@ -460,23 +506,76 @@ create() {
 		fi
 		case $i in
 			"stripesize"* | "chunksize"*) stripesize=${i##*=} ;;
-			"name"*) name="--name ${i##*=}" ;;
+			"name"*) name=${i##*=} ;;
 			"fstyp"* | "fs"*) fstyp=${i##*=} ;;
-			"size"*) size="-L ${i##*=}" ;;
-			*) error "Wrong option $i. (see: $TOOL --help)"
+			"size"*) size=${i##*=} ;;
+			*) if [ -z $vg ]; then vgname=$i; else
+				error "Wrong option $i. (see: $TOOL --help)"
+			   fi ;;
 		esac
 	done
 
-	for i in $(seq -w $MAX_VGS); do
-		name_exists vgs vg${i}
-		if [ $? -eq 0 ]; then
-			vgname="vg${i}"
-			break;
+	detect_device_group $devices
+	if [ $? -ne 0 ]; then
+		error "Some of the devices are in different groups. Please run \
+			\"$TOOL list\" to get more information on layout."
+	fi
+	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
-	done
+	elif [ $GROUP ]; then
+		vgname=$GROUP
+	fi
+	echo $vgname
+
+	# 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
+	# 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)
+		echo "lines = $lines"
+
+		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
+					echo "Yes"
+					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."
-	[ $devcount -eq 0 ] && error "No suitable device specified."
 
 	if [ "$stripesize" ]; then
 		striped="-i $devcount -I $stripesize"
@@ -486,17 +585,30 @@ create() {
 		lvname="--name $name"
 	else
 		for i in $(seq -w $MAX_VGS); do
-			name_exists "vgs $vgname" lvol${i}
-			if [ $? -eq 0 ]; then
+			$LVM lvs $vgname --separator ' ' --noheadings | grep -e " lvol${i} " &> /dev/null
+			if [ $? -ne 0 ]; then
 				name="lvol${i}"
 				lvname="--name $name"
 				break;
 			fi
 		done
 	fi
+	echo "going to create logical volume $lvname"
+
+	[ -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
 
-	dry $LVM vgcreate $VERB $vgname $devices
-	dry $LVM lvcreate $VERB $lvname $size $striped $vgname
+	if [ -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
@@ -554,7 +666,7 @@ destroy() {
 	fi
 
 	if [ -z $device ]; then
-		$LVM vgs $1
+		$LVM vgs $1 &> /dev/null
 		# Volume group has been given
 		if [ $? -eq 0 ]; then
 			dry $LVM vgremove $FORCE $1
@@ -570,14 +682,6 @@ destroy() {
 	dry $LVM lvremove $FORCE $device
 }
 
-float_math() {
-	if [ $# -le 0 ]; then
-		return
-	fi
-	result=$(LANG=C echo "scale=2; $@" | $BC -q 2> /dev/null)
-	echo $result
-}
-
 #####################################
 # Convet the size into human readable
 # form. size in KiB expected
@@ -621,7 +725,7 @@ get_ext_size() {
 	USED=$(humanize_size $used)
 	free=$((($fbcount-$rbcount)*$bsize))
 	FREE=$(humanize_size $free)
-	IFS=$OLD_IFS
+	IFS=$IFS_OLD
 }
 
 get_xfs_size() {
@@ -649,7 +753,6 @@ get_xfs_size() {
 		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)
@@ -658,15 +761,19 @@ get_xfs_size() {
 	FREE=$(humanize_size $free)
 	used=$(echo $line | cut -d' ' -f2)
 	USED=$(humanize_size $used)
-	IFS=$OLD_IFS
+	IFS=$IFS_OLD
 }
 
 detect_fs_size() {
+	if [ -z $FSTYPE ]; then
+		return
+	fi
 	case $FSTYPE in
 		ext[234]) get_ext_size ;;
 		xfs) get_xfs_size ;;
-		*) error "Filesystem $FSTYM is not supported by $TOOL"
+		*) return 1 ;;
 	esac
+	return 0
 }
 
 list_filesystems() {
@@ -680,22 +787,31 @@ list_filesystems() {
 	echo $separator
 	echo $header
 	echo $separator
-	IFS=$NL
 	c=0
-	for line in $($LVM lvs -o lv_path,lv_size --noheadings --separator ' ' --nosuffix --units k 2> /dev/null); do
+	for line in $(LANG=C $LVM lvs -o lv_path,lv_size --noheadings --separator ' ' --nosuffix --units k 2> /dev/null); do
 		line=$(echo $line | sed -e 's/^ *\//\//')
 		volume=$(echo $line | cut -d' ' -f1)
 		detect_fs $volume
 		detect_mounted
 		detect_fs_size
-		printf $format $volume $FSTYPE $FREE $USED $TOTAL $MOUNTED
+		if [ -z "$TOTAL" ]; then
+			total=$(echo $line | cut -d' ' -f2)
+			TOTAL=$(humanize_size ${total%%.})
+		fi
+		printf "$format" "$volume" "$FSTYPE" "$FREE" "$USED" "$TOTAL" "$MOUNTED"
+		volume=
+		FSTYPE=
+		FREE=
+		USED=
+		TOTAL=
+		MOUNTED=
 		c=$((c+1))
 	done
 	if [ $c -eq 0 ]; then
 		echo " No file systems suitable for managing by $TOOL found."
 	fi
 	echo $separator
-	IFS=$OLD_IFS
+	IFS=$IFS_OLD
 }
 
 list_devices() {
@@ -722,14 +838,19 @@ list_devices() {
 		used=$(echo $line | cut -d' ' -f5)
 		used=$(humanize_size ${used%%.*})
 
-		printf $format $device $free $used $total $group
+		printf "$format" "$device" "$free" "$used" "$total" "$group"
+		device=
+		free=
+		used=
+		total=
+		group=
 		c=$((c+1))
 	done
 	if [ $c -eq 0 ]; then
 		echo " No devices found in the pool."
 	fi
 	echo $separator
-	IFS=$OLD_IFS
+	IFS=$IFS_OLD
 }
 
 list_pool() {
@@ -745,25 +866,30 @@ list_pool() {
 	echo $header
 	echo $separator
 	c=0
-	for line in $(LANG=C $LVM vgs -o vg_name,pv_count,vg_size,vg_free --separator ' ' --noheadings --nosuffix --units k); 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
 		line=$(echo $line | sed -e 's/^ *//')
 		group=$(echo $line | cut -d' ' -f1)
 		devices=$(echo $line | cut -d' ' -f2)
 		total=$(echo $line | cut -d' ' -f3)
 		free=$(echo $line | cut -d' ' -f4)
-
 		used=$((${total%%.*}-${free%%.*}))
 		used=$(humanize_size ${used%%.*})
 		total=$(humanize_size ${total%%.*})
 		free=$(humanize_size ${free%%.*})
-		printf $format $group $devices $total $free $used
+
+		printf "$format" "$group" "$devices" "$free" "$used" "$total"
+		group=
+		devices=
+		total=
+		free=
+		used=
 		c=$((c+1))
 	done
 	if [ $c -eq 0 ]; then
 		echo " Pool is empty."
 	fi
 	echo $separator
-	IFS=$OLD_IFS
+	IFS=$IFS_OLD
 }
 
 #############################
@@ -804,6 +930,7 @@ diff_dates() {
 ###################
 check() {
 	detect_fs "$1"
+	[ $? -eq 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";
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 05/35] fsadm: Teach "destroy" command to take more arguments
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (3 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 04/35] fsadm: Make "create" command more vg aware Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 06/35] fsadm: Simple cleanup and comment update Lukas Czerner
                   ` (29 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

This commit teaches the destroy command to remove more groups or lvs at
one invocation by specifying more arguments.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |   59 ++++++++++++++++++++++++++++++++----------------------
 1 files changed, 35 insertions(+), 24 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 00b5019..3e9ed3b 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -125,6 +125,10 @@ error() {
 	cleanup 1
 }
 
+warn() {
+	echo "$TOOL: $@" >&2
+}
+
 dry() {
 	if [ "$DRY" -ne 0 ]; then
 		verbose "Dry execution $@"
@@ -517,8 +521,8 @@ create() {
 
 	detect_device_group $devices
 	if [ $? -ne 0 ]; then
-		error "Some of the devices are in different groups. Please run \
-			\"$TOOL list\" to get more information on layout."
+		error "Devices are in different groups. Please run "\
+			"\"$TOOL list\" to get more information on layout."
 	fi
 	if [ $GROUP ] && [ $vgname ];then
 		if [ "$vgname" != "$GROUP" ]; then
@@ -629,29 +633,19 @@ create() {
 	esac
 }
 
-#############################
-# Remove the logical volume
-# of the whole volume group
-#############################
-destroy() {
-	if [ $# -gt 2 ]; then
-		error "Wrong option for destroy. (see: $TOOL --help)"
-	fi
-
-	# help
-	if [ "$1" == "help" ]; then
-		echo "Usage: $TOOL destroy [mount point | dm device | voulume group]"
-		exit 0
-	fi
+do_destroy() {
+	item=$1
+	device=
+	MOUNTED=
 
 	# Block device has been given
-	if [ -b $1 ]; then
-		device=$1
+	if [ -b $item ]; then
+		device=$item
 	fi
 
 	# Mount point has been given
-	if [ -z $device ] && [ -d $1 ]; then
-		mp=$(echo $1 | sed -e 's/\/$//')
+	if [ -z $device ] && [ -d $item ]; then
+		mp=$(echo $item | sed -e 's/\/$//')
 
 		count=$($GREP " $mp " $PROCMOUNTS | wc -l)
 
@@ -659,22 +653,22 @@ destroy() {
 			device=$($GREP " $mp " $PROCMOUNTS | cut -d' ' -f1)
 		else
 			count=$(LANG=C $MOUNT | $GREP " $mp " | wc -l)
-			[ $count -ne 1 ] && error "Could not find device mounted at $mp"
+			[ $count -ne 1 ] && warn "Could not find device mounted at $mp" && return
 			device=$(LANG=C $MOUNT | $GREP " $mp " | cut -d' ' -f1)
 		fi
 		MOUNTED=$device
 	fi
 
 	if [ -z $device ]; then
-		$LVM vgs $1 &> /dev/null
+		$LVM vgs $item &> /dev/null
 		# Volume group has been given
 		if [ $? -eq 0 ]; then
-			dry $LVM vgremove $FORCE $1
+			dry $LVM vgremove $FORCE $item
 			return 0
 		fi
 	fi
 
-	[ -z $device ] && error "$1 is not a valid mount point, dm device nor volume group name."
+	[ -z $device ] && warn "$item is not a valid mount point, dm device nor volume group name."&& return
 
 	if [ $MOUNTED ]; then
 		try_umount
@@ -682,6 +676,23 @@ destroy() {
 	dry $LVM lvremove $FORCE $device
 }
 
+#############################
+# Remove the logical volume
+# of the whole volume group
+#############################
+destroy() {
+	# help
+	if [ "$1" == "help" ]; then
+		echo "Usage: $TOOL destroy [mount point | dm device | voulume group]"
+		exit 0
+	fi
+
+	for item in $@; do
+		do_destroy $item
+	done
+
+}
+
 #####################################
 # Convet the size into human readable
 # form. size in KiB expected
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 06/35] fsadm: Simple cleanup and comment update
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (4 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 05/35] fsadm: Teach "destroy" command to take more arguments Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 07/35] fsadm: Create "add" command Lukas Czerner
                   ` (28 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

Create new comments for various functions and update error messages.
Also remove not used name_exists() function.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |   57 ++++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 3e9ed3b..91b0de3 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -395,6 +395,10 @@ resize_xfs() {
 	fi
 }
 
+#####################################
+# Create extN filesystem with respect
+# to the striped volume
+#####################################
 make_ext() {
 	device=$1
 	fstyp=$2
@@ -415,6 +419,9 @@ make_ext() {
 	dry mkfs.$fstyp $force -b$(($bsize*1024)) $extended $device
 }
 
+############################################
+# Create a file system just using mkfs.fstyp
+############################################
 generic_make_fs() {
 	device=$1
 	fstyp=$2
@@ -452,16 +459,10 @@ resize() {
 	cleanup 0
 }
 
-name_exists() {
-	cmd=$1
-	search=$2
-	$LVM $cmd --separator ' ' --noheadings --nosuffix 2>&1 | cut -d' ' -f3 | grep $search 2>&1> /dev/null
-	if [ $? -eq 0 ]; then
-		return 1
-	fi
-	return 0
-}
-
+#################################
+# Check the device list to detect
+# if there is not multiple groups
+#################################
 detect_device_group() {
 	devices=$@
 	ret=0
@@ -633,6 +634,10 @@ create() {
 	esac
 }
 
+#############################
+# Remove the logical volume
+# of the whole volume group
+#############################
 do_destroy() {
 	item=$1
 	device=
@@ -676,10 +681,10 @@ do_destroy() {
 	dry $LVM lvremove $FORCE $device
 }
 
-#############################
-# Remove the logical volume
-# of the whole volume group
-#############################
+###############################
+# Iterate through the arguments
+# and do_destroy on them
+###############################
 destroy() {
 	# help
 	if [ "$1" == "help" ]; then
@@ -718,6 +723,10 @@ humanize_size() {
 	echo "$size $unit"
 }
 
+#############################
+# Get size of entN filesystem
+# by reading tune2fs output
+#############################
 get_ext_size() {
 	IFS=$NL
 	for i in $(LANG=C $TUNE_EXT -l "$VOLUME"); do
@@ -739,6 +748,12 @@ get_ext_size() {
 	IFS=$IFS_OLD
 }
 
+############################
+# Get size of xfs file system
+# by reading the df output or
+# examine file system with
+# xfs_db tool
+#############################
 get_xfs_size() {
 	IFS=$NL
 	if [ -z $MOUNTED ]; then
@@ -787,6 +802,10 @@ detect_fs_size() {
 	return 0
 }
 
+#############################
+# List all file systems built
+# on top of DM device
+#############################
 list_filesystems() {
 	IFS=$NL
 	format="%-20s%-8s%-13s%-13s%-13s%s\n"
@@ -825,6 +844,9 @@ list_filesystems() {
 	IFS=$IFS_OLD
 }
 
+###########################
+# List all physical volumes
+###########################
 list_devices() {
 	IFS=$NL
 
@@ -858,12 +880,15 @@ list_devices() {
 		c=$((c+1))
 	done
 	if [ $c -eq 0 ]; then
-		echo " No devices found in the pool."
+		echo " No devices found."
 	fi
 	echo $separator
 	IFS=$IFS_OLD
 }
 
+################################
+# List all pools (volume groups)
+################################
 list_pool() {
 	IFS=$NL
 
@@ -897,7 +922,7 @@ list_pool() {
 		c=$((c+1))
 	done
 	if [ $c -eq 0 ]; then
-		echo " Pool is empty."
+		echo " No pools found on the system."
 	fi
 	echo $separator
 	IFS=$IFS_OLD
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 07/35] fsadm: Create "add" command
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (5 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 06/35] fsadm: Simple cleanup and comment update Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 08/35] fsadm: Update "list" command for better alignment Lukas Czerner
                   ` (27 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

Add command allows to add devices into volume groups (pool).

Also update device listing so we see all block devices in the system
with appropriate mount points and information whether it has partitions.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |  116 +++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 97 insertions(+), 19 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 91b0de3..5dc945b 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -18,7 +18,7 @@
 #
 # Needed utilities:
 #   mount, umount, grep, readlink, blockdev, blkid, fsck, xfs_check, bc, df
-#   xfs_db
+#   xfs_db, mktemp
 #
 # ext2/ext3/ext4: resize2fs, tune2fs
 # reiserfs: resize_reiserfs, reiserfstune
@@ -230,10 +230,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
@@ -242,8 +242,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
@@ -543,7 +543,6 @@ create() {
 	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)
-		echo "lines = $lines"
 
 		if [ $lines -eq 1 ]; then
 			vgname=$(echo $vgroups | sed -e 's/^ *//' | cut -d' ' -f1)
@@ -845,13 +844,14 @@ list_filesystems() {
 }
 
 ###########################
-# List all physical volumes
+# List all non DM devices
 ###########################
 list_devices() {
 	IFS=$NL
+	tmp=$(mktemp)
 
-	format="%-13s%-13s%-13s%-13s%s\n"
-	header=$(printf $format "Device" "Free" "Used" "Total" "Group")
+	format="%-13s%-13s%-13s%-13s%-13s%s\n"
+	header=$(printf $format "Device" "Free" "Used" "Total" "Group" "Mount point")
 	separator=""
 	for i in $(seq ${#header}); do
 		separator+="-"
@@ -860,23 +860,41 @@ list_devices() {
 	echo $header
 	echo $separator
 	c=0
-	for line in $(LANG=C $LVM pvs -o pv_name,vg_name,pv_size,pv_free,pv_used --separator ' ' --noheadings --nosuffix --units k); do
-		line=$(echo $line | sed -e 's/^ *\//\//')
-		device=$(echo $line | cut -d' ' -f1)
-		group=$(echo $line | cut -d' ' -f2)
+	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 k > $tmp
+	for line in $(cat $PROCPARTITIONS | tail -n +3 | sed -e 's/^  *//' | grep -v -e "^$dmnumber"); do
+		line=$(echo $line | sed -e 's/  */ /g')
 		total=$(echo $line | cut -d' ' -f3)
 		total=$(humanize_size ${total%%.*})
-		free=$(echo $line | cut -d' ' -f4)
-		free=$(humanize_size ${free%%.*})
-		used=$(echo $line | cut -d' ' -f5)
-		used=$(humanize_size ${used%%.*})
+		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
 
-		printf "$format" "$device" "$free" "$used" "$total" "$group"
-		device=
+		if [ ! -z $line ]; then
+			line=$(echo $line | sed -e 's/^ *\//\//')
+			RVOLUME=$(echo $line | cut -d' ' -f1)
+			group=$(echo $line | cut -d' ' -f2)
+			total=$(echo $line | cut -d' ' -f3)
+			total=$(humanize_size ${total%%.*})
+			free=$(echo $line | cut -d' ' -f4)
+			free=$(humanize_size ${free%%.*})
+			used=$(echo $line | cut -d' ' -f5)
+			used=$(humanize_size ${used%%.*})
+		fi
+
+		printf "$format" "$RVOLUME" "$free" "$used" "$total" "$group" "$MOUNTED"
+		RVOLUME=
 		free=
 		used=
 		total=
 		group=
+		MOUNTED=
 		c=$((c+1))
 	done
 	if [ $c -eq 0 ]; then
@@ -884,6 +902,7 @@ list_devices() {
 	fi
 	echo $separator
 	IFS=$IFS_OLD
+	rm -f $tmp
 }
 
 ################################
@@ -951,6 +970,64 @@ list() {
 	fi
 }
 
+############################
+# Add devices into the group
+############################
+add() {
+	vg_create=0
+	tmp=$(mktemp)
+
+	for i in $@; do
+		if [ -b $i ]; then
+			devices="$devices $i"
+			devcount=$(($devcount+1))
+			continue
+		elif [ -z $vg ]; then vgname=$i; else
+			error "Wrong option $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
+			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
+	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
+}
+
 
 ####################################
 # Calclulate diff between two dates
@@ -1050,6 +1127,7 @@ do
 	  "create") COMMAND=$1; shift; ARGS=$@; break ;;
 	  "destroy") COMMAND=$1; shift; ARGS=$@; break ;;
 	  "list") COMMAND=$1; shift; ARGS=$@; break ;;
+	  "add") COMMAND=$1; shift; ARGS=$@; break ;;
 	  *) error "Wrong argument \"$1\". (see: $TOOL --help)"
 	esac
 	shift
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 08/35] fsadm: Update "list" command for better alignment
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (6 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 07/35] fsadm: Create "add" command Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 09/35] fsadm: Specify number of stripes when no device is given Lukas Czerner
                   ` (26 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

The output of list command is now dynamically aligned so it should be
well readably even in the case there are long names. It looks much
better this way.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |  206 +++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 141 insertions(+), 65 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 5dc945b..d42b759 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -807,19 +807,12 @@ detect_fs_size() {
 #############################
 list_filesystems() {
 	IFS=$NL
-	format="%-20s%-8s%-13s%-13s%-13s%s\n"
-	header=$(printf $format "Volume" "Type" "Free" "Used" "Total" "Mount point")
-	separator=""
-	for i in $(seq ${#header}); do
-		separator+="-"
-	done
-	echo $separator
-	echo $header
-	echo $separator
-	c=0
+	local c=0
 	for line in $(LANG=C $LVM lvs -o lv_path,lv_size --noheadings --separator ' ' --nosuffix --units k 2> /dev/null); do
+		c=$((c+1))
 		line=$(echo $line | sed -e 's/^ *\//\//')
 		volume=$(echo $line | cut -d' ' -f1)
+		volumes[$c]=$volume
 		detect_fs $volume
 		detect_mounted
 		detect_fs_size
@@ -827,20 +820,60 @@ list_filesystems() {
 			total=$(echo $line | cut -d' ' -f2)
 			TOTAL=$(humanize_size ${total%%.})
 		fi
-		printf "$format" "$volume" "$FSTYPE" "$FREE" "$USED" "$TOTAL" "$MOUNTED"
-		volume=
+		total[$c]=$TOTAL
+		fstype[$c]=$FSTYPE
+		free[$c]=$FREE
+		used[$c]=$USED
+		mounted[$c]=$MOUNTED
 		FSTYPE=
 		FREE=
 		USED=
 		TOTAL=
 		MOUNTED=
-		c=$((c+1))
 	done
+	IFS=$IFS_OLD
+
 	if [ $c -eq 0 ]; then
 		echo " 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
+	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 _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}
+		[ ${#_mounted} -gt "0$len_mounted" ] && len_mounted=${#_mounted}
+	done
+
+	format="%-$(($len_volume+2))s%-$(($len_fstype+2))s%-$(($len_free+2))s%-$(($len_used+2))s%-$(($len_total+2))s%-$(($len_mounted+2))s\n"
+	header=$(printf "$format" "Volume" "FS" "Free" "Used" "Total" "Mount point")
+	separator=""
+	for i in $(seq ${#header}); do
+		separator+="-"
+	done
+	echo $separator
+	printf "$format" "Volume" "FS" "Free" "Used" "Total" "Mount point"
+	echo $separator
+
+	for i in $(seq $c); do
+		printf "$format" "${volumes[$i]}" "${fstype[$i]}" "${free[$i]}" "${used[$i]}" "${total[$i]}" "${mounted[$i]}"
+	done
+
 	echo $separator
-	IFS=$IFS_OLD
 }
 
 ###########################
@@ -850,23 +883,15 @@ list_devices() {
 	IFS=$NL
 	tmp=$(mktemp)
 
-	format="%-13s%-13s%-13s%-13s%-13s%s\n"
-	header=$(printf $format "Device" "Free" "Used" "Total" "Group" "Mount point")
-	separator=""
-	for i in $(seq ${#header}); do
-		separator+="-"
-	done
-	echo $separator
-	echo $header
-	echo $separator
 	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 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)
-		total=$(humanize_size ${total%%.*})
+		_total=$(echo $line | cut -d' ' -f3)
+		total[$c]=$(humanize_size ${_total%%.*})
 		VOLUME=$(echo $line | cut -d' ' -f4)
 		RVOLUME="/dev/$(echo $line | cut -d' ' -f4)"
 		line=$($GREP -e " $RVOLUME " $tmp)
@@ -878,31 +903,65 @@ list_devices() {
 
 		if [ ! -z $line ]; then
 			line=$(echo $line | sed -e 's/^ *\//\//')
-			RVOLUME=$(echo $line | cut -d' ' -f1)
-			group=$(echo $line | cut -d' ' -f2)
-			total=$(echo $line | cut -d' ' -f3)
-			total=$(humanize_size ${total%%.*})
-			free=$(echo $line | cut -d' ' -f4)
-			free=$(humanize_size ${free%%.*})
-			used=$(echo $line | cut -d' ' -f5)
-			used=$(humanize_size ${used%%.*})
+			group[$c]=$(echo $line | cut -d' ' -f2)
+			_total=$(echo $line | cut -d' ' -f3)
+			total[$c]=$(humanize_size ${_total%%.*})
+			_free=$(echo $line | cut -d' ' -f4)
+			free[$c]=$(humanize_size ${_free%%.*})
+			_used=$(echo $line | cut -d' ' -f5)
+			used[$c]=$(humanize_size ${_used%%.*})
 		fi
-
-		printf "$format" "$RVOLUME" "$free" "$used" "$total" "$group" "$MOUNTED"
-		RVOLUME=
+		volumes[$c]=$RVOLUME
+		mounted[$c]=$MOUNTED
 		free=
 		used=
 		total=
-		group=
 		MOUNTED=
-		c=$((c+1))
 	done
+	IFS=$IFS_OLD
+	rm -f $tmp
+
 	if [ $c -eq 0 ]; then
 		echo " 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+2))s%-$(($len_used+2))s%-$(($len_total+2))s%-$(($len_group+2))s%-$(($len_mounted+2))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
-	IFS=$IFS_OLD
-	rm -f $tmp
 }
 
 ################################
@@ -910,41 +969,58 @@ list_devices() {
 ################################
 list_pool() {
 	IFS=$NL
+	c=0
+	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/^ *//')
+		group[$c]=$(echo $line | cut -d' ' -f1)
+		devices[$c]=$(echo $line | cut -d' ' -f2)
+		_total=$(echo $line | cut -d' ' -f3)
+		_free=$(echo $line | cut -d' ' -f4)
+		_used=$((${_total%%.*}-${_free%%.*}))
+		used[$c]=$(humanize_size ${_used%%.*})
+		total[$c]=$(humanize_size ${_total%%.*})
+		free[$c]=$(humanize_size ${_free%%.*})
+	done
+	IFS=$IFS_OLD
 
-	format="%-10s%-9s%-13s%-13s%s\n"
+	if [ $c -eq 0 ]; then
+		echo " 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+2))s%-$(($len_used+2))s%-$(($len_total+2))s\n"
 	header=$(printf $format "Group" "Devices" "Free" "Used" "Total")
 	separator=""
 	for i in $(seq ${#header}); do
 		separator+="-"
 	done
 	echo $separator
-	echo $header
+	printf $format "Group" "Devices" "Free" "Used" "Total"
 	echo $separator
-	c=0
-	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
-		line=$(echo $line | sed -e 's/^ *//')
-		group=$(echo $line | cut -d' ' -f1)
-		devices=$(echo $line | cut -d' ' -f2)
-		total=$(echo $line | cut -d' ' -f3)
-		free=$(echo $line | cut -d' ' -f4)
-		used=$((${total%%.*}-${free%%.*}))
-		used=$(humanize_size ${used%%.*})
-		total=$(humanize_size ${total%%.*})
-		free=$(humanize_size ${free%%.*})
-
-		printf "$format" "$group" "$devices" "$free" "$used" "$total"
-		group=
-		devices=
-		total=
-		free=
-		used=
-		c=$((c+1))
+
+	for i in $(seq $c); do
+		printf "$format" "${group[$i]}" "${devices[$i]}" "${free[$i]}" "${used[$i]}" "${total[$i]}"
 	done
-	if [ $c -eq 0 ]; then
-		echo " No pools found on the system."
-	fi
 	echo $separator
-	IFS=$IFS_OLD
 }
 
 #############################
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 09/35] fsadm: Specify number of stripes when no device is given
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (7 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 08/35] fsadm: Update "list" command for better alignment Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 10/35] fsadm: Print type of the volume in filesystem listing Lukas Czerner
                   ` (25 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

We should force user to specify number of stripes, (devices to be used)
if no device is specified on the command line when creating a new
volume.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index d42b759..f368f96 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -514,6 +514,7 @@ create() {
 			"name"*) name=${i##*=} ;;
 			"fstyp"* | "fs"*) fstyp=${i##*=} ;;
 			"size"*) size=${i##*=} ;;
+			"stripes"*) stripes=${i##*=} ;;
 			*) if [ -z $vg ]; then vgname=$i; else
 				error "Wrong option $i. (see: $TOOL --help)"
 			   fi ;;
@@ -582,9 +583,22 @@ create() {
 	[ -z "$vgname" ] && error "No suitable name for volume group found."
 
 	if [ "$stripesize" ]; then
+		if [ -z "$devices" ] && [ -z "$stripes" ]; then
+			error "Chunk size specified ($stripesize), but " \
+				"neither devices, nor number of stripes " \
+				"has been provided"
+		fi
 		striped="-i $devcount -I $stripesize"
 	fi
 
+	if [ "$stripes" ]; then
+		if [ -z "$stripesize" ]; then
+			error "Number of stripes specified, but chunk size " \
+				"has not been provided"
+		fi
+		striped="-i $stripes -I $stripesize"
+	fi
+
 	if [ "$name" ]; then
 		lvname="--name $name"
 	else
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 10/35] fsadm: Print type of the volume in filesystem listing
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (8 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 09/35] fsadm: Specify number of stripes when no device is given Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 11/35] fsadm: Add "remove" command Lukas Czerner
                   ` (24 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

Type of the volume is useful information so we should list that as well.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index f368f96..6617de0 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -822,11 +822,12 @@ detect_fs_size() {
 list_filesystems() {
 	IFS=$NL
 	local c=0
-	for line in $(LANG=C $LVM lvs -o lv_path,lv_size --noheadings --separator ' ' --nosuffix --units k 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
 		c=$((c+1))
 		line=$(echo $line | sed -e 's/^ *\//\//')
 		volume=$(echo $line | cut -d' ' -f1)
 		volumes[$c]=$volume
+		segtype[$c]=$(echo $line | cut -d' ' -f3)
 		detect_fs $volume
 		detect_mounted
 		detect_fs_size
@@ -858,33 +859,36 @@ list_filesystems() {
 	len_used=4
 	len_total=5
 	len_mounted=11
+	len_segtype=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 _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}
 		[ ${#_mounted} -gt "0$len_mounted" ] && len_mounted=${#_mounted}
 	done
 
-	format="%-$(($len_volume+2))s%-$(($len_fstype+2))s%-$(($len_free+2))s%-$(($len_used+2))s%-$(($len_total+2))s%-$(($len_mounted+2))s\n"
-	header=$(printf "$format" "Volume" "FS" "Free" "Used" "Total" "Mount point")
+	format="%-$(($len_volume+2))s%-$(($len_fstype+2))s%-$(($len_free+2))s%-$(($len_used+2))s%-$(($len_total+2))s%-$(($len_segtype+2))s%-$(($len_mounted+2))s\n"
+	header=$(printf "$format" "Volume" "FS" "Free" "Used" "Total" "Type" "Mount point")
 	separator=""
 	for i in $(seq ${#header}); do
 		separator+="-"
 	done
 	echo $separator
-	printf "$format" "Volume" "FS" "Free" "Used" "Total" "Mount point"
+	printf "$format" "Volume" "FS" "Free" "Used" "Total" "Type" "Mount point"
 	echo $separator
 
 	for i in $(seq $c); do
-		printf "$format" "${volumes[$i]}" "${fstype[$i]}" "${free[$i]}" "${used[$i]}" "${total[$i]}" "${mounted[$i]}"
+		printf "$format" "${volumes[$i]}" "${fstype[$i]}" "${free[$i]}" "${used[$i]}" "${total[$i]}" "${segtype[$i]}" "${mounted[$i]}"
 	done
 
 	echo $separator
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 11/35] fsadm: Add "remove" command
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (9 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 10/35] fsadm: Print type of the volume in filesystem listing Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-22 10:06   ` Zdenek Kabelac
  2011-09-21 16:45 ` [linux-lvm] [PATCH 12/35] fsadm: Try to avoid calling LVM in the loops Lukas Czerner
                   ` (23 subsequent siblings)
  34 siblings, 1 reply; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

Remove command allows to remove unused devices from the pool (volume
group).

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |   82 ++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 62 insertions(+), 20 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 6617de0..4a4f625 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -823,11 +823,12 @@ list_filesystems() {
 	IFS=$NL
 	local c=0
 	for line in $(LANG=C $LVM lvs -o lv_path,lv_size,segtype --noheadings --separator ' ' --nosuffix --units k 2> /dev/null); do
-		c=$((c+1))
 		line=$(echo $line | sed -e 's/^ *\//\//')
 		volume=$(echo $line | cut -d' ' -f1)
-		volumes[$c]=$volume
-		segtype[$c]=$(echo $line | cut -d' ' -f3)
+		[ "$volume" == "$last_volume" ] && continue
+		c=$((c+1))
+		local volumes[$c]=$volume
+		local segtype[$c]=$(echo $line | cut -d' ' -f3)
 		detect_fs $volume
 		detect_mounted
 		detect_fs_size
@@ -835,16 +836,17 @@ list_filesystems() {
 			total=$(echo $line | cut -d' ' -f2)
 			TOTAL=$(humanize_size ${total%%.})
 		fi
-		total[$c]=$TOTAL
-		fstype[$c]=$FSTYPE
-		free[$c]=$FREE
-		used[$c]=$USED
-		mounted[$c]=$MOUNTED
+		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
 	IFS=$IFS_OLD
 
@@ -909,7 +911,7 @@ list_devices() {
 		c=$((c+1))
 		line=$(echo $line | sed -e 's/  */ /g')
 		_total=$(echo $line | cut -d' ' -f3)
-		total[$c]=$(humanize_size ${_total%%.*})
+		local total[$c]=$(humanize_size ${_total%%.*})
 		VOLUME=$(echo $line | cut -d' ' -f4)
 		RVOLUME="/dev/$(echo $line | cut -d' ' -f4)"
 		line=$($GREP -e " $RVOLUME " $tmp)
@@ -921,16 +923,16 @@ list_devices() {
 
 		if [ ! -z $line ]; then
 			line=$(echo $line | sed -e 's/^ *\//\//')
-			group[$c]=$(echo $line | cut -d' ' -f2)
+			local group[$c]=$(echo $line | cut -d' ' -f2)
 			_total=$(echo $line | cut -d' ' -f3)
-			total[$c]=$(humanize_size ${_total%%.*})
+			local total[$c]=$(humanize_size ${_total%%.*})
 			_free=$(echo $line | cut -d' ' -f4)
-			free[$c]=$(humanize_size ${_free%%.*})
+			local free[$c]=$(humanize_size ${_free%%.*})
 			_used=$(echo $line | cut -d' ' -f5)
-			used[$c]=$(humanize_size ${_used%%.*})
+			local used[$c]=$(humanize_size ${_used%%.*})
 		fi
-		volumes[$c]=$RVOLUME
-		mounted[$c]=$MOUNTED
+		local volumes[$c]=$RVOLUME
+		local mounted[$c]=$MOUNTED
 		free=
 		used=
 		total=
@@ -991,14 +993,14 @@ list_pool() {
 	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/^ *//')
-		group[$c]=$(echo $line | cut -d' ' -f1)
-		devices[$c]=$(echo $line | cut -d' ' -f2)
+		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%%.*}))
-		used[$c]=$(humanize_size ${_used%%.*})
-		total[$c]=$(humanize_size ${_total%%.*})
-		free[$c]=$(humanize_size ${_free%%.*})
+		local used[$c]=$(humanize_size ${_used%%.*})
+		local total[$c]=$(humanize_size ${_total%%.*})
+		local free[$c]=$(humanize_size ${_free%%.*})
 	done
 	IFS=$IFS_OLD
 
@@ -1122,6 +1124,45 @@ add() {
 	fi
 }
 
+###############################
+# Remove devices form the group
+###############################
+remove() {
+	devices=$@
+	tmp=$(mktemp)
+	tmp2=$(mktemp)
+	nothing=1
+
+	$LVM vgs -o vg_name --separator ' ' --noheadings | sed -e 's/^ *//' > $tmp
+
+	[ "$(cat $tmp | wc -l)" == "0" ] && return
+
+	IFS=$NL
+	for vgname in $(cat $tmp); do
+		if [ -z $devices ]; then
+			$LVM vgreduce -a $vgname
+			nothing=0
+		else
+			$LVM vgs $vgname -o pv_name --separator ' ' --noheadings > $tmp2
+			IFS=$IFS_OLD
+			for dev in $devices; do
+				$GREP $dev $tmp2 &> /dev/null
+				if [ $? -eq 0 ]; then
+					$LVM vgreduce $vgname $dev
+					nothing=0
+				fi
+			done
+			IFS=$NL
+		fi
+	done
+	rm -f $tmp $tmp2
+	IFS=$IFS_OLD
+
+	if [ $nothing -eq 1 ]; then
+		warn "Nothing to do."
+	fi
+}
+
 
 ####################################
 # Calclulate diff between two dates
@@ -1222,6 +1263,7 @@ do
 	  "destroy") 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
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 12/35] fsadm: Try to avoid calling LVM in the loops
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (10 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 11/35] fsadm: Add "remove" command Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 13/35] fsadm: Merge "destroy" and "remove" into one command Lukas Czerner
                   ` (22 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

We should try to avoid calling LVM in the loops since it will cause
significant slowdown when the loop has many iteration (there is a lot of
devices/lvs/vgs). Instead use temporary files to store the listing
before the loop and the just grep for it.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 4a4f625..517d10d 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -468,11 +468,13 @@ 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 $devices; do
-		cur_vgname=$(LANG=C $LVM pvs -o vg_name --separator ' ' --noheadings --nosuffix --units k $i 2> /dev/null | sed -e 's/^ *//')
+		cur_vgname=$($GREP -e "$i$" $tmp | cut -d' ' -f1)
 
 		if [ -z $cur_vgname ]; then
 			NOT_IN_GROUP+="$i "
@@ -492,6 +494,7 @@ detect_device_group() {
 			vgs+=" $cur_vgname"
 		fi
 	done
+	rm -f $tmp
 	GROUP=$vgs
 	return $ret
 }
@@ -540,7 +543,7 @@ create() {
 	# 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
-	# in it
+	# 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)
@@ -602,14 +605,17 @@ create() {
 	if [ "$name" ]; then
 		lvname="--name $name"
 	else
+		tmp=$(mktemp)
+		$LVM lvs $vgname --separator ' ' --noheadings > $tmp
 		for i in $(seq -w $MAX_VGS); do
-			$LVM lvs $vgname --separator ' ' --noheadings | grep -e " lvol${i} " &> /dev/null
+			$GREP -e " lvol${i} " $tmp &> /dev/null
 			if [ $? -ne 0 ]; then
 				name="lvol${i}"
 				lvname="--name $name"
 				break;
 			fi
 		done
+		rm -f $tmp
 	fi
 	echo "going to create logical volume $lvname"
 
@@ -617,7 +623,7 @@ create() {
 
 	if [ $vg_create -eq 1 ]; then
 		dry $LVM vgcreate $VERB $vgname $devices
-	elif [ ! -z $NOT_IN_GROUP ]; then
+	elif [ ! -z "$NOT_IN_GROUP" ]; then
 		dry $LVM vgextend $VERB $vgname $NOT_IN_GROUP
 	fi
 
@@ -1097,13 +1103,7 @@ add() {
 			rm -f $tmp
 			error "Please, specify group you want to add the device into"
 		elif [ $lines -eq 0 ]; then
-			for i in $(seq -w $MAX_VGS); do
-				$LVM vgs vg${i} &> /dev/null
-				if [ $? -ne 0 ]; then
-					vgname="vg${i}"
-					break;
-				fi
-			done
+			vgname="vg001"
 			vg_create=1
 		fi
 	else
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 13/35] fsadm: Merge "destroy" and "remove" into one command
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (11 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 12/35] fsadm: Try to avoid calling LVM in the loops Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 14/35] fsadm: Allow to remove all volume groups Lukas Czerner
                   ` (21 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

We do not actually need both "remove" and "destroy" command because we
can do everything with only one "remove" command. Also it is confusing
to have both. This commit mergers the functionality of destroy to
remove.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |  526 ++++++++++++++++++++++++++++++++----------------------
 1 files changed, 310 insertions(+), 216 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 517d10d..66ecf1eb 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -66,7 +66,6 @@ DRY=0
 VERB=
 FORCE=
 EXTOFF=0
-DO_LVRESIZE=0
 FSTYPE=unknown
 VOLUME=unknown
 TEMPDIR="${TMPDIR:-/tmp}/${TOOL}_${RANDOM}$$/m"
@@ -106,7 +105,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,"
@@ -167,41 +165,130 @@ cleanup() {
 
 	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() {
+	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)
+	IFS=$IFS_OLD
+}
+
+############################
+# Get size of xfs file system
+# by reading the df output or
+# examine file system with
+# xfs_db tool
+#############################
+get_xfs_size() {
+	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)
+	IFS=$IFS_OLD
+}
+
+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
@@ -325,13 +412,15 @@ 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)"
@@ -437,13 +526,15 @@ generic_make_fs() {
 ####################
 # Resize filesystem
 ####################
-resize() {
+resize_fs() {
 	NEWSIZE=$2
 	detect_fs "$1"
 	[ $? -eq 1 ] && error "Cannot get FSTYPE of \"$VOLUME\""
 	verbose "\"$FSTYPE\" filesystem found on \"$VOLUME\""
-	is_natural $NEWSIZE
-	[ $? -ne 1 ] && error "$NEWSIZE is not valid number for file system size"
+	if [ "$NEWSIZE" ]; then
+		is_natural $NEWSIZE
+		[ $? -ne 1 ] && 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
@@ -456,7 +547,112 @@ resize() {
 	  "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=0
+
+	# Special case for the situation we have been called from within the lvresize code.
+	# How crazy is that ?:) But anyway to preserve the old behaviour it is there.
+	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
+				if [ $? -eq 0 ]; then
+					lvname=$i
+					devcount=0
+					continue
+				else
+					error "$i is not valid logical volume"
+				fi
+			fi
+			devices="$devices $i"
+			devcount=$(($devcount+1))
+			continue
+		fi
+		case $i in
+			"size=+"*) [ $size -eq 0 ] && extend=${i##*+} && size=1;;
+			"size=-"*) [ $size -eq 0 ] && shrink=${i##*-} && size=1;;
+			"size="*)  [ $size -eq 0 ] && new_size=${i##*=} && size=1;;
+			*) error "Wrong option $i. (see: $TOOL --help)";;
+		esac
+	done
+
+	[ -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
+		if [ $? -ne 0 ]; then
+			error "Devices are in different groups. Please run "\
+				"\"$TOOL list\" to get more information on layout."
+		fi
+
+		[ "$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
+
+	# If no size parameter has been provided, resize the volume to the maximum
+	if [ $size -eq 0 ]; then
+		echo "What??"
+	fi
+
+	# determine new size of the file system
+	if [ "$extend" ] || [ "$shrink" ] || [ "$new_size" ]; then
+		# only one of those variable should be set, so it is safe to do that
+		decode_size $extend $shrink $new_size 1
+		detect_fs $lvname
+		detect_fs_size
+		[ "$extend" ] && size=$(($total + $NEWSIZE))
+		[ "$shrink" ] && size=$(($total - $NEWSIZE))
+		if [ "$new_size" ]; then
+			[ $NEWSIZE -ge $total ] && extend=1 || shrink=1
+			size=$NEWSIZE
+		fi
+	fi
+
+	# Do the actual resize
+	if [ "$shrink" ]; then
+		resize_fs $lvname $size
+		# The file system might decide to alight the size a bit, so we
+		# have to resize the volume to the exact size of the file system
+		# so we do not corrupt it.
+		detect_fs_size
+		resize_lvolume $lvname $total
+	elif [ "$extend" ]; then
+		resize_lvolume $lvname $size
+		resize_fs $lvname
+	fi
 }
 
 #################################
@@ -518,7 +714,7 @@ create() {
 			"fstyp"* | "fs"*) fstyp=${i##*=} ;;
 			"size"*) size=${i##*=} ;;
 			"stripes"*) stripes=${i##*=} ;;
-			*) if [ -z $vg ]; then vgname=$i; else
+			*) if [ -z $vgname ]; then vgname=$i; else
 				error "Wrong option $i. (see: $TOOL --help)"
 			   fi ;;
 		esac
@@ -538,7 +734,6 @@ create() {
 	elif [ $GROUP ]; then
 		vgname=$GROUP
 	fi
-	echo $vgname
 
 	# Devices are not in any group so we should find some.
 	# Either create a new one, or use the existing one, if
@@ -585,20 +780,17 @@ create() {
 
 	[ -z "$vgname" ] && error "No suitable name for volume group found."
 
-	if [ "$stripesize" ]; then
-		if [ -z "$devices" ] && [ -z "$stripes" ]; then
-			error "Chunk size specified ($stripesize), but " \
-				"neither devices, nor number of stripes " \
-				"has been provided"
-		fi
-		striped="-i $devcount -I $stripesize"
-	fi
-
-	if [ "$stripes" ]; then
+	if [ "$stripesize" ] || [ "$stripes" ]; then
 		if [ -z "$stripesize" ]; then
 			error "Number of stripes specified, but chunk size " \
 				"has not been provided"
 		fi
+		if [ -z "$stripes" ] && [ $devcount -eq 0 ]; then
+			error "Chunk size specified ($stripesize), but " \
+				"neither devices, nor number of stripes " \
+				"has been provided"
+		fi
+		[ $devcount -gt 0 ] && [ -z "$stripes" ] && stripes=$devcount
 		striped="-i $stripes -I $stripesize"
 	fi
 
@@ -617,7 +809,6 @@ create() {
 		done
 		rm -f $tmp
 	fi
-	echo "going to create logical volume $lvname"
 
 	[ -z "$lvname" ] && error "No suitable name for the logical volume."
 
@@ -642,7 +833,7 @@ create() {
 	fi
 
 	case $fstyp in
-		ext[234]) make_ext $device $fstyp $devcount $stripesize ;;
+		ext[234]) make_ext $device $fstyp $stripes $stripesize ;;
 		xfs|reiserfs) generic_make_fs $device $fstyp;;
 		*)	if [ $guess -eq 1 ]; then
 				return 0
@@ -653,11 +844,40 @@ create() {
 	esac
 }
 
+###############################
+# Remove device form the group
+###############################
+remove_device() {
+	device=$@
+	tmp=$(mktemp)
+	removed=0
+
+	$LVM vgs -o vg_name --separator ' ' --noheadings | sed -e 's/^ *//' > $tmp
+	IFS=$NL
+	for vgname in $(cat $tmp); do
+		if [ -z "$device" ]; then
+			$LVM vgreduce -a $vgname
+			removed=1
+		else
+			$LVM vgs $vgname -o pv_name --separator ' ' --noheadings | $GREP -e "$device$" &> /dev/null
+			if [ $? -eq 0 ]; then
+				$LVM vgreduce $vgname $device
+				removed=1
+			fi
+		fi
+	done
+	IFS=$IFS_OLD
+	rm -f $tmp
+
+	return $removed
+}
+
 #############################
 # Remove the logical volume
-# of the whole volume group
+# volume group, or the device
+# from the group
 #############################
-do_destroy() {
+do_remove() {
 	item=$1
 	device=
 	MOUNTED=
@@ -696,129 +916,35 @@ do_destroy() {
 
 	if [ $MOUNTED ]; then
 		try_umount
+		dry $LVM lvremove $FORCE $device
+		return
+	fi
+
+	remove_device $device
+	if [ $? -eq 0 ]; then
+		dry $LVM lvremove $FORCE $device
 	fi
-	dry $LVM lvremove $FORCE $device
 }
 
 ###############################
 # Iterate through the arguments
-# and do_destroy on them
+# and do_remove on them
 ###############################
-destroy() {
+remove() {
 	# help
 	if [ "$1" == "help" ]; then
-		echo "Usage: $TOOL destroy [mount point | dm device | voulume group]"
+		echo "Usage: $TOOL remove [mount point | dm device | voulume group | device]"
 		exit 0
 	fi
 
 	for item in $@; do
-		do_destroy $item
-	done
-
-}
-
-#####################################
-# Convet the size into human readable
-# form. size in KiB expected
-#####################################
-humanize_size() {
-	size=$1
-	count=0
-	while [ ${size%%.*} -ge 1024 ] && [ $count -lt 7 ]; do
-		size=$(float_math $size/1024)
-		count=$(($count+1))
+		do_remove $item
 	done
-	case $count in
-		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"
-}
 
-#############################
-# Get size of entN filesystem
-# by reading tune2fs output
-#############################
-get_ext_size() {
-	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
-
-	bsize=$(($bsize/1024))
-	total=$(($bcount*$bsize))
-	TOTAL=$(humanize_size $total)
-	used=$((($bcount-$fbcount)*$bsize))
-	USED=$(humanize_size $used)
-	free=$((($fbcount-$rbcount)*$bsize))
-	FREE=$(humanize_size $free)
-	IFS=$IFS_OLD
-}
-
-############################
-# Get size of xfs file system
-# by reading the df output or
-# examine file system with
-# xfs_db tool
-#############################
-get_xfs_size() {
-	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
-		bsize=$(($bsize/1024))
-		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
+	if [ $# -eq 0 ]; then
+		remove_device
 	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)
-	IFS=$IFS_OLD
-}
 
-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
 }
 
 #############################
@@ -828,20 +954,17 @@ detect_fs_size() {
 list_filesystems() {
 	IFS=$NL
 	local c=0
-	for line in $(LANG=C $LVM lvs -o lv_path,lv_size,segtype --noheadings --separator ' ' --nosuffix --units k 2> /dev/null); do
+	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
-		if [ -z "$TOTAL" ]; then
-			total=$(echo $line | cut -d' ' -f2)
-			TOTAL=$(humanize_size ${total%%.})
-		fi
 		local total[$c]=$TOTAL
 		local fstype[$c]=$FSTYPE
 		local free[$c]=$FREE
@@ -868,6 +991,7 @@ list_filesystems() {
 	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]}
@@ -875,6 +999,7 @@ list_filesystems() {
 		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}
@@ -882,21 +1007,22 @@ list_filesystems() {
 		[ ${#_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_fstype+2))s%-$(($len_free+2))s%-$(($len_used+2))s%-$(($len_total+2))s%-$(($len_segtype+2))s%-$(($len_mounted+2))s\n"
-	header=$(printf "$format" "Volume" "FS" "Free" "Used" "Total" "Type" "Mount point")
+	format="%-$(($len_volume+2))s %-$(($len_lvsize+2))s%-$(($len_fstype+2))s%-$(($len_free+2))s%-$(($len_used+2))s%-$(($len_total+2))s%-$(($len_segtype+2))s%-$(($len_mounted+2))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" "FS" "Free" "Used" "Total" "Type" "Mount point"
+	printf "$format" "Volume" "LV size" "FS" "Free" "Used" "Total" "Type" "Mount point"
 	echo $separator
 
 	for i in $(seq $c); do
-		printf "$format" "${volumes[$i]}" "${fstype[$i]}" "${free[$i]}" "${used[$i]}" "${total[$i]}" "${segtype[$i]}" "${mounted[$i]}"
+		printf "$format" "${volumes[$i]}" "${lvsize[$i]}" "${fstype[$i]}" "${free[$i]}" "${used[$i]}" "${total[$i]}" "${segtype[$i]}" "${mounted[$i]}"
 	done
 
 	echo $separator
@@ -912,11 +1038,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 k > $tmp
+	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)
+		_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)"
@@ -996,7 +1122,7 @@ list_devices() {
 list_pool() {
 	IFS=$NL
 	c=0
-	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
+	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)
@@ -1072,6 +1198,13 @@ list() {
 	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
 ############################
@@ -1084,8 +1217,8 @@ add() {
 			devices="$devices $i"
 			devcount=$(($devcount+1))
 			continue
-		elif [ -z $vg ]; then vgname=$i; else
-			error "Wrong option $i. (see: $TOOL --help)"
+		elif [ -z $vgname ]; then vgname=$i; else
+			error "Wrong argument $i. (see: $TOOL --help)"
 		fi
 	done
 
@@ -1117,53 +1250,13 @@ add() {
 
 	if [ $vg_create -eq 1 ]; then
 		dry $LVM vgcreate $VERB $vgname $devices
-	elif [ ! -z $NOT_IN_GROUP ]; then
+	elif [ ! -z "$NOT_IN_GROUP" ]; then
 		dry $LVM vgextend $VERB $vgname $NOT_IN_GROUP
 	else
 		warn "Nothing to do"
 	fi
 }
 
-###############################
-# Remove devices form the group
-###############################
-remove() {
-	devices=$@
-	tmp=$(mktemp)
-	tmp2=$(mktemp)
-	nothing=1
-
-	$LVM vgs -o vg_name --separator ' ' --noheadings | sed -e 's/^ *//' > $tmp
-
-	[ "$(cat $tmp | wc -l)" == "0" ] && return
-
-	IFS=$NL
-	for vgname in $(cat $tmp); do
-		if [ -z $devices ]; then
-			$LVM vgreduce -a $vgname
-			nothing=0
-		else
-			$LVM vgs $vgname -o pv_name --separator ' ' --noheadings > $tmp2
-			IFS=$IFS_OLD
-			for dev in $devices; do
-				$GREP $dev $tmp2 &> /dev/null
-				if [ $? -eq 0 ]; then
-					$LVM vgreduce $vgname $dev
-					nothing=0
-				fi
-			done
-			IFS=$NL
-		fi
-	done
-	rm -f $tmp $tmp2
-	IFS=$IFS_OLD
-
-	if [ $nothing -eq 1 ]; then
-		warn "Nothing to do."
-	fi
-}
-
-
 ####################################
 # Calclulate diff between two dates
 #  LANG=C input is expected the
@@ -1255,12 +1348,12 @@ 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 ;;
+	  "-l"|"--lvresize") ;;
 	  "check") COMMAND=$1; shift; ARGS=$@; break ;;
 	  "resize") COMMAND=$1; shift; ARGS=$@; break ;;
 	  "create") COMMAND=$1; shift; ARGS=$@; break ;;
-	  "destroy") COMMAND=$1; shift; ARGS=$@; break ;;
 	  "list") COMMAND=$1; shift; ARGS=$@; break ;;
 	  "add") COMMAND=$1; shift; ARGS=$@; break ;;
 	  "remove") COMMAND=$1; shift; ARGS=$@; break ;;
@@ -1275,3 +1368,4 @@ fi
 
 export FSADM_RUNNING="fsadm"
 $COMMAND $ARGS
+cleanup 0
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 14/35] fsadm: Allow to remove all volume groups
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (12 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 13/35] fsadm: Merge "destroy" and "remove" into one command Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 15/35] fsadm: Make all internal math in kilobytes Lukas Czerner
                   ` (20 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

Add new argument to the "remove" command --all to remove all volume
groups.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 66ecf1eb..028bc04 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -757,7 +757,6 @@ create() {
 				vgsize=$(echo $i | sed -e 's/^ *//' | cut -d' ' -f2)
 				vgsize=${vgsize%%B}
 				if [ $vgsize -ge $new_size ]; then
-					echo "Yes"
 					vgname=$(echo $i | sed -e 's/^ *//' | cut -d' ' -f1)
 					break;
 				fi
@@ -933,11 +932,15 @@ do_remove() {
 remove() {
 	# help
 	if [ "$1" == "help" ]; then
-		echo "Usage: $TOOL remove [mount point | dm device | voulume group | device]"
+		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 $@; do
+	for item in $list; do
 		do_remove $item
 	done
 
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 15/35] fsadm: Make all internal math in kilobytes
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (13 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 14/35] fsadm: Allow to remove all volume groups Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 16/35] fsadm: Use warn for warnings in list command Lukas Czerner
                   ` (19 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

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.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |   74 ++++++++++++++++++++++++++---------------------------
 1 files changed, 36 insertions(+), 38 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 028bc04..233b4fc 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -181,15 +181,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"
@@ -210,11 +209,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)
 	IFS=$IFS_OLD
 }
@@ -241,11 +240,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
@@ -273,21 +272,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%%.*}
 }
 
@@ -423,7 +421,7 @@ resize_ext() {
 		esac
 	fi
 
-	verbose "Resizing filesystem on device \"$VOLUME\" to $NEWSIZE bytes ($BLOCKCOUNT -> $NEWBLOCKCOUNT blocks of $BLOCKSIZE bytes)"
+	verbose "Resizing filesystem on device \"$VOLUME\" to $NEWSIZE KiB ($BLOCKCOUNT -> $NEWBLOCKCOUNT blocks of $BLOCKSIZE bytes)"
 	dry "$RESIZE_EXT" $FSFORCE "$VOLUME" $NEWBLOCKCOUNT
 }
 
@@ -444,11 +442,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
 }
 
@@ -552,9 +550,9 @@ 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
@@ -740,8 +738,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)
@@ -935,7 +933,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
@@ -957,7 +955,7 @@ remove() {
 list_filesystems() {
 	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
@@ -1041,11 +1039,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
+	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)"
@@ -1125,7 +1123,7 @@ list_devices() {
 list_pool() {
 	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)
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 16/35] fsadm: Use warn for warnings in list command
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (14 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 15/35] fsadm: Make all internal math in kilobytes Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 17/35] fsadm: Handle resize if there is no file system Lukas Czerner
                   ` (18 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

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

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 233b4fc..3b47a21 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -981,7 +981,7 @@ list_filesystems() {
 	IFS=$IFS_OLD
 
 	if [ $c -eq 0 ]; then
-		echo " No file systems suitable for managing by $TOOL found."
+		warn " No file systems suitable for managing by $TOOL found."
 		return
 	fi
 
@@ -1075,7 +1075,7 @@ list_devices() {
 	rm -f $tmp
 
 	if [ $c -eq 0 ]; then
-		echo " No devices found."
+		warn " No devices found."
 		return
 	fi
 
@@ -1138,7 +1138,7 @@ list_pool() {
 	IFS=$IFS_OLD
 
 	if [ $c -eq 0 ]; then
-		echo " No pools found on the system."
+		warn " No pools found on the system."
 		return
 	fi
 
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 17/35] fsadm: Handle resize if there is no file system
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (15 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 16/35] fsadm: Use warn for warnings in list command Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 18/35] fsadm: Fsck extN before resize only if it is not mounted Lukas Czerner
                   ` (17 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

Resize the logical volume even if there is not file system on it.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |   28 ++++++++++++++++++++--------
 1 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 3b47a21..112ad62 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -630,10 +630,18 @@ resize() {
 		# only one of those variable should be set, so it is safe to do that
 		decode_size $extend $shrink $new_size 1
 		detect_fs $lvname
-		detect_fs_size
+		# What if there is not file system on the volume
+		if [ -z $FSTYP ]; then
+			total=$(LANG=C $LVM lvs -o lv_size --separator ' ' --noheadings --units k $lvname 2> /dev/null)
+			total=${total%%.*}
+		else
+			detect_fs_size
+		fi
+		[ -z $total ] && error "Could not determine the size of the volume"
 		[ "$extend" ] && size=$(($total + $NEWSIZE))
 		[ "$shrink" ] && size=$(($total - $NEWSIZE))
 		if [ "$new_size" ]; then
+			# NEWSIZE is decoded new_size
 			[ $NEWSIZE -ge $total ] && extend=1 || shrink=1
 			size=$NEWSIZE
 		fi
@@ -641,15 +649,19 @@ resize() {
 
 	# Do the actual resize
 	if [ "$shrink" ]; then
-		resize_fs $lvname $size
-		# The file system might decide to alight the size a bit, so we
-		# have to resize the volume to the exact size of the file system
-		# so we do not corrupt it.
-		detect_fs_size
-		resize_lvolume $lvname $total
+		if [ "$FSTYPE" ]; then
+			resize_fs $lvname $size
+			# The file system might decide to alight the size a bit, so we
+			# have to resize the volume to the exact size of the file system
+			# so we do not corrupt it.
+			detect_fs_size
+		fi
+		resize_lvolume $lvname $size
 	elif [ "$extend" ]; then
 		resize_lvolume $lvname $size
-		resize_fs $lvname
+		if [ "$FSTYPE" ]; then
+			resize_fs $lvname
+		fi
 	fi
 }
 
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 18/35] fsadm: Fsck extN before resize only if it is not mounted
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (16 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 17/35] fsadm: Handle resize if there is no file system Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 19/35] fsadm: Align numbers to the decimal point Lukas Czerner
                   ` (16 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 112ad62..7471377 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -407,8 +407,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
 
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 19/35] fsadm: Align numbers to the decimal point
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (17 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 18/35] fsadm: Fsck extN before resize only if it is not mounted Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 20/35] fsadm: Add simple configuration file Lukas Czerner
                   ` (15 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

In "list" command align numbers to the decimal point for better
readability.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 7471377..db0fd4b 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -1025,7 +1025,7 @@ list_filesystems() {
 		[ ${#_mounted} -gt "0$len_mounted" ] && len_mounted=${#_mounted}
 	done
 
-	format="%-$(($len_volume+2))s %-$(($len_lvsize+2))s%-$(($len_fstype+2))s%-$(($len_free+2))s%-$(($len_used+2))s%-$(($len_total+2))s%-$(($len_segtype+2))s%-$(($len_mounted+2))s\n"
+	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
@@ -1113,14 +1113,14 @@ list_devices() {
 		[ ${#_mounted} -gt "0$len_mounted" ] && len_mounted=${#_mounted}
 	done
 
-	format="%-$(($len_volume+2))s%-$(($len_free+2))s%-$(($len_used+2))s%-$(($len_total+2))s%-$(($len_group+2))s%-$(($len_mounted+2))s\n"
-	header=$(printf $format "Device" "Free" "Used" "Total" "Group" "Mount point")
+	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"
+	printf "$format" "Device" "Free" "Used" "Total" "Group" "Mount point"
 	echo $separator
 
 	for i in $(seq $c); do
@@ -1173,14 +1173,14 @@ list_pool() {
 		[ ${#_total} -gt "0$len_total" ] && len_total=${#_total}
 	done
 
-	format="%-$(($len_group+2))s%-$(($len_devices+2))s%-$(($len_free+2))s%-$(($len_used+2))s%-$(($len_total+2))s\n"
-	header=$(printf $format "Group" "Devices" "Free" "Used" "Total")
+	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"
+	printf "$format" "Group" "Devices" "Free" "Used" "Total"
 	echo $separator
 
 	for i in $(seq $c); do
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 20/35] fsadm: Add simple configuration file
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (18 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 19/35] fsadm: Align numbers to the decimal point Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 21/35] fsadm: Use DEFAULT_POOL when creating volume group Lukas Czerner
                   ` (14 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index db0fd4b..1968ee3 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -83,6 +83,8 @@ PROCDEVICES="/proc/devices"
 PROCPARTITIONS="/proc/partitions"
 NULL="$DM_DEV_DIR/null"
 
+CONFIG_PATHS="/etc/fsadm.conf ~/fsadm.conf"
+
 MAX_VGS=999
 
 IFS_OLD=$IFS
@@ -1325,6 +1327,20 @@ check() {
 	esac
 }
 
+set_default_config() {
+	[ -z "$DEFAULT_POOL" ] && DEFAULT_POOL="default_pool"
+}
+
+parse_config() {
+	for line in $(cat $1); do
+		case "$line" in
+		  "DEFAULT_POOL"*) DEFAULT_POOL=${line##*=} ;;
+		  "#"*) continue ;;
+		  *) error "Unknown value \"$line\" in configuration file"
+		esac
+	done
+}
+
 #############################
 # start point of this script
 # - parsing parameters
@@ -1365,6 +1381,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 ;;
@@ -1380,6 +1397,17 @@ 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"
+	parse_config $CONFIG
+else
+	for i in $CONFIG_PATHS; do
+		[ ! -f "$i" ] && continue
+		parse_config $i
+	done
+fi
+
 export FSADM_RUNNING="fsadm"
 $COMMAND $ARGS
 cleanup 0
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 21/35] fsadm: Use DEFAULT_POOL when creating volume group
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (19 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 20/35] fsadm: Add simple configuration file Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-22 10:09   ` Zdenek Kabelac
  2011-09-21 16:45 ` [linux-lvm] [PATCH 22/35] fsadm: Add LVOL_PREFIX configuration option Lukas Czerner
                   ` (13 subsequent siblings)
  34 siblings, 1 reply; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

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

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |   58 ++++++-----------------------------------------------
 1 files changed, 7 insertions(+), 51 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 1968ee3..a66abd1 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -748,49 +748,13 @@ 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
+		vgname=$DEFAULT_POOL
+		$LVM vgs $vgname &> /dev/null
+		if [ $? -ne 0 ]; then
+			vg_create=1
 		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 "$stripesize" ]; then
@@ -1242,17 +1206,9 @@ 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"
+		vgname=$DEFAULT_POOL
+		grep $vgname $tmp &> /dev/null
+		if [ $? -ne 0 ]; then
 			vg_create=1
 		fi
 	else
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 22/35] fsadm: Add LVOL_PREFIX configuration option
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (20 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 21/35] fsadm: Use DEFAULT_POOL when creating volume group Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 23/35] fsadm: Only use readlink if link is provided Lukas Czerner
                   ` (12 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

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.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index a66abd1..63600eb 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -776,9 +776,9 @@ create() {
 		tmp=$(mktemp)
 		$LVM lvs $vgname --separator ' ' --noheadings > $tmp
 		for i in $(seq -w $MAX_VGS); do
-			$GREP -e " lvol${i} " $tmp &> /dev/null
+			$GREP -e " ${LVOL_PREFIX}${i} " $tmp &> /dev/null
 			if [ $? -ne 0 ]; then
-				name="lvol${i}"
+				name="${LVOL_PREFIX}${i}"
 				lvname="--name $name"
 				break;
 			fi
@@ -1285,12 +1285,14 @@ check() {
 
 set_default_config() {
 	[ -z "$DEFAULT_POOL" ] && DEFAULT_POOL="default_pool"
+	[ -z "$LVOL_PREFIX" ] && LVOL_PREFIX="lvol"
 }
 
 parse_config() {
 	for line in $(cat $1); do
 		case "$line" in
 		  "DEFAULT_POOL"*) DEFAULT_POOL=${line##*=} ;;
+		  "LVOL_PREFIX"*) LVOL_PREFIX=${line##*=} ;;
 		  "#"*) continue ;;
 		  *) error "Unknown value \"$line\" in configuration file"
 		esac
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 23/35] fsadm: Only use readlink if link is provided
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (21 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 22/35] fsadm: Add LVOL_PREFIX configuration option Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 24/35] fsadm: Remove unnecessary modification of PATH variable Lukas Czerner
                   ` (11 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 63600eb..3a00063 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -295,8 +295,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
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 24/35] fsadm: Remove unnecessary modification of PATH variable
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (22 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 23/35] fsadm: Only use readlink if link is provided Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 25/35] fsadm: Allow to specify size without "size=" prefix in "resize" Lukas Czerner
                   ` (10 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

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.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 3a00063..55b4688 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -32,9 +32,6 @@
 
 TOOL=$(basename $0)
 
-_SAVEPATH=$PATH
-PATH=/sbin:/usr/sbin:/bin:/usr/sbin:$PATH
-
 # utilities
 TUNE_EXT=tune2fs
 RESIZE_EXT=resize2fs
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 25/35] fsadm: Allow to specify size without "size=" prefix in "resize"
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (23 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 24/35] fsadm: Remove unnecessary modification of PATH variable Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 26/35] fsadm: Allow to specify lv in vg/lv format Lukas Czerner
                   ` (9 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

For backwards compatibility we should allow to specify plain number as a
new size in resize command.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 55b4688..f354fec 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -587,9 +587,12 @@ resize() {
 			continue
 		fi
 		case $i in
-			"size=+"*) [ $size -eq 0 ] && extend=${i##*+} && size=1;;
-			"size=-"*) [ $size -eq 0 ] && shrink=${i##*-} && size=1;;
-			"size="*)  [ $size -eq 0 ] && new_size=${i##*=} && size=1;;
+			"size=+"*)	[ $size -eq 0 ] && extend=${i##*+} && size=1;;
+			+[[:digit:]]*)	[ $size -eq 0 ] && extend=${i##*+} && size=1;;
+			"size=-"*)	[ $size -eq 0 ] && shrink=${i##*-} && size=1;;
+			-[[:digit:]]*)	[ $size -eq 0 ] && shrink=${i##*-} && size=1;;
+			"size="*) 	[ $size -eq 0 ] && new_size=${i##*=} && size=1;;
+			[[:digit:]]*) 	[ $size -eq 0 ] && new_size=${i} && size=1;;
 			*) error "Wrong option $i. (see: $TOOL --help)";;
 		esac
 	done
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 26/35] fsadm: Allow to specify lv in vg/lv format
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (24 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 25/35] fsadm: Allow to specify size without "size=" prefix in "resize" Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 27/35] fsadm: error out when no size is provided in resize Lukas Czerner
                   ` (8 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

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

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |   24 +++++++++++++++---------
 1 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index f354fec..ba78533 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -288,10 +288,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\""
@@ -303,6 +300,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
@@ -532,6 +535,7 @@ resize_fs() {
 	[ $? -eq 1 ] && error "Cannot get FSTYPE of \"$VOLUME\""
 	verbose "\"$FSTYPE\" filesystem found on \"$VOLUME\""
 	if [ "$NEWSIZE" ]; then
+		decode_size $NEWSIZE
 		is_natural $NEWSIZE
 		[ $? -ne 1 ] && error "$NEWSIZE is not valid number for file system size"
 	fi
@@ -571,15 +575,16 @@ 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
 				if [ $? -eq 0 ]; then
-					lvname=$i
+					lvname=$VOLUME
 					devcount=0
 					continue
 				else
-					error "$i is not valid logical volume"
+					error "$VOLUME is not valid logical volume"
 				fi
 			fi
 			devices="$devices $i"
@@ -861,8 +866,9 @@ do_remove() {
 	MOUNTED=
 
 	# Block device has been given
-	if [ -b $item ]; then
-		device=$item
+	get_volume $item
+	if [ -b $VOLUME ]; then
+		device=$VOLUME
 	fi
 
 	# Mount point has been given
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 27/35] fsadm: error out when no size is provided in resize
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (25 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 26/35] fsadm: Allow to specify lv in vg/lv format Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 28/35] fsadm: Umount ext2 file system prior resize Lukas Czerner
                   ` (7 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index ba78533..e11f7f9 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -602,6 +602,10 @@ resize() {
 		esac
 	done
 
+	if [ $size -eq 0 ]; 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. "\
@@ -630,11 +634,6 @@ resize() {
 		fi
 	fi
 
-	# If no size parameter has been provided, resize the volume to the maximum
-	if [ $size -eq 0 ]; then
-		echo "What??"
-	fi
-
 	# determine new size of the file system
 	if [ "$extend" ] || [ "$shrink" ] || [ "$new_size" ]; then
 		# only one of those variable should be set, so it is safe to do that
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 28/35] fsadm: Umount ext2 file system prior resize
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (26 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 27/35] fsadm: error out when no size is provided in resize Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-22 10:28   ` Zdenek Kabelac
  2011-09-21 16:45 ` [linux-lvm] [PATCH 29/35] lvresize: Specify --resize-fs-only when going to use fsadm resize Lukas Czerner
                   ` (6 subsequent siblings)
  34 siblings, 1 reply; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

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

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index e11f7f9..87c9bbc 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -411,6 +411,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
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 29/35] lvresize: Specify --resize-fs-only when going to use fsadm resize
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (27 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 28/35] fsadm: Umount ext2 file system prior resize Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 30/35] test: add helper to compute aligned lv size Lukas Czerner
                   ` (5 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

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.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 tools/lvresize.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/tools/lvresize.c b/tools/lvresize.c
index ccd6c6e..959c953 100644
--- a/tools/lvresize.c
+++ b/tools/lvresize.c
@@ -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) {
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 30/35] test: add helper to compute aligned lv size
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (28 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 29/35] lvresize: Specify --resize-fs-only when going to use fsadm resize Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 31/35] fsadm: Add help for new commands and update man page Lukas Czerner
                   ` (4 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

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

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 test/lib/utils.sh |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/test/lib/utils.sh b/test/lib/utils.sh
index 4c3cd93..c555e45 100644
--- a/test/lib/utils.sh
+++ b/test/lib/utils.sh
@@ -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
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 31/35] fsadm: Add help for new commands and update man page
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (29 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 30/35] test: add helper to compute aligned lv size Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 32/35] fsadm: Update authorship of the fsadm Lukas Czerner
                   ` (3 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

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.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 man/fsadm.8.in   |  323 ++++++++++++++++++++++++++++++++++++++++++++++++++----
 scripts/fsadm.sh |  136 ++++++++++++++++++++---
 2 files changed, 418 insertions(+), 41 deletions(-)

diff --git a/man/fsadm.8.in b/man/fsadm.8.in
index b7bd0a0..62fbb1a 100644
--- a/man/fsadm.8.in
+++ b/man/fsadm.8.in
@@ -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 default_pool/lvol001
+logical volume by 10G we can simply call
+.PP
+    \fB fsadm resize size=-10G default_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 default_pool/lvol001 /dev/sde /dev/sdf
+.PP
+To
+.B remove
+the above logical volume we can use the following command:
+.PP
+    \fB fsadm remove default_pool/lvol001
+.PP
+or we can simply remove the whole pool, wiping all logical volumes in it:
+.PP
+    \fB fsadm remove default_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_POOL
+Default pool that will be used in
+.B fsadm
+command if not pool is specified. It defaults to "default_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),
diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 87c9bbc..95b26e2 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -89,27 +89,107 @@ IFS_OLD=$IFS
 NL='
 '
 
-tool_usage() {
-	echo "${TOOL}: Utility to resize or check the filesystem on a device"
-	echo
+check_usage() {
 	echo "  ${TOOL} [options] check device"
 	echo "    - Check the filesystem on device using fsck"
 	echo
-	echo "  ${TOOL} [options] resize device [new_size[BKMGTPE]]"
-	echo "    - Change the size of the filesystem on device to new_size"
+}
+
+resize_usage() {
+	echo "  ${TOOL} [options] resize [resize options] [device...] volume"
+	echo "    - Change the size of the logical volume and file system on it"
 	echo
-	echo "  Options:"
-	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 "    -n | --dry-run      Print commands without running them"
-	echo "    -y | --yes          Answer \"yes\" at any prompts"
+	echo "  Resize options:"
+	echo "    size=n        Resize the volume to the given size."
+	echo "    size=+n       Extend the volume by the given size."
+	echo "    size=-n       Shrink the volume by the given size."
+	echo "    [device...]   Devices to use when extending the volume. If the"
+	echo "                  device is not in any pool, it is added to the"
+	echo "                  volume's pool prior to the extension."
+	echo "    A suffix K, M, G, T, P, E can be used to define size units."
 	echo
-	echo "  new_size - Absolute number of filesystem blocks to be in the filesystem,"
-	echo "             or an absolute size using a suffix (in powers of 1024)."
-	echo "             If new_size is not supplied, the whole device is used."
+}
 
+create_usage() {
+	echo "  ${TOOL} [options] create [create options] [device...] [pool]"
+	echo "    - Create a new logical volume from the pool"
+	echo
+	echo "  Create options:"
+	echo "    stripesize=n     Gives the number of kilobytes for the granularity"
+	echo "                     of stripes (see man 8 fsadm for more information)."
+	echo "                     This is optional and if not given, linear logical"
+	echo "                     volume will be created."
+	echo "    name=n           The name for the new logical volume. Without this"
+	echo "                     option the default names of "lvol#" will be generated"
+	echo "                     where # is internal number of the logical volume"
+	echo "                     The default can be changed by setting LVOL_PREFIX"
+	echo "                     fsadm configuration file."
+	echo "    fstyp=n | fs=n   Gives the file system type to create on the new"
+	echo "                     logical volume. Supported file systems are (ext3,"
+	echo "                     ext4, xfs). This is optional and if not given"
+	echo "                     file system will not be created."
+	echo "    size=n           Gives the size to allocate for the new logical volume"
+	echo "                     A size suffix K, M, G, T, P, E can be used to define"
+	echo "                     units (see man 8 fsadm for more information). This"
+	echo "                     is optional if if not given maximum size will be"
+	echo "                     used."
+	echo "    stripes=n        Gives the number of stripes. This is equal to the"
+	echo "                     number of physical volumes to scatter the logical"
+	echo "                     volume. This is optional and if stripesize is set"
+	echo "                     and multiple devices are provided stripes is"
+	echo "                     determined automatically."
+	echo
+}
+
+list_usage() {
+	echo "  ${TOOL} [options] list [filesystems|fs|devices|dev|pool]"
+	echo "    - List devices, file systems and pools in your system"
+	echo
+	echo "  List options:"
+	echo "    filesystems | fs   List all logical volumes."
+	echo "    devices | dev      List all devices except the ones create by DM."
+	echo "    pool               List all available pools to create new logical"
+	echo "                       volumes from."
+	echo
+}
+
+add_usage() {
+	echo "  ${TOOL} [options] add device [device...] [pool]"
+	echo "    - Add one, or more devices into the defined, or default pool"
+	echo
+}
+
+remove_usage() {
+	echo "  ${TOOL} [options] remove [remove options] item [item...]"
+	echo "    - Remove one, or more logical volumes, devices, or pools defined"
+	echo "      by item"
+	echo
+	echo "  Remove options:"
+	echo "    item   Can be logical volume, mount point, device in the pool"
+	echo "             or name of the pool to remove."
+	echo "    --all    Removes all pools and corresponding logical volumes"
+	echo
+
+}
+
+tool_usage() {
+	echo "${TOOL}: Utility to manage logical volumes"
+	echo
+	check_usage
+	resize_usage
+	create_usage
+	list_usage
+	add_usage
+	remove_usage
+	echo "  Options:"
+	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 "    -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
 }
 
@@ -570,6 +650,10 @@ resize_lvolume() {
 }
 
 resize() {
+	if [ "$1" == "--help" ]; then
+		resize_usage
+		exit
+	fi
 	local size=0
 
 	# Special case for the situation we have been called from within the lvresize code.
@@ -733,11 +817,14 @@ create() {
 			continue
 		fi
 		case $i in
-			"stripesize"* | "chunksize"*) stripesize=${i##*=} ;;
+			"stripesize"*) stripesize=${i##*=} ;;
 			"name"*) name=${i##*=} ;;
 			"fstyp"* | "fs"*) fstyp=${i##*=} ;;
 			"size"*) size=${i##*=} ;;
 			"stripes"*) stripes=${i##*=} ;;
+			"--help") create_usage
+				  exit
+				  ;;
 			*) if [ -z $vgname ]; then vgname=$i; else
 				error "Wrong option $i. (see: $TOOL --help)"
 			   fi ;;
@@ -920,9 +1007,9 @@ do_remove() {
 ###############################
 remove() {
 	# help
-	if [ "$1" == "help" ]; then
-		echo "Usage: $TOOL remove [mount point | dm device | voulume group | device | --all]"
-		exit 0
+	if [ "$1" == "--help" ]; then
+		remove_usage
+		exit
 	elif [ "$1" == "--all" ]; then
 		list="$(LANG=C $LVM vgs -o vg_name --separator ' ' --noheadings --nosuffix --units k 2> /dev/null)"
 	else
@@ -1177,6 +1264,9 @@ list() {
 			"filesystems" | "fs") list_filesystems ;;
 			"devices" | "dev") list_devices ;;
 			"pool") list_pool ;;
+			"--help") list_usage
+				  exit
+				  ;;
 			*) error "Wrong option $i. (see: $TOOL --help)"
 		esac
 	done
@@ -1201,6 +1291,10 @@ detect_volume_group() {
 # Add devices into the group
 ############################
 add() {
+	if [ "$1" == "--help" ]; then
+		add_usage
+		exit
+	fi
 	vg_create=0
 	tmp=$(mktemp)
 
@@ -1254,6 +1348,10 @@ diff_dates() {
 # Check filesystem
 ###################
 check() {
+	if [ "$1" == "--help" ]; then
+		check_usage
+		exit
+	fi
 	detect_fs "$1"
 	[ $? -eq 1 ] && error "Cannot get FSTYPE of \"$VOLUME\""
 	verbose "\"$FSTYPE\" filesystem found on \"$VOLUME\""
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 32/35] fsadm: Update authorship of the fsadm
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (30 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 31/35] fsadm: Add help for new commands and update man page Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 33/35] test: Add test for fsadm add command Lukas Czerner
                   ` (2 subsequent siblings)
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

Add authorship into the fsadm header since the fsadm tool has been
significantly changed.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 scripts/fsadm.sh |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
index 95b26e2..d732525 100755
--- a/scripts/fsadm.sh
+++ b/scripts/fsadm.sh
@@ -14,7 +14,12 @@
 #
 # Author: Zdenek Kabelac <zkabelac@redhat.com>
 #
-# Script for resizing devices (usable for LVM resize)
+# Author: Lukas Czerner <lczerner@redhat.com>
+#    - 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, bc, df
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 33/35] test: Add test for fsadm add command
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (31 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 32/35] fsadm: Update authorship of the fsadm Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 34/35] test: Add test for fsadm create command Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 35/35] test: Add test for fsadm resize command Lukas Czerner
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

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

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 test/t-fsadm-add.sh |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 48 insertions(+), 0 deletions(-)
 create mode 100644 test/t-fsadm-add.sh

diff --git a/test/t-fsadm-add.sh b/test/t-fsadm-add.sh
new file mode 100644
index 0000000..c2d310e
--- /dev/null
+++ b/test/t-fsadm-add.sh
@@ -0,0 +1,48 @@
+#!/bin/bash
+# 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
+# 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_POOL=$vg1
+
+pool1=$vg2
+pool2=$vg3
+
+# Create default pool with all devices at once
+fsadm add $TEST_DEVS
+check vg_field $DEFAULT_POOL pv_count $DEV_COUNT
+fsadm -f remove $DEFAULT_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_POOL pv_count $DEV_COUNT
+fsadm -f remove $DEFAULT_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
+check vg_field $DEFAULT_POOL pv_count 3
+check vg_field $pool1 pv_count 3
+check vg_field $pool2 pv_count 4
+
+# Simple remove check
+fsadm -f remove $pool1 $pool2
+check vg_field $pool3 pv_count 4
+fsadm -f remove $pool3
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 34/35] test: Add test for fsadm create command
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (32 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 33/35] test: Add test for fsadm add command Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  2011-09-21 16:45 ` [linux-lvm] [PATCH 35/35] test: Add test for fsadm resize command Lukas Czerner
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

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.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 test/t-fsadm-create.sh |  112 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 112 insertions(+), 0 deletions(-)
 create mode 100644 test/t-fsadm-create.sh

diff --git a/test/t-fsadm-create.sh b/test/t-fsadm-create.sh
new file mode 100644
index 0000000..ccf3f9f
--- /dev/null
+++ b/test/t-fsadm-create.sh
@@ -0,0 +1,112 @@
+#!/bin/bash
+# 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
+# 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_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
+check lv_field $DEFAULT_POOL/$lvol1 pv_count $DEV_COUNT
+fsadm -f remove $DEFAULT_POOL
+
+# Create the group first and then create volume using the whole group
+fsadm add $TEST_DEVS
+fsadm create
+check lv_field $DEFAULT_POOL/$lvol1 pv_count $DEV_COUNT
+fsadm -f remove $DEFAULT_POOL
+
+# Create a logical volume of fixed size
+size=$(($DEV_SIZE*6))
+fsadm create size=${size}M $TEST_DEVS
+size=$(align_size_up $size)
+check lv_field $DEFAULT_POOL/$lvol1 lv_size ${size}.00m
+fsadm -f remove $DEFAULT_POOL
+
+# Create a striped logical volume
+fsadm create stripesize=32 $TEST_DEVS
+check lv_field $DEFAULT_POOL/$lvol1 stripesize 32.00k
+fsadm  -f remove $DEFAULT_POOL
+
+# Create several volumes with different parameters
+fsadm  add $TEST_DEVS
+fsadm create stripesize=32 stripes=$(($DEV_COUNT/2)) size=$(($DEV_SIZE*2))M
+fsadm create stripesize=8 stripes=$((DEV_COUNT)) size=$(($DEV_SIZE*3))M
+fsadm create
+check lv_field $DEFAULT_POOL/$lvol1 stripesize 32.00k
+check lv_field $DEFAULT_POOL/$lvol1 stripes $(($DEV_COUNT/2))
+check lv_field $DEFAULT_POOL/$lvol2 stripesize 8.00k
+check lv_field $DEFAULT_POOL/$lvol2 stripes $DEV_COUNT
+check lv_field $DEFAULT_POOL/$lvol3 segtype linear
+fsadm  -f remove $DEFAULT_POOL
+
+# Create several volumes with different parameters from different groups
+fsadm add $dev1 $dev2 $dev3 $pool1
+fsadm create $pool1
+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
+check lv_field $DEFAULT_POOL/$lvol1 stripesize 8.00k
+check lv_field $pool1/$lvol1 pv_count 3
+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_POOL/$lvol1 pv_count $DEV_COUNT
+	fsadm -f check ${DEFAULT_POOL}/$lvol1
+	_do_fsck $fs ${DEFAULT_POOL}/$lvol1
+	fsadm  -f remove $DEFAULT_POOL
+
+	fsadm create fs=$fs stripesize=32 size=$(($DEV_SIZE*6))M $TEST_DEVS
+	check lv_field $DEFAULT_POOL/$lvol1 pv_count $DEV_COUNT
+	check lv_field $DEFAULT_POOL/$lvol1 stripesize 32.00k
+	_do_fsck $fs ${DEFAULT_POOL}/$lvol1
+	fsadm  -f remove $DEFAULT_POOL
+
+	fsadm add $TEST_DEVS
+	fsadm create fs=$fs stripesize=8 stripes=$((DEV_COUNT/5)) size=$(($DEV_SIZE*2))M
+	check lv_field $DEFAULT_POOL/$lvol1 stripes $(($DEV_COUNT/5))
+	check lv_field $DEFAULT_POOL/$lvol1 stripesize 8.00k
+	_do_fsck $fs ${DEFAULT_POOL}/$lvol1
+	fsadm  -f remove $DEFAULT_POOL
+done
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [linux-lvm] [PATCH 35/35] test: Add test for fsadm resize command
  2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
                   ` (33 preceding siblings ...)
  2011-09-21 16:45 ` [linux-lvm] [PATCH 34/35] test: Add test for fsadm create command Lukas Czerner
@ 2011-09-21 16:45 ` Lukas Czerner
  34 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-21 16:45 UTC (permalink / raw)
  To: zkabelac; +Cc: Lukas Czerner, dchinner, rwheeler, linux-lvm

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.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 test/t-fsadm-resize.sh |  113 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 113 insertions(+), 0 deletions(-)
 create mode 100644 test/t-fsadm-resize.sh

diff --git a/test/t-fsadm-resize.sh b/test/t-fsadm-resize.sh
new file mode 100644
index 0000000..59c3e75
--- /dev/null
+++ b/test/t-fsadm-resize.sh
@@ -0,0 +1,113 @@
+#!/bin/bash
+# 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
+# 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_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_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 -y resize size=${size}M ${DM_DEV_DIR}/$DEFAULT_VOLUME
+	size=$(align_size_up $size)
+	check lv_field $DEFAULT_POOL/$lvol1 lv_size ${size}.00m
+
+	# xfs does not support shrinking (xfs only grows big!! :))
+	if [ "$fs" != "xfs" ]; then
+		fsadm -f -y -v resize size=-$(($TEST_MAX_SIZE/4))M $DEFAULT_VOLUME
+		size=$(align_size_up $(($size-($TEST_MAX_SIZE/4))))
+		check lv_field $DEFAULT_POOL/$lvol1 lv_size ${size}.00m
+	fi
+	fsadm -y -v resize size=+$(($TEST_MAX_SIZE/5))M $DEFAULT_VOLUME
+	size=$(align_size_up $(($size+($TEST_MAX_SIZE/5))))
+	check lv_field $DEFAULT_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_POOL/$lvol1 lv_size ${size}.00m
+
+fsadm -y 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_POOL/$lvol1 lv_size ${size}.00m
+
+fsadm -f -y resize size=-$(($TEST_MAX_SIZE/2))M $DEFAULT_VOLUME
+size=$(align_size_up $(($size-($TEST_MAX_SIZE/2))))
+check lv_field $DEFAULT_POOL/$lvol1 lv_size ${size}.00m
+
+fsadm -y resize size=$(($TEST_MAX_SIZE/2))M $DEFAULT_VOLUME
+size=$(align_size_up $(($TEST_MAX_SIZE/2)))
+check lv_field $DEFAULT_POOL/$lvol1 lv_size ${size}.00m
+fsadm -f remove $DEFAULT_POOL
+
+fsadm create $dev1
+check lv_field $DEFAULT_POOL/$lvol1 pv_count 1
+devs=${TEST_DEVS##*$dev1 }
+fsadm resize size=+$((TEST_MAX_SIZE/2))M $DEFAULT_VOLUME $devs
+check lv_field $DEFAULT_POOL/$lvol1 pv_count $DEV_COUNT
+fsadm -f remove $DEFAULT_POOL
+
+fsadm create $dev1
+check lv_field $DEFAULT_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_POOL/$lvol1 pv_count $DEV_COUNT
+fsadm -f remove $DEFAULT_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_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_POOL
+done
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* Re: [linux-lvm] [PATCH 01/35] fsadm: Add "create" command
  2011-09-21 16:45 ` [linux-lvm] [PATCH 01/35] fsadm: Add "create" command Lukas Czerner
@ 2011-09-21 19:00   ` Stephane Chazelas
  2011-09-22  9:28     ` Lukas Czerner
  0 siblings, 1 reply; 48+ messages in thread
From: Stephane Chazelas @ 2011-09-21 19:00 UTC (permalink / raw)
  To: LVM general discussion and development
  Cc: Lukas Czerner, dchinner, rwheeler, zkabelac

2011-09-21 18:45:20 +0200, Lukas Czerner:
> Create command provides the functionality of creating a new logical
> volumes including defined file system.
> 
> This commit also changes the way how are commands recognised an
> executed. The new approach is more suitable for bigger number of
> commands.

There are so many shell scripting bad practices in there that I
thought I had to reply.

> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> ---
>  scripts/fsadm.sh |  140 ++++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 files changed, 131 insertions(+), 9 deletions(-)
> 
> diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
> index c8cc5e0..42c7da4 100755
> --- a/scripts/fsadm.sh
> +++ b/scripts/fsadm.sh
> @@ -29,7 +29,7 @@
>  #   2 break detected
>  #   3 unsupported online filesystem check for given mounted fs
>  
> -TOOL=fsadm
> +TOOL=$(basename $0)

Leaving a variable unquoted has a very special meaning in shell.
If we were looking to a perl equivalent of $var that would be:

map glob, split $IFS_regex, $var

With the default value of $IFS:

$ ls
bar  foo
$ perl -le '$var="a *"; print for map glob, split " ", $var'
a
bar
foo
$ sh -c 'var="a *"; printf "%s\n" $var'
a
bar
foo

That is it is a special operator that triggers word splitting
(split()) and filename generation (glob()).

Variables should alway be quoted unless you've got a very good
reason not to.

Also,

It's either

cmd "$option_or_argument"
or
cmd -- "$argument"

So above:

TOOL=$(basename -- "$0")

>  
>  _SAVEPATH=$PATH

Why would you need to save $PATH?

>  PATH=/sbin:/usr/sbin:/bin:/usr/sbin:$PATH
> @@ -76,6 +76,8 @@ REMOUNT=
>  PROCMOUNTS="/proc/mounts"
>  NULL="$DM_DEV_DIR/null"
>  
> +MAX_VGS=999
> +
>  IFS_OLD=$IFS

doing

IFS_OLD=$IFS
...
IFS=$OLD_IFS

doesn't have the expected behavior if IFS was previously unset
(allowed by LSB and POSIX).

Use subshells or local (LSB but not POSIX).

>  # without bash $'\n'
>  NL='
> @@ -122,6 +124,14 @@ dry() {
>  	fi
>  	verbose "Executing $@"
>  	$@

$@ doesn't make sense. It should either be

IFS=" "
eval "$*"
(the concatenation of the positional parameters taken as a
command line)

or more likely (without knowing the context)

"$@"

The position parameters are to be interpreted as the arguments
to a simple command.


> +	if [ $? -ne 0 ]; then
> +		error "FAILED. Exitting!"
> +	fi

"$@" || error ...

or

if ! "$@"; then
  error ...
fi

> +}
> +
> +is_natural() {
> +	test "$1" -ge 0 &> /dev/null && return 1


&> is a bashism.

And depending on the shell, that is not guaranteed to do what
you think it does. for instance, it could say that "1+1", "1 ",
"1.2",  are natural numbers.

Also, you seem to have it backward.

It will return 1 (failure/false) if the number is natural.

is_natural()
  case "$1" in
    ("" | *[!0-9]*) return 1;;
    (*) return 0;;
  esac

>  }
>  
>  cleanup() {
> @@ -365,12 +375,42 @@ resize_xfs() {
>  	fi
>  }
>  
> +make_ext() {
> +	device=$1
> +	fstyp=$2
> +	stripe=$3
> +	stripesize=$4
> +	bsize=4
> +
> +	if [ "$YES" ]; then
> +		force="-F"
> +	fi
> +	stride=$(($stripesize/$bsize))
> +	stripewidth=$(($stride*$stripe))
> +
> +	dry mkfs.$fstyp $force -b$(($bsize*1024)) -E stride=${stride},stripe-width=${stripewidth} $device

Again, I think that should be either:

set --
[ -n "$YES" ] &&
  set -- -F
dry "mkfs.$fstyp" "$@" -b "$(($bsize*1024))" -E stride="${stride},stripe-width=${stripewidth}" "$device"

or:

force=
[ -n "$YES" ] &&
  force=-F

eval 'dry "mkfs.$fstyp" '"$force"' -b "$(($bsize*1024))" -E stride="${stride},stripe-width=${stripewidth}" "$device"'

[...]
> +	is_natural $NEWSIZE
> +	[ $? -ne 1 ] && error "$NEWSIZE is not valid number for file system size"

With a fixed is_natural,

is_natural "$NEWSIZE" || error ...

[...]
> +	for i in $@; do

for i do
 ...


> +		if [ -b $i ]; then

...and so on.

-- 
Stephane

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [linux-lvm] [PATCH 01/35] fsadm: Add "create" command
  2011-09-21 19:00   ` Stephane Chazelas
@ 2011-09-22  9:28     ` Lukas Czerner
  2011-09-22 10:01       ` Zdenek Kabelac
  2011-09-22 10:56       ` Stephane Chazelas
  0 siblings, 2 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-22  9:28 UTC (permalink / raw)
  To: Stephane Chazelas
  Cc: Lukas Czerner, zkabelac, rwheeler, dchinner,
	LVM general discussion and development

On Wed, 21 Sep 2011, Stephane Chazelas wrote:

> 2011-09-21 18:45:20 +0200, Lukas Czerner:
> > Create command provides the functionality of creating a new logical
> > volumes including defined file system.
> > 
> > This commit also changes the way how are commands recognised an
> > executed. The new approach is more suitable for bigger number of
> > commands.
> 
> There are so many shell scripting bad practices in there that I
> thought I had to reply.

Great, that's what I need, Thanks!

> 
> > 
> > Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> > ---
> >  scripts/fsadm.sh |  140 ++++++++++++++++++++++++++++++++++++++++++++++++++----
> >  1 files changed, 131 insertions(+), 9 deletions(-)
> > 
> > diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
> > index c8cc5e0..42c7da4 100755
> > --- a/scripts/fsadm.sh
> > +++ b/scripts/fsadm.sh
> > @@ -29,7 +29,7 @@
> >  #   2 break detected
> >  #   3 unsupported online filesystem check for given mounted fs
> >  
> > -TOOL=fsadm
> > +TOOL=$(basename $0)
> 
> Leaving a variable unquoted has a very special meaning in shell.
> If we were looking to a perl equivalent of $var that would be:
> 
> map glob, split $IFS_regex, $var
> 
> With the default value of $IFS:
> 
> $ ls
> bar  foo
> $ perl -le '$var="a *"; print for map glob, split " ", $var'
> a
> bar
> foo
> $ sh -c 'var="a *"; printf "%s\n" $var'
> a
> bar
> foo
> 
> That is it is a special operator that triggers word splitting
> (split()) and filename generation (glob()).
> 
> Variables should alway be quoted unless you've got a very good
> reason not to.

Ok, good point.

> 
> Also,
> 
> It's either
> 
> cmd "$option_or_argument"
> or
> cmd -- "$argument"
> 
> So above:
> 
> TOOL=$(basename -- "$0")
> 
> >  
> >  _SAVEPATH=$PATH
> 
> Why would you need to save $PATH?

Yeah, that is something I would like to know as well :) I have actually
removed this in patch 24 since it does not make sense to change PATH in
this script.

> 
> >  PATH=/sbin:/usr/sbin:/bin:/usr/sbin:$PATH
> > @@ -76,6 +76,8 @@ REMOUNT=
> >  PROCMOUNTS="/proc/mounts"
> >  NULL="$DM_DEV_DIR/null"
> >  
> > +MAX_VGS=999
> > +
> >  IFS_OLD=$IFS
> 
> doing
> 
> IFS_OLD=$IFS
> ...
> IFS=$OLD_IFS
> 
> doesn't have the expected behavior if IFS was previously unset
> (allowed by LSB and POSIX).
> 
> Use subshells or local (LSB but not POSIX).
> 
> >  # without bash $'\n'
> >  NL='
> > @@ -122,6 +124,14 @@ dry() {
> >  	fi
> >  	verbose "Executing $@"
> >  	$@
> 
> $@ doesn't make sense. It should either be
> 
> IFS=" "
> eval "$*"
> (the concatenation of the positional parameters taken as a
> command line)
> 
> or more likely (without knowing the context)
> 
> "$@"
> 
> The position parameters are to be interpreted as the arguments
> to a simple command.
> 
> 
> > +	if [ $? -ne 0 ]; then
> > +		error "FAILED. Exitting!"
> > +	fi
> 
> "$@" || error ...

I guess it is just a matter of taste.

> 
> or
> 
> if ! "$@"; then
>   error ...
> fi

That is really not very readable.

> 
> > +}
> > +
> > +is_natural() {
> > +	test "$1" -ge 0 &> /dev/null && return 1
> 
> 
> &> is a bashism.

Probably (#!/bin/bash) :)

> 
> And depending on the shell, that is not guaranteed to do what
> you think it does. for instance, it could say that "1+1", "1 ",
> "1.2",  are natural numbers.
> 
> Also, you seem to have it backward.
> 
> It will return 1 (failure/false) if the number is natural.

It seems it's a bit weird, thanks!

> 
> is_natural()
>   case "$1" in
>     ("" | *[!0-9]*) return 1;;
>     (*) return 0;;
>   esac
> 
> >  }
> >  
> >  cleanup() {
> > @@ -365,12 +375,42 @@ resize_xfs() {
> >  	fi
> >  }
> >  
> > +make_ext() {
> > +	device=$1
> > +	fstyp=$2
> > +	stripe=$3
> > +	stripesize=$4
> > +	bsize=4
> > +
> > +	if [ "$YES" ]; then
> > +		force="-F"
> > +	fi
> > +	stride=$(($stripesize/$bsize))
> > +	stripewidth=$(($stride*$stripe))
> > +
> > +	dry mkfs.$fstyp $force -b$(($bsize*1024)) -E stride=${stride},stripe-width=${stripewidth} $device
> 
> Again, I think that should be either:
> 
> set --
> [ -n "$YES" ] &&
>   set -- -F
> dry "mkfs.$fstyp" "$@" -b "$(($bsize*1024))" -E stride="${stride},stripe-width=${stripewidth}" "$device"
> 
> or:
> 
> force=
> [ -n "$YES" ] &&
>   force=-F
> 
> eval 'dry "mkfs.$fstyp" '"$force"' -b "$(($bsize*1024))" -E stride="${stride},stripe-width=${stripewidth}" "$device"'

Ok, I do understand the that I should rather use 'eval', but I do not
understand why you're trying to get rid of the 'if', it is a bit longer,
so what ? But is is also more obvious.

> 
> [...]
> > +	is_natural $NEWSIZE
> > +	[ $? -ne 1 ] && error "$NEWSIZE is not valid number for file system size"
> 
> With a fixed is_natural,
> 
> is_natural "$NEWSIZE" || error ...
> 
> [...]
> > +	for i in $@; do
> 
> for i do

I am not sure what is wrong with that, it is more obvious and it works
just fine.

>  ...
> 
> 
> > +		if [ -b $i ]; then
> 
> ...and so on.
> 
> 

Thanks!
-Lukas

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [linux-lvm] [PATCH 01/35] fsadm: Add "create" command
  2011-09-22  9:28     ` Lukas Czerner
@ 2011-09-22 10:01       ` Zdenek Kabelac
  2011-09-22 10:27         ` Lukas Czerner
  2011-09-22 10:56       ` Stephane Chazelas
  1 sibling, 1 reply; 48+ messages in thread
From: Zdenek Kabelac @ 2011-09-22 10:01 UTC (permalink / raw)
  To: Lukas Czerner; +Cc: LVM general discussion and development

Dne 22.9.2011 11:28, Lukas Czerner napsal(a):
> On Wed, 21 Sep 2011, Stephane Chazelas wrote:
> 
>> 2011-09-21 18:45:20 +0200, Lukas Czerner:
>>> Create command provides the functionality of creating a new logical
>>> volumes including defined file system.
>>>
>>> This commit also changes the way how are commands recognised an
>>> executed. The new approach is more suitable for bigger number of
>>> commands.
>>
>> There are so many shell scripting bad practices in there that I
>> thought I had to reply.
> 
> Great, that's what I need, Thanks!
> 
>>
 So above:
>>
>> TOOL=$(basename -- "$0")
>>
>>>  
>>>  _SAVEPATH=$PATH
>>
>> Why would you need to save $PATH?
> 
> Yeah, that is something I would like to know as well :) I have actually
> removed this in patch 24 since it does not make sense to change PATH in
> this script.

When we are at this one - this commit definitely does not belong into the
fsadm update patch set - such patch should be submitted as a separate
unrelated patch and not 'well' hidden in the middle - and there are more such
things which should probably go separately - especially if they present some
bugfixes and should be merged before this large fsadm gets slowly upstream.

Zdenek

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [linux-lvm] [PATCH 11/35] fsadm: Add "remove" command
  2011-09-21 16:45 ` [linux-lvm] [PATCH 11/35] fsadm: Add "remove" command Lukas Czerner
@ 2011-09-22 10:06   ` Zdenek Kabelac
  2011-09-22 10:36     ` Lukas Czerner
  0 siblings, 1 reply; 48+ messages in thread
From: Zdenek Kabelac @ 2011-09-22 10:06 UTC (permalink / raw)
  To: Lukas Czerner; +Cc: linux-lvm

Dne 21.9.2011 18:45, Lukas Czerner napsal(a):
> Remove command allows to remove unused devices from the pool (volume
> group).
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> ---
>  scripts/fsadm.sh |   82 ++++++++++++++++++++++++++++++++++++++++-------------
>  1 files changed, 62 insertions(+), 20 deletions(-)
> 
> diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
> index 6617de0..4a4f625 100755
> --- a/scripts/fsadm.sh
> +++ b/scripts/fsadm.sh
> @@ -823,11 +823,12 @@ list_filesystems() {
>  	IFS=$NL
>  	local c=0
>  	for line in $(LANG=C $LVM lvs -o lv_path,lv_size,segtype --noheadings --separator ' ' --nosuffix --units k 2> /dev/null); do
> -		c=$((c+1))
>  		line=$(echo $line | sed -e 's/^ *\//\//')
>  		volume=$(echo $line | cut -d' ' -f1)
> -		volumes[$c]=$volume
> -		segtype[$c]=$(echo $line | cut -d' ' -f3)
> +		[ "$volume" == "$last_volume" ] && continue
> +		c=$((c+1))
> +		local volumes[$c]=$volume
> +		local segtype[$c]=$(echo $line | cut -d' ' -f3)
>  		detect_fs $volume
>  		detect_mounted
>  		detect_fs_size

Could you please update/cleanup the patch set - so it doesn't rework same code
multiple times over and over ?
(It's a waste of time to review new code, which gets replaced several times
during the whole patch set)

Thanks

Zdenek

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [linux-lvm] [PATCH 21/35] fsadm: Use DEFAULT_POOL when creating volume group
  2011-09-21 16:45 ` [linux-lvm] [PATCH 21/35] fsadm: Use DEFAULT_POOL when creating volume group Lukas Czerner
@ 2011-09-22 10:09   ` Zdenek Kabelac
  0 siblings, 0 replies; 48+ messages in thread
From: Zdenek Kabelac @ 2011-09-22 10:09 UTC (permalink / raw)
  To: Lukas Czerner; +Cc: linux-lvm

Dne 21.9.2011 18:45, Lukas Czerner napsal(a):
> I user did not specified group, use DEFAULT_POOL from configuration
> file.
> 

Since we start to use 'thin pool' terminology with thin target - it needs
probably more specific name -   DEFAULT_VG_POOL, DEFAULT_DEVICE_POOL ??

Zdenek

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [linux-lvm] [PATCH 01/35] fsadm: Add "create" command
  2011-09-22 10:01       ` Zdenek Kabelac
@ 2011-09-22 10:27         ` Lukas Czerner
  0 siblings, 0 replies; 48+ messages in thread
From: Lukas Czerner @ 2011-09-22 10:27 UTC (permalink / raw)
  To: Zdenek Kabelac; +Cc: Lukas Czerner, LVM general discussion and development

On Thu, 22 Sep 2011, Zdenek Kabelac wrote:

> Dne 22.9.2011 11:28, Lukas Czerner napsal(a):
> > On Wed, 21 Sep 2011, Stephane Chazelas wrote:
> > 
> >> 2011-09-21 18:45:20 +0200, Lukas Czerner:
> >>> Create command provides the functionality of creating a new logical
> >>> volumes including defined file system.
> >>>
> >>> This commit also changes the way how are commands recognised an
> >>> executed. The new approach is more suitable for bigger number of
> >>> commands.
> >>
> >> There are so many shell scripting bad practices in there that I
> >> thought I had to reply.
> > 
> > Great, that's what I need, Thanks!
> > 
> >>
>  So above:
> >>
> >> TOOL=$(basename -- "$0")
> >>
> >>>  
> >>>  _SAVEPATH=$PATH
> >>
> >> Why would you need to save $PATH?
> > 
> > Yeah, that is something I would like to know as well :) I have actually
> > removed this in patch 24 since it does not make sense to change PATH in
> > this script.
> 
> When we are at this one - this commit definitely does not belong into the
> fsadm update patch set - such patch should be submitted as a separate
> unrelated patch and not 'well' hidden in the middle - and there are more such
> things which should probably go separately - especially if they present some
> bugfixes and should be merged before this large fsadm gets slowly upstream.
> 
> Zdenek
> 

The patch removing the PATH think is in the separate path, and I
mentioned above. Regarding more bug fixes, I am not sure what do you
mean specifically, some of them are separate patches and some of them
might be unfortunately part of other patches.

The reason is simple, I have extended fsadm from 494 lines to 1486 lines
with series of patches, improvements and new features. I would be painful
to identify each and every bug fix and create separate patch, which will
go through separate merging process especially since it is a single file
script.

Also the patch is not "well" hidden, it is just a part of the series and
it is done after all the big changes I have made.

Thanks!
-Lukas

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [linux-lvm] [PATCH 28/35] fsadm: Umount ext2 file system prior resize
  2011-09-21 16:45 ` [linux-lvm] [PATCH 28/35] fsadm: Umount ext2 file system prior resize Lukas Czerner
@ 2011-09-22 10:28   ` Zdenek Kabelac
  0 siblings, 0 replies; 48+ messages in thread
From: Zdenek Kabelac @ 2011-09-22 10:28 UTC (permalink / raw)
  To: Lukas Czerner; +Cc: linux-lvm

Dne 21.9.2011 18:45, Lukas Czerner napsal(a):
> ext2 does NOT support online resize, so we should attempt to umount the
> file system first.
> 


Hmm - wondering when this has changed, as in the time it's been developed I
think it has worked - thought it's been some experimental kernel support.
It seems to be even mentioned in many man pages around that ext2 should work
online, but my current 3.0 refuses to do it - so it's probably ok to disable
it for ext2.  (So currently user needs to add  '-e' option to proceed).

Zdenek

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [linux-lvm] [PATCH 11/35] fsadm: Add "remove" command
  2011-09-22 10:06   ` Zdenek Kabelac
@ 2011-09-22 10:36     ` Lukas Czerner
  2011-09-22 10:43       ` Zdenek Kabelac
  0 siblings, 1 reply; 48+ messages in thread
From: Lukas Czerner @ 2011-09-22 10:36 UTC (permalink / raw)
  To: Zdenek Kabelac; +Cc: Lukas Czerner, linux-lvm

On Thu, 22 Sep 2011, Zdenek Kabelac wrote:

> Dne 21.9.2011 18:45, Lukas Czerner napsal(a):
> > Remove command allows to remove unused devices from the pool (volume
> > group).
> > 
> > Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> > ---
> >  scripts/fsadm.sh |   82 ++++++++++++++++++++++++++++++++++++++++-------------
> >  1 files changed, 62 insertions(+), 20 deletions(-)
> > 
> > diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
> > index 6617de0..4a4f625 100755
> > --- a/scripts/fsadm.sh
> > +++ b/scripts/fsadm.sh
> > @@ -823,11 +823,12 @@ list_filesystems() {
> >  	IFS=$NL
> >  	local c=0
> >  	for line in $(LANG=C $LVM lvs -o lv_path,lv_size,segtype --noheadings --separator ' ' --nosuffix --units k 2> /dev/null); do
> > -		c=$((c+1))
> >  		line=$(echo $line | sed -e 's/^ *\//\//')
> >  		volume=$(echo $line | cut -d' ' -f1)
> > -		volumes[$c]=$volume
> > -		segtype[$c]=$(echo $line | cut -d' ' -f3)
> > +		[ "$volume" == "$last_volume" ] && continue
> > +		c=$((c+1))
> > +		local volumes[$c]=$volume
> > +		local segtype[$c]=$(echo $line | cut -d' ' -f3)
> >  		detect_fs $volume
> >  		detect_mounted
> >  		detect_fs_size
> 
> Could you please update/cleanup the patch set - so it doesn't rework same code
> multiple times over and over ?
> (It's a waste of time to review new code, which gets replaced several times
> during the whole patch set)

Unfortunately that is what I have mentioned in the patch 0. The main
problem is that the fsadm is one file script and while adding other
functionality I was building helpers and yes also changing other parts
of the code.

It was not possible for me make one feature, be done with it and move to
the another feature. Some of them actually supports each other and not
having to rebase with every change made my work significantly easier. So
at this point I am not going to rework the patches, since it would take
more time that actually write that code :).

But, since it is a single file script, and I changed the whole thing
significantly (there is not much left from the original code) I can
create one big patch covering all the new features, so it will be easier
for you to review it. Will that be acceptable for you ?

Thanks!
-Lukas

> 
> Thanks
> 
> Zdenek

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [linux-lvm] [PATCH 11/35] fsadm: Add "remove" command
  2011-09-22 10:36     ` Lukas Czerner
@ 2011-09-22 10:43       ` Zdenek Kabelac
  2011-09-22 10:52         ` Lukas Czerner
  0 siblings, 1 reply; 48+ messages in thread
From: Zdenek Kabelac @ 2011-09-22 10:43 UTC (permalink / raw)
  To: Lukas Czerner; +Cc: linux-lvm

Dne 22.9.2011 12:36, Lukas Czerner napsal(a):
> On Thu, 22 Sep 2011, Zdenek Kabelac wrote:
> 
>> Dne 21.9.2011 18:45, Lukas Czerner napsal(a):
>>> Remove command allows to remove unused devices from the pool (volume
>>> group).
>>>
>>> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
>>> ---
>>>  scripts/fsadm.sh |   82 ++++++++++++++++++++++++++++++++++++++++-------------
>>>  1 files changed, 62 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
>>> index 6617de0..4a4f625 100755
>>> --- a/scripts/fsadm.sh
>>> +++ b/scripts/fsadm.sh
>>> @@ -823,11 +823,12 @@ list_filesystems() {
>>>  	IFS=$NL
>>>  	local c=0
>>>  	for line in $(LANG=C $LVM lvs -o lv_path,lv_size,segtype --noheadings --separator ' ' --nosuffix --units k 2> /dev/null); do
>>> -		c=$((c+1))
>>>  		line=$(echo $line | sed -e 's/^ *\//\//')
>>>  		volume=$(echo $line | cut -d' ' -f1)
>>> -		volumes[$c]=$volume
>>> -		segtype[$c]=$(echo $line | cut -d' ' -f3)
>>> +		[ "$volume" == "$last_volume" ] && continue
>>> +		c=$((c+1))
>>> +		local volumes[$c]=$volume
>>> +		local segtype[$c]=$(echo $line | cut -d' ' -f3)
>>>  		detect_fs $volume
>>>  		detect_mounted
>>>  		detect_fs_size
>>
>> Could you please update/cleanup the patch set - so it doesn't rework same code
>> multiple times over and over ?
>> (It's a waste of time to review new code, which gets replaced several times
>> during the whole patch set)
> 
> Unfortunately that is what I have mentioned in the patch 0. The main
> problem is that the fsadm is one file script and while adding other
> functionality I was building helpers and yes also changing other parts
> of the code.
> 
> It was not possible for me make one feature, be done with it and move to
> the another feature. Some of them actually supports each other and not
> having to rebase with every change made my work significantly easier. So
> at this point I am not going to rework the patches, since it would take
> more time that actually write that code :).
> 
> But, since it is a single file script, and I changed the whole thing
> significantly (there is not much left from the original code) I can
> create one big patch covering all the new features, so it will be easier
> for you to review it. Will that be acceptable for you ?


Yes - I could imagine in certain cases it might be useful to squash several
patches together to improve readability here.

But before you change some code - please look at how it has looked and do not
put problem which were already resolved.

Few notes for script writing:  use  "$VAR"  (since path may contain spaces)
Preferred way for using shell test is  'test '  instead of []


Zdenek

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [linux-lvm] [PATCH 11/35] fsadm: Add "remove" command
  2011-09-22 10:43       ` Zdenek Kabelac
@ 2011-09-22 10:52         ` Lukas Czerner
  2011-09-22 11:41           ` Zdenek Kabelac
  0 siblings, 1 reply; 48+ messages in thread
From: Lukas Czerner @ 2011-09-22 10:52 UTC (permalink / raw)
  To: Zdenek Kabelac; +Cc: Lukas Czerner, linux-lvm

On Thu, 22 Sep 2011, Zdenek Kabelac wrote:

> Dne 22.9.2011 12:36, Lukas Czerner napsal(a):
> > On Thu, 22 Sep 2011, Zdenek Kabelac wrote:
> > 
> >> Dne 21.9.2011 18:45, Lukas Czerner napsal(a):
> >>> Remove command allows to remove unused devices from the pool (volume
> >>> group).
> >>>
> >>> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> >>> ---
> >>>  scripts/fsadm.sh |   82 ++++++++++++++++++++++++++++++++++++++++-------------
> >>>  1 files changed, 62 insertions(+), 20 deletions(-)
> >>>
> >>> diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
> >>> index 6617de0..4a4f625 100755
> >>> --- a/scripts/fsadm.sh
> >>> +++ b/scripts/fsadm.sh
> >>> @@ -823,11 +823,12 @@ list_filesystems() {
> >>>  	IFS=$NL
> >>>  	local c=0
> >>>  	for line in $(LANG=C $LVM lvs -o lv_path,lv_size,segtype --noheadings --separator ' ' --nosuffix --units k 2> /dev/null); do
> >>> -		c=$((c+1))
> >>>  		line=$(echo $line | sed -e 's/^ *\//\//')
> >>>  		volume=$(echo $line | cut -d' ' -f1)
> >>> -		volumes[$c]=$volume
> >>> -		segtype[$c]=$(echo $line | cut -d' ' -f3)
> >>> +		[ "$volume" == "$last_volume" ] && continue
> >>> +		c=$((c+1))
> >>> +		local volumes[$c]=$volume
> >>> +		local segtype[$c]=$(echo $line | cut -d' ' -f3)
> >>>  		detect_fs $volume
> >>>  		detect_mounted
> >>>  		detect_fs_size
> >>
> >> Could you please update/cleanup the patch set - so it doesn't rework same code
> >> multiple times over and over ?
> >> (It's a waste of time to review new code, which gets replaced several times
> >> during the whole patch set)
> > 
> > Unfortunately that is what I have mentioned in the patch 0. The main
> > problem is that the fsadm is one file script and while adding other
> > functionality I was building helpers and yes also changing other parts
> > of the code.
> > 
> > It was not possible for me make one feature, be done with it and move to
> > the another feature. Some of them actually supports each other and not
> > having to rebase with every change made my work significantly easier. So
> > at this point I am not going to rework the patches, since it would take
> > more time that actually write that code :).
> > 
> > But, since it is a single file script, and I changed the whole thing
> > significantly (there is not much left from the original code) I can
> > create one big patch covering all the new features, so it will be easier
> > for you to review it. Will that be acceptable for you ?
> 
> 
> Yes - I could imagine in certain cases it might be useful to squash several
> patches together to improve readability here.
> 
> But before you change some code - please look at how it has looked and do not
> put problem which were already resolved.

I am sorry, but I am not sure what you have in mind. Cold you be more
specific ?

> 
> Few notes for script writing:  use  "$VAR"  (since path may contain spaces)
> Preferred way for using shell test is  'test '  instead of []

Yes, as Stephane already said, using "$var" is really what we need to
use. I'll change it. But I am really not sure why would using 'test' be
preferred over [] ? doing things like [ -b "$dev" ] or similar seems
just better readable for me. But maybe I am missing something.

> 
> 
> Zdenek
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [linux-lvm] [PATCH 01/35] fsadm: Add "create" command
  2011-09-22  9:28     ` Lukas Czerner
  2011-09-22 10:01       ` Zdenek Kabelac
@ 2011-09-22 10:56       ` Stephane Chazelas
  1 sibling, 0 replies; 48+ messages in thread
From: Stephane Chazelas @ 2011-09-22 10:56 UTC (permalink / raw)
  To: Lukas Czerner
  Cc: zkabelac, rwheeler, dchinner, LVM general discussion and development

2011-09-22 11:28:36 +0200, Lukas Czerner:
[...]
> > > +	if [ $? -ne 0 ]; then
> > > +		error "FAILED. Exitting!"
> > > +	fi
> > 
> > "$@" || error ...
> 
> I guess it is just a matter of taste.
> 
> > 
> > or
> > 
> > if ! "$@"; then
> >   error ...
> > fi
> 
> That is really not very readable.
[...]

That is how shell works. The syntax is

if
  list-of-commands
then
  other-list-of-commands
else
  yet-another-list-of-commands
fi

I find [ "$?" -ne 0 ] above very confusion and illegible myself.

Why would you run another command that fails if the last one
succeeded? 

> > > +}
> > > +
> > > +is_natural() {
> > > +	test "$1" -ge 0 &> /dev/null && return 1
> > 
> > 
> > &> is a bashism.
> 
> Probably (#!/bin/bash) :)

But it is called "fsadm.sh", that's misleading.

[...]
> > Again, I think that should be either:
> > 
> > set --
> > [ -n "$YES" ] &&
> >   set -- -F
> > dry "mkfs.$fstyp" "$@" -b "$(($bsize*1024))" -E stride="${stride},stripe-width=${stripewidth}" "$device"
> > 
> > or:
> > 
> > force=
> > [ -n "$YES" ] &&
> >   force=-F
> > 
> > eval 'dry "mkfs.$fstyp" '"$force"' -b "$(($bsize*1024))" -E stride="${stride},stripe-width=${stripewidth}" "$device"'
> 
> Ok, I do understand the that I should rather use 'eval', but I do not
> understand why you're trying to get rid of the 'if', it is a bit longer,
> so what ? But is is also more obvious.

"if" is fine, it was just to save me some typing.

> > 
> > [...]
> > > +	is_natural $NEWSIZE
> > > +	[ $? -ne 1 ] && error "$NEWSIZE is not valid number for file system size"
> > 
> > With a fixed is_natural,
> > 
> > is_natural "$NEWSIZE" || error ...
> > 
> > [...]
> > > +	for i in $@; do
> > 
> > for i do
> 
> I am not sure what is wrong with that, it is more obvious and it works
> just fine.

for i in "$@"; do

would have been fine. Again, I can't think of any circumstance
where leaving $@ unquoted would make sense.

-- 
Stephane

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [linux-lvm] [PATCH 11/35] fsadm: Add "remove" command
  2011-09-22 10:52         ` Lukas Czerner
@ 2011-09-22 11:41           ` Zdenek Kabelac
  0 siblings, 0 replies; 48+ messages in thread
From: Zdenek Kabelac @ 2011-09-22 11:41 UTC (permalink / raw)
  To: Lukas Czerner; +Cc: linux-lvm

Dne 22.9.2011 12:52, Lukas Czerner napsal(a):
> On Thu, 22 Sep 2011, Zdenek Kabelac wrote:
> 
>> Dne 22.9.2011 12:36, Lukas Czerner napsal(a):
>>> On Thu, 22 Sep 2011, Zdenek Kabelac wrote:
>>>
>>>> Dne 21.9.2011 18:45, Lukas Czerner napsal(a):
>>>>> Remove command allows to remove unused devices from the pool (volume
>>>>> group).
>>>>>
>>>>> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
>>>>> ---
>>>>>  scripts/fsadm.sh |   82 ++++++++++++++++++++++++++++++++++++++++-------------
>>>>>  1 files changed, 62 insertions(+), 20 deletions(-)
>>>>>
>>>>> diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
>>>>> index 6617de0..4a4f625 100755
>>>>> --- a/scripts/fsadm.sh
>>>>> +++ b/scripts/fsadm.sh
>>>>> @@ -823,11 +823,12 @@ list_filesystems() {
>>>>>  	IFS=$NL
>>>>>  	local c=0
>>>>>  	for line in $(LANG=C $LVM lvs -o lv_path,lv_size,segtype --noheadings --separator ' ' --nosuffix --units k 2> /dev/null); do
>>>>> -		c=$((c+1))
>>>>>  		line=$(echo $line | sed -e 's/^ *\//\//')
>>>>>  		volume=$(echo $line | cut -d' ' -f1)
>>>>> -		volumes[$c]=$volume
>>>>> -		segtype[$c]=$(echo $line | cut -d' ' -f3)
>>>>> +		[ "$volume" == "$last_volume" ] && continue
>>>>> +		c=$((c+1))
>>>>> +		local volumes[$c]=$volume
>>>>> +		local segtype[$c]=$(echo $line | cut -d' ' -f3)
>>>>>  		detect_fs $volume
>>>>>  		detect_mounted
>>>>>  		detect_fs_size
>>>>
>>>> Could you please update/cleanup the patch set - so it doesn't rework same code
>>>> multiple times over and over ?
>>>> (It's a waste of time to review new code, which gets replaced several times
>>>> during the whole patch set)
>>>
>>> Unfortunately that is what I have mentioned in the patch 0. The main
>>> problem is that the fsadm is one file script and while adding other
>>> functionality I was building helpers and yes also changing other parts
>>> of the code.
>>>
>>> It was not possible for me make one feature, be done with it and move to
>>> the another feature. Some of them actually supports each other and not
>>> having to rebase with every change made my work significantly easier. So
>>> at this point I am not going to rework the patches, since it would take
>>> more time that actually write that code :).
>>>
>>> But, since it is a single file script, and I changed the whole thing
>>> significantly (there is not much left from the original code) I can
>>> create one big patch covering all the new features, so it will be easier
>>> for you to review it. Will that be acceptable for you ?
>>
>>
>> Yes - I could imagine in certain cases it might be useful to squash several
>> patches together to improve readability here.
>>
>> But before you change some code - please look at how it has looked and do not
>> put problem which were already resolved.
> 
> I am sorry, but I am not sure what you have in mind. Cold you be more
> specific ?

i.e. those  "$var" changes which were already fixed upstream.


> 
>>
>> Few notes for script writing:  use  "$VAR"  (since path may contain spaces)
>> Preferred way for using shell test is  'test '  instead of []
> 
> Yes, as Stephane already said, using "$var" is really what we need to
> use. I'll change it. But I am really not sure why would using 'test' be
> preferred over [] ? doing things like [ -b "$dev" ] or similar seems
> just better readable for me. But maybe I am missing something.

Well it's the preferred shell coding style used in lvm for lines like:

test 1 -eq 0 &&

instead of

[ 1 -eq 0 ] &&

And while it's not the case of bash - in some older non bash shells it's been
more often 'test' was shell builtin, and '[' is externally executed program.
(Yet, that's why bashism should be eliminated - some distros even
intentionally replace bash with something faster & lighter).
AFAIK in all cases we could probably avoid usage of bash specific extensions.
Have you tested your script on some older distros? (i.e. upstream fsadm.sh has
couple hacks around commands where its syntax changed over the time)

Zdenek

^ permalink raw reply	[flat|nested] 48+ messages in thread

end of thread, other threads:[~2011-09-22 11:41 UTC | newest]

Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-21 16:45 [linux-lvm] [RFC][PATCH 00/35] fsadm update Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 01/35] fsadm: Add "create" command Lukas Czerner
2011-09-21 19:00   ` Stephane Chazelas
2011-09-22  9:28     ` Lukas Czerner
2011-09-22 10:01       ` Zdenek Kabelac
2011-09-22 10:27         ` Lukas Czerner
2011-09-22 10:56       ` Stephane Chazelas
2011-09-21 16:45 ` [linux-lvm] [PATCH 02/35] fsadm: Add "destroy" command Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 03/35] fsadm: Add "list" command Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 04/35] fsadm: Make "create" command more vg aware Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 05/35] fsadm: Teach "destroy" command to take more arguments Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 06/35] fsadm: Simple cleanup and comment update Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 07/35] fsadm: Create "add" command Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 08/35] fsadm: Update "list" command for better alignment Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 09/35] fsadm: Specify number of stripes when no device is given Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 10/35] fsadm: Print type of the volume in filesystem listing Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 11/35] fsadm: Add "remove" command Lukas Czerner
2011-09-22 10:06   ` Zdenek Kabelac
2011-09-22 10:36     ` Lukas Czerner
2011-09-22 10:43       ` Zdenek Kabelac
2011-09-22 10:52         ` Lukas Czerner
2011-09-22 11:41           ` Zdenek Kabelac
2011-09-21 16:45 ` [linux-lvm] [PATCH 12/35] fsadm: Try to avoid calling LVM in the loops Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 13/35] fsadm: Merge "destroy" and "remove" into one command Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 14/35] fsadm: Allow to remove all volume groups Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 15/35] fsadm: Make all internal math in kilobytes Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 16/35] fsadm: Use warn for warnings in list command Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 17/35] fsadm: Handle resize if there is no file system Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 18/35] fsadm: Fsck extN before resize only if it is not mounted Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 19/35] fsadm: Align numbers to the decimal point Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 20/35] fsadm: Add simple configuration file Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 21/35] fsadm: Use DEFAULT_POOL when creating volume group Lukas Czerner
2011-09-22 10:09   ` Zdenek Kabelac
2011-09-21 16:45 ` [linux-lvm] [PATCH 22/35] fsadm: Add LVOL_PREFIX configuration option Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 23/35] fsadm: Only use readlink if link is provided Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 24/35] fsadm: Remove unnecessary modification of PATH variable Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 25/35] fsadm: Allow to specify size without "size=" prefix in "resize" Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 26/35] fsadm: Allow to specify lv in vg/lv format Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 27/35] fsadm: error out when no size is provided in resize Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 28/35] fsadm: Umount ext2 file system prior resize Lukas Czerner
2011-09-22 10:28   ` Zdenek Kabelac
2011-09-21 16:45 ` [linux-lvm] [PATCH 29/35] lvresize: Specify --resize-fs-only when going to use fsadm resize Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 30/35] test: add helper to compute aligned lv size Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 31/35] fsadm: Add help for new commands and update man page Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 32/35] fsadm: Update authorship of the fsadm Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 33/35] test: Add test for fsadm add command Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 34/35] test: Add test for fsadm create command Lukas Czerner
2011-09-21 16:45 ` [linux-lvm] [PATCH 35/35] test: Add test for fsadm resize command Lukas Czerner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.