[GRUB PARTUUID PATCH V7 0/4] Add PARTUUID detection support

[GRUB PARTUUID PATCH V7 0/4] Add PARTUUID detection support

[Nicholas Vinson]

Changes from Patch v6:
    - Corrected spelling and grammatical errors in description text for
      GRUB_ENABLE_LINUX_PARTUUID
    - Moved disk->partition save & restore logic to beginning and end of
      probe_partuuid()
    - Fixed formatting errors in probe_partuuid

Changes from Patch v5:
    - Added sign-off by lines
    - Fixed formatting errors found by Daniel Kiper

Changes from Patch v4:
    - Updated grub.texi to reflect new behavior for
      GRUB_ENABLE_LINUX_PARTUUID

    - Updated 10_linux.in logic to favor the PARTUUID when
      GRUB_ENABLE_LINUX_PARTUUID is enabled and GRUB_DISABLE_LINUX_UUID
      is disabled.

Changes from Patch v3:
    - Removed flex-2.6.3 compatibility patch

    - Removed Steve Kenton's patch

Changes from Patch v2:
    - Added flex-2.6.3 compatibility patch

    - Fixed a GPT partition read error

    - Added Steve Kenton's patch

    - Changed struct grub_part_gpt_type name to struct
      grub_part_gpt_part_guid

    - Changed grub_part_gpt_type_t typedef name to grub_part_gpt_guid_t

    - Added sprint_gpt_guid to Steve Kenton's patch

    - Updated v1 and Steve Kenton's patch to use similar methods when
      reading partition GUIDs.

Changes from Patch v1:
    - Added GRUB_ENABLE_LINUX_PARTUUID variable description to grub.texi

    - Removed added gpt_part_guid copy logic from
      grub_gpt_partition_map_iterate()

    - Removed added NT disk signature copy logic from
      grub_partition_msdos_iterate()

    - Removed modifications to partition number increment logic

    - Removed added guid union definition.

    - Added GRUB_ENABLE_LINUX_PARTUUID to grub-mkconfig.in export list

    - Moved PRINT_GPT_PARTTYPE printing logic to print_gpt_guid()
      function in grub-probe.c

    - Updated PRINT_GPT_PARTTYPE case to call print_gpt_guid() function
      in grub-probe.c.

    - Created probe_partuuid() function in grub-probe.c

    - Updated print == PRINT_PARTUUID check logic in probe() to call
      probe_partuuid().

    - Updated UUID logic in 10_linux.in to enable root=PARTUUID feature
      only if GRUB_DISABLE_LINUX_UUID is not set to true,
      and GRUB_DEVICE_PARTUUID is not empty, GRUB_ENABLE_LINUX_PARTUUID
      is set to true.

Hello,

This is a request to 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 specifying 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.

Thanks,
Nicholas Vinson

Nicholas Vinson (4):
  Centralize guid prints
  Update grub_gpt_partentry
  Add PARTUUID detection support to grub-probe
  Update grub script template files

 docs/grub.texi               |  9 +++++
 grub-core/disk/ldm.c         |  2 +-
 grub-core/partmap/gpt.c      |  4 +--
 include/grub/gpt_partition.h |  8 ++---
 util/grub-install.c          |  2 +-
 util/grub-mkconfig.in        |  3 ++
 util/grub-probe.c            | 78 +++++++++++++++++++++++++++++++++++---------
 util/grub.d/10_linux.in      | 12 +++++--
 8 files changed, 92 insertions(+), 26 deletions(-)

-- 
2.16.3

[GRUB PARTUUID PATCH V7 1/4] Centralize guid prints

[Nicholas Vinson]

Define print_gpt_guid(), so there is a central function for printing
GUID strings.  This change is a precursor for later patches which rely
on this logic.

Signed-off-by: Nicholas Vinson <nvinson234@gmail.com>
---
 util/grub-probe.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/util/grub-probe.c b/util/grub-probe.c
index 8ac527d2f..21cb80fbe 100644
--- a/util/grub-probe.c
+++ b/util/grub-probe.c
@@ -129,6 +129,20 @@ get_targets_string (void)
   return str;
 }
 
