20 Feb
2011
20 Feb
'11
6:37 p.m.
From what I can tell in my mail client, it seems like this will be a nice cleanup of some very old code :)
thanks.
On Sun, 20 Feb 2011 12:30:10 -0500
pyther@pyther.net wrote:
> find_usable_devs will looks for both partitioned and non-partitioned
> block devices. The blockdevice will be excluded from the list if it is
> an extended partition (not logical) or part of a LVM or RAID
> configuration.
typos and not using current tense in commit message.
"look", not looks.
for consistency, i would not try to shorten the function names.
i.e. find_usable_bdevs -> find_usable_blockdevices.
code gets read 95% time, written 5% of the time ;)
it's also not very relevant to know what it looks for. what is relevant is what the function returns.
and it returns partitions themselves as well.
> ---
> src/core/libs/lib-blockdevices-filesystems.sh | 150 +++++++++++++------------
> src/core/libs/lib-ui-interactive.sh | 4 +-
> 2 files changed, 80 insertions(+), 74 deletions(-)
>
> diff --git a/src/core/libs/lib-blockdevices-filesystems.sh b/src/core/libs/lib-blockdevices-filesystems.sh
> index ac749a3..1b10927 100644
> --- a/src/core/libs/lib-blockdevices-filesystems.sh
> +++ b/src/core/libs/lib-blockdevices-filesystems.sh
> @@ -206,99 +206,105 @@ getlabel() {
> # find partitionable blockdevices
> # $1 extra things to echo for each device (optional) (backslash escapes will get interpreted)
> finddisks() {
> - workdir="$PWD"
> - if cd /sys/block 2>/dev/null
> - then
> - # ide devices
> - for dev in $(ls | egrep '^hd')
> - do
> - if [ "$(cat $dev/device/media)" = "disk" ]
> - then
> - echo -ne "/dev/$dev $1"
> - fi
> - done
> - #scsi/sata devices, and virtio blockdevices (/dev/vd*)
> - for dev in $(ls | egrep '^[sv]d')
> - do
> - # TODO: what is the significance of 5? ASKDEV
> - if [ "$(cat $dev/device/type)" != "5" ]
> - then
> - echo -ne "/dev/$dev $1"
> + shopt -s nullglob
> +
> + # Block Devices
> + if [[ -d /sys/block ]]; then
> + for dev in /sys/block/*; do
since nullglob is set, I think you can remove all if [[ -d $dir ]] checks. the net result is the same:
$ shopt -s nullglob; for foo in /dev/doesnotexist/*; do echo $foo; done
$
> + if [[ -f $dev/device/type ]]; then
> + local type
> + read type < /sys/block/${dev##*/}/device/type
> + read size < /sys/block/${dev##*/}/size
> + # Type 5 is a ROM Device
maybe mention what a ROM device is
> + if [[ $type != 5 ]] && (( $size > 0 )); then
maybe a comment that size 0 can happen for empty card readers
> + source "$dev/uevent"
> + echo -ne "/dev/$DEVNAME $1"
> + unset DEVNAME
> + fi
> fi
> done
> fi
> +
> # cciss controllers
> - if cd /dev/cciss 2>/dev/null
> - then
> - for dev in $(ls | egrep -v 'p')
> - do
> - echo -ne "/dev/cciss/$dev $1"
> + if [[ -d /dev/cciss ]]; then
> + for dev in /dev/cciss/*; do
> + if [[ $dev != *[[:digit:]]p[[:digit:]]* ]]; then
> + echo "$dev $1"
> + fi
> done
> fi
> - # Smart 2 controllers
> - if cd /dev/ida 2>/dev/null
> - then
> - for dev in $(ls | egrep -v 'p')
> - do
> - echo -ne "/dev/ida/$dev $1"
> + # Smart 2 Controller
> + if [[ -d /dev/ida ]]; then
> + for dev in /dev/ida/*; do
> + if [[ $dev != *[[:digit:]]p[[:digit:]]* ]]; then
> + echo "$dev $1"
> + fi
> done
> fi
> - cd "$workdir"
> + shopt -u nullglob
> }
>
> +
> # find block devices, both partionable or not (i.e. partitions themselves)
> # $1 extra things to echo for each partition (optional) (backslash escapes will get interpreted)
> -findblockdevices() {
> - workdir="$PWD"
> - for devpath in $(finddisks)
> - do
> - disk=$(basename $devpath)
> - echo -ne "/dev/$disk $1"
> - cd /sys/block/$disk
> - shopt -s nullglob
> - for part in $disk*
> - do
> - # check if not already assembled to a raid device. TODO: what is the significance of the 5? ASKDEV
> - if [ -n "$part" ] && ! grep -q $part /proc/mdstat 2>/dev/null && ! fstype 2>/dev/null </dev/$part | grep -q lvm2 && ! sfdisk -c /dev/$disk $(echo $part | sed -e "s#$disk##g") 2>/dev/null | grep -q '5'
> - then
> - if [ -d $part ]
> - then
> - echo -ne "/dev/$part $1"
> - fi
> +# find usable block devices
you should merge this comment in the comments above.
> +find_usable_bdevs() {
> + shopt -s nullglob
> +
> + local parts
> +
> + # Cycle through all non-partitional block devices (sda, sdb, etc...)
what's non-partitional? maybe "non-partition" or "block devices which are not partitions"
> + # Look for partitions and include them only if they are not part of a
> + # RAID or LVM. Also, exclude the extended partition
- what happens when a non-partition block device is part of RAID/LVM ?
- extended partitionS
- explain *why* we do this: although we strongly encourage users to setup LVM in AIF, we also want to make the life a bit easier for those who set up LVM/softraid before running AIF
> + for devpath in $(finddisks); do
> + hidebldev=
how about just "hide"? or "hide_blockdevice" but we already know we're in the context of blockdevices.
> + unset parts
> +
> + # Glob allows for following matches:
> + # /dev/sda -> /dev/sda1
> + # /dev/cciss/c0d1 -> /dev/cciss/c0d1p1
> + for dev in ${devpath}*[[:digit:]]*; do
> + local disk="${dev%%[[:digit:]]*}"
> + local partnum="${dev##*[[:alpha:]]}"
> +
> + # Don't display extened partition (not logical parts)
extended
> + sfdisk -c "$disk" "$partnum" 2>/dev/null | grep -q '5' && continue;
how about:
[ "$(sfdisk -c "$disk" "$partnum" 2>/dev/null)" = 5 ] && continue;
?
> +
> + # Don't list parts that are part of RAID or LVM volumes
> + if grep -qsw "${dev##*/}" /proc/mdstat || { pvscan -s 2>/dev/null | grep -q "$dev"; }; then
> + hidebldev="True"
> + else
> + parts+=("$dev")
> fi
> done
> - shopt -u nullglob
> +
> + # If we have no partitions then print the root of the block device
> + # Otherwise print the partitions
> + if [[ -z $hidebldev ]] && (( ${#parts[@]} < 1 )); then
> + echo -ne "$devpath $1"
> + elif (( ${#parts[@]} )); then
> + for part in ${parts[@]}; do
> + echo -ne "$part $1"
> + done
> + fi
> done
> +
> # mapped devices
> - for devpath in $(ls /dev/mapper 2>/dev/null | grep -v control)
> - do
> - echo -ne "/dev/mapper/$devpath $1"
> + for devpath in /dev/mapper/*; do
> + # Exclude /control directory and other non-block files
non-block files such as?
> + if [[ -b $devpath ]]; then
> + echo -ne "$devpath $1"
> + fi
> done
> +
> # raid md devices
> - for devpath in $(ls -d /dev/md* | grep '[0-9]' 2>/dev/null)
> - do
> - if grep -qw $(echo $devpath /proc/mdstat | sed -e 's|/dev/||g')
> - then
> + for devpath in /dev/md[0-9]*; do
> + if grep -qw "${devpath//\/dev\//}" /proc/mdstat; then
> echo -ne "$devpath $1"
> fi
> done
> - # cciss controllers
> - if cd /dev/cciss 2>/dev/null
> - then
> - for dev in $(ls | egrep 'p')
> - do
> - echo -ne "/dev/cciss/$dev $1"
> - done
> - fi
> - # Smart 2 controllers
> - if cd /dev/ida 2>/dev/null
> - then
> - for dev in $(ls | egrep 'p')
> - do
> - echo -ne "/dev/ida/$dev $1"
> - done
> - fi
> - cd "$workdir"
> +
> + shopt -u nullglob
> }