Patchwork boot-directdisk: mount root by MBR disk signature for Linux 3.8+

login
register
mail settings
Submitter Jonathan Liu
Date May 23, 2013, 4:31 a.m.
Message ID <1369283514-8837-1-git-send-email-net147@gmail.com>
Download mbox | patch
Permalink /patch/50355/
State Superseded, archived
Headers show

Comments

Jonathan Liu - May 23, 2013, 4:31 a.m.
Signed-off-by: Jonathan Liu <net147@gmail.com>
---
 meta/classes/boot-directdisk.bbclass | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)
Darren Hart - June 7, 2013, 11:28 p.m.
Hi Jon,

Please include a complete commit message. For specifics on contributing
patches, please see:

https://wiki.yoctoproject.org/wiki/Contribution_Guidelines

What is the problem? What is the cause? How are you addressing it? How
was it tested?

Is this 3.8 feature you are referring to something must be configured
into the kernel? Or is it always present?

There is also the concern with dual kernel systems (but that perhaps can
wait until that is commonplace and the infrastructure is in place).

See inline for additional comments:

On 05/22/2013 09:31 PM, Jonathan Liu wrote:
> Signed-off-by: Jonathan Liu <net147@gmail.com>
> ---
>  meta/classes/boot-directdisk.bbclass | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/meta/classes/boot-directdisk.bbclass b/meta/classes/boot-directdisk.bbclass
> index 3169043..736bee0 100644
> --- a/meta/classes/boot-directdisk.bbclass
> +++ b/meta/classes/boot-directdisk.bbclass
> @@ -23,7 +23,8 @@ do_bootdirectdisk[depends] += "dosfstools-native:do_populate_sysroot \
>                                 syslinux:do_populate_sysroot \
>                                 syslinux-native:do_populate_sysroot \
>                                 parted-native:do_populate_sysroot \
> -                               mtools-native:do_populate_sysroot "
> +                               mtools-native:do_populate_sysroot \
> +                               util-linux-native:do_populate_sysroot "
>  
>  PACKAGES = " "
>  EXCLUDE_FROM_WORLD = "1"
> @@ -58,6 +59,22 @@ build_boot_dd() {
>  	# done in blocks, thus the mod by 16 instead of 32.
>  	BLOCKS=$(expr $BLOCKS + $(expr 16 - $(expr $BLOCKS % 16)))
>  
> +	KERNEL_VERSION=$(grep '^VERSION\s*=' ${STAGING_KERNEL_DIR}/Makefile | grep -o '[0-9]*$')
> +	KERNEL_PATCHLEVEL=$(grep '^PATCHLEVEL\s*=' ${STAGING_KERNEL_DIR}/Makefile | grep -o '[0-9]*$')


Could you use LINUX_VERSION here and void delving into the Linux sources
directly?


> +
> +	rm -rf $IMAGE
> +	dd if=/dev/zero of=$IMAGE bs=512 seek=1 count=0
> +	parted $IMAGE mklabel msdos
> +
> +	# If using Linux 3.8 or later, mount root by MBR disk signature
> +	if [[ $KERNEL_VERSION -eq 3 && $KERNEL_PATCHLEVEL -ge 8 ]] || [ $KERNEL_VERSION -gt 3 ]; then


[[ is a bashism, use portable tests please, for example:

if ([ $KERNEL_VERSION -eq 3 ] && [$KERNEL_PATCHLEVEL -ge 8 ]) || [
$KERNEL_VERSION -gt 3 ]; then

This is easily tested by writing test.sh and executing with bash and
dash and ensuring consistent behavior.

Is the version sufficient? No CONFIG_ value to test for?

> +		# The disk signature generated by GNU Parted isn't really random, so regenerate it
> +		echo -ne "$(uuidgen | sed 's/-//g;s/\(..\)/\\x&/g')" | dd of=$IMAGE bs=1 seek=440 count=4 conv=notrunc
> +
> +		DISK_SIGNATURE=$(hexdump -s 440 -n 4 -e '4 1 "%02x\n" "\n"' $IMAGE | tac | paste -sd '')

Seems like it would be more efficient to just store this in a variable
initially
rather than opening, seeking, and dumping the value that you just wrote
in...

> +		sed -i "s|\broot=[^ ]*|root=PARTUUID=$DISK_SIGNATURE-02|" $HDDDIR/syslinux.cfg


In bootimg.bbclass I have been abstracting out the boot loader so it is
not explicitly referenced. Something like that is in order here. For
example, an EFI boot-directdisk (which admittedly doesn't exist yet)
would not have a syslinux.cfg but would still need the change. Consider
using the syslinux class to abstract this necessary change. Then I can
add the gummiboot change when EFI support is added.

--
Darren

> +	fi
> +
>  	mkdosfs -n ${BOOTDD_VOLUME_ID} -S 512 -C $HDDIMG $BLOCKS 
>  	mcopy -i $HDDIMG -s $HDDDIR/* ::/
>  
> @@ -71,10 +88,8 @@ build_boot_dd() {
>  	END3=`expr \( $ROOTFSBLOCKS \* 1024 \) + $END1`
>  
>  	echo $ROOTFSBLOCKS $TOTALSIZE $END1 $END2 $END3
> -	rm -rf $IMAGE
>  	dd if=/dev/zero of=$IMAGE bs=1024 seek=$TOTALSIZE count=1
>  
> -	parted $IMAGE mklabel msdos
>  	parted $IMAGE mkpart primary fat16 0 ${END1}B
>  	parted $IMAGE unit B mkpart primary ext2 ${END2}B ${END3}B
>  	parted $IMAGE set 1 boot on 
>
Jonathan Liu - June 8, 2013, 1:03 p.m.
Hi Darren,

On 8/06/2013 9:28 AM, Darren Hart wrote:
> Hi Jon,
>
> Please include a complete commit message. For specifics on contributing
> patches, please see:
>
> https://wiki.yoctoproject.org/wiki/Contribution_Guidelines
>
> What is the problem? What is the cause? How are you addressing it? How
> was it tested?
>
> Is this 3.8 feature you are referring to something must be configured
> into the kernel? Or is it always present?
>
> There is also the concern with dual kernel systems (but that perhaps can
> wait until that is commonplace and the infrastructure is in place).
Ok.
> See inline for additional comments:
>
> On 05/22/2013 09:31 PM, Jonathan Liu wrote:
>> Signed-off-by: Jonathan Liu<net147@gmail.com>
>> ---
>>   meta/classes/boot-directdisk.bbclass | 21 ++++++++++++++++++---
>>   1 file changed, 18 insertions(+), 3 deletions(-)
>>
>> diff --git a/meta/classes/boot-directdisk.bbclass b/meta/classes/boot-directdisk.bbclass
>> index 3169043..736bee0 100644
>> --- a/meta/classes/boot-directdisk.bbclass
>> +++ b/meta/classes/boot-directdisk.bbclass
>> @@ -23,7 +23,8 @@ do_bootdirectdisk[depends] += "dosfstools-native:do_populate_sysroot \
>>                                  syslinux:do_populate_sysroot \
>>                                  syslinux-native:do_populate_sysroot \
>>                                  parted-native:do_populate_sysroot \
>> -                               mtools-native:do_populate_sysroot "
>> +                               mtools-native:do_populate_sysroot \
>> +                               util-linux-native:do_populate_sysroot "
>>   
>>   PACKAGES = " "
>>   EXCLUDE_FROM_WORLD = "1"
>> @@ -58,6 +59,22 @@ build_boot_dd() {
>>   	# done in blocks, thus the mod by 16 instead of 32.
>>   	BLOCKS=$(expr $BLOCKS + $(expr 16 - $(expr $BLOCKS % 16)))
>>   
>> +	KERNEL_VERSION=$(grep '^VERSION\s*=' ${STAGING_KERNEL_DIR}/Makefile | grep -o '[0-9]*$')
>> +	KERNEL_PATCHLEVEL=$(grep '^PATCHLEVEL\s*=' ${STAGING_KERNEL_DIR}/Makefile | grep -o '[0-9]*$')
> Could you use LINUX_VERSION here and void delving into the Linux sources
> directly?
It's not available in the context of the image recipe if I do bitbake -e 
core-image-minimal.
>> +
>> +	rm -rf $IMAGE
>> +	dd if=/dev/zero of=$IMAGE bs=512 seek=1 count=0
>> +	parted $IMAGE mklabel msdos
>> +
>> +	# If using Linux 3.8 or later, mount root by MBR disk signature
>> +	if [[ $KERNEL_VERSION -eq 3 && $KERNEL_PATCHLEVEL -ge 8 ]] || [ $KERNEL_VERSION -gt 3 ]; then
> [[ is a bashism, use portable tests please, for example:
>
> if ([ $KERNEL_VERSION -eq 3 ] && [$KERNEL_PATCHLEVEL -ge 8 ]) || [
> $KERNEL_VERSION -gt 3 ]; then
>
> This is easily tested by writing test.sh and executing with bash and
> dash and ensuring consistent behavior.
Ok.
> Is the version sufficient? No CONFIG_ value to test for?
It needs CONFIG_BLOCK so I will test for that.
>> +		# The disk signature generated by GNU Parted isn't really random, so regenerate it
>> +		echo -ne "$(uuidgen | sed 's/-//g;s/\(..\)/\\x&/g')" | dd of=$IMAGE bs=1 seek=440 count=4 conv=notrunc
>> +
>> +		DISK_SIGNATURE=$(hexdump -s 440 -n 4 -e '4 1 "%02x\n" "\n"' $IMAGE | tac | paste -sd '')
> Seems like it would be more efficient to just store this in a variable
> initially
> rather than opening, seeking, and dumping the value that you just wrote
> in...
Ok.
>> +		sed -i "s|\broot=[^ ]*|root=PARTUUID=$DISK_SIGNATURE-02|" $HDDDIR/syslinux.cfg
> In bootimg.bbclass I have been abstracting out the boot loader so it is
> not explicitly referenced. Something like that is in order here. For
> example, an EFI boot-directdisk (which admittedly doesn't exist yet)
> would not have a syslinux.cfg but would still need the change. Consider
> using the syslinux class to abstract this necessary change. Then I can
> add the gummiboot change when EFI support is added.
Do you mean adding a function to the syslinux class which updates root= 
in syslinux.cfg previously created by build_syslinux_cfg()?

> --
> Darren
>
>> +	fi
>> +
>>   	mkdosfs -n ${BOOTDD_VOLUME_ID} -S 512 -C $HDDIMG $BLOCKS
>>   	mcopy -i $HDDIMG -s $HDDDIR/* ::/
>>   
>> @@ -71,10 +88,8 @@ build_boot_dd() {
>>   	END3=`expr \( $ROOTFSBLOCKS \* 1024 \) + $END1`
>>   
>>   	echo $ROOTFSBLOCKS $TOTALSIZE $END1 $END2 $END3
>> -	rm -rf $IMAGE
>>   	dd if=/dev/zero of=$IMAGE bs=1024 seek=$TOTALSIZE count=1
>>   
>> -	parted $IMAGE mklabel msdos
>>   	parted $IMAGE mkpart primary fat16 0 ${END1}B
>>   	parted $IMAGE unit B mkpart primary ext2 ${END2}B ${END3}B
>>   	parted $IMAGE set 1 boot on
>>
Regards,
Jonathan
Darren Hart - June 10, 2013, 4:26 p.m.
On 06/08/2013 06:03 AM, Jonathan Liu wrote:
> Hi Darren,
> 

>>> +		sed -i "s|\broot=[^ ]*|root=PARTUUID=$DISK_SIGNATURE-02|" $HDDDIR/syslinux.cfg
>> In bootimg.bbclass I have been abstracting out the boot loader so it is
>> not explicitly referenced. Something like that is in order here. For
>> example, an EFI boot-directdisk (which admittedly doesn't exist yet)
>> would not have a syslinux.cfg but would still need the change. Consider
>> using the syslinux class to abstract this necessary change. Then I can
>> add the gummiboot change when EFI support is added.
> Do you mean adding a function to the syslinux class which updates root= 
> in syslinux.cfg previously created by build_syslinux_cfg()?

In the words of a wise director at a large technology company, "I meant
whatever is the most intelligent." :-)

This could be a new function in syslinux.bbclass. It could also be a
parameter to build_syslinux_cfg which would allow it to be used from
either bootimg.bbclass, boot-directdisk.bbclass, with or without EFI,
with various kernel versions, etc. I suspect some parameterization will
result in the cleanest, most maintainable approach, but investigation is
required.

Thanks!

Darren

> 
>> --
>> Darren
>>
>>> +	fi
>>> +
>>>   	mkdosfs -n ${BOOTDD_VOLUME_ID} -S 512 -C $HDDIMG $BLOCKS
>>>   	mcopy -i $HDDIMG -s $HDDDIR/* ::/
>>>   
>>> @@ -71,10 +88,8 @@ build_boot_dd() {
>>>   	END3=`expr \( $ROOTFSBLOCKS \* 1024 \) + $END1`
>>>   
>>>   	echo $ROOTFSBLOCKS $TOTALSIZE $END1 $END2 $END3
>>> -	rm -rf $IMAGE
>>>   	dd if=/dev/zero of=$IMAGE bs=1024 seek=$TOTALSIZE count=1
>>>   
>>> -	parted $IMAGE mklabel msdos
>>>   	parted $IMAGE mkpart primary fat16 0 ${END1}B
>>>   	parted $IMAGE unit B mkpart primary ext2 ${END2}B ${END3}B
>>>   	parted $IMAGE set 1 boot on
>>>
> Regards,
> Jonathan
>
Jonathan Liu - July 9, 2013, 1:58 p.m.
On 11/06/2013 2:26 AM, Darren Hart wrote:
>
> On 06/08/2013 06:03 AM, Jonathan Liu wrote:
>> Hi Darren,
>>
>>>> +		sed -i "s|\broot=[^ ]*|root=PARTUUID=$DISK_SIGNATURE-02|" $HDDDIR/syslinux.cfg
>>> In bootimg.bbclass I have been abstracting out the boot loader so it is
>>> not explicitly referenced. Something like that is in order here. For
>>> example, an EFI boot-directdisk (which admittedly doesn't exist yet)
>>> would not have a syslinux.cfg but would still need the change. Consider
>>> using the syslinux class to abstract this necessary change. Then I can
>>> add the gummiboot change when EFI support is added.
>> Do you mean adding a function to the syslinux class which updates root=
>> in syslinux.cfg previously created by build_syslinux_cfg()?
> In the words of a wise director at a large technology company, "I meant
> whatever is the most intelligent." :-)
>
> This could be a new function in syslinux.bbclass. It could also be a
> parameter to build_syslinux_cfg which would allow it to be used from
> either bootimg.bbclass, boot-directdisk.bbclass, with or without EFI,
> with various kernel versions, etc. I suspect some parameterization will
> result in the cleanest, most maintainable approach, but investigation is
> required.
>
> Thanks!
>
> Darren
Submitted V3 patch which sets SYSLINUX_ROOT appropriately instead of 
modifying syslinux.cfg directly.

Regards,
Jonathan

>
>>> --
>>> Darren
>>>
>>>> +	fi
>>>> +
>>>>    	mkdosfs -n ${BOOTDD_VOLUME_ID} -S 512 -C $HDDIMG $BLOCKS
>>>>    	mcopy -i $HDDIMG -s $HDDDIR/* ::/
>>>>    
>>>> @@ -71,10 +88,8 @@ build_boot_dd() {
>>>>    	END3=`expr \( $ROOTFSBLOCKS \* 1024 \) + $END1`
>>>>    
>>>>    	echo $ROOTFSBLOCKS $TOTALSIZE $END1 $END2 $END3
>>>> -	rm -rf $IMAGE
>>>>    	dd if=/dev/zero of=$IMAGE bs=1024 seek=$TOTALSIZE count=1
>>>>    
>>>> -	parted $IMAGE mklabel msdos
>>>>    	parted $IMAGE mkpart primary fat16 0 ${END1}B
>>>>    	parted $IMAGE unit B mkpart primary ext2 ${END2}B ${END3}B
>>>>    	parted $IMAGE set 1 boot on
>>>>
>> Regards,
>> Jonathan
>>

Patch

diff --git a/meta/classes/boot-directdisk.bbclass b/meta/classes/boot-directdisk.bbclass
index 3169043..736bee0 100644
--- a/meta/classes/boot-directdisk.bbclass
+++ b/meta/classes/boot-directdisk.bbclass
@@ -23,7 +23,8 @@  do_bootdirectdisk[depends] += "dosfstools-native:do_populate_sysroot \
                                syslinux:do_populate_sysroot \
                                syslinux-native:do_populate_sysroot \
                                parted-native:do_populate_sysroot \
-                               mtools-native:do_populate_sysroot "
+                               mtools-native:do_populate_sysroot \
+                               util-linux-native:do_populate_sysroot "
 
 PACKAGES = " "
 EXCLUDE_FROM_WORLD = "1"
@@ -58,6 +59,22 @@  build_boot_dd() {
 	# done in blocks, thus the mod by 16 instead of 32.
 	BLOCKS=$(expr $BLOCKS + $(expr 16 - $(expr $BLOCKS % 16)))
 
+	KERNEL_VERSION=$(grep '^VERSION\s*=' ${STAGING_KERNEL_DIR}/Makefile | grep -o '[0-9]*$')
+	KERNEL_PATCHLEVEL=$(grep '^PATCHLEVEL\s*=' ${STAGING_KERNEL_DIR}/Makefile | grep -o '[0-9]*$')
+
+	rm -rf $IMAGE
+	dd if=/dev/zero of=$IMAGE bs=512 seek=1 count=0
+	parted $IMAGE mklabel msdos
+
+	# If using Linux 3.8 or later, mount root by MBR disk signature
+	if [[ $KERNEL_VERSION -eq 3 && $KERNEL_PATCHLEVEL -ge 8 ]] || [ $KERNEL_VERSION -gt 3 ]; then
+		# The disk signature generated by GNU Parted isn't really random, so regenerate it
+		echo -ne "$(uuidgen | sed 's/-//g;s/\(..\)/\\x&/g')" | dd of=$IMAGE bs=1 seek=440 count=4 conv=notrunc
+
+		DISK_SIGNATURE=$(hexdump -s 440 -n 4 -e '4 1 "%02x\n" "\n"' $IMAGE | tac | paste -sd '')
+		sed -i "s|\broot=[^ ]*|root=PARTUUID=$DISK_SIGNATURE-02|" $HDDDIR/syslinux.cfg
+	fi
+
 	mkdosfs -n ${BOOTDD_VOLUME_ID} -S 512 -C $HDDIMG $BLOCKS 
 	mcopy -i $HDDIMG -s $HDDDIR/* ::/
 
@@ -71,10 +88,8 @@  build_boot_dd() {
 	END3=`expr \( $ROOTFSBLOCKS \* 1024 \) + $END1`
 
 	echo $ROOTFSBLOCKS $TOTALSIZE $END1 $END2 $END3
-	rm -rf $IMAGE
 	dd if=/dev/zero of=$IMAGE bs=1024 seek=$TOTALSIZE count=1
 
-	parted $IMAGE mklabel msdos
 	parted $IMAGE mkpart primary fat16 0 ${END1}B
 	parted $IMAGE unit B mkpart primary ext2 ${END2}B ${END3}B
 	parted $IMAGE set 1 boot on