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

[Darren Hart <dvhart@linux.intel.com>]

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 
> 

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Technical Lead - Linux Kernel