Re: [PATCH] grub2: add support to search for partuuid

[Nick Vinson]

On 5/27/19 1:07 AM, Michael Grzeschik wrote:
> With this feature the grub-shell is able to search for partuuid strings.
> This reduces the complexity to find the correct boot partition on
> systems with unspecified device connectivity.
> 
> Signed-off-by: Michael Grzeschik 
> ---
>  grub-core/Makefile.core.def  |  5 +
>  grub-core/commands/search.c  | 25 -
>  grub-core/commands/search_partuuid.c |  5 +
>  grub-core/commands/search_wrap.c |  8 +++-
>  grub-core/partmap/msdos.c|  9 +
>  include/grub/disk.h  |  2 ++
>  6 files changed, 52 insertions(+), 2 deletions(-)
>  create mode 100644 grub-core/commands/search_partuuid.c
> 
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 474a63e68..208feb934 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1070,6 +1070,11 @@ module = {
>common = commands/search_file.c;
>  };
>  
> +module = {
> +  name = search_partuuid;
> +  common = commands/search_partuuid.c;
> +};
> +
>  module = {
>name = search_fs_uuid;
>common = commands/search_uuid.c;
> diff --git a/grub-core/commands/search.c b/grub-core/commands/search.c
> index ed090b3af..3ac93943b 100644
> --- a/grub-core/commands/search.c
> +++ b/grub-core/commands/search.c
> @@ -72,7 +72,26 @@ iterate_device (const char *name, void *data)
>  #define compare_fn grub_strcmp
>  #endif
>  
> -#ifdef DO_SEARCH_FILE
> +#ifdef (DO_SEARCH_PARTUUID)

Typo here.  DO_SEARCH_PARTUUID should not be wrapped with parentheses.

> +{
> +  grub_disk_t disk;
> +  char *part_uuid;
> +  char *uuid;
> +
> +  disk = grub_disk_open (name);
> +  if (disk && disk->partition)
> + {
> +  part_uuid = grub_xasprintf ("%08x-%02x", disk->nt_disk_sig,
> +  disk->partition->index + 1);
> +  if (compare_fn (part_uuid, ctx->key) == 0)
> + found = 1;
> +  if (part_uuid)
> +grub_free (part_uuid);
> +}
> +  if (disk)
> +grub_disk_close (disk);
> +}

Did you intend to omit GPT support for this command?  Asking because it
would be a bit weird in my opinion to add support for PARTUUIDs but omit
the most commonly used partition scheme that actually supports them.

Thanks,
Nicholas Vinson

> +#elif defined (DO_SEARCH_FILE)
>  {
>char *buf;
>grub_file_t file;
> @@ -313,6 +332,8 @@ static grub_command_t cmd;
>  
>  #ifdef DO_SEARCH_FILE
>  GRUB_MOD_INIT(search_fs_file)
> +#elif defined (DO_SEARCH_PARTUUID)
> +GRUB_MOD_INIT(search_partuuid)
>  #elif defined (DO_SEARCH_FS_UUID)
>  GRUB_MOD_INIT(search_fs_uuid)
>  #else
> @@ -327,6 +348,8 @@ GRUB_MOD_INIT(search_label)
>  
>  #ifdef DO_SEARCH_FILE
>  GRUB_MOD_FINI(search_fs_file)
> +#elif defined (DO_SEARCH_PARTUUID)
> +GRUB_MOD_FINI(search_partuuid)
>  #elif defined (DO_SEARCH_FS_UUID)
>  GRUB_MOD_FINI(search_fs_uuid)
>  #else
> diff --git a/grub-core/commands/search_partuuid.c 
> b/grub-core/commands/search_partuuid.c
> new file mode 100644
> index 0..e4aa20b5f
> --- /dev/null
> +++ b/grub-core/commands/search_partuuid.c
> @@ -0,0 +1,5 @@
> +#define DO_SEARCH_PARTUUID 1
> +#define FUNC_NAME grub_search_partuuid
> +#define COMMAND_NAME "search.partuuid"
> +#define HELP_MESSAGE N_("Search devices by PARTUUID. If VARIABLE is 
> specified, the first device found is set to a variable.")
> +#include "search.c"
> diff --git a/grub-core/commands/search_wrap.c 
> b/grub-core/commands/search_wrap.c
> index d7fd26b94..ba36dbac2 100644
> --- a/grub-core/commands/search_wrap.c
> +++ b/grub-core/commands/search_wrap.c
> @@ -36,6 +36,8 @@ static const struct grub_arg_option options[] =
>   0, 0},
>  {"fs-uuid",  'u', 0, N_("Search devices by a filesystem 
> UUID."),
>   0, 0},
> +{"partuuid", 'p', 0, N_("Search devices by a PARTUUID."),
> + 0, 0},
>  {"set",  's', GRUB_ARG_OPTION_OPTIONAL,
>   N_("Set a variable to the first device found."), N_("VARNAME"),
>   ARG_TYPE_STRING},
> @@ -71,6 +73,7 @@ enum options
>  SEARCH_FILE,
>  SEARCH_LABEL,
>  SEARCH_FS_UUID,
> +SEARCH_PARTUUID,
>  SEARCH_SET,
>  SEARCH_NO_FLOPPY,
>  SEARCH_HINT,
> @@ -186,6 +189,9 @@ grub_cmd_search (grub_extcmd_context_t ctxt, int argc, 
> char **args)
>else if (state[SEARCH_FS_UUID].set)
>  grub_search_fs_uuid (id, var, state[SEARCH_NO_FLOPPY].set,
>hints, nhints);
> +  else if (state[SEARCH_PARTUUID].set)
> +grub_search_partuuid (id, var, state[SEARCH_NO_FLOPPY].set,
> +  hints, nhints);
>else if (state[SEARCH_FILE].set)
>  grub_search_fs_file (id, var, state[SEARCH_NO_FLOPPY].set, 
>hints, nhints);
> @@ -207,7 +213,7 @@ GRUB_MOD_INIT(search)
> N_("[-f|-l|-u|-s|-n] [--hint HINT [--hint HINT] ...]"
>" 

Re: [PATCH v2] Optimization calculation expression of FOR_MODULES(var)

[Nick Vinson]

You cannot remove sizeof(A) from the expression, as doing so may cause
the updated expression to compute a different answer.

Consider ((H*)var)->size = 64, B = 32, and A = 64.

In the original form, you have the expression (64 + 64 - 1) / 64 * 64 /
32 which equals 2.
In your proposal, you have the expression (64 + 64 - 1) / 32 which equals 3.

This could result in incorrect behavior later on.  If I were Daniel, I
would want to see something that shows the change in behavior does not
result in incorrect behavior.

Regards,
Nicholas Vinson

On 4/11/19 1:39 AM, Milo Wenxiang Niu wrote:
> From: Milo Wenxiang X Niu 
> 
> It's just to remove the common factor: "sizeof (grub_addr_t)"  from the 
> numerator and denominator of the fractional expression of next var.
> Let me explain it:
> Shortly:
>   H: struct grub_module_header ;
>   B: grub_uint32_t ;
>   A: grub_addr_t;
> 
> Thus, original expression can be expressed as:
>   var = (H *)((B*)var) + ( offset_exp ))
> ++
>  (((H*)var)->size + sizeof(A) - 1)sizeof(A)  |
> offset_exp = - * --- |
>   sizeof(A)   sizeof(B)  |
> ++
> Remove the common factor "sizeof(A)" of fractional offset_exp, we got the 
> result.
> offset_exp_new = ((H*)->size + sizeof(A) - 1) / sizeof(B)
>  = ((struct grub_module_header *) var)->size + sizeof 
> (grub_addr_t) -1) / sizeof (grub_uint32_t)
> 
> so:
> var =(H *)(((B*)var) + ( (((H*)var)->size + sizeof(A) - 1) / sizeof(B) ))
> That's what I do.
> 
> Still, the new offset express is meaningfull:
> *numerator: ((struct grub_module_header *) var)->size + sizeof 
> (grub_addr_t) -1)
>   it present the offset value united by byte.
>   *denominator: sizeof (grub_uint32_t)
>   it's means "struct grub_module_header" aligned by 
> sizeof(grub_uint32_t)
> ---
>  include/grub/kernel.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/grub/kernel.h b/include/grub/kernel.h
> index 133a37c..b257224 100644
> --- a/include/grub/kernel.h
> +++ b/include/grub/kernel.h
> @@ -104,7 +104,7 @@ extern grub_addr_t EXPORT_VAR (grub_modbase);
>var && (grub_addr_t) var \
>  < (grub_modbase + (((struct grub_module_info *) grub_modbase)->size));   
>  \
>var = (struct grub_module_header *)
> \
> -(((grub_uint32_t *) var) + struct grub_module_header *) var)->size + 
> sizeof (grub_addr_t) - 1) / sizeof (grub_addr_t)) * (sizeof (grub_addr_t) / 
> sizeof (grub_uint32_t
> +(((grub_uint32_t *) var) + struct grub_module_header *) var)->size + 
> sizeof (grub_addr_t) - 1) / sizeof (grub_uint32_t
>  
>  grub_addr_t grub_modules_get_end (void);
>  
> 



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Add knobs to allow non-initrd boot config

[Nick Vinson]

Paul,

Let me start of by saying that I am a contributor and do not have any
authority to approve or deny patches to GRUB.  That said I do have a few
comments below that I would like for you to consider.

On 2/15/19 3:07 AM, Paul Menzel wrote:
> Date: Thu, 10 Nov 2016 13:44:25 -0500
> 
> Add GRUB_FORCE_PARTUUID and GRUB_DISABLE_INITRD configuration knobs to allow
> users to generate GRUB menu entries booting directly to the kernel, without
> using an initramfs.
> 
> Signed-off-by: Mathieu Trudel-Lapierre 
> [1. Upstream patch from Ubuntu repository
> https://code.launchpad.net/ubuntu/+source/grub2
>  2. Fix typo s/Then/When/]
> Signed-off-by: Paul Menzel 
> ---
>  docs/grub.texi  | 13 +
>  util/grub-mkconfig.in   |  4 +++-
>  util/grub.d/10_linux.in | 11 +--
>  3 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/grub.texi b/docs/grub.texi
> index ecaba9d5c..6520d9f87 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -1522,6 +1522,19 @@ This option may be set to a list of GRUB module names 
> separated by spaces.
>  Each module will be loaded as early as possible, at the start of
>  @file{grub.cfg}.
>  
> +@item GRUB_FORCE_PARTUUID
> +This option forces the root disk entry to be the specified PARTUUID instead
> +of whatever would be used instead. This is useful when you control the
> +partitioning of the disk but cannot guarantee what the actual hardware will
> +be, for example in virtual machine images.
> +Setting this option to @samp{12345678-01} will produce:
> +root=PARTUUID=12345678-01
> 
> +@item GRUB_DISABLE_INITRD
> +When set to @samp{true}, this option prevents an initrd to be used at boot
> +time, regardless of whether one is detected or not. @command{grub-mkconfig}
> +will therefore not generate any initrd lines.
> +
>  @end table
>  
>  The following options are still accepted for compatibility with existing
> diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
> index 2360e..265cf959b 100644
> --- a/util/grub-mkconfig.in
> +++ b/util/grub-mkconfig.in
> @@ -238,7 +238,9 @@ export GRUB_DEFAULT \
>GRUB_ENABLE_CRYPTODISK \
>GRUB_BADRAM \
>GRUB_OS_PROBER_SKIP_LIST \
> -  GRUB_DISABLE_SUBMENU
> +  GRUB_DISABLE_SUBMENU \
> +  GRUB_FORCE_PARTUUID \
> +  GRUB_DISABLE_INITRD
>  
>  if test "x${grub_cfg}" != "x"; then
>rm -f "${grub_cfg}.new"
> diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
> index 4532266be..d4498f106 100644
> --- a/util/grub.d/10_linux.in
> +++ b/util/grub.d/10_linux.in
> @@ -139,11 +139,18 @@ linux_entry ()
>  printf '%s\n' "${prepare_boot_cache}" | sed "s/^/$submenu_indentation/"
>fi
>message="$(gettext_printf "Loading Linux %s ..." ${version})"
> -  sed "s/^/$submenu_indentation/" << EOF
> +  if [ x"$GRUB_FORCE_PARTUUID" = x ]; then
> +sed "s/^/$submenu_indentation/" << EOF

Since grub has the option GRUB_DISABLE_LINUX_PARTUUID which is expected
to control whether `root=PARTUUID=...` is used, perhaps it would be
better to add something like:

GRUB_DEVICE_PARTUUID=${GRUB_FORCE_PARTUUID:-${GRUB_DEVICE_PARTUUID}}

near the beginning of 10_linux.in?

>   echo'$(echo "$message" | grub_quote)'
>   linux   ${rel_dirname}/${basename} 
> root=${linux_root_device_thisversion} ro ${args}
>  EOF
> -  if test -n "${initrd}" ; then
> +  else
> +sed "s/^/$submenu_indentation/" << EOF
> + echo'$(echo "$message" | grub_quote)'
> + linux   ${rel_dirname}/${basename} 
> root=$PARTUUID=${GRUB_FORCE_PARTUUID} ro ${args}
> +EOF
> +  fi
> +  if test -n "${initrd}" && [ x"$GRUB_DISABLE_INITRD" != xtrue ]; then

Perhaps instead of an all-or-nothing approach something similar to how
the GRUB_EARLY_INITRD_LINUX_STOCK and GRUB_EARLY_INITRD_LINUX_CUSTOM
variables are used could be adopted?

My thinking here is a user could set something like
GRUB_INITRD_LINUX_STOCK="" and GRUB_EARLY_INITRD_LINUX_STOCK="".  That
would allow for the same effect while also giving distros and users
greater flexibility in how early initrd and regular initrd images are named.

This also allows early initrds to still be used without normal initrds
to load CPU microcode updates at boot without having to recompile the
kernel on update.

However, for GRUB_EARLY_INITRD_LINUX_STOCK, this might require a small
code change as

`if [ "x${GRUB_EARLY_INITRD_LINUX_STOCK}" = "x" ]; then`

would probably need to be something like

`if ["x${GRUB_EARLY_INITRD_LINUX_STOCK+is_set} = "x"]; then`.

Thanks,
Nicholas Vinson

>  # TRANSLATORS: ramdisk isn't identifier. Should be translated.
>  message="$(gettext_printf "Loading initial ramdisk ...")"
>  initrd_path=
> 
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org

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

[Nick Vinson]

On 04/17/2018 06:36 AM, Daniel Kiper wrote:
> On Mon, Apr 16, 2018 at 10:36:26PM -0700, Nicholas Vinson wrote:
>> Update grub-mkconfig.in and 10_linux.in to support grub-probe's new
>> partuuid target.  Update grub.texi documentation.  The following table
>> shows how GRUB_DISABLE_LINUX_UUID, GRUB_DISABLE_LINUX_PARTUUID, and
>> initramfs detection interact:
>>
>> Initramfs  GRUB_DISABLE_LINUX_PARTUUID  GRUB_DISABLE_LINUX_UUID  Linux Root
>> detected   Set  Set  ID Method
>>
>> false  falsefalsepart UUID
>> false  falsetrue part UUID
>> false  true falsedev name
>> false  true true dev name
>> true   falsefalsefs UUID
>> true   falsetrue part UUID
>> true   true falsefs UUID
>> true   true true dev name
>>
>> Note: GRUB_DISABLE_LINUX_PARTUUID and GRUB_DISABLE_LINUX_UUID equate to
>>   'false' when unset or set to any value other than 'true'.
>>   GRUB_DISABLE_LINUX_PARTUUID defaults to 'true'.
>> Signed-off-by: Nicholas Vinson 
>> ---
>>  docs/grub.texi  | 67 ++---
>>  util/grub-mkconfig.in   |  3 ++
>>  util/grub.d/10_linux.in | 22 ++--
>>  util/grub.d/20_linux_xen.in | 22 ++--
>>  4 files changed, 104 insertions(+), 10 deletions(-)
>>
>> diff --git a/docs/grub.texi b/docs/grub.texi
>> index 0f2ab91fc..6aa65552f 100644
>> --- a/docs/grub.texi
>> +++ b/docs/grub.texi
>> @@ -1214,10 +1214,11 @@ GRUB is configured using @file{grub.cfg}, usually 
>> located under
>>  need to write the whole thing by hand.
>>
>>  @menu
>> -* Simple configuration::Recommended for most users
>> -* Shell-like scripting::For power users and developers
>> -* Multi-boot manual config::For non-standard multi-OS scenarios
>> -* Embedded configuration::  Embedding a configuration file into GRUB
>> +* Simple configuration::Recommended for most users
>> +* Root Identifcation Heuristics::   Summary on how the root file system is 
>> identified.
>> +* Shell-like scripting::For power users and developers
>> +* Multi-boot manual config::For non-standard multi-OS scenarios
>> +* Embedded configuration::  Embedding a configuration file into GRUB
>>  @end menu
>>
>>
>> @@ -1425,6 +1426,17 @@ 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_DISABLE_LINUX_PARTUUID
>> +If @command{grub-mkconfig} cannot identify the root filesystem via its
>> +universally-unique indentifier (UUID), @command{grub-mkconfig} can use the 
>> UUID
>> +of the partition containing the filesystem to identify the root filesystem 
>> to
>> +the Linux kernel via a @samp{root=PARTUUID=...} kernel parameter.  This is 
>> not
>> +as reliable as using the filesystem UUID, but is more reliable than using 
>> the
>> +Linux device names. When @samp{GRUB_DISABLE_LINUX_PARTUUID} is set to
>> +@samp{false}, the Linux kernel version must be 2.6.37 (3.10 for systems 
>> using
>> +the MSDOS partition scheme) or newer.  This option defaults to @samp{true}. 
>>  To
>> +enable the use of partition UUIDs, set this option to @samp{false}.
>> +
>>  @item GRUB_DISABLE_RECOVERY
>>  If this option is set to @samp{true}, disable the generation of recovery
>>  mode menu entries.
>> @@ -1556,6 +1568,53 @@ edit the scripts in @file{/etc/grub.d} directly.
>>  menu entries; simply type the menu entries you want to add at the end of
>>  that file, making sure to leave at least the first two lines intact.
>>
>> +@node Root Identifcation Heuristics
>> +@section Root Identifcation Heuristics
>> +If the target operating system uses the Linux kernel, 
>> @command{grub-mkconfig}
>> +attempts to identify the root file system via a heuristic algoirthm.  This
>> +algorithm selects the identification method of the root file system by
>> +considering three factors.  The first is if an initrd for the target 
>> operating
>> +system is also present.  The second is @samp{GRUB_DISABLE_LINUX_UUID} and 
>> if set
>> +to @samp{true}, prevents @command{grub-mkconfig} from identifying the root 
>> file
>> +system by its UUID.  The third is @samp{GRUB_DISABLE_LINUX_PARTUUID} and if 
>> set
>> +to @samp{true}, prevents @command{grub-mkconfig} from identifying the root 
>> file
>> +system via the UUID of its enclosing partition.  If the variables are 
>> assigned
>> +any other value, that value is considered equivalent to @samp{false}.  The
>> +variables are also 

Re: [GRUB PARTUUID PATCH V9 4/5] Update grub script template files

[Nick Vinson]

On 04/16/2018 04:47 AM, Daniel Kiper wrote:
> On Wed, Apr 11, 2018 at 07:45:01AM -0700, Nick Vinson wrote:
>> On 04/11/2018 01:31 AM, Daniel Kiper wrote:
>>> On Tue, Apr 10, 2018 at 08:00:04PM -0700, Nick Vinson wrote:
>>>> On 04/10/2018 01:52 PM, Daniel Kiper wrote:
>>>>> On Sat, Apr 07, 2018 at 04:28:13PM -0700, Nicholas Vinson wrote:
>>>>>> Update grub-mkconfig.in and 10_linux.in to support grub-probe's new
>>>>>> partuuid target.  Update grub.texi documentation.  The following table
>>>>>> shows how GRUB_DISABLE_LINUX_UUID, GRUB_DISABLE_LINUX_PARTUUID, and
>>>>>> initramfs detection interact:
>>>>>>
>>>>>> Initramfs  GRUB_DISABLE_LINUX_PARTUUID  GRUB_DISABLE_LINUX_UUID  Linux 
>>>>>> Root
>>>>>> detected   Set  Set  ID 
>>>>>> Method
>>>>>>
>>>>>> False  FalseFalsepart 
>>>>>> UUID
>>>>>> False  FalseTrue part 
>>>>>> UUID
>>>>>> False  True Falsedev name
>>>>>> False  True True dev name
>>>>>> True   FalseFalsefs UUID
>>>>>> True   FalseTrue part 
>>>>>> UUID
>>>>>> True   True Falsefs UUID
>>>>>> True   True True dev name
>>>>>
>>>>> What will happen if GRUB_DISABLE_LINUX_PARTUUID and/or 
>>>>> GRUB_DISABLE_LINUX_UUID
>>>>> are not set? I think that you can avoid that by setting defaults. You do 
>>>>> that
>>>>> for GRUB_DISABLE_LINUX_PARTUUID in next patch but GRUB_DISABLE_LINUX_UUID
>>>>> does not have any default.
>>>>>
>>>>
>>>> If they're not set, then that's the same as them being set to 'False'.
>>>> I should have worded my table above a bit differently and used Yes/No
>>>> instead of True/False as that is really what it is trying to convey.
>>>
>>> IMO it will be more confusing. I think that I would use lowercase
>>> false/true as it is used in the script and below the table I would
>>> add a note that  == false or something like that.
>>
>> Ack.  I will update the commit comment.
> 
> Thanks. May I ask you to put similar table into docs/grub.texi?

Done.

Thanks,
Nicholas Vinson
> 
> Daniel
> 

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [GRUB PARTUUID PATCH V9 4/5] Update grub script template files

[Nick Vinson]

On 04/11/2018 01:31 AM, Daniel Kiper wrote:
> On Tue, Apr 10, 2018 at 08:00:04PM -0700, Nick Vinson wrote:
>> On 04/10/2018 01:52 PM, Daniel Kiper wrote:
>>> On Sat, Apr 07, 2018 at 04:28:13PM -0700, Nicholas Vinson wrote:
>>>> Update grub-mkconfig.in and 10_linux.in to support grub-probe's new
>>>> partuuid target.  Update grub.texi documentation.  The following table
>>>> shows how GRUB_DISABLE_LINUX_UUID, GRUB_DISABLE_LINUX_PARTUUID, and
>>>> initramfs detection interact:
>>>>
>>>> Initramfs  GRUB_DISABLE_LINUX_PARTUUID  GRUB_DISABLE_LINUX_UUID  Linux Root
>>>> detected   Set  Set  ID Method
>>>>
>>>> False  FalseFalsepart UUID
>>>> False  FalseTrue part UUID
>>>> False  True Falsedev name
>>>> False  True True dev name
>>>> True   FalseFalsefs UUID
>>>> True   FalseTrue part UUID
>>>> True   True Falsefs UUID
>>>> True   True True dev name
>>>
>>> What will happen if GRUB_DISABLE_LINUX_PARTUUID and/or 
>>> GRUB_DISABLE_LINUX_UUID
>>> are not set? I think that you can avoid that by setting defaults. You do 
>>> that
>>> for GRUB_DISABLE_LINUX_PARTUUID in next patch but GRUB_DISABLE_LINUX_UUID
>>> does not have any default.
>>>
>>
>> If they're not set, then that's the same as them being set to 'False'.
>> I should have worded my table above a bit differently and used Yes/No
>> instead of True/False as that is really what it is trying to convey.
> 
> IMO it will be more confusing. I think that I would use lowercase
> false/true as it is used in the script and below the table I would
> add a note that  == false or something like that.

Ack.  I will update the commit comment.

> 
>> I.E. If there is no initramfs detected and GRUB_DISABLE_LINUX_PARTUUID
>> is not set and GRUB_DISABLE_LINUX_UUID is not set, set the kernel root
>> variable to "root=PARTUUID=...".
>>
>> In the scripts, the only value explicitly checked for is 'true'.
>> Anything else (including unset) is considered false.
> 
> OK, perfect!
> 
>>>> Signed-off-by: Nicholas Vinson <nvinson...@gmail.com>
>>>> ---
>>>>  docs/grub.texi  | 10 ++
>>>>  util/grub-mkconfig.in   |  3 +++
>>>>  util/grub.d/10_linux.in | 18 +++---
>>>
>>> Could you update util/grub.d/20_linux_xen.in too?
>>
>> Let me see what I can do.  I have never worked with xen before, so I do
>> not know much about it.
> 
> This should be easy. However, if you are not sure please drop me a line.
> 
>>>>  3 files changed, 28 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/docs/grub.texi b/docs/grub.texi
>>>> index 65b4bbeda..06f0afe45 100644
>>>> --- a/docs/grub.texi
>>>> +++ b/docs/grub.texi
>>>> @@ -1424,6 +1424,16 @@ 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_DISABLE_LINUX_PARTUUID
>>>> +If @command{grub-mkconfig} cannot identify the root filesystem via its
>>>> +universally-unique indentifier (UUID), @command{grub-mkconfig} will use 
>>>> the UUID
>>>> +of the partition containing the filesystem to identify the root 
>>>> filesystem to
>>>> +the Linux kernel via a @samp{root=PARTUUID=...} kernel parameter.  This 
>>>> is not
>>>> +as reliable as using the filesystem UUID, but is more reliable than using 
>>>> the
>>>> +Linux device names.  When enabled, this option requires the Linux kernel 
>>>> version
>>>
>>> s/When enabled, /When enabled, GRUB_DISABLE_LINUX_PARTUUID set to false, /
>>> or something like that. Otherwise it is unclear.
>>
>> I'll clean up the wording in the next release.
> 
> Thanks a lot!
> 
> By the way, should not we merge patches 4 and 5 into one patch?

We should.

I split it so that if #5 proved controversial for some reason and
received a lot of resistance, it could be dropped and the remainder of
the patchset could be merged.

There does not seem to be any issue with setting
GRUB_DISABLE_LINUX_PARTUUID to a default value, so there is no reason to
keep it separate.

Thanks,
Nicholas Vinson

> 
> Daniel
> 

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [GRUB PARTUUID PATCH V9 0/5] Add PARTUUID detection support

[Nick Vinson]

On 04/10/2018 01:56 PM, Daniel Kiper wrote:
> On Sat, Apr 07, 2018 at 04:28:09PM -0700, Nicholas Vinson wrote:
>> Changes from Patch v8:
>> - Renamed GRUB_ENABLE_LINUX_PARTUUID to GRUB_DISABLE_LINUX_PARTUUID
>> - Updated the 10_linux logic so GRUB_ENABLE_LINUX_PARTUUID and
>> GRUB_ENABLE_LINUX_UUID would behave more independently.
>> - Documented interactions of GRUB_DISABLE_LINUX_UUID,
>> GRUB_DISABLE_LINUX_PARTUUID, and initramfs detection in commit
>> message.
>> - Added optional patch that sets GRUB_DISABLE_LINUX_PARTUUID to true
>> by default.
>> - Fixed merge conflicts between this patchset and upstream's master
>> branch.
> 
> Could you provide summary for current version not only for previous ones?

I can add a summary section before the change log.  I'll make sure to do
so with the next patchset.

Thanks,
Nicholas Vinson

> 
> Anyway, I think that most of the patches are now in good shape.
> I am looking forward for next version.
> 
> Thank you for doing the work.
> 
> Daniel
> 

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [GRUB PARTUUID PATCH V9 4/5] Update grub script template files

[Nick Vinson]

On 04/10/2018 01:52 PM, Daniel Kiper wrote:
> On Sat, Apr 07, 2018 at 04:28:13PM -0700, Nicholas Vinson wrote:
>> Update grub-mkconfig.in and 10_linux.in to support grub-probe's new
>> partuuid target.  Update grub.texi documentation.  The following table
>> shows how GRUB_DISABLE_LINUX_UUID, GRUB_DISABLE_LINUX_PARTUUID, and
>> initramfs detection interact:
>>
>> Initramfs  GRUB_DISABLE_LINUX_PARTUUID  GRUB_DISABLE_LINUX_UUID  Linux Root
>> detected   Set  Set  ID Method
>>
>> False  FalseFalsepart UUID
>> False  FalseTrue part UUID
>> False  True Falsedev name
>> False  True True dev name
>> True   FalseFalsefs UUID
>> True   FalseTrue part UUID
>> True   True Falsefs UUID
>> True   True True dev name
> 
> What will happen if GRUB_DISABLE_LINUX_PARTUUID and/or GRUB_DISABLE_LINUX_UUID
> are not set? I think that you can avoid that by setting defaults. You do that
> for GRUB_DISABLE_LINUX_PARTUUID in next patch but GRUB_DISABLE_LINUX_UUID
> does not have any default.
> 

If they're not set, then that's the same as them being set to 'False'.
I should have worded my table above a bit differently and used Yes/No
instead of True/False as that is really what it is trying to convey.
I.E. If there is no initramfs detected and GRUB_DISABLE_LINUX_PARTUUID
is not set and GRUB_DISABLE_LINUX_UUID is not set, set the kernel root
variable to "root=PARTUUID=...".

In the scripts, the only value explicitly checked for is 'true'.
Anything else (including unset) is considered false.

>> Signed-off-by: Nicholas Vinson 
>> ---
>>  docs/grub.texi  | 10 ++
>>  util/grub-mkconfig.in   |  3 +++
>>  util/grub.d/10_linux.in | 18 +++---
> 
> Could you update util/grub.d/20_linux_xen.in too?

Let me see what I can do.  I have never worked with xen before, so I do
not know much about it.

> 
>>  3 files changed, 28 insertions(+), 3 deletions(-)
>>
>> diff --git a/docs/grub.texi b/docs/grub.texi
>> index 65b4bbeda..06f0afe45 100644
>> --- a/docs/grub.texi
>> +++ b/docs/grub.texi
>> @@ -1424,6 +1424,16 @@ 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_DISABLE_LINUX_PARTUUID
>> +If @command{grub-mkconfig} cannot identify the root filesystem via its
>> +universally-unique indentifier (UUID), @command{grub-mkconfig} will use the 
>> UUID
>> +of the partition containing the filesystem to identify the root filesystem 
>> to
>> +the Linux kernel via a @samp{root=PARTUUID=...} kernel parameter.  This is 
>> not
>> +as reliable as using the filesystem UUID, but is more reliable than using 
>> the
>> +Linux device names.  When enabled, this option requires the Linux kernel 
>> version
> 
> s/When enabled, /When enabled, GRUB_DISABLE_LINUX_PARTUUID set to false, /
> or something like that. Otherwise it is unclear.

I'll clean up the wording in the next release.

Thanks,
Nicholas Vinson

> 
> Daniel
> 

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

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

[Nick Vinson]

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

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

[Nick Vinson]

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 
>> ---
>>  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 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -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), _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), ) == 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
> 



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

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