+static int
+print_gpt_guid (grub_gpt_part_guid_t guid)
+{
+  guid.data1 = grub_le_to_cpu32 (guid.data1);
+  guid.data2 = grub_le_to_cpu16 (guid.data2);
+  guid.data3 = grub_le_to_cpu16 (guid.data3);
+
+  return grub_printf ("%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
+		      guid.data1, guid.data2, guid.data3, guid.data4[0],
+		      guid.data4[1], guid.data4[2], guid.data4[3],
+		      guid.data4[4], guid.data4[5], guid.data4[6],
+		      guid.data4[7]);
+}
+
 static void
 do_print (const char *x, void *data)
 {
@@ -641,21 +655,7 @@ probe (const char *path, char **device_names, char delim)
 
               if (grub_disk_read (dev->disk, p->offset, p->index,
                                   sizeof (gptdata), &gptdata) == 0)
-                {
-                  grub_gpt_part_type_t gpttype;
-                  gpttype.data1 = grub_le_to_cpu32 (gptdata.type.data1);
-                  gpttype.data2 = grub_le_to_cpu16 (gptdata.type.data2);
-                  gpttype.data3 = grub_le_to_cpu16 (gptdata.type.data3);
-                  grub_memcpy (gpttype.data4, gptdata.type.data4, 8);
-
-                  grub_printf ("%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
-                               gpttype.data1, gpttype.data2,
-                               gpttype.data3, gpttype.data4[0], 
-                               gpttype.data4[1], gpttype.data4[2],
-                               gpttype.data4[3], gpttype.data4[4],
-                               gpttype.data4[5], gpttype.data4[6],
-                               gpttype.data4[7]);
-                }
+		print_gpt_guid(gptdata.type);
               dev->disk->partition = p;
             }
           putchar (delim);
-- 
2.16.3

[GRUB PARTUUID PATCH V7 2/4] Update grub_gpt_partentry

[Nicholas Vinson]

Rename grub_gpt_part_type to grub_gpt_part_guid and update
grub_gpt_partentry to use this type for both the partition type GUID
string and the partition GUID string entries.  This change ensures that
the two GUID fields are handled more consistently and helps to simplify
the changes needed to add Linux partition GUID support.

Signed-off-by: Nicholas Vinson <nvinson234@gmail.com>
---
 grub-core/disk/ldm.c         | 2 +-
 grub-core/partmap/gpt.c      | 4 ++--
 include/grub/gpt_partition.h | 8 ++++----
 util/grub-install.c          | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/grub-core/disk/ldm.c b/grub-core/disk/ldm.c
index 0f978ad05..2a22d2d6c 100644
--- a/grub-core/disk/ldm.c
+++ b/grub-core/disk/ldm.c
@@ -135,7 +135,7 @@ msdos_has_ldm_partition (grub_disk_t dsk)
   return has_ldm;
 }
 
-static const grub_gpt_part_type_t ldm_type = GRUB_GPT_PARTITION_TYPE_LDM;
+static const grub_gpt_part_guid_t ldm_type = GRUB_GPT_PARTITION_TYPE_LDM;
 
 /* Helper for gpt_ldm_sector.  */
 static int
diff --git a/grub-core/partmap/gpt.c b/grub-core/partmap/gpt.c
index 83bcba779..103f6796f 100644
--- a/grub-core/partmap/gpt.c
+++ b/grub-core/partmap/gpt.c
@@ -33,10 +33,10 @@ static grub_uint8_t grub_gpt_magic[8] =
     0x45, 0x46, 0x49, 0x20, 0x50, 0x41, 0x52, 0x54
   };
 
-static const grub_gpt_part_type_t grub_gpt_partition_type_empty = GRUB_GPT_PARTITION_TYPE_EMPTY;
+static const grub_gpt_part_guid_t grub_gpt_partition_type_empty = GRUB_GPT_PARTITION_TYPE_EMPTY;
 
 #ifdef GRUB_UTIL
-static const grub_gpt_part_type_t grub_gpt_partition_type_bios_boot = GRUB_GPT_PARTITION_TYPE_BIOS_BOOT;
+static const grub_gpt_part_guid_t grub_gpt_partition_type_bios_boot = GRUB_GPT_PARTITION_TYPE_BIOS_BOOT;
 #endif
 
 /* 512 << 7 = 65536 byte sectors.  */
diff --git a/include/grub/gpt_partition.h b/include/grub/gpt_partition.h
index 1b32f6725..354fe2246 100644
--- a/include/grub/gpt_partition.h
+++ b/include/grub/gpt_partition.h
@@ -22,14 +22,14 @@
 #include <grub/types.h>
 #include <grub/partition.h>
 
