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

[Jonathan Liu <net147@gmail.com>]

On 10/07/2013 7:48 AM, Darren Hart wrote:
> On Wed, 2013-07-10 at 00:12 +1000, Jonathan Liu wrote:
>> The root device is currently set as /dev/hda2. However, this is
>> only correct if it's the first IDE drive. If booting off the first SATA
>> drive instead, it would be /dev/sda2. It's not the first drive, neither
>> /dev/hda2 or /dev/sda2 would be correct.
>>
>> The solution to this has typically been to use the filesystem UUID to
>> specify the root device but this requires extra support in the initrd.
>> Linux 3.8 introduces the ability to specify the root device using the
>> MBR disk signature and the partition number which is much simpler to
>> use and avoids the extra overhead of an initrd.
>>
>> This change uses the MBR disk signature to specify the root device when
>> using Linux 3.8+ and CONFIG_BLOCK (required for root=PARTUUID=)
>> is enabled in the kernel.
>>
>> This has been tested with QEMU x86 and Intel Desktop Board D2500HN using
>> an image recipe inheriting boot-directdisk and core-image.
>>
>> Signed-off-by: Jonathan Liu <net147@gmail.com>
>> ---
>>   meta/classes/boot-directdisk.bbclass | 30 +++++++++++++++++++++++++++++-
>>   1 file changed, 29 insertions(+), 1 deletion(-)
>>
>> diff --git a/meta/classes/boot-directdisk.bbclass b/meta/classes/boot-directdisk.bbclass
>> index efeadab..f97060f 100644
>> --- a/meta/classes/boot-directdisk.bbclass
>> +++ b/meta/classes/boot-directdisk.bbclass
>> @@ -35,7 +35,9 @@ BOOTDD_EXTRA_SPACE ?= "16384"
>>   # Get the build_syslinux_cfg() function from the syslinux class
>>   
>>   AUTO_SYSLINUXCFG = "1"
>> -SYSLINUX_ROOT ?= "root=/dev/sda2"
>> +SYSLINUX_FIXED_ROOT = "root=/dev/sda2"
>> +SYSLINUX_UUID_ROOT = "root=PARTUUID=${DISK_SIGNATURE}-02"
> Why  '=' and not '?=' for the above? Shouldn't the user be able to
> override these?
Ok.
>
>> +SYSLINUX_ROOT ?= "${@'${SYSLINUX_UUID_ROOT}' if has_kernel_mbr_partuuid_support(d) else '${SYSLINUX_FIXED_ROOT}'}"
>>   SYSLINUX_TIMEOUT ?= "10"
>>   
>>   inherit syslinux
>> @@ -98,6 +100,10 @@ build_boot_dd() {
>>   	parted $IMAGE set 1 boot on
>>   	parted $IMAGE print
>>   
>> +	# Disk signature generated by parted isn't really random, so use our own generated signature
>> +	echo -ne "$(echo ${DISK_SIGNATURE} | fold -w 2 | tac | paste -sd '' | sed 's/\(..\)/\\x&/g')" | \
>> +		dd of=$IMAGE bs=1 seek=440 conv=notrunc
> Oi! So, can "fold -w 2 | tac | paste -sd '' be replaced with "rev" from
> the standard util-linux package?
>
> And the sed command... just insert "\x" every two characters? That
> doesn't seem to add any more randomness.... maybe I'm missing the point
> here?
fold -w 2 | tac | paste -sd '' is to reverse the hex bytes. the sed 
command is to convert the hex bytes aabbccdd to \xaa\xbb\xcc\xdd and 
pass to echo -ne to convert the hex bytes to binary. I can remove fold 
-w 2 | tac | paste -sd '' and add rev after converting to binary.
>
>
>> +
>>   	OFFSET=`expr $END2 / 512`
>>   	dd if=${STAGING_DATADIR}/syslinux/mbr.bin of=$IMAGE conv=notrunc
>>   	dd if=$HDDIMG of=$IMAGE conv=notrunc seek=1 bs=512
>> @@ -113,4 +119,26 @@ python do_bootdirectdisk() {
>>       bb.build.exec_func('build_boot_dd', d)
>>   }
>>   
>> +def generate_disk_signature():
>> +    import uuid
>> +    return str(uuid.uuid4())[:8]
>> +
>> +def get_kernel_version(d):
>> +    import subprocess
>> +    version_cmd = r"grep '^VERSION\s*=' '%s/Makefile' | grep -o '[0-9]*$'" % (d.getVar("STAGING_KERNEL_DIR", True))
>> +    version = int(subprocess.Popen(version_cmd, shell=True, stdout=subprocess.PIPE).communicate()[0])
>> +    patchlevel_cmd = r"grep '^PATCHLEVEL\s*=' '%s/Makefile' | grep -o '[0-9]*$'" % (d.getVar("STAGING_KERNEL_DIR", True))
>> +    patchlevel = int(subprocess.Popen(patchlevel_cmd, shell=True, stdout=subprocess.PIPE).communicate()[0])
>> +    return (version, patchlevel)
>> +
>> +def has_kernel_config_option(option, d):
>> +    import subprocess
>> +    grep_cmd = r"grep '^CONFIG_%s=y$' '%s/.config'" % (option, d.getVar("STAGING_KERNEL_DIR", True))
>> +    return subprocess.call(grep_cmd, shell=True, stdout=subprocess.PIPE) == 0
> Is there a specific kernel option to test for the MBR disk signature
> support or is that inherent in the block drivers now? It would be nice
> if we could skip the kernel version check, that code looks dangerously
> fragile (no fault of yours).
The commit which adds the support is 
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?id=d33b98fc82b0908e91fb05ae081acaed7323f9d2. 
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/init/do_mounts.c?id=refs/tags/v3.8.13 
indicates PARTUUID= support is only enabled with CONFIG_BLOCK.
Alternatively, instead of checking kernel version I could just add an 
option to enable the PARTUUID support and add a comment in the class 
that you need Linux 3.8+ with CONFIG_BLOCK. It would be disabled by 
default in that case and if they are using an older kernel version with 
the feature manually backported, they can explicitly enable it.
>
>> +
>> +def has_kernel_mbr_partuuid_support(d):
>> +    return get_kernel_version(d) >= (3, 8) and has_kernel_config_option("BLOCK", d)
>> +
>> +DISK_SIGNATURE := "${@generate_disk_signature()}"
>> +
>>   addtask bootdirectdisk before do_build
Regards,
Jonathan