[Nick Vinson]

On 03/22/2018 06:51 AM, Daniel Kiper wrote:
> On Sun, Mar 18, 2018 at 12:06:02PM -0700, Nicholas Vinson wrote:
>> Add PARTUUID detection support grub-probe for MBR and GPT partition
>> schemes.
>>
>> Signed-off-by: Nicholas Vinson 
>> ---
>>  util/grub-probe.c | 51 +++
>>  1 file changed, 51 insertions(+)
>>
>> diff --git a/util/grub-probe.c b/util/grub-probe.c
>> index 21cb80fbe..0ae73c591 100644
>> --- a/util/grub-probe.c
>> +++ b/util/grub-probe.c
>> @@ -28,6 +28,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -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)
>> +{
> 
> Define p here:
>   grub_partition_t p = disk->partition;

done.

> 
>> +  /*
>> +   * 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;
> 
> Somthing is wrong with formating here and there. Please fix this.

fixed.  I forgot to tell vim not to expand tab characters.

> 
>> +grub_partition_t p = disk->partition;
> 
> Drop this...
> 

dropped.

>> +disk->partition = p->parent;
>> +
>> +if (grub_disk_read (disk, 0, GRUB_BOOT_MACHINE_WINDOWS_NT_MAGIC,
>> +sizeof(nt_disk_sig), _disk_sig) == 0)
>> +
>> +grub_printf ("%08x-%02x",
>> + grub_le_to_cpu32(nt_disk_sig), 1 + p->number);
>> +disk->partition = p;
> 
> ...and above line...
> 

done.

>> +}
>> +  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), ) == 0)
>> +print_gpt_guid(gptdata.guid);
>> +
>> +  disk->partition = p;
> 
> ...and above line...

done.

> 
>> +}
> 
> ...and add this here:
> disk->partition = p;

added.

Thanks,
Nicholas Vinson

> 
> Daniel
> 



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

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

[Nick Vinson]

On 03/22/2018 06:56 AM, Daniel Kiper wrote:
> On Sun, Mar 18, 2018 at 12:06:03PM -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 
>> ---
>>  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..b6a44eb6b 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 imappropriate.  However,
> 
> s/imappropriate/inappropriate/

fixed

> 
>> +this option is only supported in Linux kernel versions least 2.6.37 (3.10 
>> for
> 
> s/least/at least/?

I think s/least// would sound better than s/least/at least/.  Thoughts?

Thanks,
Nicholas Vinson

> 
> Otherwise LGTM.
> 
> Daniel
> 



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

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

[Nick Vinson]

On 02/20/2018 09:45 AM, Daniel Kiper wrote:
> On Sun, Feb 04, 2018 at 11:47:37AM -0800, Nicholas Vinson wrote:
>> Update grub-mkconfig.in and 10_linux.in to support grub-probe's new
>> partuuid target.  Update grub.texi documenation.
> 
> As earlier lack of SOB. Otherwise LGTM.

Will be added in V6.

Thanks,
Nicholas Vinson
> 
> Daniel
> 



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

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

[Nick Vinson]

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 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -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), _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), ) == 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
> 



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [GRUB PARTUUID PATCH V5 1/3] Update grub_gpt_partentry; centralize guid prints

[Nick Vinson]

Sorry for the delay in responding.  I just now saw these replies.

On 02/20/2018 09:30 AM, Daniel Kiper wrote:
> On Sun, Feb 04, 2018 at 11:47:35AM -0800, Nicholas Vinson wrote:
>> To help clean the code and simplify the code in util/grub-probe.c, this
>> patch renames grub_gpt_part_type to grub_gpt_part_guid and updates
>> grub_gpt_partentry to use this type for both the partition type GUID
>> string and the partition GUID string entries.
>>
>> This patch also moves the GUID printing logic in util/grub-probe.c to a
>> separate function.  This change allows the partuuid logic in the next
>> commit to use the same printing logic without having to completely
>> duplicate it.
> 
> One logical change per patch please...

Done.  I've split this patch into two.

> And please add your SOB next time.

Done.  I'll submit a version 6 shortly.

Thanks,
Nicholas Vinson
> 
> Daniel
> 



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [GRUB PARTUUID PATCH V4 0/3] Add PARTUUID detection support

[Nick Vinson]

Hello,

I submitted this patch set back in May and have not heard anything.  I
noticed it also has not been merged into the GRUB tree.  Is there some
issue with the patches that I still need to fix?  If so, please let me
know so I can make the corrections and submit an updated patch set.

Thanks,
Nicholas Vinson

On 05/14/2017 09:26 AM, Nicholas Vinson wrote:
> This is an updated patch set for PARTUUID support.  I've retested
> against GRUB 2.03 and found no errors.  Because including Steve Kenton's
> patches in the last iteration caused confusion, I have removed them from
> this version.  I have also removed the flex-2.6.3 compatibility patch as
> flex-2.6.4 has been released and is not affected by the same issues.
> 
> Thanks,
> Nicholas Vinson
> 
> 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 (3):
>   Update grub_gpt_partentry; centralize guid prints
>   Add PARTUUID detection support to grub-probe
>   Update grub script template files
> 
>  docs/grub.texi   | 13 +++
>  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| 81 
> 
>  util/grub.d/10_linux.in  | 13 +--
>  8 files changed, 101 insertions(+), 25 deletions(-)
> 



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [GRUB PARTUUID PATCH V3 0/5] Add PARTUUID detection support

[Nick Vinson]

I would like to remind grub-devel of this patch set.  Since the last
time I submitted this, Flex-2.6.4 has been released, so Part 1 of this
patch set can now be dropped.  Part 5 is optional and can be removed if
the development team does not approve of its changes.

Finally, this is the version that included Steve Kenton's work as
requested by Andrei Borzenkov.  Steve Kenton's work is Part #4.

If necessary, I can resubmit the patch set with parts 1 and 4 (and 5)
removed.

Please let me know how you would like me to proceed.

Thanks,
Nicholas Vinson

On 02/26/2017 04:20 PM, Nicholas Vinson wrote:
> 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):
>   Fix flex-2.6.3 incompatbility issue
>   Add PARTUUID detection support to grub-probe
>   Update grub script template files
>   Harmonize patches
> 
> Steve Kenton (1):
>   Support both EFI and NT Disk Signature for passing to kernel as
> root=PARTUUID=$val
> 
>  docs/grub.texi   | 13 +++
>  grub-core/commands/probe.c   | 78 ++
>  grub-core/disk/ldm.c |  2 +-
>  grub-core/partmap/gpt.c  |  4 +--
>  grub-core/script/yylex.l | 22 +---
>  include/grub/gpt_partition.h |  8 ++---
>  util/grub-install.c  |  2 +-
>  util/grub-mkconfig.in|  3 ++
>  util/grub-probe.c| 81 
> 
>  util/grub.d/10_linux.in  | 13 +--
>  10 files changed, 197 insertions(+), 29 deletions(-)
> 



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 1/1] add --partuuid to probe

[Nick Vinson]

I'll need to check it against the 2.02 sources, but even if it needs
updating, it shouldn't be too hard to do it.  I can add your changes to
the patchset.

-Nick

On 02/14/2017 11:39 AM, Steve Kenton wrote:
> Nick,
> 
> What is the state of your patch set? I think mine is much smaller if you
> want to roll it into yours and resubmit. We were told all along to wait
> until after 2.02, which should to be very soon, so it looks like it's
> show time!
> 
> Steve Kenton
> 
> 
> On 02/14/2017 07:12 PM, Andrei Borzenkov wrote:
>> 14.02.2017 21:00, Steve Kenton пишет:
>>> Support both EFI and NT Disk Signature for passing to kernel as
>>> root=PARTUUID=$val
>>>
>> Yes, I guess we need to add it finally. Unfortunately it is too late for
>> 2.02, but it should go after in release. There were also patches for
>> grub-probe and we also need to support it in search to be on par with
>> filesystem UUID. May be if you could prepare consolidated patch set it
>> would be great.
>>
>> I apologize that it tool so long. Thank you for not giving up!
>>
>>> Signed-off-by: Steve Kenton 
>>> ---
>>> It's been six months so I thought I'd resend this so it does not get
>>> lost
>>> in case I get hit by a meteor or something before the next release
>>>
>>>   grub-core/commands/probe.c | 53
>>> ++
>>>   1 file changed, 53 insertions(+)
>>>
>>> diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
>>> index cf2793e..3afc8b8 100644
>>> --- a/grub-core/commands/probe.c
>>> +++ b/grub-core/commands/probe.c
>> Please also add patch for manual.
>>
>>> @@ -45,6 +45,7 @@ static const struct grub_arg_option options[] =
>>>   {"fs",'f', 0, N_("Determine filesystem type."), 0, 0},
>>>   {"fs-uuid",'u', 0, N_("Determine filesystem UUID."), 0,
>>> 0},
>>>   {"label",'l', 0, N_("Determine filesystem label."), 0, 0},
>>> +{"partuuid",'g', 0, N_("Determine partition GUID/UUID."), 0,
>>> 0}, /* GUID but Linux kernel calls it "PARTUUID" */
>>>   {0, 0, 0, 0, 0, 0}
>>> };
>>>   @@ -154,6 +155,58 @@ grub_cmd_probe (grub_extcmd_context_t ctxt,
>>> int argc, char **args)
>>> grub_device_close (dev);
>>> return GRUB_ERR_NONE;
>>>   }
>>> +  if (state[6].set)
>>> +{
>>> +  char *partuuid = NULL; /* NULL to silence a spurious GCC
>>> warning */
>>> +  grub_uint8_t diskbuf[16];
>>> +  if (dev->disk && dev->disk->partition)
>>> +{
>>> +  grub_partition_t p = dev->disk->partition;
>>> +  if (!grub_strcmp (p->partmap->name, "msdos"))
>>> +{
>>> +  const int diskid_offset = 440; /* location in MBR */
>>> +  dev->disk->partition = p->parent;
>>> +  /* little-endian 4-byte NT disk signature */
>>> +  err = grub_disk_read (dev->disk, 0, diskid_offset, 4,
>>> diskbuf);
>>> +  dev->disk->partition = p;
>>> +  if (err)
>>> +return grub_errno;
>>> +  partuuid = grub_xasprintf ("%02x%02x%02x%02x-%02x",
>>> + diskbuf[3], diskbuf[2], diskbuf[1], diskbuf[0],
>>> + p->number + 1); /* one based partition number */
>>> +}
>>> +  else if (!grub_strcmp (p->partmap->name, "gpt"))
>>> +{
>>> +  const int guid_offset = 16; /* location in entry */
>>> +  dev->disk->partition = p->parent;
>>> +  /* little-endian 16-byte EFI partition GUID */
>>> +  err = grub_disk_read (dev->disk, p->offset, p->index +
>>> guid_offset, 16, diskbuf);
>>> +  dev->disk->partition = p;
>>> +  if (err)
>>> +return grub_errno;
>>> +  partuuid = grub_xasprintf
>>> ("%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
>>> + diskbuf[3], diskbuf[2], diskbuf[1], diskbuf[0],
>>> + diskbuf[5], diskbuf[4],
>>> + diskbuf[7], diskbuf[6],
>>> + diskbuf[8], diskbuf[9],
>>> + diskbuf[10], diskbuf[11], diskbuf[12],
>>> diskbuf[13], diskbuf[14], diskbuf[15]);
>>> +}
>>> +  else
>>> +return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
>>> +   N_("partition map %s does not support partition
>>> UUIDs"),
>>> +   dev->disk->partition->partmap->name);
>>> +}
>>> +  else
>>> +partuuid = grub_strdup (""); /* a freeable empty string */
>>> +
>>> +  if (state[0].set)
>>> +grub_env_set (state[0].arg, partuuid);
>>> +  else
>>> +grub_printf ("%s", partuuid);
>>> +  grub_free (partuuid);
>>> +  grub_device_close (dev);
>>> +  return GRUB_ERR_NONE;
>>> +}
>>> grub_device_close (dev);
>>> return grub_error (GRUB_ERR_BAD_ARGUMENT, "unrecognised target");
>>>   }
>>>
> 



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org

