[GRUB PARTUUID PATCH 0/2] GRUB: Add PARTUUID Detection Support

[GRUB PARTUUID PATCH 0/2] GRUB: Add PARTUUID Detection Support

[Nicholas Vinson]

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 (2):
  Add PARTUUID detection support to grub-probe
  Update grub script template files

 grub-core/partmap/gpt.c   |  2 ++
 grub-core/partmap/msdos.c | 12 ++++++++--
 include/grub/partition.h  |  9 +++++++-
 util/grub-mkconfig.in     |  2 ++
 util/grub-probe.c         | 58 +++++++++++++++++++++++++++++++++++++++++++++++
 util/grub.d/10_linux.in   | 11 +++++++--
 6 files changed, 89 insertions(+), 5 deletions(-)

-- 
2.9.0

[GRUB PARTUUID PATCH 1/2] Add PARTUUID detection support to grub-probe

[Nicholas Vinson]

Add PARTUUID detection to grub-probe.  The grub-probe utility is used by
grub-mkconfig to determine the filesystem [GU]UID, so updating it to be
able to return partition [GU]UIDs seemed like the natural choice.  The
other obvious choice was to rely on Linux userland tools and /dev file
structure which would added to the runtime dependencies of grub-probe.

Signed-off-by: Nicholas Vinson <nvinson234@gmail.com>
---
 grub-core/partmap/gpt.c   |  2 ++
 grub-core/partmap/msdos.c | 12 ++++++++--
 include/grub/partition.h  |  9 +++++++-
 util/grub-probe.c         | 58 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 78 insertions(+), 3 deletions(-)

