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

[Daniel Kiper]

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.

> +         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.

> +         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.

> +       disk->partition = p;
> +     }
> +    }
> +}
> +

Otherwise patch LGMT.

Daniel