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 [email protected] 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 > }
