[Fedora-livecd-list] Bug 587411 - livecd-iso-to-disk --format fails with SD/MMC partitions

[Frederick Grose]

Sorry for 'jumping the gun'.  Here's a more fully debugged version:

        --Fred

On Wed, May 12, 2010 at 4:35 PM, Frederick Grose <fgrose at gmail.com> wrote:

> On Mon, May 10, 2010 at 6:27 PM, Alan Pevec <apevec at gmail.com> wrote:

>  On Mon, May 10, 2010 at 10:20 PM, David Huff <dhuff at redhat.com> wrote:
>
>> > Frederick's patch fixes issues with SD cards that do not have a standard
>
>> > partitioning layout and requires part0 ie: /dev/mmcblk0
>
>>
> issue is that partitions on mmc device use separator p i.e. 1st
>
>> partition is mmcblk0p1
>
>>
> > However the proposed patch requires users specify a
>
>> > partition for the storage device or have --format option require a
>
>> > partition number.
>
>>
> --format will create one, first partition, so it doesn't make sense to
> specify the partition number
>

Yes,  currently we don't support multiple partition devices.  That will be

> another bug and patch upcoming.

>

> > Any comments on this?
>
>> >
>
>> > Also Frederick can you please resend the patch to the list, that way I
>
>> > can apply it and retain all header info for the commit.
>
>>
> patch in BZ also merged checkLVMActive() which is now committed
>
>>
> I don't like special case for mmc:
>
>> [[ "${path/*mmc*/yes}" = yes ]] && mmc=p
>
>>
> instead, why not simply detect partition separator here:
>
>>
> path=/dev/`basename $path`
>
>> partnum0=${path##$device}
>
>> partnum=${partnum0/#p/}
>
>> if [ "$partnum" = "$partnum0" ]; then
>
>>  separator=""
>
>> else
>
>>  separator="p"
>
>> fi
>
>> ...
> USBDEV=${device}${separator}${partnum:-1}
>

A problem arises because users are allowed to --reset-mbr --format a
partition by simply supplying a device node such as /dev/sdb.  This means
the code must determine an appropriate partition name when one is not
supplied. So the suggested test fails for the case where "... --format
/dev/mmcblk0 ..." is supplied to livecd-iso-to-disk.

>
The path test works, but assumes that the separator is used, and apparently
some older systems did not use a 'p' separator.  So, better to just read
from /proc/partitions.

>
The following patch is proposed--but note: It has NOT been tested on EFI
code paths.

>
      --Fred


diff --git a/tools/livecd-iso-to-disk.sh b/tools/livecd-iso-to-disk.sh

> index f4d0855..ad85839 100755

> --- a/tools/livecd-iso-to-disk.sh

> +++ b/tools/livecd-iso-to-disk.sh

> @@ -47,21 +47,24 @@ getdisk() {

>         return

>      fi

>

> -    p=$(udevadm info -q path -n $DEV)

> -    if [ -e /sys/$p/device ]; then

> - device=$(basename /sys/$p)

> +    path=$(udevadm info -q path -n $DEV)

> +    if [ -e /sys/$path/device ]; then

> +    device=$(basename /sys/$path)

>      else

> - device=$(basename $(readlink -f /sys/$p/../))

> +    device=$(basename $(readlink -f /sys/$path/../))

>      fi

>      if [ ! -e /sys/block/$device -o ! -e /dev/$device ]; then

> - echo "Error finding block device of $DEV.  Aborting!"

> - exitclean

> +    echo "Error finding block device of $DEV.  Aborting!"

> +    exitclean

>      fi

> -

> -    device="/dev/$device"

> +

>      # FIXME: weird dev names could mess this up I guess

> -    p=/dev/`basename $p`

> -    partnum=${p##$device}

> +    set $( awk '$4 ~ /'$device'.*[0-9]$/ {print $4}' /proc/partitions )

> +    partition=$1

> +    path=$(basename $path)

> +    partnum=${path##$device}

> +    [[ -z "$partnum" ]] && partnum=${partition##$device}

> +    device="/dev/$device"

>  }

>

>  resetMBR() {

> @@ -157,16 +160,18 @@ createGPTLayout() {

>      echo "WARNING: THIS WILL DESTROY ANY DATA ON $device!!!"

>      echo "Press Enter to continue or ctrl-c to abort"

>      read

> -    umount ${device}? &> /dev/null

> +    umount ${device}* &> /dev/null

>      /sbin/parted --script $device mklabel gpt

>      partinfo=$(LC_ALL=C /sbin/parted --script -m $device "unit b print"
|grep ^$device:)

>      size=$(echo $partinfo |cut -d : -f 2 |sed -e 's/B$//')

>      /sbin/parted --script $device unit b mkpart '"EFI System Partition"'
fat32 17408 $(($size - 17408)) set 1 boot on

> -    USBDEV=${device}1

>      # Sometimes automount can be _really_ annoying.

>      echo "Waiting for devices to settle..."

>      /sbin/udevadm settle

>      sleep 5

> +    partition=$( awk '$4 ~ /'${device#/dev/}'.*[0-9]$/ {print $4}'
/proc/partitions )

> +    partnum=${partition##${device#/dev/}}

> +    USBDEV=${device}${partnum}

>      umount $USBDEV &> /dev/null

>      /sbin/mkdosfs -n LIVE $USBDEV

>      USBLABEL="UUID=$(/sbin/blkid -s UUID -o value $USBDEV)"

> @@ -179,16 +184,18 @@ createMSDOSLayout() {

>      echo "WARNING: THIS WILL DESTROY ANY DATA ON $device!!!"

>      echo "Press Enter to continue or ctrl-c to abort"

>      read

> -    umount ${device}? &> /dev/null

> +    umount ${device}* &> /dev/null

>      /sbin/parted --script $device mklabel msdos

>      partinfo=$(LC_ALL=C /sbin/parted --script -m $device "unit b print"
|grep ^$device:)

>      size=$(echo $partinfo |cut -d : -f 2 |sed -e 's/B$//')

>      /sbin/parted --script $device unit b mkpart primary fat32 17408
$(($size - 17408)) set 1 boot on

> -    USBDEV=${device}1

>      # Sometimes automount can be _really_ annoying.

>      echo "Waiting for devices to settle..."

>      /sbin/udevadm settle

>      sleep 5

> +    partition=$( awk '$4 ~ /'${device#/dev/}'.*[0-9]$/ {print $4}'
/proc/partitions )

> +    partnum=${partition##${device#/dev/}}

> +    USBDEV=${device}${partnum}

>      umount $USBDEV &> /dev/null

>      /sbin/mkdosfs -n LIVE $USBDEV
     USBLABEL="UUID=$(/sbin/blkid -s UUID -o value $USBDEV)"
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.fedoraproject.org/pipermail/livecd/attachments/20100512/bf930579/attachment.html