Re: [PATCH] mkefidisk: Create interactive menu for the script

[Saul Wold <sgw@linux.intel.com>]

On 09/04/2015 03:19 PM, Bruno Bottazzini wrote:
> If it is passed only the image parameter, it will show some interactive
> options to help to conclude the script.
>
> With this patch it will detect devices available in the machine, asking
> to choose what it is wanted to be flashed.
>
> it will suggest the target device. If it is a SD card the suggestion
> is /dev/mmcblk0. If it is a pendrive it will suggest to use /dev/sda.
>
> Signed-off-by: Bruno Bottazzini <bruno.bottazzini@intel.com>
> ---
>   scripts/contrib/mkefidisk.sh | 63 ++++++++++++++++++++++++++++++++++++++++----
>   1 file changed, 58 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/contrib/mkefidisk.sh b/scripts/contrib/mkefidisk.sh
> index 55f72b0..66e0e58 100755
> --- a/scripts/contrib/mkefidisk.sh
> +++ b/scripts/contrib/mkefidisk.sh
> @@ -152,8 +152,55 @@ unmount() {
>   # Parse and validate arguments
>   #
>   if [ $# -lt 3 ] || [ $# -gt 4 ]; then
> -	usage
> -	exit 1
> +    if [ $# -eq 1 ]; then
> +        AVAILABLE_DISK=`lsblk | grep "disk" | cut -f 1 -d " "`
> +        X=0
> +        for disk in `echo $AVAILABLE_DISK`; do
> +            mounted=`blkid -o list | grep "/dev/$disk" | grep "not mounted"`
Is there any way to not call blkid multiple time (ie save the output and 
use it once?


Are the lsblk and blkid commands guaranteed to be installed on most 
machines?

Also this seems to have matched my root disk, which again concerns me, I 
am still not convinced that a selection system like this is safe.

> +            if [ -n "$mounted" ]; then
> +                UNMOUNTED_AVAILABLES="$UNMOUNTED_AVAILABLES /dev/$disk\n"

I think the \n is not needed here, what shell are you using, with Bash I 
get the following output:

0 - /dev/sda
1 - /dev/sdb
2 - /dev/mmcblk0
Choose unmounted device number: 1

Choose a name of the device that will be boot from
Recommended name is: /dev/sda\n
Is target device okay? [y/N]: y

Usage: mkefidisk.sh [-v] DEVICE HDDIMG TARGET_DEVICE
        -v: Verbose debug
        DEVICE: The device to write the image to, e.g. /dev/sdh
        HDDIMG: The hddimg file to generate the efi disk from
        TARGET_DEVICE: The device the target will boot from, e.g. 
/dev/mmcblk0
Device /dev/sdb\n cannot be found

Note the \n above in the choose a name and then the trailing \n here 
with the device not being found.



> +                info "$X - /dev/$disk"
> +                X=`expr $X + 1`
> +            fi
> +        done
> +        if [ $X -eq 0 ]; then
> +            die "No unmounted device found."
> +        fi
> +        read -p "Choose unmounted device number: " DISK_NUMBER
> +        X=0
> +        for line in `echo $UNMOUNTED_AVAILABLES`; do
> +            if [ $DISK_NUMBER -eq $X ]; then
> +                DISK_TO_BE_FLASHED=$line
> +                break
> +            else
> +                X=`expr $X + 1`
> +            fi
> +        done
> +        if [ -z "$DISK_TO_BE_FLASHED" ]; then
> +            die "Option \"$DISK_NUMBER\" is invalid. Choose a valid option"
> +        else
> +            if [ -z `echo $DISK_TO_BE_FLASHED | grep "mmc"` ]; then
> +                TARGET_TO_BE_BOOT="/dev/sda"
> +            else
> +                TARGET_TO_BE_BOOT="/dev/mmcblk0"
> +            fi
> +        fi
> +        echo ""
> +        echo "Choose a name of the device that will be boot from"
> +        echo -n "Recommended name is: "
> +        info "$TARGET_TO_BE_BOOT\n"

You are still outputting the \n here

> +        read -p "Is target device okay? [y/N]: " RESPONSE
> +        if [ "$RESPONSE" != "y" ]; then
> +            read -p "Choose target device name: " TARGET_TO_BE_BOOT
> +        fi
> +        echo ""
> +        if [ -z "$TARGET_TO_BE_BOOT" ]; then
> +            die "Error: choose a valid target name"
> +        fi
> +    else
> +        usage
> +	    exit 1
> +    fi
>   fi
>
>   if [ "$1" = "-v" ]; then
> @@ -162,9 +209,15 @@ if [ "$1" = "-v" ]; then
>   	shift
>   fi
>
> -DEVICE=$1
> -HDDIMG=$2
> -TARGET_DEVICE=$3
> +if [ -z "$AVAILABLE_DISK" ]; then
> +    DEVICE=$1
> +    HDDIMG=$2
> +    TARGET_DEVICE=$3
> +else
> +    DEVICE=$DISK_TO_BE_FLASHED
> +    HDDIMG=$1
> +    TARGET_DEVICE=$TARGET_TO_BE_BOOT
> +fi
>
This change I like, if done with checking for 2 parameter (image and 
disk to flash, the 3 paramter can be determined as you have with best 
guess of /dev/sda if it's an sd and mmc if it's a /dev/mmc flash device.

Sau!


>   LINK=$(readlink $DEVICE)
>   if [ $? -eq 0 ]; then
>