[OE-core] [PATCH] mkefidisk: Create interactive menu for the script

[Bottazzini, Bruno]

On Tue, 2015-09-08 at 08:15 -0700, Saul Wold wrote:
> 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 at 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?

Unfortunately no, both outputs are different.

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

I double checked it before sending this patch and the answer was yes.

> 
> 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.

This is odd. My output shows only unmounted disks.

> 
> > +            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

This output is strange mine shows only (without sda and sdb which i do
have them also, but mounted):
        contrib > ./mkefidisk.sh image
        0 - /dev/mmcblk0
        Choose unmounted device number:

Are you sure you have updated the script with the changes? What distro
are you using?

> 
> 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.

You are right I am removing \n. Strangely here it works perfectly:
        0 - /dev/sdc
        1 - /dev/mmcblk0
        Choose unmounted device number: 1
        
        Choose a name of the device that will be boot from
        Recommended name is: /dev/mmcblk0
        Is target device okay? [y/N]: y


> 
> 
> 
> > +                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

Removed too

Thank you a lot,

Bruno

> 
> > +        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
> >