diff --git a/grub-core/partmap/gpt.c b/grub-core/partmap/gpt.c
index 83bcba7..fd0bbef 100644
--- a/grub-core/partmap/gpt.c
+++ b/grub-core/partmap/gpt.c
@@ -99,6 +99,8 @@ grub_gpt_partition_map_iterate (grub_disk_t disk,
       if (grub_memcmp (&grub_gpt_partition_type_empty, &entry.type,
 		       sizeof (grub_gpt_partition_type_empty)))
 	{
+          grub_memcpy(part.guid.gpt, entry.guid, sizeof(part.guid.gpt));
+
 	  /* Calculate the first block and the size of the partition.  */
 	  part.start = grub_le_to_cpu64 (entry.start) << sector_log;
 	  part.len = (grub_le_to_cpu64 (entry.end)
diff --git a/grub-core/partmap/msdos.c b/grub-core/partmap/msdos.c
index 6d4b455..79bb5b2 100644
--- a/grub-core/partmap/msdos.c
+++ b/grub-core/partmap/msdos.c
@@ -169,6 +169,13 @@ grub_partition_msdos_iterate (grub_disk_t disk,
 	if (mbr.entries[i].flag & 0x7f)
 	  return grub_error (GRUB_ERR_BAD_PART_TABLE, "bad boot flag");
 
+      /*
+       * Copy off the NT Disk signature.  Linux uses this to compute the
+       * PARTUUID.  The disk signature is 440 bytes in and 4 bytes long.
+       */
+      if (p.offset == 0)
+        grub_memcpy(p.guid.mbr, mbr.code + 440, sizeof(p.guid.mbr));
+
       /* Analyze DOS partitions.  */
       for (p.index = 0; p.index < 4; p.index++)
 	{
@@ -191,7 +198,7 @@ grub_partition_msdos_iterate (grub_disk_t disk,
 	  if (! grub_msdos_partition_is_empty (e->type)
 	      && ! grub_msdos_partition_is_extended (e->type))
 	    {
-	      p.number++;
+	      p.guid.mbr[4] = ++p.number;
 
 	      if (hook (disk, &p, hook_data))
 		return grub_errno;
@@ -199,7 +206,7 @@ grub_partition_msdos_iterate (grub_disk_t disk,
 	  else if (p.number < 3)
 	    /* If this partition is a logical one, shouldn't increase the
 	       partition number.  */
-	    p.number++;
+	    p.guid.mbr[4] = ++p.number;
 	}
 
       /* Find an extended partition.  */
@@ -209,6 +216,7 @@ grub_partition_msdos_iterate (grub_disk_t disk,
 
 	  if (grub_msdos_partition_is_extended (e->type))
 	    {
+              p.guid.mbr[4] = 4 + i; // logical partitions start with 4.
 	      p.offset = ext_offset
 		+ (grub_le_to_cpu32 (e->start)
 		   << (disk->log_sector_size - GRUB_DISK_SECTOR_BITS));
diff --git a/include/grub/partition.h b/include/grub/partition.h
index 7adb7ec..4a80bf7 100644
--- a/include/grub/partition.h
+++ b/include/grub/partition.h
@@ -66,6 +66,13 @@ struct grub_partition
   /* The partition number.  */
   int number;
 
+  /* Unique partition GUID. */
+  union
+  {
+      grub_uint8_t gpt[16];
+      grub_uint8_t mbr[5];
+  } guid;
+
   /* The start sector (relative to parent).  */
   grub_disk_addr_t start;
 
@@ -84,7 +91,7 @@ struct grub_partition
   /* The type partition map.  */
   grub_partition_map_t partmap;
 
-  /* The type of partition whne it's on MSDOS.
+  /* The type of partition when it's on MSDOS.
      Used for embedding detection.  */
   grub_uint8_t msdostype;
 };
diff --git a/util/grub-probe.c b/util/grub-probe.c
index 8ac527d..5ea9c4c 100644
--- a/util/grub-probe.c
+++ b/util/grub-probe.c
@@ -62,6 +62,7 @@ enum {
   PRINT_DRIVE,
   PRINT_DEVICE,
   PRINT_PARTMAP,
+  PRINT_PARTUUID,
   PRINT_ABSTRACTION,
   PRINT_CRYPTODISK_UUID,
   PRINT_HINT_STR,
@@ -85,6 +86,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",
@@ -617,6 +619,62 @@ probe (const char *path, char **device_names, char delim)
       else if (print == PRINT_CRYPTODISK_UUID)
 	probe_cryptodisk_uuid (dev->disk, delim);
 
+      else if (print == PRINT_PARTUUID && dev->disk->partition)
+        {
+          grub_uint8_t be_guid[16];
+          int i;
+          int guid_len;
+          if (strcmp(dev->disk->partition->partmap->name, "gpt") == 0)
+            {
+              guid_len = sizeof(dev->disk->partition->guid.gpt);
+              /**
+               * The GUID disk format is LE(4) LE(2) LE(2) BE(8).
+               * where LE(n) means n-bytes little-endian formatted and
+               * BE(n) means n-bytes big-endian formatted.
+               */
+              for(i = 3; i >= 0; i--)
+                {
+                  be_guid[3 - i] = dev->disk->partition->guid.gpt[i];
+                }
+              for (i = 1; i >= 0; i--)
+                {
+                  be_guid[5 - i] = dev->disk->partition->guid.gpt[i + 4];
+                  be_guid[7 - i] = dev->disk->partition->guid.gpt[i + 6];
+                }
+              for (i = 7; i >= 0; i--)
+                {
+                  be_guid[i + 8] = dev->disk->partition->guid.gpt[i + 8];
+                }
+            }
+          else if (strcmp(dev->disk->partition->partmap->name, "msdos") == 0)
+            {
+              guid_len = sizeof(dev->disk->partition->guid.mbr);
+              /*
+               * First 4 bytes are in LE order and need to be swapped them to BE
+               * order.
+               */
+              for(i = 3; i >= 0; i--)
+                {
+                  be_guid[3 - i] = dev->disk->partition->guid.mbr[i];
+                }
+                /* Adjust the last number so that it is 1-indexed. */
+                be_guid[4] = dev->disk->partition->guid.mbr[4] + 1;
+            }
+            for (i = 0; i < guid_len; i++)
+              {
+                switch(i)
+                  {
+                  case 4:
+                  case 6:
+                  case 8:
+                  case 10:
+                    printf("-");
+                  default:
+                    printf("%02x", be_guid[i]);
+                  }
+              }
+          putchar(delim);
+        }
       else if (print == PRINT_PARTMAP)
 	/* Check if dev->disk itself is contained in a partmap.  */
 	probe_partmap (dev->disk, delim);
-- 
2.9.0

[GRUB PARTUUID PATCH 2/2] Update grub script template files

[Nicholas Vinson]

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

Signed-off-by: Nicholas Vinson <nvinson234@gmail.com>
---
 util/grub-mkconfig.in   |  2 ++
 util/grub.d/10_linux.in | 11 +++++++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
index f8496d2..fc42462 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`"
@@ -182,6 +183,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 \
diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
index de9044c..8081fdb 100644
--- a/util/grub.d/10_linux.in
+++ b/util/grub.d/10_linux.in
@@ -220,8 +220,15 @@ while [ "x$list" != "x" ] ; do
     gettext_printf "Found initrd image: %s\n" "${dirname}/${initrd}" >&2
   elif test -z "${initramfs}" ; then
     # "UUID=" and "ZFS=" magic is parsed by initrd or initramfs.  Since there's
-    # no initrd or builtin initramfs, it can't work here.
-    linux_root_device_thisversion=${GRUB_DEVICE}
+    # no initrd or builtin initramfs, it can't work here.  However, if
+    # GRUB_DEVICE_PARTUUID is not empty we can use that here if
+    # GRUD_DISABLE_LINUX_UUID is not set to true.
+    if [ "x${GRUB_DISABLE_LINUX_UUID}" != "xtrue" ]
+        && [ "x${GRUB_DEVICE_PARTUUID}" != "x" ]; then
+        linux_root_device_thisversion="PARTUUID=${GRUB_DEVICE_PARTUUID}"
+    else
+        linux_root_device_thisversion=${GRUB_DEVICE}
+    fi
   fi
 
   if [ "x$is_top_level" = xtrue ] && [ "x${GRUB_DISABLE_SUBMENU}" != xy ]; then
-- 
2.9.0

Re: [GRUB PARTUUID PATCH 2/2] Update grub script template files

[Nicholas Vinson]

Thanks,
Nicholas Vinson

Signed-off-by: Nicholas Vinson <nvinson234@gmail.com>
---
diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
index 5a78513..dc9bab0 100644
--- a/util/grub.d/10_linux.in
+++ b/util/grub.d/10_linux.in
@@ -224,11 +224,11 @@ while [ "x$list" != "x" ] ; do
   elif test -z "${initramfs}" ; then
     # "UUID=" and "ZFS=" magic is parsed by initrd or initramfs.  Since there's
     # no initrd or builtin initramfs, it can't work here.  However, if
     # GRUB_DEVICE_PARTUUID is not empty we can use that here if
     # GRUD_DISABLE_LINUX_UUID is not set to true.
-    if [ "x${GRUB_DISABLE_LINUX_UUID}" != "xtrue" ]
+    if [ "x${GRUB_DISABLE_LINUX_UUID}" != "xtrue" ]	\
         && [ "x${GRUB_DEVICE_PARTUUID}" != "x" ]; then
         linux_root_device_thisversion="PARTUUID=${GRUB_DEVICE_PARTUUID}"
     else
         linux_root_device_thisversion=${GRUB_DEVICE}
     fi

Re: [GRUB PARTUUID PATCH 2/2] Update grub script template files

[Nick Vinson]

My apologies.  It looks like most of my reply was cut-off when this
email was sent.  I'm pasting the missing part below.

Thanks,
Nicholas Vinson

> I found a minor error in my original patch for 10_linux.in.  I created the patch
> below to fix it.  My apologies for not noticing it in the original
> submission.

On 07/19/2016 07:07 AM, Nicholas Vinson wrote:
> Thanks,
> Nicholas Vinson
> 
> Signed-off-by: Nicholas Vinson <nvinson234@gmail.com>
> ---
> diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
> index 5a78513..dc9bab0 100644
> --- a/util/grub.d/10_linux.in
> +++ b/util/grub.d/10_linux.in
> @@ -224,11 +224,11 @@ while [ "x$list" != "x" ] ; do
>    elif test -z "${initramfs}" ; then
>      # "UUID=" and "ZFS=" magic is parsed by initrd or initramfs.  Since there's
>      # no initrd or builtin initramfs, it can't work here.  However, if
>      # GRUB_DEVICE_PARTUUID is not empty we can use that here if
>      # GRUD_DISABLE_LINUX_UUID is not set to true.
> -    if [ "x${GRUB_DISABLE_LINUX_UUID}" != "xtrue" ]
> +    if [ "x${GRUB_DISABLE_LINUX_UUID}" != "xtrue" ]	\
>          && [ "x${GRUB_DEVICE_PARTUUID}" != "x" ]; then
>          linux_root_device_thisversion="PARTUUID=${GRUB_DEVICE_PARTUUID}"
>      else
>          linux_root_device_thisversion=${GRUB_DEVICE}
>      fi
> 

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

[Andrei Borzenkov]

20.06.2016 04:37, Nicholas Vinson пишет:
> Add PARTUUID detection to grub-probe.  The grub-probe utility is used by
> grub-mkconfig to determine the filesystem [GU]UID, so updating it to be
> able to return partition [GU]UIDs seemed like the natural choice.  The
> other obvious choice was to rely on Linux userland tools and /dev file
> structure which would added to the runtime dependencies of grub-probe.
> 
> Signed-off-by: Nicholas Vinson <nvinson234@gmail.com>
> ---
>  grub-core/partmap/gpt.c   |  2 ++
>  grub-core/partmap/msdos.c | 12 ++++++++--
>  include/grub/partition.h  |  9 +++++++-
>  util/grub-probe.c         | 58 +++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 78 insertions(+), 3 deletions(-)
> 
> diff --git a/grub-core/partmap/gpt.c b/grub-core/partmap/gpt.c
> index 83bcba7..fd0bbef 100644
> --- a/grub-core/partmap/gpt.c
> +++ b/grub-core/partmap/gpt.c
> @@ -99,6 +99,8 @@ grub_gpt_partition_map_iterate (grub_disk_t disk,
>        if (grub_memcmp (&grub_gpt_partition_type_empty, &entry.type,
>  		       sizeof (grub_gpt_partition_type_empty)))
>  	{
> +          grub_memcpy(part.guid.gpt, entry.guid, sizeof(part.guid.gpt));
> +
>  	  /* Calculate the first block and the size of the partition.  */
>  	  part.start = grub_le_to_cpu64 (entry.start) << sector_log;
>  	  part.len = (grub_le_to_cpu64 (entry.end)
> diff --git a/grub-core/partmap/msdos.c b/grub-core/partmap/msdos.c
> index 6d4b455..79bb5b2 100644
> --- a/grub-core/partmap/msdos.c
> +++ b/grub-core/partmap/msdos.c
> @@ -169,6 +169,13 @@ grub_partition_msdos_iterate (grub_disk_t disk,
>  	if (mbr.entries[i].flag & 0x7f)
>  	  return grub_error (GRUB_ERR_BAD_PART_TABLE, "bad boot flag");
>  
> +      /*
> +       * Copy off the NT Disk signature.  Linux uses this to compute the
> +       * PARTUUID.  The disk signature is 440 bytes in and 4 bytes long.
> +       */
> +      if (p.offset == 0)
> +        grub_memcpy(p.guid.mbr, mbr.code + 440, sizeof(p.guid.mbr));
> +
>        /* Analyze DOS partitions.  */
>        for (p.index = 0; p.index < 4; p.index++)
>  	{
> @@ -191,7 +198,7 @@ grub_partition_msdos_iterate (grub_disk_t disk,
>  	  if (! grub_msdos_partition_is_empty (e->type)
>  	      && ! grub_msdos_partition_is_extended (e->type))
>  	    {
> -	      p.number++;
> +	      p.guid.mbr[4] = ++p.number;
>  
>  	      if (hook (disk, &p, hook_data))
>  		return grub_errno;
> @@ -199,7 +206,7 @@ grub_partition_msdos_iterate (grub_disk_t disk,
>  	  else if (p.number < 3)
>  	    /* If this partition is a logical one, shouldn't increase the
>  	       partition number.  */
> -	    p.number++;
> +	    p.guid.mbr[4] = ++p.number;
>  	}
>  
>        /* Find an extended partition.  */
> @@ -209,6 +216,7 @@ grub_partition_msdos_iterate (grub_disk_t disk,
>  
>  	  if (grub_msdos_partition_is_extended (e->type))
>  	    {
> +              p.guid.mbr[4] = 4 + i; // logical partitions start with 4.
>  	      p.offset = ext_offset
>  		+ (grub_le_to_cpu32 (e->start)
>  		   << (disk->log_sector_size - GRUB_DISK_SECTOR_BITS));
> diff --git a/include/grub/partition.h b/include/grub/partition.h
> index 7adb7ec..4a80bf7 100644
> --- a/include/grub/partition.h
> +++ b/include/grub/partition.h
> @@ -66,6 +66,13 @@ struct grub_partition
>    /* The partition number.  */
>    int number;
>  
> +  /* Unique partition GUID. */
> +  union
> +  {
> +      grub_uint8_t gpt[16];
> +      grub_uint8_t mbr[5];
> +  } guid;
> +
>    /* The start sector (relative to parent).  */
>    grub_disk_addr_t start;
>  
> @@ -84,7 +91,7 @@ struct grub_partition
>    /* The type partition map.  */
>    grub_partition_map_t partmap;
>  
> -  /* The type of partition whne it's on MSDOS.
> +  /* The type of partition when it's on MSDOS.
>       Used for embedding detection.  */
>    grub_uint8_t msdostype;
>  };

As long as this is used by grub-probe only there is no need to bloat
boot time code.

> diff --git a/util/grub-probe.c b/util/grub-probe.c
> index 8ac527d..5ea9c4c 100644
> --- a/util/grub-probe.c
> +++ b/util/grub-probe.c
> @@ -62,6 +62,7 @@ enum {
>    PRINT_DRIVE,
>    PRINT_DEVICE,
>    PRINT_PARTMAP,
> +  PRINT_PARTUUID,
>    PRINT_ABSTRACTION,
>    PRINT_CRYPTODISK_UUID,
>    PRINT_HINT_STR,
> @@ -85,6 +86,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",
> @@ -617,6 +619,62 @@ probe (const char *path, char **device_names, char delim)
>        else if (print == PRINT_CRYPTODISK_UUID)
>  	probe_cryptodisk_uuid (dev->disk, delim);
>  
> +      else if (print == PRINT_PARTUUID && dev->disk->partition)
> +        {
> +          grub_uint8_t be_guid[16];
> +          int i;
> +          int guid_len;
> +          if (strcmp(dev->disk->partition->partmap->name, "gpt") == 0)
> +            {
> +              guid_len = sizeof(dev->disk->partition->guid.gpt);
> +              /**
> +               * The GUID disk format is LE(4) LE(2) LE(2) BE(8).
> +               * where LE(n) means n-bytes little-endian formatted and
> +               * BE(n) means n-bytes big-endian formatted.
> +               */
> +              for(i = 3; i >= 0; i--)
> +                {
> +                  be_guid[3 - i] = dev->disk->partition->guid.gpt[i];
> +                }
> +              for (i = 1; i >= 0; i--)
> +                {
> +                  be_guid[5 - i] = dev->disk->partition->guid.gpt[i + 4];
> +                  be_guid[7 - i] = dev->disk->partition->guid.gpt[i + 6];
> +                }
> +              for (i = 7; i >= 0; i--)
> +                {
> +                  be_guid[i + 8] = dev->disk->partition->guid.gpt[i + 8];
> +                }
> +            }
> +          else if (strcmp(dev->disk->partition->partmap->name, "msdos") == 0)
> +            {
> +              guid_len = sizeof(dev->disk->partition->guid.mbr);
> +              /*
> +               * First 4 bytes are in LE order and need to be swapped them to BE
> +               * order.
> +               */
> +              for(i = 3; i >= 0; i--)
> +                {
> +                  be_guid[3 - i] = dev->disk->partition->guid.mbr[i];
> +                }
> +                /* Adjust the last number so that it is 1-indexed. */
> +                be_guid[4] = dev->disk->partition->guid.mbr[4] + 1;
> +            }
> +            for (i = 0; i < guid_len; i++)
> +              {
> +                switch(i)
> +                  {
> +                  case 4:
> +                  case 6:
> +                  case 8:
> +                  case 10:
> +                    printf("-");
> +                  default:
> +                    printf("%02x", be_guid[i]);
> +                  }
> +              }
> +          putchar(delim);
> +        }
>        else if (print == PRINT_PARTMAP)
>  	/* Check if dev->disk itself is contained in a partmap.  */
>  	probe_partmap (dev->disk, delim);
> 

You can simply read partition table and build UUID in place. See
PRINT_GPT_PARTTYPE as example.

Re: [GRUB PARTUUID PATCH 2/2] Update grub script template files

[Andrei Borzenkov]

20.06.2016 04:37, Nicholas Vinson пишет:
> Update grub-mkconfig.in and 10_linux.in to support grub-probe's new
> partuuid target.
> 
> Signed-off-by: Nicholas Vinson <nvinson234@gmail.com>
> ---
>  util/grub-mkconfig.in   |  2 ++
>  util/grub.d/10_linux.in | 11 +++++++++--
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
> index f8496d2..fc42462 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`"
> @@ -182,6 +183,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 \
> diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
> index de9044c..8081fdb 100644
> --- a/util/grub.d/10_linux.in
> +++ b/util/grub.d/10_linux.in
> @@ -220,8 +220,15 @@ while [ "x$list" != "x" ] ; do
>      gettext_printf "Found initrd image: %s\n" "${dirname}/${initrd}" >&2
>    elif test -z "${initramfs}" ; then
>      # "UUID=" and "ZFS=" magic is parsed by initrd or initramfs.  Since there's
> -    # no initrd or builtin initramfs, it can't work here.
> -    linux_root_device_thisversion=${GRUB_DEVICE}
> +    # no initrd or builtin initramfs, it can't work here.  However, if
> +    # GRUB_DEVICE_PARTUUID is not empty we can use that here if
> +    # GRUD_DISABLE_LINUX_UUID is not set to true.
> +    if [ "x${GRUB_DISABLE_LINUX_UUID}" != "xtrue" ]
> +        && [ "x${GRUB_DEVICE_PARTUUID}" != "x" ]; then
> +        linux_root_device_thisversion="PARTUUID=${GRUB_DEVICE_PARTUUID}"

Well, PARTUUID appeared first in 2.6.37 and MSDOS "UUID" in 3.10.
Unfortunately we have no way to check for it, even as fragile as stored
kernel config. So I am not sure we should do it by default. And if we
add some extra knob to turn it on, as you mentioned yourself, you can
simply add root=PARTUUID=xxx to stored kernel command line.

One more consideration is that reinstalling in existing partition will
invalidate FS UUID stored in grub.cfg which is arguably the right thing
because you now have something different there, but PARTUUID will most
likely remain the same.

> +    else
> +        linux_root_device_thisversion=${GRUB_DEVICE}
> +    fi
>    fi
>  
>    if [ "x$is_top_level" = xtrue ] && [ "x${GRUB_DISABLE_SUBMENU}" != xy ]; then
> 

Re: [GRUB PARTUUID PATCH 2/2] Update grub script template files

[Nick Vinson]

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



On 07/26/2016 10:01 PM, Andrei Borzenkov wrote:
> 20.06.2016 04:37, Nicholas Vinson пишет:
>> Update grub-mkconfig.in and 10_linux.in to support grub-probe's new
>> partuuid target.
>>
>> Signed-off-by: Nicholas Vinson <nvinson234@gmail.com>
>> ---
>>  util/grub-mkconfig.in   |  2 ++
>>  util/grub.d/10_linux.in | 11 +++++++++--
>>  2 files changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
>> index f8496d2..fc42462 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`"
>> @@ -182,6 +183,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 \
>> diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
>> index de9044c..8081fdb 100644
>> --- a/util/grub.d/10_linux.in
>> +++ b/util/grub.d/10_linux.in
>> @@ -220,8 +220,15 @@ while [ "x$list" != "x" ] ; do
>>      gettext_printf "Found initrd image: %s\n" "${dirname}/${initrd}" >&2
>>    elif test -z "${initramfs}" ; then
>>      # "UUID=" and "ZFS=" magic is parsed by initrd or initramfs.  Since there's
>> -    # no initrd or builtin initramfs, it can't work here.
>> -    linux_root_device_thisversion=${GRUB_DEVICE}
>> +    # no initrd or builtin initramfs, it can't work here.  However, if
>> +    # GRUB_DEVICE_PARTUUID is not empty we can use that here if
>> +    # GRUD_DISABLE_LINUX_UUID is not set to true.
>> +    if [ "x${GRUB_DISABLE_LINUX_UUID}" != "xtrue" ]
>> +        && [ "x${GRUB_DEVICE_PARTUUID}" != "x" ]; then
>> +        linux_root_device_thisversion="PARTUUID=${GRUB_DEVICE_PARTUUID}"
> 
> Well, PARTUUID appeared first in 2.6.37 and MSDOS "UUID" in 3.10.
> Unfortunately we have no way to check for it, even as fragile as stored
> kernel config. So I am not sure we should do it by default. And if we
> add some extra knob to turn it on, as you mentioned yourself, you can
> simply add root=PARTUUID=xxx to stored kernel command line.

I've done that using /etc/default/grub and it works, but you end up with
two root=... entries in your kernel command line.  I could have gone
back and edited the grub.cfg to clean up the command-line.

I will adjust the logic so that using PARTUUID is an opt-in feature
instead of an opt-out feature.  Downstream distributions would then be
able to adjust GRUB's defaults in their own packaging and turn this
feature on by default if it made sense for them to do so.

> 
> One more consideration is that reinstalling in existing partition will
> invalidate FS UUID stored in grub.cfg which is arguably the right thing
> because you now have something different there, but PARTUUID will most
> likely remain the same.

I am sorry, but I am not sure I understand the issue you are raising here.

If I boot Linux without an initramfs, I must specify where the root
partition (or device, if the device has no partitions) is.  GRUB
currently does this by setting root to something like '/dev/sda3'.  That
said, this is a Linux limitation not GRUB's.  Linux is unable to use the
FS UUID this early in the boot process.

If I have an initramfs, I *may* be able to use the FS UUID.  In this
case, it all depends on the implementation of my initramfs.  However,
GRUB's current behavior is to assume that all initramfs implementations
understand how to handle UUIDs, that the UUID is the FS UUID, and are
able to mount rootfs properly.

I have no interest in debating if that behavior is correct or not, nor
do I have any interest in changing it.  As currently written, my patch
should favor the FS UUID when an initramfs is detected, the PARTUUID
when it is not, and the Linux device naming scheme when the UUID feature
is disabled or the PARTUUID could not be found.

The rationale for favoring PARTUUID over the linux device name is
because the PARTUUID would allow a system to boot if the device with the
root partition is ever renamed.  In other words, if the device was sda
when grub.cfg was created, but somehow became sdb (or the other way
around), the system would sill be able to boot because the kernel would
still be able to find /sbin/init.  Whereas with the linux device name,
the boot would fail because it would be looking at the wrong device for
/sbin/init (FS UUID provides the same protections, but in theory allows
use to reorder partitions on a single device and still boot successfully
without having to regenerate the grub.cfg).

If there is something I am missing, let me know, so I can address it.

Thanks,
Nicholas Vinson

> 
>> +    else
>> +        linux_root_device_thisversion=${GRUB_DEVICE}
>> +    fi
>>    fi
>>  
>>    if [ "x$is_top_level" = xtrue ] && [ "x${GRUB_DISABLE_SUBMENU}" != xy ]; then
>>
> 


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