3:20 p.m.
New subject: Bug 587411 - livecd-iso-to-disk --format fails with SD/MMC partitions
On 05/04/2010 10:26 PM, Frederick Grose wrote:
Comments solicited on
https://bugzilla.redhat.com/show_bug.cgi?id=587411.
--Fred
Frederick's patch fixes issues with SD cards that do not have a standard
partitioning layout and requires part0 ie: /dev/mmcblk0
However the proposed patch requires users specify a
partition for the storage device or have --format option require a
partition number.
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.
-D
5:27 p.m.
On Mon, May 10, 2010 at 10:20 PM, David Huff <dhuff(a)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
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}
...
3:35 p.m.
On Mon, May 10, 2010 at 6:27 PM, Alan Pevec <apevec(a)gmail.com> wrote:
On Mon, May 10, 2010 at 10:20 PM, David Huff <dhuff(a)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..f92fe2b 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=/dev/$(basename $path)
+ partnum=${path##$device}
+ [[ -z "$partnum" ]] && partnum=${partition##$device}
+ device="/dev/$device"
}
resetMBR() {
@@ -157,16 +160,17 @@ 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
+ partnum=$( awk '$4 ~ /'${device#/}'.*[0-9]$/ {print $4}'
/proc/partitions )
+ USBDEV=${device}${partnum}
umount $USBDEV &> /dev/null
/sbin/mkdosfs -n LIVE $USBDEV
USBLABEL="UUID=$(/sbin/blkid -s UUID -o value $USBDEV)"
@@ -179,16 +183,17 @@ 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
+ partnum=$( awk '$4 ~ /'${device#/}'.*[0-9]$/ {print $4}'
/proc/partitions )
+ USBDEV=${device}${partnum}
umount $USBDEV &> /dev/null
/sbin/mkdosfs -n LIVE $USBDEV
USBLABEL="UUID=$(/sbin/blkid -s UUID -o value $USBDEV)"
4:22 p.m.
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(a)gmail.com> wrote:
> > 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
On Mon, May 10, 2010 at 6:27 PM, Alan Pevec <apevec(a)gmail.com>
wrote:
On Mon, May 10, 2010 at 10:20 PM, David Huff
<dhuff(a)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.
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)"
5100
days inactive
5107
days old
livecd@lists.fedoraproject.org
4 comments
3 participants
participants (3)
-
Alan Pevec -
David Huff -
Frederick Grose