Re: [PATCH] boot-directdisk: mount root by MBR disk signature for Linux 3.8+

[Jonathan Liu <net147@gmail.com>]

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