grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [GRUB PARTUUID PATCH V5 2/3] Add PARTUUID detection support to grub-


From: Nick Vinson
Subject: Re: [GRUB PARTUUID PATCH V5 2/3] Add PARTUUID detection support to grub-probe
Date: Sun, 18 Mar 2018 11:59:56 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0

On 02/20/2018 09:41 AM, Daniel Kiper wrote:
> On Sun, Feb 04, 2018 at 11:47:36AM -0800, Nicholas Vinson wrote:
>> Add PARTUUID detection support grub-probe for MBR and GPT partition
>> schemes.  The Linux kernel supports mounting the root filesystem by
>> Linux device name or by the Partition [GU]UID.  GRUB's mkconfig,
>> however, currently only supports specifing the rootfs in the kernel
>> command-line by Linux device name unless an initramfs is also present.
>> When an initramfs is present GRUB's mkconfig will set the kernel's root
>> parameter value to either the Linux device name or to the filesystem
>> [GU]UID.
>>
>> Therefore, the only way to protect a Linux system from failing to boot
>> when its Linux storage device names change is to either manually edit
>> grub.cfg or /etc/default/grub and append root=PARTUUID=xxx to the
>> command-line or create an initramfs that understands how to mount
>> devices by filesystem [G]UID and let grub-mkconfig pass the filesystem
>> [GU]UID to the initramfs.
>>
>> The goal of this patch set is to enable root=PARTUUID=xxx support in
>> grub-mkconfig, so that users don't have to manually edit
>> /etc/default/grub or grub.cfg, or create an initramfs for the sole
>> purpose of having a robust bootloader configuration for Linux.
>> ---
>>  util/grub-probe.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 51 insertions(+)
>>
>> diff --git a/util/grub-probe.c b/util/grub-probe.c
>> index 21cb80fbe..3656e32e8 100644
>> --- a/util/grub-probe.c
>> +++ b/util/grub-probe.c
>> @@ -28,6 +28,7 @@
>>  #include <grub/partition.h>
>>  #include <grub/msdos_partition.h>
>>  #include <grub/gpt_partition.h>
>> +#include <grub/i386/pc/boot.h>
>>  #include <grub/emu/hostdisk.h>
>>  #include <grub/emu/getroot.h>
>>  #include <grub/term.h>
>> @@ -62,6 +63,7 @@ enum {
>>    PRINT_DRIVE,
>>    PRINT_DEVICE,
>>    PRINT_PARTMAP,
>> +  PRINT_PARTUUID,
>>    PRINT_ABSTRACTION,
>>    PRINT_CRYPTODISK_UUID,
>>    PRINT_HINT_STR,
>> @@ -85,6 +87,7 @@ static const char *targets[] =
>>      [PRINT_DRIVE]              = "drive",
>>      [PRINT_DEVICE]             = "device",
>>      [PRINT_PARTMAP]            = "partmap",
>> +    [PRINT_PARTUUID]           = "partuuid",
>>      [PRINT_ABSTRACTION]        = "abstraction",
>>      [PRINT_CRYPTODISK_UUID]    = "cryptodisk_uuid",
>>      [PRINT_HINT_STR]           = "hints_string",
>> @@ -181,6 +184,48 @@ probe_partmap (grub_disk_t disk, char delim)
>>      }
>>  }
>>
>> +static void
>> +probe_partuuid (grub_disk_t disk, char delim)
>> +{
>> +  /*
>> +   * Nested partitions not supported for now.
>> +   * Non-nested partitions must have disk->partition->parent == NULL
>> +   */
>> +  if (disk->partition && disk->partition->parent == NULL)
>> +    {
>> +      if (strcmp(disk->partition->partmap->name, "msdos") == 0)
>> +    {
>> +        /*
>> +         * The partition GUID for MSDOS is the partition number (starting
>> +         * with 1) prepended with the NT disk signature.
>> +         */
>> +            grub_uint32_t nt_disk_sig;
> 
> Here you use spaces...
> 
>> +        grub_partition_t p = disk->partition;
> 
> ...and here tab and spaces. I prefer the latter.
> Please fix this.
> 

Fixed.

>> +        disk->partition = p->parent;
>> +
>> +        if (grub_disk_read (disk, 0, GRUB_BOOT_MACHINE_WINDOWS_NT_MAGIC,
>> +                            sizeof(nt_disk_sig), &nt_disk_sig) == 0)
>> +          {
>> +            grub_printf ("%08x-%02x",
>> +                         grub_le_to_cpu32(nt_disk_sig), 1 + p->number);
>> +          }
> 
> These curly brackets are not needed. Please remove them and leave empty
> line before next instruction.
> 

Curly braces removed and a new line added.

>> +        disk->partition = p;
>> +    }
>> +      else if (strcmp(disk->partition->partmap->name, "gpt") == 0)
>> +    {
>> +      grub_partition_t p = disk->partition;
>> +      struct grub_gpt_partentry gptdata;
>> +
>> +      disk->partition = p->parent;
>> +
>> +      if (grub_disk_read (disk, p->offset, p->index,
>> +                          sizeof(gptdata), &gptdata) == 0)
>> +        print_gpt_guid(gptdata.guid);
> 
> Please add empty line here.
> 

Added.  These changes will also be included in version 6.
Thanks,
Nicholas Vinson

>> +      disk->partition = p;
>> +    }
>> +    }
>> +}
>> +
> 
> Otherwise patch LGMT.
> 
> Daniel
> 

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]