Re: Grub module to return partuuid of a device such as (hd0, gpt1) at boot time

[Nick Vinson]

On 08/14/2016 07:58 PM, sken...@ou.edu wrote:
> I think yours is in Linux user space while mine is grub boot time. I
> sent a second concept patch based on feedback and suggestions for a
> small change to grub probe command to set environment variable to guid
> of partition. Gpt partition guid is array of 16 uchar which is a bother.
> Do you access it directly too?

Yes I read that from the disk as well.  If you want to see my approach,
it's in the probe_partuuid() function (see:
http://lists.gnu.org/archive/html/grub-devel/2016-08/msg00019.html)

There's also an older version of this patch set that I submitted back in
June.  You may find the review comments helpful.  The link to the
comments are:
http://lists.gnu.org/archive/html/grub-devel/2016-07/msg00043.html

Thanks,
Nicholas Vinson

> 
> On August 14, 2016 9:43:14 PM CDT, Nick Vinson <nvin...@comcast.net> wrote:
> 
> Steve,
> 
> I'm not part of the mailing list and I only periodically check it.
> However, I was wondering if your request is the same as the code request
> that I made earlier this month
> (http://lists.gnu.org/archive/html/grub-devel/2016-08/msg00018.html).
> If not could you tell me how it's different?  If they're different, it
> might be worthwhile to combine our efforts into a single code change.
> 
> Let me know what you think,
> Nicholas Vinson
> 
> 
> -- 
> Sent from my Android device with K-9 Mail. Please excuse my brevity.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Grub module to return partuuid of a device such as (hd0, gpt1) at boot time

[Nick Vinson]

Steve,

I'm not part of the mailing list and I only periodically check it.
However, I was wondering if your request is the same as the code request
that I made earlier this month
(http://lists.gnu.org/archive/html/grub-devel/2016-08/msg00018.html).
If not could you tell me how it's different?  If they're different, it
might be worthwhile to combine our efforts into a single code change.

Let me know what you think,
Nicholas Vinson

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

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

[Nick Vinson]

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

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

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel