* [GRUB PARTUUID PATCH V8 1/4] Centralize guid prints
2018-03-28 15:32 [GRUB PARTUUID PATCH V8 0/4] Add PARTUUID detection support Nicholas Vinson
@ 2018-03-28 15:32 ` Nicholas Vinson
2018-03-28 15:32 ` [GRUB PARTUUID PATCH V8 2/4] Update grub_gpt_partentry Nicholas Vinson
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Nicholas Vinson @ 2018-03-28 15:32 UTC (permalink / raw)
To: grub-devel
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>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [GRUB PARTUUID PATCH V8 2/4] Update grub_gpt_partentry
2018-03-28 15:32 [GRUB PARTUUID PATCH V8 0/4] Add PARTUUID detection support Nicholas Vinson
2018-03-28 15:32 ` [GRUB PARTUUID PATCH V8 1/4] Centralize guid prints Nicholas Vinson
@ 2018-03-28 15:32 ` Nicholas Vinson
2018-03-28 15:32 ` [GRUB PARTUUID PATCH V8 3/4] Add PARTUUID detection support to grub-probe Nicholas Vinson
2018-03-28 15:32 ` [GRUB PARTUUID PATCH V8 4/4] Update grub script template files Nicholas Vinson
3 siblings, 0 replies; 8+ messages in thread
From: Nicholas Vinson @ 2018-03-28 15:32 UTC (permalink / raw)
To: grub-devel
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>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [GRUB PARTUUID PATCH V8 3/4] Add PARTUUID detection support to grub-probe
2018-03-28 15:32 [GRUB PARTUUID PATCH V8 0/4] Add PARTUUID detection support Nicholas Vinson
2018-03-28 15:32 ` [GRUB PARTUUID PATCH V8 1/4] Centralize guid prints Nicholas Vinson
2018-03-28 15:32 ` [GRUB PARTUUID PATCH V8 2/4] Update grub_gpt_partentry Nicholas Vinson
@ 2018-03-28 15:32 ` Nicholas Vinson
2018-04-04 20:09 ` Daniel Kiper
2018-03-28 15:32 ` [GRUB PARTUUID PATCH V8 4/4] Update grub script template files Nicholas Vinson
3 siblings, 1 reply; 8+ messages in thread
From: Nicholas Vinson @ 2018-03-28 15:32 UTC (permalink / raw)
To: grub-devel
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [GRUB PARTUUID PATCH V8 4/4] Update grub script template files
2018-03-28 15:32 [GRUB PARTUUID PATCH V8 0/4] Add PARTUUID detection support Nicholas Vinson
` (2 preceding siblings ...)
2018-03-28 15:32 ` [GRUB PARTUUID PATCH V8 3/4] Add PARTUUID detection support to grub-probe Nicholas Vinson
@ 2018-03-28 15:32 ` Nicholas Vinson
2018-04-04 20:23 ` Daniel Kiper
3 siblings, 1 reply; 8+ messages in thread
From: Nicholas Vinson @ 2018-03-28 15:32 UTC (permalink / raw)
To: grub-devel
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [GRUB PARTUUID PATCH V8 4/4] Update grub script template files
2018-03-28 15:32 ` [GRUB PARTUUID PATCH V8 4/4] Update grub script template files Nicholas Vinson
@ 2018-04-04 20:23 ` Daniel Kiper
2018-04-05 6:16 ` Nick Vinson
0 siblings, 1 reply; 8+ messages in thread
From: Daniel Kiper @ 2018-04-04 20:23 UTC (permalink / raw)
To: nvinson234; +Cc: grub-devel
On Wed, Mar 28, 2018 at 08:32:54AM -0700, Nicholas Vinson wrote:
> 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" ] \
Hmmm... Have you tested this patch with GRUB_DISABLE_LINUX_UUID=true and
GRUB_ENABLE_LINUX_PARTUUID=true? I do not think so. LINUX_ROOT_DEVICE will
be set to ${GRUB_DEVICE}. I have a feeling that this is not what you expect.
Additionally, what will happen if GRUB_DISABLE_LINUX_UUID=true and
GRUB_ENABLE_LINUX_PARTUUID=false? Well, this should work right now
but please test it after fixing above mentioned issue.
Should not we do s/GRUB_ENABLE_LINUX_PARTUUID/GRUB_DISABLE_LINUX_PARTUUID/?
Then the variable will have similar meaning to GRUB_DISABLE_LINUX_UUID.
Hence, all will be less confusing.
> + || ( ! 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
Daniel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [GRUB PARTUUID PATCH V8 4/4] Update grub script template files
2018-04-04 20:23 ` Daniel Kiper
@ 2018-04-05 6:16 ` Nick Vinson
0 siblings, 0 replies; 8+ messages in thread
From: Nick Vinson @ 2018-04-05 6:16 UTC (permalink / raw)
To: Daniel Kiper; +Cc: grub-devel
[-- Attachment #1.1: Type: text/plain, Size: 5716 bytes --]
On 04/04/2018 01:23 PM, Daniel Kiper wrote:
> On Wed, Mar 28, 2018 at 08:32:54AM -0700, Nicholas Vinson wrote:
>> 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" ] \
>
> Hmmm... Have you tested this patch with GRUB_DISABLE_LINUX_UUID=true and
> GRUB_ENABLE_LINUX_PARTUUID=true? I do not think so. LINUX_ROOT_DEVICE will
> be set to ${GRUB_DEVICE}. I have a feeling that this is not what you expect.
The idea was for GRUB_DISABLE_LINUX_UUID to act as a controlling
variable with this setup.
I reviewed the description for the variable, and having it act that way
doesn't make sense. I will rewrite this part and make sure the two
variables act as independently as possible.
The one exception, however, would be when GRUB_DISABLE_LINUX_UUID is
unset and GRUB_ENABLE_LINUX_PARTUUID is set. In such a case, I'll have
the code favor the fs UUID over the partition UUID. I think this would
cause the least amount of surprise since the initramfs images I've seen
prefer the fs UUID.
>
> Additionally, what will happen if GRUB_DISABLE_LINUX_UUID=true and
> GRUB_ENABLE_LINUX_PARTUUID=false? Well, this should work right now
> but please test it after fixing above mentioned issue.
It should favor the Linux device name. I'll make sure that behavior is
preserved.
>
> Should not we do s/GRUB_ENABLE_LINUX_PARTUUID/GRUB_DISABLE_LINUX_PARTUUID/?
> Then the variable will have similar meaning to GRUB_DISABLE_LINUX_UUID.
> Hence, all will be less confusing.
I can return the name to GRUB_DISABLE_LINUX_PARTUUID. When I originally
used that name, there was concerns about maintaining compatibility with
older kernel versions. However, I think I can use the name
GRUB_DISABLE_LINUX_PARTUUID while maintaining that compatibility.
Thanks,
Nicholas Vinson
>
>> + || ( ! 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
>
> Daniel
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread