[arch-releng] [PATCH] Reworte finddisks and findblockdevices. Renamed findblockdevices to find_usable_devs
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. --- 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 + 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 + if [[ $type != 5 ]] && (( $size > 0 )); then + 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 +find_usable_bdevs() { + shopt -s nullglob + + local parts + + # Cycle through all non-partitional block devices (sda, sdb, etc...) + # Look for partitions and include them only if they are not part of a + # RAID or LVM. Also, exclude the extended partition + for devpath in $(finddisks); do + hidebldev= + 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) + sfdisk -c "$disk" "$partnum" 2>/dev/null | grep -q '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 + 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 } diff --git a/src/core/libs/lib-ui-interactive.sh b/src/core/libs/lib-ui-interactive.sh index b244a6e..bf19ef2 100644 --- a/src/core/libs/lib-ui-interactive.sh +++ b/src/core/libs/lib-ui-interactive.sh @@ -557,7 +557,7 @@ interactive_filesystems() { if [ ! -f $TMP_BLOCKDEVICES ] || ! ask_yesno "Previous blockdevice definitions found:\n`cat $TMP_BLOCKDEVICES`\n\ Use these as a starting point? Make sure your disk(s) are partitioned correctly so your definitions can be applied. Pick 'no' when in doubt to start from scratch" no then - findblockdevices 'raw no_label no_fs\n' > $TMP_BLOCKDEVICES + find_usable_bdevs 'raw no_label no_fs\n' > $TMP_BLOCKDEVICES fi [ -z "$PART_ACCESS" ] && PART_ACCESS=dev @@ -885,7 +885,7 @@ interactive_grub() { # Create and edit the grub menu.lst interactive_grub_menulst - DEVS="$(findblockdevices '_ ')" + DEVS="$(find_usable_bdevs '_ ')" if [ "$DEVS" = " " ]; then notify "No hard drives were found" return 1 -- 1.7.4.1
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
> }
On 02/20/2011 01:37 PM, Dieter Plaetinck wrote:
From what I can tell in my mail client, it seems like this will be a nice cleanup of some very old code :) thanks.
Here is a diff of the changes after implementing your suggestions. The most notable change is that I now check to see if the root block device (sda, sdb) is part of a raid or lvm setup. I attempted to fix the commit message: Reworte finddisks and findblockdevices. Renamed findblockdevices to find_usable_devs find_usable_devs returns a list of partitioned and non-partitioned block devices. These devices include SCSI/Sata, CCISS, IDA, LVM, SOFT RAID devices. Devices in LVM and SOFT RAID configurations are excluded from this list. diff --git a/src/core/libs/lib-blockdevices-filesystems.sh b/src/core/libs/lib-blockdevices-filesystems.sh index 1b10927..d9c28ff 100644 --- a/src/core/libs/lib-blockdevices-filesystems.sh +++ b/src/core/libs/lib-blockdevices-filesystems.sh @@ -209,53 +209,49 @@ finddisks() { shopt -s nullglob # Block Devices - if [[ -d /sys/block ]]; then - for dev in /sys/block/*; do - 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 - if [[ $type != 5 ]] && (( $size > 0 )); then - source "$dev/uevent" - echo -ne "/dev/$DEVNAME $1" - unset DEVNAME - fi + for dev in /sys/block/*; do + if [[ -f $dev/device/type ]]; then + local type + read type < /sys/block/${dev##*/}/device/type + # Block Devices with size =< 0 may be an empty card reader + read size < /sys/block/${dev##*/}/size + # Type 5 is a ROM Device - Optical Drives + if [[ $type != 5 ]] && (( $size > 0 )); then + source "$dev/uevent" + echo -ne "/dev/$DEVNAME $1" + unset DEVNAME fi - done - fi + fi + done # cciss controllers - if [[ -d /dev/cciss ]]; then - for dev in /dev/cciss/*; do - if [[ $dev != *[[:digit:]]p[[:digit:]]* ]]; then - echo "$dev $1" - fi - done - fi + for dev in /dev/cciss/*; do + if [[ $dev != *[[:digit:]]p[[:digit:]]* ]]; then + echo "$dev $1" + fi + done + # 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 + for dev in /dev/ida/*; do + if [[ $dev != *[[:digit:]]p[[:digit:]]* ]]; then + echo "$dev $1" + fi + done + shopt -u nullglob } -# find block devices, both partionable or not (i.e. partitions themselves) +# find usable devices, both partionable or not (i.e. partitions themselves) # $1 extra things to echo for each partition (optional) (backslash escapes will get interpreted) -# find usable block devices -find_usable_bdevs() { +find_usable_blockdevices() { shopt -s nullglob local parts - # Cycle through all non-partitional block devices (sda, sdb, etc...) + # Cycle through all root block devices (sda, sdb, etc...) # Look for partitions and include them only if they are not part of a - # RAID or LVM. Also, exclude the extended partition + # RAID or LVM configuration. Also, exclude extended partitions for devpath in $(finddisks); do hidebldev= unset parts @@ -268,7 +264,7 @@ find_usable_bdevs() { local partnum="${dev##*[[:alpha:]]}" # Don't display extened partition (not logical parts) - sfdisk -c "$disk" "$partnum" 2>/dev/null | grep -q '5' && continue; + [ "$(sfdisk -c "$disk" "$partnum" 2>/dev/null)" = 5 ] && continue; # Don't list parts that are part of RAID or LVM volumes + # Although we strongly encourage users to setup LVM in AIF, we want to make + # life a bit easier for those who have setup LVM/softraid before running AIF if grep -qsw "${dev##*/}" /proc/mdstat || { pvscan -s 2>/dev/null | grep -q "$dev"; }; then @@ -278,10 +274,13 @@ find_usable_bdevs() { fi done - # 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" + # If hidebldev is not set and we have no partitions then + # Echo root block device if root block device is not part of a LVM or RAID configuration + # Otherwise echo the partitions + if [[ -z $hidebldev ]] && (( ! ${#parts[@]} )); then + if ! grep -qsw "${devpath##*/}" /proc/mdstat && ! pvscan -s 2>/dev/null | grep -q "$devpath"; then + echo -ne "$devpath $1" + fi elif (( ${#parts[@]} )); then for part in ${parts[@]}; do echo -ne "$part $1" diff --git a/src/core/libs/lib-ui-interactive.sh b/src/core/libs/lib-ui-interactive.sh index bf19ef2..64be570 100644 --- a/src/core/libs/lib-ui-interactive.sh +++ b/src/core/libs/lib-ui-interactive.sh @@ -557,7 +557,7 @@ interactive_filesystems() { if [ ! -f $TMP_BLOCKDEVICES ] || ! ask_yesno "Previous blockdevice definitions found:\n`cat $TMP_BLOCKDEVICES`\n\ Use these as a starting point? Make sure your disk(s) are partitioned correctly so your definitions can be applied. Pick 'no' when in doubt to start from scratch" no then - find_usable_bdevs 'raw no_label no_fs\n' > $TMP_BLOCKDEVICES + find_usable_blockdevices 'raw no_label no_fs\n' > $TMP_BLOCKDEVICES fi [ -z "$PART_ACCESS" ] && PART_ACCESS=dev @@ -885,7 +885,7 @@ interactive_grub() { # Create and edit the grub menu.lst interactive_grub_menulst - DEVS="$(find_usable_bdevs '_ ')" + DEVS="$(find_usable_blockdevices '_ ')" if [ "$DEVS" = " " ]; then notify "No hard drives were found" return 1
On Sun, 20 Feb 2011 15:09:38 -0500 Matthew Gyurgyik <pyther@pyther.net> wrote:
Here is a diff of the changes after implementing your suggestions. (..) I attempted to fix the commit message: Reworte finddisks and findblockdevices. Renamed findblockdevices to find_usable_devs
fix the typo's and make it current tense, double check you fixed everything I asked, resubmit and I'll have another look. Dieter
participants (3)
-
Dieter Plaetinck
-
Matthew Gyurgyik
-
pyther＠pyther.net