-struct grub_gpt_part_type
+struct grub_gpt_part_guid
 {
   grub_uint32_t data1;
   grub_uint16_t data2;
   grub_uint16_t data3;
   grub_uint8_t data4[8];
 } __attribute__ ((aligned(8)));
-typedef struct grub_gpt_part_type grub_gpt_part_type_t;
+typedef struct grub_gpt_part_guid grub_gpt_part_guid_t;
 
 #define GRUB_GPT_PARTITION_TYPE_EMPTY \
   { 0x0, 0x0, 0x0, \
@@ -70,8 +70,8 @@ struct grub_gpt_header
 
 struct grub_gpt_partentry
 {
-  grub_gpt_part_type_t type;
-  grub_uint8_t guid[16];
+  grub_gpt_part_guid_t type;
+  grub_gpt_part_guid_t guid;
   grub_uint64_t start;
   grub_uint64_t end;
   grub_uint64_t attrib;
diff --git a/util/grub-install.c b/util/grub-install.c
index 690f180c5..78d0138cb 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -714,7 +714,7 @@ is_prep_partition (grub_device_t dev)
       if (grub_disk_read (dev->disk, p->offset, p->index,
 			  sizeof (gptdata), &gptdata) == 0)
 	{
-	  const grub_gpt_part_type_t template = {
+	  const grub_gpt_part_guid_t template = {
 	    grub_cpu_to_le32_compile_time (0x9e1a2d38),
 	    grub_cpu_to_le16_compile_time (0xc612),
 	    grub_cpu_to_le16_compile_time (0x4316),
-- 
2.16.3

[GRUB PARTUUID PATCH V7 3/4] Add PARTUUID detection support to grub-probe

[Nicholas Vinson]

Add PARTUUID detection support grub-probe for MBR and GPT partition
schemes.

Signed-off-by: Nicholas Vinson <nvinson234@gmail.com>
---
 util/grub-probe.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/util/grub-probe.c b/util/grub-probe.c
index 21cb80fbe..48ef1e2ec 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,45 @@ probe_partmap (grub_disk_t disk, char delim)
     }
 }
 
+static void
+probe_partuuid (grub_disk_t disk, char delim)
+{
+  grub_partition_t p = disk->partition;
+  /*
+   * 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;
+	    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);
+	}
+      else if (strcmp(disk->partition->partmap->name, "gpt") == 0)
+	{
+	  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);
+	}
+    }
+  disk->partition = p;
+}
+
 static void
 probe_cryptodisk_uuid (grub_disk_t disk, char delim)
 {
@@ -635,6 +677,12 @@ probe (const char *path, char **device_names, char delim)
 	/* Check if dev->disk itself is contained in a partmap.  */
 	probe_partmap (dev->disk, delim);
 
+      else if (print == PRINT_PARTUUID)
+	{
+	  probe_partuuid (dev->disk, delim);
+	  putchar (delim);
+	}
+
       else if (print == PRINT_MSDOS_PARTTYPE)
 	{
 	  if (dev->disk->partition
-- 
2.16.3

[GRUB PARTUUID PATCH V7 4/4] Update grub script template files

[Nicholas Vinson]

Update grub-mkconfig.in and 10_linux.in to support grub-probe's new
partuuid target.  Update grub.texi documenation.

Signed-off-by: Nicholas Vinson <nvinson234@gmail.com>
---
 docs/grub.texi          |  9 +++++++++
 util/grub-mkconfig.in   |  3 +++
 util/grub.d/10_linux.in | 12 +++++++++---
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/docs/grub.texi b/docs/grub.texi
index 65b4bbeda..019ce5d03 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -1424,6 +1424,15 @@ the Linux kernel, using a @samp{root=UUID=...} kernel parameter.  This is
 usually more reliable, but in some cases it may not be appropriate.  To
 disable the use of UUIDs, set this option to @samp{true}.
 
+@item GRUB_ENABLE_LINUX_PARTUUID
+Setting this option to @samp{true} changes the behavior of
+@command{grub-mkconfig} so that it identifies the device containing the root
+filesystem by the partition UUID via the @samp{root=PARTUUID=...} kernel
+parameter.  This is desirable for Linux systems where identifying the root
+filesystem by its filesystem UUID or device name is inappropriate.  However,
+this option is only supported in Linux kernel versions 2.6.37 (3.10 for systems
+using the MSDOS partition scheme) or newer.
+
 @item GRUB_DISABLE_RECOVERY
 If this option is set to @samp{true}, disable the generation of recovery
 mode menu entries.
diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
index 35ef583b0..9a1f92bdf 100644
--- a/util/grub-mkconfig.in
+++ b/util/grub-mkconfig.in
@@ -134,6 +134,7 @@ fi
 # Device containing our userland.  Typically used for root= parameter.
 GRUB_DEVICE="`${grub_probe} --target=device /`"
 GRUB_DEVICE_UUID="`${grub_probe} --device ${GRUB_DEVICE} --target=fs_uuid 2> /dev/null`" || true
+GRUB_DEVICE_PARTUUID="`${grub_probe} --device ${GRUB_DEVICE} --target=partuuid 2> /dev/null`" || true
 
 # Device containing our /boot partition.  Usually the same as GRUB_DEVICE.
 GRUB_DEVICE_BOOT="`${grub_probe} --target=device /boot`"
@@ -188,6 +189,7 @@ if [ "x${GRUB_ACTUAL_DEFAULT}" = "xsaved" ] ; then GRUB_ACTUAL_DEFAULT="`"${grub
 # override them.
 export GRUB_DEVICE \
   GRUB_DEVICE_UUID \
+  GRUB_DEVICE_PARTUUID \
   GRUB_DEVICE_BOOT \
   GRUB_DEVICE_BOOT_UUID \
   GRUB_FS \
@@ -223,6 +225,7 @@ export GRUB_DEFAULT \
   GRUB_TERMINAL_OUTPUT \
   GRUB_SERIAL_COMMAND \
   GRUB_DISABLE_LINUX_UUID \
+  GRUB_ENABLE_LINUX_PARTUUID \
   GRUB_DISABLE_RECOVERY \
   GRUB_VIDEO_BACKEND \
   GRUB_GFXMODE \
diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
index faedf74e1..53de33bea 100644
--- a/util/grub.d/10_linux.in
+++ b/util/grub.d/10_linux.in
@@ -45,12 +45,18 @@ esac
 
 # btrfs may reside on multiple devices. We cannot pass them as value of root= parameter
 # and mounting btrfs requires user space scanning, so force UUID in this case.
-if [ "x${GRUB_DEVICE_UUID}" = "x" ] || [ "x${GRUB_DISABLE_LINUX_UUID}" = "xtrue" ] \
-    || ! test -e "/dev/disk/by-uuid/${GRUB_DEVICE_UUID}" \
+if [ "x${GRUB_DEVICE_UUID}" = "x" ] && [ "x${GRUB_DEVICE_PARTUUID}" = "x" ] \
+    || [ "x${GRUB_DISABLE_LINUX_UUID}" = "xtrue" ] \
+    || ( ! test -e "/dev/disk/by-uuid/${GRUB_DEVICE_UUID}" \
+        && ! test -e "/dev/disk/by-partuuid/${GRUB_DEVICE_PARTUUID}" ) \
     || ( test -e "${GRUB_DEVICE}" && uses_abstraction "${GRUB_DEVICE}" lvm ); then
   LINUX_ROOT_DEVICE=${GRUB_DEVICE}
-else
+elif [ "x${GRUB_ENABLE_LINUX_PARTUUID}" != "xtrue" ] \
+    || [ "x${GRUB_DEVICE_PARTUUID}" = "x" ] \
+    || ! test -e "/dev/disk/by-partuuid/${GRUB_DEVICE_PARTUUID}"; then
   LINUX_ROOT_DEVICE=UUID=${GRUB_DEVICE_UUID}
+else
+  LINUX_ROOT_DEVICE=PARTUUID=${GRUB_DEVICE_PARTUUID}
 fi
 
 case x"$GRUB_FS" in
-- 
2.16.3

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

[Daniel Kiper]

On Mon, Mar 26, 2018 at 11:07:58PM -0700, Nicholas Vinson wrote:
> Add PARTUUID detection support grub-probe for MBR and GPT partition
> schemes.
>
> Signed-off-by: Nicholas Vinson <nvinson234@gmail.com>
> ---
>  util/grub-probe.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>
> diff --git a/util/grub-probe.c b/util/grub-probe.c
> index 21cb80fbe..48ef1e2ec 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,45 @@ probe_partmap (grub_disk_t disk, char delim)
>      }
>  }
>
> +static void
> +probe_partuuid (grub_disk_t disk, char delim)
> +{
> +  grub_partition_t p = disk->partition;

Lack of empty line.

> +  /*
> +   * 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;
> +	    disk->partition = p->parent;
> +
> +	    if (grub_disk_read (disk, 0, GRUB_BOOT_MACHINE_WINDOWS_NT_MAGIC,
> +				sizeof(nt_disk_sig), &nt_disk_sig) == 0)
> +

Redundant empty line.

> +		grub_printf ("%08x-%02x",
> +			     grub_le_to_cpu32(nt_disk_sig), 1 + p->number);
> +	}
> +      else if (strcmp(disk->partition->partmap->name, "gpt") == 0)
> +	{
> +	  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);
> +	}

Why "disk->partition = p;" is not here?
Because compiler complains if it is here?

Anyway, if I know the reason for above I can fix
earlier ntipicks myself before commiting this patch.

Well, "disk->partition = p->parent;" can be before
"if (strcmp(disk->partition->partmap->name, "msdos") == 0)".
Am I right?

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

Daniel

Re: [GRUB PARTUUID PATCH V7 0/4] Add PARTUUID detection support

[Daniel Kiper]

Hi,

On Mon, Mar 26, 2018 at 11:07:55PM -0700, Nicholas Vinson wrote:

[...]

> Hello,
>
> This is a request to 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 specifying 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.

Patches are in good shape right now. I have some questions WRT 3rd patch.
Please reply. If I am OK with explanations and there is no objections
I will this patch series in a week or so.

Daniel

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

[Nick Vinson]

[-- Attachment #1.1: Type: text/plain, Size: 3758 bytes --]

On 03/27/2018 01:52 PM, Daniel Kiper wrote:
> On Mon, Mar 26, 2018 at 11:07:58PM -0700, Nicholas Vinson wrote:
>> Add PARTUUID detection support grub-probe for MBR and GPT partition
>> schemes.
>>
>> Signed-off-by: Nicholas Vinson <nvinson234@gmail.com>
>> ---
>>  util/grub-probe.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 48 insertions(+)
>>
>> diff --git a/util/grub-probe.c b/util/grub-probe.c
>> index 21cb80fbe..48ef1e2ec 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,45 @@ probe_partmap (grub_disk_t disk, char delim)
>>      }
>>  }
>>
>> +static void
>> +probe_partuuid (grub_disk_t disk, char delim)
>> +{
>> +  grub_partition_t p = disk->partition;
> 
> Lack of empty line.

added an empty line.

> 
>> +  /*
>> +   * 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;
>> +	    disk->partition = p->parent;
>> +
>> +	    if (grub_disk_read (disk, 0, GRUB_BOOT_MACHINE_WINDOWS_NT_MAGIC,
>> +				sizeof(nt_disk_sig), &nt_disk_sig) == 0)
>> +
> 
> Redundant empty line.

removed the empty line.
> 
>> +		grub_printf ("%08x-%02x",
>> +			     grub_le_to_cpu32(nt_disk_sig), 1 + p->number);
>> +	}
>> +      else if (strcmp(disk->partition->partmap->name, "gpt") == 0)
>> +	{
>> +	  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);
>> +	}
> 
> Why "disk->partition = p;" is not here?
> Because compiler complains if it is here?
> 

I misread my own diff and thought you had asked for me to put it below
instead of here.  Either location works, so it's not too critical where
it's put.

> Anyway, if I know the reason for above I can fix
> earlier ntipicks myself before commiting this patch.>
> Well, "disk->partition = p->parent;" can be before
> "if (strcmp(disk->partition->partmap->name, "msdos") == 0)".
> Am I right?
> 

Yes, but it'll require some changes to the checks to make such a change
work.  For example, "disk->partition->partmap->name" would have to be
changed to "p->partmap->name"

I'll send a second email with an updated patch for this commit.  If you
would like me to regenerate the entire patch set, please let me know.

Thanks,
Nicholas Vinson

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[GRUB PARTUUID PATCH V7.1 3/4] Add PARTUUID detection support to grub-probe

[Nicholas Vinson]

Add PARTUUID detection support grub-probe for MBR and GPT partition
schemes.

Signed-off-by: Nicholas Vinson <nvinson234@gmail.com>
---
 util/grub-probe.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/util/grub-probe.c b/util/grub-probe.c
index 21cb80fbe..8a013548a 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,43 @@ probe_partmap (grub_disk_t disk, char delim)
     }
 }
 
+static void
+probe_partuuid (grub_disk_t disk, char delim)
+{
+  grub_partition_t p = disk->partition;
+
+  /*
+   * Nested partitions not supported for now.
+   * Non-nested partitions must have disk->partition->parent == NULL
+   */
+  if (p && p->parent == NULL)
+    {
+      disk->partition = p->parent;
+      if (strcmp(p->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;
+
+	    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);
+	}
+      else if (strcmp(p->partmap->name, "gpt") == 0)
+	{
+	  struct grub_gpt_partentry gptdata;
+
+	  if (grub_disk_read (disk, p->offset, p->index,
+			      sizeof(gptdata), &gptdata) == 0)
+	    print_gpt_guid(gptdata.guid);
+	}
+      disk->partition = p;
+    }
+}
+
 static void
 probe_cryptodisk_uuid (grub_disk_t disk, char delim)
 {
@@ -635,6 +675,12 @@ probe (const char *path, char **device_names, char delim)
 	/* Check if dev->disk itself is contained in a partmap.  */
 	probe_partmap (dev->disk, delim);
 
+      else if (print == PRINT_PARTUUID)
+	{
+	  probe_partuuid (dev->disk, delim);
+	  putchar (delim);
+	}
+
       else if (print == PRINT_MSDOS_PARTTYPE)
 	{
 	  if (dev->disk->partition
-- 
2.16.3

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

[Daniel Kiper]

On Tue, Mar 27, 2018 at 09:30:10PM -0700, Nick Vinson wrote:
> On 03/27/2018 01:52 PM, Daniel Kiper wrote:
> > On Mon, Mar 26, 2018 at 11:07:58PM -0700, Nicholas Vinson wrote:
> >> Add PARTUUID detection support grub-probe for MBR and GPT partition
> >> schemes.
> >>
> >> Signed-off-by: Nicholas Vinson <nvinson234@gmail.com>
> >> ---
> >>  util/grub-probe.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 48 insertions(+)
> >>
> >> diff --git a/util/grub-probe.c b/util/grub-probe.c
> >> index 21cb80fbe..48ef1e2ec 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,45 @@ probe_partmap (grub_disk_t disk, char delim)
> >>      }
> >>  }
> >>
> >> +static void
> >> +probe_partuuid (grub_disk_t disk, char delim)
> >> +{
> >> +  grub_partition_t p = disk->partition;
> >
> > Lack of empty line.
>
> added an empty line.
>
> >
> >> +  /*
> >> +   * 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;
> >> +	    disk->partition = p->parent;
> >> +
> >> +	    if (grub_disk_read (disk, 0, GRUB_BOOT_MACHINE_WINDOWS_NT_MAGIC,
> >> +				sizeof(nt_disk_sig), &nt_disk_sig) == 0)
> >> +
> >
> > Redundant empty line.
>
> removed the empty line.
> >
> >> +		grub_printf ("%08x-%02x",
> >> +			     grub_le_to_cpu32(nt_disk_sig), 1 + p->number);
> >> +	}
> >> +      else if (strcmp(disk->partition->partmap->name, "gpt") == 0)
> >> +	{
> >> +	  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);
> >> +	}
> >
> > Why "disk->partition = p;" is not here?
> > Because compiler complains if it is here?
>
> I misread my own diff and thought you had asked for me to put it below
> instead of here.  Either location works, so it's not too critical where
> it's put.

Great! So, please put it there where I have asked for earlier.

> > Anyway, if I know the reason for above I can fix
> > earlier ntipicks myself before commiting this patch.>
> > Well, "disk->partition = p->parent;" can be before
> > "if (strcmp(disk->partition->partmap->name, "msdos") == 0)".
> > Am I right?
>
> Yes, but it'll require some changes to the checks to make such a change
> work.  For example, "disk->partition->partmap->name" would have to be
> changed to "p->partmap->name"

I am OK with that change.

> I'll send a second email with an updated patch for this commit.  If you
> would like me to regenerate the entire patch set, please let me know.

Entire patchset please. If you do not change anything in other patches
you can add my RB to them.

Daniel