cleanup in blockdevice size stuff. correct usage of units etc. inspired by FS#12949 - "hdparm -I" fails in VMware 0001-Using-fdisk-instead-of-hdparm-to-get-disc-capacity.patch

3 files changed, 33 insertions, 23 deletions

diff --git a/src/core/libs/lib-blockdevices-filesystems.sh b/src/core/libs/lib-blockdevices-filesystems.sh
index 1cbfcbe..28fb51c 100644
--- a/src/core/libs/lib-blockdevices-filesystems.sh
+++ b/src/core/libs/lib-blockdevices-filesystems.sh
@@ -736,24 +736,31 @@ get_filesystem_program ()
 
 
 # $1 blockdevice
-# $2 standard SI for 1000*n, IEC for 1024*n (optional. defaults to SI)
-# --> Note that if you do SI on a partition, you get the size of the entire disk, so for now you need IEC for single partitions
-# output will be in $BLOCKDEVICE_SIZE in MB/MiB
+# $2 unit: B, KiB, kB, MiB, MB, GiB or GB.  defaults to B (we follow IEEE 1541-2002 )
+# output will be in $BLOCKDEVICE_SIZE
 # WARNING: hdparm works - by design - only for ide/sata. not scsi et al
-# TODO: clean up all disk size related stuff.  see http://bugs.archlinux.org/task/12949
 get_blockdevice_size ()
 {
 	[ -b "$1" ] || die_error "get_blockdevice_size needs a blockdevice as \$1 ($1 given)"
-	standard=${2:-SI}
-
-	if [ "$standard" = SI ]
-	then
-		BLOCKDEVICE_SIZE=$(hdparm -I $1 | grep -F '1000*1000' | sed "s/^.*:[ \t]*\([0-9]*\) MBytes.*$/\1/")
-	elif [ "$standard" = IEC ]
+	unit=${2:-B}
+	allowed_units=(B KiB kB MiB MB GiB GB)
+	if ! is_in $unit "${allowed_units[@]}"
 	then
-		#NOTE: unreliable method: on some interwebs they say 1 block = 512B, on other internets they say 1 block = 1kiB.  1kiB seems to work for me.  don't sue me if it doesn't for you
-		#blocks=`fdisk -s $1` || show_warning "Fdisk problem" "Something failed when trying to do fdisk -s $1"
-		#BLOCKDEVICE_SIZE=$(($blocks/1024))
-		BLOCKDEVICE_SIZE=$((`fdisk -l $1 | sed -n '2p' | cut -d' ' -f5`/1024))
+		die_error "Unrecognized unit $unit!"
 	fi
+
+	# NOTES about older, deprecated methods:
+	# - BLOCKDEVICE_SIZE=$(hdparm -I $1 | grep -F '1000*1000' | sed "s/^.*:[ \t]*\([0-9]*\) MBytes.*$/\1/") # if you do this on a partition, you get the size of the entire disk ! + hdparm only supports sata and ide. not scsi.
+	# - unreliable method: on some interwebs they say 1 block = 512B, on other internets they say 1 block = 1kiB.  1kiB seemed to work for me.
+	# blocks=`fdisk -s $1` || show_warning "Fdisk problem" "Something failed when trying to do fdisk -s $1"
+	# BLOCKDEVICE_SIZE=$(($blocks/1024))
+	#
+	bytes=$((`fdisk -l $1 2>/dev/null | sed -n '2p' | cut -d' ' -f5`))
+	[ $unit = B ]   && BLOCKDEVICE_SIZE=$bytes
+	[ $unit = KiB ] && BLOCKDEVICE_SIZE=$(($bytes/2*10))  # /1024
+	[ $unit = kB ]  && BLOCKDEVICE_SIZE=$(($bytes/10**3)) # /1000
+	[ $unit = MiB ] && BLOCKDEVICE_SIZE=$(($bytes/2*20))  # ...
+	[ $unit = MB ]  && BLOCKDEVICE_SIZE=$(($bytes/10**6))
+	[ $unit = GiB ] && BLOCKDEVICE_SIZE=$(($bytes/2*30))
+	[ $unit = GB ]  && BLOCKDEVICE_SIZE=$(($bytes/10**9))
 }
diff --git a/src/core/libs/lib-ui-interactive.sh b/src/core/libs/lib-ui-interactive.sh
index 8fdc5dc..8076a0e 100644
--- a/src/core/libs/lib-ui-interactive.sh
+++ b/src/core/libs/lib-ui-interactive.sh
@@ -113,7 +113,7 @@ interactive_autoprepare()
 
 	DISC=${DISC// /} # strip all whitespace.  we need this for some reason.TODO: find out why
 
-	get_blockdevice_size $DISC SI
+	get_blockdevice_size $DISC MB
 	FSOPTS=
 	which `get_filesystem_program ext2`     &>/dev/null && FSOPTS="$FSOPTS ext2 Ext2"
 	which `get_filesystem_program ext3`     &>/dev/null && FSOPTS="$FSOPTS ext3 Ext3"
@@ -449,9 +449,9 @@ interactive_filesystems() {
 				fs_display=${fs//;target/}
 				[ "$label" != no_label ] && label_display="($label)"
 				[ "$label"  = no_label ] && label_display=
-				if [ -b "${part/+/}" ] && get_blockdevice_size ${part/+/} IEC # test -b <-- exit's 0, test -b '' exits >0.
+				if [ -b "${part/+/}" ] && get_blockdevice_size ${part/+/} MB # test -b <-- exit's 0, test -b '' exits >0.
 				then
-					infostring="${type},${BLOCKDEVICE_SIZE}MB${label_display}->$fs_display" # add size in MB for existing blockdevices (eg not for mapper devices that are not yet created yet) #TODO: ${BLOCKDEVICE_SIZE} is empty sometimes?
+					infostring="${type},${BLOCKDEVICE_SIZE}MB${label_display}->$fs_display" # add size in MB for existing blockdevices (eg not for mapper devices that are not yet created yet)
 				else
 					infostring="${type}${label_display}->$fs_display"
 				fi
diff --git a/src/core/libs/lib-ui.sh b/src/core/libs/lib-ui.sh
index 0664817..3a40eb2 100644
--- a/src/core/libs/lib-ui.sh
+++ b/src/core/libs/lib-ui.sh
@@ -129,14 +129,17 @@ printk()
 
 
 # TODO: pass disks as argument to decouple backend logic
-# Get a list of available disks for use in the "Available disks" dialogs. This
-# will print the disks as follows, getting size info from hdparm:
-#   /dev/sda: 640133 MBytes (640 GB)
-#   /dev/sdb: 640135 MBytes (640 GB)
+# Get a list of available disks for use in the "Available disks" dialogs.
+# Something like:
+#   /dev/sda: 640133 MB (640 GB)
+#   /dev/sdb: 640135 MB (640 GB)
 _getavaildisks()
 {
-    # NOTE: to test as non-root, stick in a 'sudo' before the hdparm call
-    for i in $(finddisks); do echo -n "$i: "; hdparm -I $i | grep -F '1000*1000' | sed "s/.*1000:[ \t]*\(.*\)/\1/"; echo "\n"; done
+    for i in $(finddisks)
+    do
+	get_blockdevice_size $i MB
+	echo "$i: $BLOCKDEVICE_SIZE MB ($(($BLOCKDEVICE_SIZE/1000)) GB)\n"
+    done
 }